From 4b03a6bfe9cfdf66c1c1e9466aae7c568586aa89 Mon Sep 17 00:00:00 2001 From: Paul Spangler Date: Fri, 17 Sep 2021 16:14:07 -0500 Subject: [PATCH] Rewrite to make shell completion pull option prefixes from the parser --- src/click/core.py | 3 ++ src/click/parser.py | 11 ++++--- src/click/shell_completion.py | 27 ++++++++-------- tests/test_commands.py | 59 ++++++++++++++++++++++++++++++++++ tests/test_context.py | 8 +++++ tests/test_parser.py | 15 +++++++++ tests/test_shell_completion.py | 26 +++++++++++++++ 7 files changed, 130 insertions(+), 19 deletions(-) diff --git a/src/click/core.py b/src/click/core.py index 9ba738ba5..cc5e5e766 100644 --- a/src/click/core.py +++ b/src/click/core.py @@ -292,6 +292,8 @@ def __init__( #: must be never propagated to another arguments. This is used #: to implement nested parsing. self.protected_args: t.List[str] = [] + #: the collected prefixes of the command's options. + self.opt_prefixes: t.Set[str] = set(parent.opt_prefixes) if parent else set() if obj is None and parent is not None: obj = parent.obj @@ -1385,6 +1387,7 @@ def parse_args(self, ctx: Context, args: t.List[str]) -> t.List[str]: ) ctx.args = args + ctx.opt_prefixes.update(parser.opt_prefixes) return args def invoke(self, ctx: Context) -> t.Any: diff --git a/src/click/parser.py b/src/click/parser.py index 2d5a2ed7b..cb602c322 100644 --- a/src/click/parser.py +++ b/src/click/parser.py @@ -278,6 +278,8 @@ def __init__(self, ctx: t.Optional["Context"] = None) -> None: #: second mode where it will ignore it and continue processing #: after shifting all the unknown options into the resulting args. self.ignore_unknown_options = False + #: The collected prefixes of the options added to the parser. + self.opt_prefixes = {"-", "--"} if ctx is not None: self.allow_interspersed_args = ctx.allow_interspersed_args @@ -285,7 +287,6 @@ def __init__(self, ctx: t.Optional["Context"] = None) -> None: self._short_opt: t.Dict[str, Option] = {} self._long_opt: t.Dict[str, Option] = {} - self._opt_prefixes = {"-", "--"} self._args: t.List[Argument] = [] def add_option( @@ -307,7 +308,7 @@ def add_option( """ opts = [normalize_opt(opt, self.ctx) for opt in opts] option = Option(obj, opts, dest, action=action, nargs=nargs, const=const) - self._opt_prefixes.update(option.prefixes) + self.opt_prefixes.update(option.prefixes) for opt in option._short_opts: self._short_opt[opt] = option for opt in option._long_opts: @@ -360,7 +361,7 @@ def _process_args_for_options(self, state: ParsingState) -> None: # prefixes are valid. if arg == "--": return - elif arg[:1] in self._opt_prefixes and arglen > 1: + elif arg[:1] in self.opt_prefixes and arglen > 1: self._process_opts(arg, state) elif self.allow_interspersed_args: state.largs.append(arg) @@ -482,7 +483,7 @@ def _get_value_from_state( if ( option.obj._flag_needs_value and isinstance(next_rarg, str) - and next_rarg[:1] in self._opt_prefixes + and next_rarg[:1] in self.opt_prefixes and len(next_rarg) > 1 ): # The next arg looks like the start of an option, don't @@ -519,7 +520,7 @@ def _process_opts(self, arg: str, state: ParsingState) -> None: # (applies to "--foo" for instance), we do not dispatch to the # short option code and will instead raise the no option # error. - if arg[:2] not in self._opt_prefixes: + if arg[:2] not in self.opt_prefixes: self._match_short_opt(arg, state) return diff --git a/src/click/shell_completion.py b/src/click/shell_completion.py index 7da020020..6706febf7 100644 --- a/src/click/shell_completion.py +++ b/src/click/shell_completion.py @@ -448,17 +448,16 @@ def _is_incomplete_argument(ctx: Context, param: Parameter) -> bool: ) -def _start_of_option(value: str) -> bool: +def _start_of_option(ctx: Context, value: str) -> bool: """Check if the value looks like the start of an option.""" if not value: return False c = value[0] - # Allow "/" since that starts a path. - return not c.isalnum() and c != "/" + return c in ctx.opt_prefixes -def _is_incomplete_option(args: t.List[str], param: Parameter) -> bool: +def _is_incomplete_option(ctx: Context, args: t.List[str], param: Parameter) -> bool: """Determine if the given parameter is an option that needs a value. :param args: List of complete args before the incomplete value. @@ -476,7 +475,7 @@ def _is_incomplete_option(args: t.List[str], param: Parameter) -> bool: if index + 1 > param.nargs: break - if _start_of_option(arg): + if _start_of_option(ctx, arg): last_option = arg return last_option is not None and last_option in param.opts @@ -551,25 +550,25 @@ def _resolve_incomplete( # split and discard the "=" to make completion easier. if incomplete == "=": incomplete = "" - elif "=" in incomplete and _start_of_option(incomplete): + elif "=" in incomplete and _start_of_option(ctx, incomplete): name, _, incomplete = incomplete.partition("=") args.append(name) + # The "--" marker tells Click to stop treating values as options + # even if they start with the option character. If it hasn't been + # given and the incomplete arg looks like an option, the current + # command will provide option name completions. + if "--" not in args and _start_of_option(ctx, incomplete): + return ctx.command, incomplete + params = ctx.command.get_params(ctx) # If the last complete arg is an option name with an incomplete # value, the option will provide value completions. for param in params: - if _is_incomplete_option(args, param): + if _is_incomplete_option(ctx, args, param): return param, incomplete - # The "--" marker tells Click to stop treating values as options - # even if they start with the option character. If it hasn't been - # given and the incomplete arg looks like an option, the current - # command will provide option name completions. - if "--" not in args and _start_of_option(incomplete): - return ctx.command, incomplete - # It's not an option name or value. The first argument without a # parsed value will provide value completions. for param in params: diff --git a/tests/test_commands.py b/tests/test_commands.py index bf6a5db82..698ee42bc 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -352,3 +352,62 @@ def deprecated_cmd(): result = runner.invoke(deprecated_cmd) assert "DeprecationWarning:" in result.output + + +def test_command_parse_args_collects_option_prefixes(): + @click.command() + @click.option("+p", is_flag=True) + @click.option("!e", is_flag=True) + def test(p, e): + pass + + ctx = click.Context(test) + test.parse_args(ctx, []) + + assert ctx.opt_prefixes == {"-", "--", "+", "!"} + + +def test_group_parse_args_collects_base_option_prefixes(): + @click.group() + @click.option("~t", is_flag=True) + def group(t): + pass + + @group.command() + @click.option("+p", is_flag=True) + def command1(p): + pass + + @group.command() + @click.option("!e", is_flag=True) + def command2(e): + pass + + ctx = click.Context(group) + group.parse_args(ctx, ["command1", "+p"]) + + assert ctx.opt_prefixes == {"-", "--", "~"} + + +def test_group_invoke_collects_used_option_prefixes(runner): + opt_prefixes = set() + + @click.group() + @click.option("~t", is_flag=True) + def group(t): + pass + + @group.command() + @click.option("+p", is_flag=True) + @click.pass_context + def command1(ctx, p): + nonlocal opt_prefixes + opt_prefixes = ctx.opt_prefixes + + @group.command() + @click.option("!e", is_flag=True) + def command2(e): + pass + + runner.invoke(group, ["command1"]) + assert opt_prefixes == {"-", "--", "~", "+"} diff --git a/tests/test_context.py b/tests/test_context.py index 98f083570..785a55709 100644 --- a/tests/test_context.py +++ b/tests/test_context.py @@ -365,3 +365,11 @@ def cli(ctx, option): rv = runner.invoke(cli, standalone_mode=False, **invoke_args) assert rv.return_value == expect + + +def test_propagate_opt_prefixes(): + parent = click.Context(click.Command("test")) + parent.opt_prefixes = {"-", "--", "!"} + ctx = click.Context(click.Command("test2"), parent=parent) + + assert ctx.opt_prefixes == {"-", "--", "!"} diff --git a/tests/test_parser.py b/tests/test_parser.py index 964f9c826..d1365e733 100644 --- a/tests/test_parser.py +++ b/tests/test_parser.py @@ -1,5 +1,7 @@ import pytest +import click +from click.parser import OptionParser from click.parser import split_arg_string @@ -15,3 +17,16 @@ ) def test_split_arg_string(value, expect): assert split_arg_string(value) == expect + + +def test_parser_default_prefixes(): + parser = OptionParser() + assert parser.opt_prefixes == {"-", "--"} + + +def test_parser_collects_prefixes(): + ctx = click.Context(click.Command("test")) + parser = OptionParser(ctx) + click.Option("+p", is_flag=True).add_to_parser(parser, ctx) + click.Option("!e", is_flag=True).add_to_parser(parser, ctx) + assert parser.opt_prefixes == {"-", "--", "+", "!"} diff --git a/tests/test_shell_completion.py b/tests/test_shell_completion.py index 0a8bb5b86..57729f5b6 100644 --- a/tests/test_shell_completion.py +++ b/tests/test_shell_completion.py @@ -117,12 +117,38 @@ def test_choice_special_characters(): assert _get_words(cli, ["-c"], "!2") == ["!2"] +def test_choice_conflicting_prefix(): + cli = Command( + "cli", + params=[ + Option(["-c"], type=Choice(["!1", "!2", "+3"])), + Option(["+p"], is_flag=True), + ], + ) + assert _get_words(cli, ["-c"], "") == ["!1", "!2", "+3"] + assert _get_words(cli, ["-c"], "+") == ["+p"] + + def test_option_count(): cli = Command("cli", params=[Option(["-c"], count=True)]) assert _get_words(cli, ["-c"], "") == [] assert _get_words(cli, ["-c"], "-") == ["--help"] +def test_option_optional(): + cli = Command( + "cli", + add_help_option=False, + params=[ + Option(["--name"], is_flag=False, flag_value="value"), + Option(["--flag"], is_flag=True), + ], + ) + assert _get_words(cli, ["--name"], "") == [] + assert _get_words(cli, ["--name"], "-") == ["--flag"] + assert _get_words(cli, ["--name", "--flag"], "-") == [] + + @pytest.mark.parametrize( ("type", "expect"), [(File(), "file"), (Path(), "file"), (Path(file_okay=False), "dir")],