-
-
Notifications
You must be signed in to change notification settings - Fork 672
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
🐛 Allow colon in zsh autocomplete values and descriptions #988
Conversation
Thanks for the PR! We'll review this and get back to you. Ideally, we'd love to also add a unit test for this (but that's not always easy with the autocompletion functionality) |
Agree, I took a look at the current completion tests, but they're tightly integrated into the tutorial, and couldn't think of a change for those that wasn't somewhat contrived. Open to guidance here as well. |
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.
I could confirm the bug in a zsh
shell.
On master
:
typer main.py run --name [TAB]
alpine -- hello:fake image: for testing
nvidia/cuda -- 10.0-devel-ubuntu18.04
Note that the options are wrongly split and the alpine:latest
option is missing because it got collapsed into alpine:hello
With this PR:
typer main.py run --name [TAB]
alpine:hello -- fake image: for testing
alpine:latest -- latest alpine image
nvidia/cuda:10.0-devel-ubuntu18.04
Which is what you would expect.
I added tests for this use-case, the zsh
ones are failing on master
and succeeding on this branch.
I'll leave the final review up to Tiangolo.
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.
env={ | ||
**os.environ, | ||
"_COLON_EXAMPLE.PY_COMPLETE": "complete_bash", | ||
"COMP_WORDS": "colon_example.py --name alpine:hell ", |
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.
I have struggled with Alpine hell too 😂
Allow for colons in zsh completion values and descriptions, related to #162
Note that because of the
startswith
check escaping the completion options in the client side is not an option, as a value that includes a:
forincomplete
in will not pass this check.Note that we need two levels of esacping, one for zsh, and one to preserve the escape for zsh
Tested by updating the
demo.py
example in the linked issue to the following:use the provided docker image/setup to test, first that the colons show as values and descriptiosn correctly
and that an incomplete value with a colon generates options