Repository navigation
Conversation
chrikrah
left a comment
There was a problem hiding this comment.
Nothing here blocks a merge from my side, @b1ackout. Swapping the order only changes help text that Rich rewrites. Fish and PowerShell pass help to Rich without escaping it, so zsh was the only shell where escaping ran before rendering.
$ python -m pytest tests/test_completion -q # head 4eb4b89, CPython 3.12, rich installed
79 passed, 7 skipped in 48.76s
# typer/_completion_classes.py from a5882de, tests kept
$ python -m pytest tests/test_completion/test_sanitization.py -q
FAILED ...test_zsh_format_completion_renders_help_before_escaping[:warning: Spin up [bold]now[/bold]-...]
1 failed, 5 passed in 0.22s
# probe: ZshComplete.format_completion on help with $, backticks, quotes, a backslash, "1:2:3" and "\[literal]"
# all five print byte-identical output at a5882de and 4eb4b89
$ ruff check typer/_completion_classes.py tests/test_completion/test_sanitization.py # ruff 0.17.3
All checks passed!
# not run: a live zsh session, the full suite
Would one of those probe cases be worth adding to the parametrize list, so the next change to escape() has a guard for $ too?
Thanks for the review and for running the probe. I'll add a $/backtick case to the parametrize list as a guard for escape()! |
…ape() in the test
Summary
ZshComplete.format_completionProblem
Discussion: #1996
ZshComplete.format_completionescapes:to\\:(as zsh's_describerequires) before the help text is rendered by Rich.Rich then no longer recognises emoji shortcodes such as
:warning:, so the escaped text is shown literally in the zsh completion list instead of the emoji.Fix
The help text is now passed through
_sanitize_help_text()first, and the rendered result is escaped, so there is nothing left for the escaping to break.Colons in plain help text (e.g.
Note: careful) are still escaped. Bash, fish and PowerShell completion are unchanged.Tests
Added to
tests/test_completion/test_sanitization.py::warning: Spin up [bold]now[/bold]->⚠ Spin up now)The emoji case fails without the fix.
AI usage
I used Claude Code to help write the test. I reviewed the change and ran the tests myself.
Validation results:
tests/test_completionandtests/test_others.py: 107 passed, 8 skippedscripts/lint.shandscripts/test.shpassed