-
Couldn't load subscription status.
- Fork 651
[rush] Add support for pass-through parameters #5409
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
base: main
Are you sure you want to change the base?
Conversation
|
@microsoft-github-policy-service agree |
| @@ -0,0 +1,33 @@ | |||
| /** | |||
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.
Delete this. I'm guessing this was from an old fork.
| "changes": [ | ||
| { | ||
| "packageName": "@microsoft/rush", | ||
| "comment": "Add support for remainder arguments with new allowRemainderArguments option", |
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.
| "comment": "Add support for remainder arguments with new allowRemainderArguments option", | |
| "comment": "Add support for remainder arguments with new `allowRemainderArguments` option in a global, bulk, and phased command configurations in `common/config/rush/command-line.json`, |
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.
Sure thing!
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.
I want to see precise documentation on what this:
rush some-command --some-unknown foo bar --known-flag baz --known-integer 5 -- whatever
parses as, complete with unit tests that handle it. The unit tests should probably be on CommandLineParser itself, since it doesn't have any test cases related to handling of mixing known and unknown parameters.
The remainder parsing behavior I want to see is "everything after a -- token, not including the -- token itself" is the "remainder", If you have unrecognized parameters before a -- token that should still throw an error.
| }, | ||
| "allowRemainderArguments": { | ||
| "title": "Allow Remainder Arguments", | ||
| "description": "If true, this command will accept additional command-line arguments that appear after the \"--\" separator. Everything after \"--\" (not including the \"--\" itself) will be passed through to the shell command or npm script. Any unrecognized arguments before \"--\" will still cause an error. Example: \"rush my-command --known-flag value -- --arbitrary --args here\" will pass \"--arbitrary --args here\" to the script, but \"rush my-command --unknown-flag\" will fail with an error about the unrecognized parameter.", |
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.
This is the behavior I want, but I don't think that's how ts-command-line currently handles this, so this most likely needs some changes in ts-command-line as part of this PR.
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.
Sure thing. I've pushed some changes.
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.
I think the latest is mostly there, but getting a puzzling error in CI/CD:
--[ FAILURE: @rushstack/ts-command-line (test) ]-------------[ 6.06 seconds ]--
[test:jest] ● Test suite failed to run
[test:jest]
[test:jest] TypeError: Converting circular structure to JSON
[test:jest] --> starting at object with constructor 'Array'
[test:jest] | index 0 -> object with constructor 'ActionHelp'
[test:jest] | property 'container' -> object with constructor 'ArgumentGroup'
[test:jest] --- property '_actions' closes the circle
[test:jest] at stringify (<anonymous>)
[test:jest]
[test:jest] at messageParent (../../common/temp/default/node_modules/.pnpm/[email protected]/node_modules/jest-worker/build/workers/messageParent.js:29:19)```
Summary
This implementation adds support for allowRemainderArguments property in Rush custom commands, enabling commands to accept and pass through arbitrary additional command-line arguments to shell commands or npm scripts. This addresses GitHub issue #4492.
Details
How it was tested
Added some tests.
Impacted documentation