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

The -- mechanism for splitting arguments doesn't get forwarded correctly #25

Open
simonw opened this issue Sep 6, 2023 · 6 comments
Open

Comments

@simonw
Copy link

simonw commented Sep 6, 2023

Spotted this for my llm command, which uses click-default-group to set the default command to llm prompt.

If you run this:

llm -- '--help means what?'

You get this error:

Usage: llm prompt [OPTIONS] [PROMPT]
Try 'llm prompt --help' for help.

Error: No such option: --help means what?

But if you run the full command instead:

llm prompt -- '--help means what?'

You get the expected answer:

"--help" is a command that can be used in various computer programs and ...

Bug reported here:

@simonw
Copy link
Author

simonw commented Sep 6, 2023

Related code search in the Click source code: https://github.com/search?q=repo%3Apallets%2Fclick+%2F%5B%5E-%5D--%5B%5E-%5D%2F&type=code - had to search for repo:pallets/click /[^-]--[^-]/.

@simonw
Copy link
Author

simonw commented Sep 6, 2023

Here's a test that illustrates the bug:

@click.group(cls=DefaultGroup, default="baz", invoke_without_command=True)
def cli_with_argument_and_option():
    pass


@cli_with_argument_and_option.command()
@click.argument("argument")
@click.option("--option")
def baz(argument, option):
    click.echo(f"baz: argument: {argument}, option: {option}")


def test_cli_with_argument_and_option():
    result = r.invoke(
        cli_with_argument_and_option, ["argument", "--option", "option"]
    )
    assert result.output == "baz: argument: argument, option: option\n"
    # Now try with a -- prefixed argument
    result2 = r.invoke(
        cli_with_argument_and_option, ["--", "--starts-with-hyphens"]
    )
    assert result2.output == "baz: argument: --starts-with-hyphens, option: None\n"

Currently fails like this:

>       assert result2.output == "baz: argument: --starts-with-hyphens, option: None\n"
E       AssertionError: assert 'Usage: cli-w...ith-hyphens\n' == 'baz: argumen...ption: None\n'
E         - baz: argument: --starts-with-hyphens, option: None
E         + Usage: cli-with-argument-and-option baz [OPTIONS] ARGUMENT
E         + Try 'cli-with-argument-and-option baz --help' for help.
E         + 
E         + Error: No such option: --starts-with-hyphens

@simonw
Copy link
Author

simonw commented Sep 6, 2023

Stepping through with the debugger it gets to here:

> /Users/simon/.local/share/virtualenvs/click-default-group-mhmnP7bF/lib/python3.10/site-packages/click/core.py(1644)parse_args()
-> rest = super().parse_args(ctx, args)
(Pdb) list
1639 	    def parse_args(self, ctx: Context, args: t.List[str]) -> t.List[str]:
1640 	        if not args and self.no_args_is_help and not ctx.resilient_parsing:
1641 	            echo(ctx.get_help(), color=ctx.color)
1642 	            ctx.exit()
1643 	
1644 ->	        rest = super().parse_args(ctx, args)
1645 	
1646 	        if self.chain:
1647 	            ctx.protected_args = rest
1648 	            ctx.args = []
1649 	        elif rest:
(Pdb) args
self = <DefaultGroup cli-with-argument-and-option>
ctx = <click.core.Context object at 0x105a9f280>
args = ['--', '--starts-with-hyphens']

Before that rest = super().parse_args(ctx, args) line args is ['--', '--starts-with-hyphens']

But after that line it is ['--starts-with-hyphens'].

@simonw
Copy link
Author

simonw commented Sep 6, 2023

More debugger:

> /Users/simon/.local/share/virtualenvs/click-default-group-mhmnP7bF/lib/python3.10/site-packages/click/parser.py(335)parse_args()
-> state = ParsingState(args)
(Pdb) list
330  	        for the parsed options and arguments as well as the leftover
331  	        arguments if there are any.  The order is a list of objects as they
332  	        appear on the command line.  If arguments appear multiple times they
333  	        will be memorized multiple times as well.
334  	        """
335  ->	        state = ParsingState(args)
336  	        try:
337  	            self._process_args_for_options(state)
338  	            self._process_args_for_args(state)
339  	        except UsageError:
340  	            if self.ctx is None or not self.ctx.resilient_parsing:
(Pdb) n
> /Users/simon/.local/share/virtualenvs/click-default-group-mhmnP7bF/lib/python3.10/site-packages/click/parser.py(336)parse_args()
-> try:
(Pdb) state
<click.parser.ParsingState object at 0x107cdb130>
(Pdb) args
self = <click.parser.OptionParser object at 0x107cdb190>
args = ['--', '--starts-with-hyphens']
(Pdb) n
> /Users/simon/.local/share/virtualenvs/click-default-group-mhmnP7bF/lib/python3.10/site-packages/click/parser.py(337)parse_args()
-> self._process_args_for_options(state)
(Pdb) args
self = <click.parser.OptionParser object at 0x107cdb190>
args = ['--', '--starts-with-hyphens']
(Pdb) n
> /Users/simon/.local/share/virtualenvs/click-default-group-mhmnP7bF/lib/python3.10/site-packages/click/parser.py(338)parse_args()
-> self._process_args_for_args(state)
(Pdb) args
self = <click.parser.OptionParser object at 0x107cdb190>
args = ['--starts-with-hyphens']

So the call to self._process_args_for_options(state) left that -- in place... but that call to self._process_args_for_args(state) was the thing that removed -- from the args list.

Here's that code in Click: https://github.com/pallets/click/blob/874ca2bc1c30d93a4ac6e36a15ed685eafe89097/src/click/parser.py#L326-L342

@simonw
Copy link
Author

simonw commented Sep 6, 2023

I think I found it:

> /Users/simon/.local/share/virtualenvs/click-default-group-mhmnP7bF/lib/python3.10/site-packages/click/parser.py(358)_process_args_for_options()
-> arglen = len(arg)
(Pdb) list
353  	        state.rargs = []
354  	
355  	    def _process_args_for_options(self, state: ParsingState) -> None:
356  	        while state.rargs:
357  	            arg = state.rargs.pop(0)
358  ->	            arglen = len(arg)
359  	            # Double dashes always handled explicitly regardless of what
360  	            # prefixes are valid.
361  	            if arg == "--":
362  	                return

It looks like that bit of code there is popping off that -- (removing it from the args) list, then noticing that it is a double hyphen and shortcutting itself with that return.

@simonw
Copy link
Author

simonw commented Sep 6, 2023

... and that's as far as I got. I'm lost in Click internals now and I'm not sure how to fix this.

Here's my diff from where I was investigating with that test and a breakpoint:

diff --git a/click_default_group.py b/click_default_group.py
index 735c54b..f743ce7 100644
--- a/click_default_group.py
+++ b/click_default_group.py
@@ -77,9 +77,17 @@ class DefaultGroup(click.Group):
         self.default_cmd_name = cmd_name
 
     def parse_args(self, ctx, args):
+        separator_index = None
+        original_args = args[:]
+        if "--" in args:
+            # Remember where it was so we can put it back later
+            separator_index = args.index("--")
         if not args and self.default_if_no_args:
             args.insert(0, self.default_cmd_name)
-        return super(DefaultGroup, self).parse_args(ctx, args)
+        updated_args = super(DefaultGroup, self).parse_args(ctx, args)
+        if separator_index is not None:
+            breakpoint()
+        return updated_args
 
     def get_command(self, ctx, cmd_name):
         if cmd_name not in self.commands:
diff --git a/test.py b/test.py
index f90b80d..1e23b50 100644
--- a/test.py
+++ b/test.py
@@ -28,6 +28,32 @@ def bar():
 r = CliRunner()
 
 
+@click.group(cls=DefaultGroup, default="baz", invoke_without_command=True)
+def cli_with_argument_and_option():
+    pass
+
+
+@cli_with_argument_and_option.command()
+@click.argument("argument")
+@click.option("--option")
+def baz(argument, option):
+    click.echo(f"baz: argument: {argument}, option: {option}")
+
+
+def test_cli_with_argument_and_option():
+    result = r.invoke(
+        cli_with_argument_and_option, ["argument", "--option", "option"]
+    )
+    assert result.output == "baz: argument: argument, option: option\n"
+    # Now try with a -- prefixed argument
+    result2 = r.invoke(
+        cli_with_argument_and_option, ["--", "--starts-with-hyphens"]
+        , catch_exceptions=False
+    )
+    assert result2.output == "baz: argument: --starts-with-hyphens, option: None\n"
+
+
+
 def test_default_command_with_arguments():
     assert r.invoke(cli, ['--foo', 'foooo']).output == 'foooo\n'
     assert 'no such option' in r.invoke(cli, ['-x']).output.lower()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant