-
Notifications
You must be signed in to change notification settings - Fork 374
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[main] Test has incorrect errors for -x 1 2 3 4
#2392
Comments
I think this may be a bug (even on main). First, the test input is bad. The successful test a couple up shows how an option's argument with a minimum arity greater than 1 should be tested. Specifically the test input should be: Now with that, said, even once that is corrected that will only solve your first three "Unrecognized command or argument" errors from your screenshot. You are then left with two "Option '-x' expects a single argument but 4 were provided." errors. I would propose both the error message is incorrect, as well as the fact that there are two errors (I agree there really should only be one). This is not something new with powerhouse, this is existing behavior on main. |
For this test to succeed, it would be required to set the option's command-line-api/src/System.CommandLine/CliOption.cs Lines 85 to 99 in 4457da4
(Commit and discussion related to making this disabled by default during past SCL development: #1199, #1162) |
There were two problems addressed here. First, the call to Diagram was causing the second error to be added to the ParseResult.Errors collection. The diagram call, was being invoked by the debugger as part of the ParseResult.ToString() method. Causing the number of errors to change when inspecting a parse result. The second issue was the error message text. It was implying that the option expected a single value, when its arity allowed for more.
Fixes dotnet#2392. This is a port from the fix on main. However due to additional work that is currently being done on Powerderhouse it is forcing the creation of the ArgumentConversionResult as part of parsing. This is due to the invocation SymbolResultTree.GetValueResultDictionary() which forces the evaluation of all ArgumentConversionResults. This is adding an additional test to make sure that the behavior is not re-introduced. This also fixes a few NRT issues and a comment typo.
The test I used to confirm unexpected behavior
The test is:
I modified the test only to get an interim variable for
parseResult
,When I do this, the error list is
I encountered this because for Powderhouse I encountered issues with options with collection types and I believe I am using the same or funcionally equivalent code - and this test passes. In exploring why my new tests were failing, I realized this test does not appear to be succeeding at what the CLI and the commandline request.
I believe this test should result in a single parse error - an unexpected argument only on the 4.
The text was updated successfully, but these errors were encountered: