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

Shell completion does't parse escaped slashes or spaces correctly #1708

Closed
samschott opened this issue Nov 9, 2020 · 9 comments · Fixed by #1709
Closed

Shell completion does't parse escaped slashes or spaces correctly #1708

samschott opened this issue Nov 9, 2020 · 9 comments · Fixed by #1709
Labels
f:completion feature: shell completion
Milestone

Comments

@samschott
Copy link
Contributor

samschott commented Nov 9, 2020

When a value with an escape string, for instance my\ file, is passed to a click.ParamType, the incomplete value which is passed to click.ParamType.shell_complete will only be "my\ " instead of my file.

Expected Behavior

import click

@click.command()
@click.argument("name", type=click.Choice(["my file", "my folder", "my\\ file", "other"]))
def cli(name):
    click.echo(name)

if __name__ == '__main__':
    cli()

Completing my\ f should be interpreted as the Python string "my f" and match the first two values.

$ example my\ f<TAB>
my\ file
my\ folder

Actual Behavior

Completing my\ f is interpreted as the string "my\\ f" and matches the third value.

$ example my\ f<TAB>
my\\\ file

Environment

  • Python version: 3.8.6
  • Click version: 8.0.0.dev0
@davidism

This comment has been minimized.

@davidism davidism closed this as completed Nov 9, 2020
@samschott

This comment has been minimized.

@davidism davidism changed the title Shell completion does not handle escape sequences Shell completion does't parse escaped slashes or spaces correctly Nov 9, 2020
@davidism
Copy link
Member

davidism commented Nov 9, 2020

I've updated the issue with a simpler example and direct explanations for expected and actual behavior.

The issue seems to be with how split_arg_string handles slashes and spaces. It's trying to emulate how sys.argv would be populated to apply it to the CWORDS env var, but doesn't get it right in all cases.

Adding some debugging code to ZshComplete.get_completion_args, when my\ f is completed:

os.environ['COMP_WORDS']='example my\\ f'
cwords=['example', 'my\\', 'f']
cword=1
args=[]
incomplete='my\\'

The shell is passing my\ a, which is the Python string "my\\ f". This is getting parsed into two values "my\\" and "f". Instead, it should get parsed into one value "my\\ f".

You can find some other weird behaviors by tab completing earlier, or with a single opening quote, or within quotes. The splitter should be able to handle these.

That's about the extent of the time I can devote to this for now though. Your best bet for getting this fixed before 8.0 is released is to make a PR.

@davidism davidism reopened this Nov 9, 2020
@davidism davidism added the f:completion feature: shell completion label Nov 9, 2020
@samschott
Copy link
Contributor Author

samschott commented Nov 9, 2020

Ok, thanks. Yes, that example was a bit of a mess.

From a quick look, split_arg_string attempts to do something similar to the quite complex shlex.split. Any reason why you are not using the latter? Bad performance?

@davidism
Copy link
Member

davidism commented Nov 9, 2020

Because shlex.split expects a complete command line string, but completion by definition needs to work with an incomplete string.

shlex.split("example my\\")
ValueError: No escaped character
shlex.split("example 'my ")
ValueError: No closing quotation

@davidism
Copy link
Member

davidism commented Nov 9, 2020

You might be able to get around that by manually iterating over a shlex.shlex object and doing some cleanup when it raises an error.

@samschott
Copy link
Contributor Author

samschott commented Nov 9, 2020

I was about to propose exactly that! Any remainder after a ValueError can be taken as an incomplete argument.

@davidism davidism reopened this Nov 10, 2020
@davidism
Copy link
Member

davidism commented Nov 10, 2020

Those two examples should be the only cases where shlex.split gets into a failed state, so I think handling those cases with shlex.shelx should be enough.

@samschott
Copy link
Contributor Author

samschott commented Nov 10, 2020

Hopefully. Also, unpaired quotes should ideally be preserved during the completion process: An input of my\ f should be completed to my\ file while an input of 'my f should be completed to 'my file'.

I'll need to think a bit about this and see if can submit a PR that fixes more than it breaks. But not tonight, it's already late here.

@davidism davidism added this to the 8.0.0 milestone Nov 10, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
f:completion feature: shell completion
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants