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

Sanitize command name in fish shell generated completion file #297

Merged
merged 3 commits into from
Dec 12, 2022

Conversation

riton
Copy link
Contributor

@riton riton commented Dec 12, 2022

Description of the problem

This P.R tries to fix the behavior described in poetry's Fish completions raise errors but still works issue.

Software versions

I'm running

❯ pip freeze |grep -E 'poetry|cleo'
cleo==2.0.1
poetry==1.3.1
poetry-core==1.4.0
poetry-plugin-export==1.2.0
❯ python -V
Python 3.10.6

Description of the patch

A --goodbye option was added to the spaced command in the cleo test suite. Adding this option mimics the problem described in poetry's Fish completions raise errors but still works issue.

fish shell fixture was modified to what I consider a valid solution to the problem.

Generated fish completion file before and after the patch:

before the patch

[...]
complete -c script -A -n '__fish_seen_subcommand_from 'spaced command'' -l goodbye -d ''
[...]

after the patch

[...]
complete -c script -A -n '__fish_seen_subcommand_from \'spaced command\'' -l goodbye -d ''
[...]

Modified test suite passes successfully.

❯ poetry run pytest -q tests
[...]
246 passed, 1 skipped in 5.37s

I've tested this patch to generate the poetry fish completion file and it seems to solve the problem.

* This triggers a bug in 'fish completion' script as described in https://github.com/python-poetry/poetry/issues/5929#issuecomment-1345580021
* Also adapt bash, zsh and fish sample output
* Those test should currently fail
* We should escape quotes within command with space to prevent
  ❯ complete -c poetry -A -n '__fish_seen_subcommand_from 'env remove'' -l all -d 'Remove all managed virtual environments associated with the project.'
    complete: too many arguments
* Should produce something like
  ❯ complete -c poetry -A -n '__fish_seen_subcommand_from \'env remove\'' -l all -d 'Remove all managed virtual environments associated with the project.'
@Secrus
Copy link
Member

Secrus commented Dec 12, 2022

Thanks for contributing!

@Secrus Secrus merged commit 6cc808d into python-poetry:main Dec 12, 2022
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

Successfully merging this pull request may close these issues.

2 participants