Skip to content
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] Add ScopedCommandLineAction class #3364

Merged
merged 19 commits into from
May 10, 2022

Conversation

D4N14L
Copy link
Member

@D4N14L D4N14L commented Apr 21, 2022

Summary

Adds the ScopedCommandLineAction class, which allows for scoping on commands to influence the parsing of parameters used.

Details

The ScopedCommandLineAction class was implemented using a 2-stage parsing technique. All scoped arguments must be provided as positional arguments specified after the pseudo-argument --. This pseudo-argument is required when specifying scoped arguments, and forces all following arguments to be interpreted as positional. These positional arguments are dropped into the ScopedCommandLineAction.remainder property, where they are then fed to a secondary parser to validate against the defined scoped commands. This allows for argparse to validate and parse the underlying scoped arguments as if they were provided to a top-level action, and makes help documentation clear and understandable in this scenario.

Underlying scoped arguments can be varied depending on the arguments provided to the owning action by the caller. This allows for two different scopes to have different arguments, eg:

<tool> <action> --scope A -- --scopedFlag1 works because --scopedFlag1 is defined only when --scope is set to A
<tool> <action> --scope B -- --scopedFlag2 works because --scopedFlag2 is defined only when --scope is set to B
<tool> <action> --scope A -- --scopedFlag2 does not work because --scopedFlag2 is not defined when --scope is set to A

Additionally, scopes are not necessarily required on scoped commands (this is left up to the developer to allow or prevent), eg:

<tool> <action> --scope A runs the action limited to scope A
<tool> <action> runs the action against all scopes

As mentioned, --help documentation is well-defined and easy to understand, eg.

> example scoped-action --help
usage: example scoped-action [-h] [--scope SCOPE] ...

a longer description

Positional arguments:
  "..."          Scoped parameters. Must be prefixed with "--", ex. "-- 
                 --scopedParameter foo --scopedFlag". For more information 
                 on available scoped parameters, use "-- --help".

Optional arguments:
  -h, --help     Show this help message and exit.

Optional scoping arguments:
  --scope SCOPE  The scope.

> example scoped-action --scope foo -- --help
usage: example scoped-action --scope foo -- [-h] [--scoped-arg SCOPED]

a longer description

Optional arguments:
  -h, --help           Show this help message and exit.
  --scoped-arg SCOPED  The scoped argument.

For more information on available unscoped parameters, use "example 
scoped-action --help"

How it was tested

Added tests and snapshot tests to ensure that validation of parameters works as expected, and that the provided action onExecute method gets called with all required parameters populated.

@sachinjoseph
Copy link
Member

Would a syntax like --scope A:"--param1 value1 --param2 value2" --scope B:"--param3 value3 --param4 value4" be a good idea to pass params to multiple scopes (A and B in this case), and while still remaining very explicit about which scope to pass the params/values to? To be able to escape quotes (for example, say value1 contains spaces) may be a challenge though

@D4N14L
Copy link
Member Author

D4N14L commented Apr 25, 2022

Would a syntax like --scope A:"--param1 value1 --param2 value2" --scope B:"--param3 value3 --param4 value4" be a good idea to pass params to multiple scopes (A and B in this case), and while still remaining very explicit about which scope to pass the params/values to? To be able to escape quotes (for example, say value1 contains spaces) may be a challenge though

Hmmm.... The nice part about the approach taken in the PR is that duplicate parameters across different scopes (which provide their own parameters) are handled naturally, since the args would simply be available in the onExecute method. It removes the verbosity that would be associated with scoped arguments that are common across most scopes (ex. if scopes A, B, C, D are selected, and A, B, C accept --arg1 and D does not, it would not be required to pass the arg to each individual scope while omitting it from the last).

Also, the design is such that you use the action to implement a single scope, with multiple different scoping parameters progressively narrowing the scope (ex. --to <A> --except <B> --only-changed selecting everything up to A that has changed, and excluding B) or progressively expanding the scope (ex. --to <A> --from <B> --only <C> selecting everything up to A, everything after B, and C). There's only one "scope", and the breadth of that scope changes with the scoping arguments.

The way it is currently designed, the action allows the developer the flexibility to choose how the scope should be used and how scoped parameters should be applied within the selected scope. It's explicitly not choosing to make any statements on which arguments apply to which scope values, because IMO that should be decided by the developer.

@iclanton iclanton merged commit fd8c362 into microsoft:main May 10, 2022
@iclanton iclanton deleted the user/danade/ScopedCommandLine branch May 10, 2022 01:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants