-
Notifications
You must be signed in to change notification settings - Fork 3
Add support for selecting subcommands as flags #228
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -304,6 +304,7 @@ class DynamicGroup(click.RichGroup): | |
| subcommand_filter: A function that takes a subcommand name and returns a boolean indicating whether the | ||
| subcommand should be included in the list of subcommands. | ||
| search_path_finder: A function that returns a list of directories to search for subcommands. | ||
| add_subcommand_selectors: Whether to add flag selectors for each subcommand. | ||
|
|
||
| Other parameters: | ||
| *args: Additional positional arguments to pass to the [`Group`][click.Group] constructor. | ||
|
|
@@ -319,13 +320,15 @@ def __init__( | |
| allow_external_plugins: bool | None = None, | ||
| subcommand_filter: Callable[[str], bool] | None = None, | ||
| search_path_finder: Callable[[], list[str]] | None = None, | ||
| add_subcommand_selectors: bool | None = None, | ||
| **kwargs: Any, | ||
| ) -> None: | ||
| super().__init__(*args, **kwargs) | ||
|
|
||
| self.allow_external_plugins = allow_external_plugins | ||
| self.__subcommand_filter = subcommand_filter | ||
| self.__search_path_finder = search_path_finder | ||
| self.__add_subcommand_selectors = add_subcommand_selectors | ||
| self.__subcommand_cache: list[str] | None = None | ||
|
|
||
| self.__dynamic_path_id = 0 | ||
|
|
@@ -418,6 +421,26 @@ def _lazy_load(cls, cmd_name: str, path: str) -> DynamicCommand | DynamicGroup: | |
|
|
||
| return cmd_object | ||
|
|
||
| def get_params(self, ctx: DynamicContext) -> list[click.Parameter]: # type: ignore[override] | ||
| # https://github.com/pallets/click/pull/2784 | ||
| # https://github.com/pallets/click/blob/8.1.7/src/click/core.py#L1255 | ||
| params = [*self.params, *ctx.dynamic_params] | ||
|
|
||
| # Add a flag for each subcommand | ||
| if self.__add_subcommand_selectors: | ||
| params.extend( | ||
| click.Option(param_decls=[f"--{command}"], is_flag=True) for command in self.list_commands(ctx) | ||
| ) | ||
|
Comment on lines
+430
to
+433
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of specifying an option on the group to add all subcommands as flags, would it be possible to add an Within one group, we might not always want to have all subcommands available as flags.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alternatively, would it make sense to create an entirely new subclass of class FlagGroup(DynamicGroup):
# Implement it such that all commands in this group automatically get registered as flags
def cmd(...):
# Here we could implement cmd such that a `FlagGroup` always implements the behavior in your example: running all subcommands sequentially by default, and only selecting some of them if flags have been passed |
||
|
|
||
| if (help_option := self.get_help_option(ctx)) is not None: | ||
| params.append(help_option) | ||
|
|
||
| return params | ||
|
|
||
| def get_selected_subcommands(self, ctx: DynamicContext, selections: dict[str, bool]) -> list[str]: | ||
| subcommands = [_normalize_cmd_name(command) for command, selected in selections.items() if selected] | ||
| return subcommands or self.list_commands(ctx) | ||
|
|
||
|
|
||
| def _normalize_cmd_name(cmd_name: str) -> str: | ||
| return cmd_name.replace("_", "-") | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -125,3 +125,91 @@ def cmd(app): | |
| encoding="utf-8", | ||
| ), | ||
| ] | ||
|
|
||
|
|
||
| def test_subcommand_selectors(dda, helpers, temp_dir): | ||
| commands_dir = temp_dir / ".dda" / "extend" / "commands" | ||
| commands_dir.ensure_dir() | ||
| commands_dir.joinpath("run").ensure_dir() | ||
| commands_dir.joinpath("run", "__init__.py").write_text( | ||
| helpers.dedent( | ||
| """ | ||
| import click | ||
| from dda.cli.base import dynamic_group | ||
|
|
||
| @dynamic_group( | ||
| invoke_without_command=True, | ||
| add_subcommand_selectors=True, | ||
| ) | ||
| @click.pass_context | ||
| def cmd(ctx, **selections): | ||
| if not ctx.invoked_subcommand: | ||
| ctx.obj.display(" ".join(cmd.get_selected_subcommands(ctx, selections))) | ||
| """ | ||
| ) | ||
| ) | ||
|
Comment on lines
+135
to
+150
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Instead of writing at runtime, why not include a new file in |
||
| commands_dir.joinpath("run", "foo").ensure_dir() | ||
| commands_dir.joinpath("run", "foo", "__init__.py").write_text( | ||
| helpers.dedent( | ||
| """ | ||
| from dda.cli.base import dynamic_command, pass_app | ||
|
|
||
| @dynamic_command() | ||
| @pass_app | ||
| def cmd(app): | ||
| app.display("foo command") | ||
| """ | ||
| ) | ||
| ) | ||
| commands_dir.joinpath("run", "bar").ensure_dir() | ||
| commands_dir.joinpath("run", "bar", "__init__.py").write_text( | ||
| helpers.dedent( | ||
| """ | ||
| from dda.cli.base import dynamic_command, pass_app | ||
|
|
||
| @dynamic_command() | ||
| @pass_app | ||
| def cmd(app): | ||
| app.display("bar command") | ||
| """ | ||
| ) | ||
| ) | ||
|
|
||
| # Select all by default | ||
| with temp_dir.as_cwd(): | ||
| result = dda("run") | ||
|
|
||
| result.check( | ||
| exit_code=0, | ||
| stdout=helpers.dedent( | ||
| """ | ||
| bar foo | ||
| """ | ||
| ), | ||
| ) | ||
|
|
||
| # Select specific subcommands | ||
| with temp_dir.as_cwd(): | ||
| result = dda("run", "--foo") | ||
|
|
||
| result.check( | ||
| exit_code=0, | ||
| stdout=helpers.dedent( | ||
| """ | ||
| foo | ||
| """ | ||
| ), | ||
| ) | ||
|
|
||
| # Run subcommand directly | ||
| with temp_dir.as_cwd(): | ||
| result = dda("run", "foo") | ||
|
|
||
| result.check( | ||
| exit_code=0, | ||
| stdout=helpers.dedent( | ||
| """ | ||
| foo command | ||
| """ | ||
| ), | ||
| ) | ||
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.
nit: Add
@overridefor clarity (maybe with the link to the source, as is added below)