Issue 2279

[samcoppock]

Fix issue #2279

The test covers both the --option value and --option=value formats

[samcoppock]

@dotnet-policy-service agree

[KalleOlaviNiemitalo]

Fix issue 2279

This syntax is not recognised by GitHub. Please see Linking a pull request to an issue

[samcoppock]

@KalleOlaviNiemitalo

Tests are fixed, please review again

[elgonzo]

My apologies for barging into the conversion like that, but please note that this is not a proper fix, as it is introducing another bug. = can be a valid character in an option/argument value, however the PR and the test included therein do not consider this fact.

Without the suggested PR, a CLI like outer inner --optionOne -=Yay=- would be consumed without any troubles. But with the PR here, an exception will be thrown in the ParseResult.WillAcceptAnArgument method.

To reproduce, it is enough to change the CLI string the test is using from

var commandLine = "inner --optionOne argument1 --optionTwo=argument2";

to

var commandLine = "inner --optionOne -=Yay=-";

It is important to note that the occurrence of this issue is conditional on the option/argument value containing = being the last token in the CLI string.

A proper fix would ideally also address the related issue that the GetCompletions() method seems to not like the last option/argument value in a CLI string having white-spaces when the CLI parameters have been passed to the Parse method as a single string only. Regardless of whether this PR is applied or not, parseResult.GetCompletions() will throw when a commandline string like "inner --optionOne \"foo bar\"" is provided.

[samcoppock]

My apologies for barging into the conversion like that, but please note that this is not a proper fix, as it is introducing another bug. = can be a valid character in an option/argument value, however the PR and the test included therein do not consider this fact.

Without the suggested PR, a CLI like outer inner --optionOne -=Yay=- would be consumed without any troubles. But with the PR here, an exception will be thrown in the ParseResult.WillAcceptAnArgument method.

To reproduce, it is enough to change the CLI string the test is using from

var commandLine = "inner --optionOne argument1 --optionTwo=argument2";

to

var commandLine = "inner --optionOne -=Yay=-";

It is important to note that the occurrence of this issue is conditional on the option/argument value containing = being the last token in the CLI string.

A proper fix would ideally also address the related issue that the GetCompletions() method seems to not like the last option/argument value in a CLI string having white-spaces when the CLI parameters have been passed to the Parse method as a single string only. Regardless of whether this PR is applied or not, parseResult.GetCompletions() will throw when a commandline string like "inner --optionOne \"foo bar\"" is provided.

I did think of that

the = character is a special character which has to be escaped if used on the CLI

https://mywiki.wooledge.org/BashGuide/SpecialCharacters

I did test this during development but did not include a test for this

[elgonzo]

the = character is a special character which has to be escaped if used on the CLI

Incorrect in the sense that this is not generally true. Whether = is a special character is specific to particular shells or particular shell commands only (or whatever particular script language is being used to execute a program while passing commandline arguments to it). For example, neither the command prompts of cmd.exe nor Powershell (arguably the two most popular shells in the Windows world) treat = as a special character when they appear as part of the commandline parameters for a program invocation.

[KalleOlaviNiemitalo]

Even if = is a special character for the shell and has to be escaped to make the shell treat it as a literal character -- this escaping is then consumed by the shell and not visible to the System.CommandLine library. So the treatment of incoming characters in System.CommandLine should not depend on whether they are special for a shell. (In contrast, the output of dotnet-suggest could usefully depend on such things, if it knows which shell is being used.)

[samcoppock]

var commandLine = "inner --optionOne -=Yay=-";

the "var commandLine = "inner --optionOne -=Yay=-";" issue has been fixed

[samcoppock]

@KalleOlaviNiemitalo @elgonzo @radical @shanselman

can you guys take another look at this please