[ts-command-line] Fix env var name shown in EnvironmentVariableParser error messages#5831
Open
LeSingh1 wants to merge 1 commit into
Open
Conversation
…e in error messages When a list parameter's environment variable contained an invalid JSON array, the two error messages in EnvironmentVariableParser.parseAsList accidentally interpolated the variable's value (environmentValue) where the variable's name (envVarName) was intended. The result was confusing messages like "The [u environment variable value looks like a JSON array but failed to parse" instead of "The MY_VAR environment variable value looks like a JSON array but failed to parse". Update the two affected template strings to use envVarName, and correct the corresponding test assertion and snapshot that were written to match the old (wrong) output.
LeSingh1
requested review from
apostolisms,
dmichon-msft,
iclanton,
jxanthony and
octogonz
as code owners
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When parseAsList encounters a malformed JSON array in an environment variable, it throws
two possible error messages. Both messages were accidentally interpolating the variable's
value (environmentValue) instead of the variable's name (envVarName). This means a user
who set MY_VAR='[u' would see:
The [u environment variable value looks like a JSON array but failed to parse: ...
instead of the expected:
The MY_VAR environment variable value looks like a JSON array but failed to parse: ...
The fix is a two-character change in each template string, swapping environmentValue for
envVarName. The existing test assertion and snapshot were written against the wrong output,
so those are corrected to match the intended behavior.
I noticed this while reading EnvironmentVariableParser.ts. No open issue existed for it
so I'm filing the fix directly. Happy to open a tracking issue first if that's preferred.