Skip to content

🐛 Allow colon in zsh autocomplete values and descriptions#988

Merged
tiangolo merged 4 commits intofastapi:masterfrom
snapbug:allow-colons-zsh-completions
Nov 7, 2024
Merged

🐛 Allow colon in zsh autocomplete values and descriptions#988
tiangolo merged 4 commits intofastapi:masterfrom
snapbug:allow-colons-zsh-completions

Conversation

@snapbug
Copy link
Copy Markdown
Contributor

@snapbug snapbug commented Sep 13, 2024

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 : for incomplete 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:

import typer

image_desc = [
    ("alpine:latest", "latest alpine image"),
    ("alpine:hello", "fake image: for testing"),
    ("nvidia/cuda:10.0-devel-ubuntu18.04", ""),
]

def _complete(incomplete: str) -> str:
    for (image, desc) in image_desc:
        if image.startswith(incomplete):
            yield image, desc

app = typer.Typer()

@app.command()
def image(name: str = typer.Option(autocompletion=_complete)):
    typer.echo(name)

if __name__ == "__main__":
    app()

use the provided docker image/setup to test, first that the colons show as values and descriptiosn correctly

0252e3324b42# typer demo.py run --name [TAB]
alpine:hello                        -- fake image: for testing
alpine:latest                       -- latest alpine image
nvidia/cuda:10.0-devel-ubuntu18.04

and that an incomplete value with a colon generates options

0252e3324b42# typer demo.py run --name alpine:[TAB]
alpine:hello   -- fake image: for testing
alpine:latest  -- latest alpine image

@svlandeg svlandeg added the bug Something isn't working label Sep 16, 2024
@svlandeg svlandeg changed the title Allow colon in zsh autocomplete values and descriptions 🐛 Allow colon in zsh autocomplete values and descriptions Sep 16, 2024
@svlandeg
Copy link
Copy Markdown
Member

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)

@svlandeg svlandeg self-assigned this Sep 16, 2024
@snapbug
Copy link
Copy Markdown
Contributor Author

snapbug commented Sep 16, 2024

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.

Copy link
Copy Markdown
Member

@svlandeg svlandeg left a 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.

@svlandeg svlandeg removed their assignment Sep 17, 2024
Copy link
Copy Markdown
Member

@tiangolo tiangolo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thank you @snapbug! 🚀

And thank you @svlandeg for the help, guidance, tests, review. 🙇

This will be available in version 0.12.6 in the next few hours. 🚀

env={
**os.environ,
"_COLON_EXAMPLE.PY_COMPLETE": "complete_bash",
"COMP_WORDS": "colon_example.py --name alpine:hell ",
Copy link
Copy Markdown
Member

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 😂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working shell / zsh

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants