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
fix: parser-configuration should work well with generated completion script #2332
base: main
Are you sure you want to change the base?
Conversation
Long story. This is not quite right, but works in simple cases. I think there are less code changes required. The strip-dashed spelling appearing in the "aliases" is a bit of cheat, it only appears in the parse results and is not recognised on the command-line and does not match what is on the command-line. I added some trace statements to double-check my expectations. (I have a At this original line:
To cope with the author changing the parser configuration, we need to be flexible about what the property got called in the parse results. Later at this point:
Here, we still want to check for the original unaltered So requestCompletions should stay just a boolean, and only that initial calculation changes. I think you can just change |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See long comment! I think this is close, but doing too much.
This assumption is ok. The cause of the bug you observed is that the option name got changed in the parse results by the parser configuration settings, and fooled yargs. |
e762ac2
to
095fdd8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @AgentEnder
Good description with the PR, and added a test covering multiple configurations. Nice work. |
Thanks for the help @shadowspawn! Once it is merged, do you know when the next release would be? I've admittedly not paid too much attention to the yargs release schedule in the past. |
I am guessing next release 1-2 months after last release (which was 2023-04-27). |
There is an incompatibility between the parser configuration options and the completion feature within yargs. Setting strip-dashed: true in the parser configuration causes the completion feature to fail.
This is because the completion feature relies on passing '--get-yargs-completions' verbatim to yargs. Within the Completion class, the completionKey is explicitly set to get-yargs-completions. Within the YargsInstance, we explicitly check the following condition: completionKey in argv, in this location: main/lib/yargs-factory.ts#L2054
Since the argv has been parsed already at this point, the argument is now 'getYargsCompletions' instead of 'get-yargs-completions'. This causes the completion feature to never be called.
Fixes: #2330