fix(cli): make Shift+letter actually type a capital under modifyOtherKeys - #94385
Open
MwC-Trexx wants to merge 3 commits into
Open
fix(cli): make Shift+letter actually type a capital under modifyOtherKeys#94385MwC-Trexx wants to merge 3 commits into
MwC-Trexx wants to merge 3 commits into
Conversation
…Keys NousResearch#87511 mapped ESC[27;2;<code>~ / CSI-u to the uppercase character, but prompt_toolkit still inserts event.data (the raw escape bytes) for character-valued ANSI_SEQUENCES entries. Shift+letter therefore leaks [27;2;…~] into the classic CLI on Ghostty/iTerm/WezTerm/kitty (NousResearch#87390). Narrow KeyPress.data to the character for single-char table entries via a defensive Vt100Parser._call_handler wrap. Assert on submitted prompt text (not parsed keys) so this class of bug cannot regress silently. Refs NousResearch#87511, NousResearch#87390, NousResearch#86866, NousResearch#87785.
The literal-key data patch rewrote insert_text for every single-char key. Stock Vt100Parser blanks insert_text on keys after the first in a multi-key ANSI match (Alt+letter → (Escape, "a") with "" on the letter). Reviving that blank made Shift+Alt+letter type the capital. Only rewrite when insert_text is non-empty and differs from the key. Pin with Alt / Shift+Alt regression tests.
Collaborator
Related to #87785: both repair the same prompt_toolkit self-insert path. This revision explicitly preserves blank insert_text on multi-key Alt and Shift+Alt tails; maintainers should choose or consolidate the two approaches. |
Note that Keys is a str Enum so the isinstance(key, Keys) exclusion is load-bearing, and soften the docstring so it no longer claims nothing else changes shape (multi-key blanked tails are intentionally preserved).
Author
|
Thanks for taking a look. Pushed a small follow-up on this same PR (no new PR):
Happy to coordinate with #87785 if that is preferred as the landing vehicle. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ANSI_SEQUENCESentries insert the character, not the matched escape bytesinsert_texton multi-key ANSI tails (Alt / Shift+Alt must not type the letter)Motivation
#87511 mapped
ESC[27;2;<code>~/ESC[<code>;2uto uppercase letters, but the classic CLI still leaks raw CSI into the buffer on Ghostty (and iTerm2 / WezTerm / kitty). prompt_toolkit buildsKeyPress(key=<table value>, data=<matched bytes>)and the defaultKeys.Anybinding insertsevent.data— so mapping fixed what the key is, not what it types.Verified on current
mainbefore this change: Shift+W thenhatsubmits\x1b[27;2;87~hat. After:What.Changes
hermes_cli/pt_input_extras.py—_install_literal_key_data_patch()(defensive getattr, marker on wrapper, only rewrites non-emptyinsert_text != key, called at end ofinstall_modify_other_keys_aliases())tests/cli/test_modify_other_keys_insert_text.py— asserts submitted text via realPromptSession+ pipe input; includes Alt/Shift+Alt non-insert guardsRelated
Test plan
./venv/bin/python -m pytest tests/cli/test_modify_other_keys_insert_text.py tests/cli/test_modify_other_keys_aliases.py -q— 233 passed\x1b[27;2;87~hat \x1b[27;2;72~appens→What Happens[27;2;…~leak; Shift+Enter still newline; Alt chords unchangedNotes for reviewers
Keysis astrEnum —not isinstance(key, Keys)is load-bearing even though every current Keys value is multi-char_call_handleris patched defensively (getattr + try); missing/renamed API degrades to no-op without breaking later installers