-
Notifications
You must be signed in to change notification settings - Fork 607
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
[ts-command-line] Prevent ambiguous abbreviations of parameters defined on parent tool or action #4405
Merged
D4N14L
merged 21 commits into
microsoft:main
from
D4N14L:user/danade/PreventAmbiguousAbbreviations
Oct 30, 2023
Merged
[ts-command-line] Prevent ambiguous abbreviations of parameters defined on parent tool or action #4405
D4N14L
merged 21 commits into
microsoft:main
from
D4N14L:user/danade/PreventAmbiguousAbbreviations
Oct 30, 2023
Conversation
This file contains 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
D4N14L
requested review from
iclanton,
octogonz,
apostolisms and
dmichon-msft
as code owners
October 23, 2023 20:27
dmichon-msft
approved these changes
Oct 23, 2023
libraries/ts-command-line/src/providers/CommandLineParameterProvider.ts
Outdated
Show resolved
Hide resolved
…er/danade/PreventAmbiguousAbbreviations
iclanton
reviewed
Oct 25, 2023
common/changes/@rushstack/heft/user-danade-PreventAmbiguousAbbreviations_2023-10-24-18-11.json
Outdated
Show resolved
Hide resolved
...s/@rushstack/ts-command-line/user-danade-PreventAmbiguousAbbreviations_2023-10-23-20-01.json
Outdated
Show resolved
Hide resolved
libraries/ts-command-line/src/providers/ScopedCommandLineAction.ts
Outdated
Show resolved
Hide resolved
libraries/ts-command-line/src/providers/CommandLineParameterProvider.ts
Outdated
Show resolved
Hide resolved
iclanton
reviewed
Oct 26, 2023
...s/@rushstack/ts-command-line/user-danade-PreventAmbiguousAbbreviations_2023-10-23-20-01.json
Outdated
Show resolved
Hide resolved
Co-authored-by: Ian Clanton-Thuon <[email protected]>
…er/danade/PreventAmbiguousAbbreviations
iclanton
approved these changes
Oct 30, 2023
…er/danade/PreventAmbiguousAbbreviations
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Consider parameters defined on the parent tool and (if applicable) parent action when preventing ambiguous parameter usage or ambiguous parameter abbreviations.
Details
Previously, it was possible to define a parameter on a parent tool or action, and not have an error thrown when the child action declares either an identical parameter or a parameter that has an ambiguous abbreviation when considering the parent parameters.
Ex. Given a tool
heft
with a defined parameter--debug
and a defined actiontest
with a defined parameter--debug
:heft --debug test
andheft test --debug
are valid invocations of theheft
tool, but the parameters would be handled differentlyheft --debug test --debug
would also be considered a valid commandThis also applies to the automatic abbreviations that are supported by
ts-command-line
.Ex. Given a tool
heft
with a defined parameter--debug
and a defined actiontest
with a defined parameter--debug-mode
:heft --debug test
andheft test --debug
are valid invocations of theheft
tool, but the parameters would be handled differentlyheft --debug test --debug
would also be considered a valid commandWith this change, parent parameters are considered when looking for ambiguous parameter references.
Ex. Given a tool
heft
with a defined parameter--debug
and a defined actiontest
:--debug
on thetest
action will throw an error regarding duplicate parameters--debug-mode
on thetest
action is validheft --debug test
is still validheft test --debug
will throw an error regarding ambiguous parameter usageheft test --debug-
is valid, as it is an unambiguous abbreviation of the--debug-mode
parameterHow it was tested
Added a bunch of unit tests.
Impacted documentation
May require updating website docs on
ts-command-line