streampager: fix Shift keys (G, @, capitals) broken in Ghostty#1364
Open
vegerot wants to merge 1 commit into
Open
streampager: fix Shift keys (G, @, capitals) broken in Ghostty#1364vegerot wants to merge 1 commit into
vegerot wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes streampager key handling under Ghostty/libghostty when modifyOtherKeys=2 is enabled by normalizing Shift-modified Char(...) inputs so existing keymap bindings (which are stored as Char(...)+NONE) still match.
Changes:
- Normalize
KeyEventcandidates inScreen::dispatch_keyto try Shift-stripped variants forChar(...)keys. - Apply the same Shift normalization behavior in
Prompt::dispatch_keyso shifted text keys (e.g.,@, capital letters) are accepted in the prompt.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| eden/scm/lib/third-party/streampager/src/screen.rs | Adds key-candidate normalization in pager navigation dispatch to handle Ghostty’s Shift-modified text key encoding. |
| eden/scm/lib/third-party/streampager/src/prompt.rs | Extends prompt key dispatch to try Shift-stripped character variants so shifted characters can be entered. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1265
to
+1280
| fn normalize_key_candidates(key: KeyEvent) -> Vec<KeyEvent> { | ||
| use termwiz::input::{KeyCode, Modifiers}; | ||
| let mut out = Vec::with_capacity(4); | ||
| out.push(key.clone()); | ||
| if key.modifiers.contains(Modifiers::SHIFT) { | ||
| if let KeyCode::Char(c) = key.key { | ||
| let mut no_shift = key.modifiers; | ||
| no_shift.remove(Modifiers::SHIFT); | ||
| out.push(KeyEvent { key: KeyCode::Char(c), modifiers: no_shift }); | ||
| if c.is_ascii_lowercase() { | ||
| out.push(KeyEvent { key: KeyCode::Char(c.to_ascii_uppercase()), modifiers: no_shift }); | ||
| } | ||
| } | ||
| } | ||
| out | ||
| } |
Comment on lines
+374
to
+388
| let mut cands = Vec::with_capacity(4); | ||
| cands.push(key.clone()); | ||
| if key.modifiers.contains(Modifiers::SHIFT) { | ||
| if let KeyCode::Char(c) = key.key { | ||
| let mut ns = key.modifiers; | ||
| ns.remove(Modifiers::SHIFT); | ||
| cands.push(KeyEvent { key: KeyCode::Char(c), modifiers: ns }); | ||
| if c.is_ascii_lowercase() { | ||
| cands.push(KeyEvent { key: KeyCode::Char(c.to_ascii_uppercase()), modifiers: ns }); | ||
| } | ||
| } | ||
| } | ||
| for (mods, k) in cands.into_iter().map(|ke| (ke.modifiers, ke.key)) { | ||
| let value_width = width - self.prompt.width() - 4; | ||
| let action = match (mods, k.clone()) { |
|
This pull request has been imported. If you are a Meta employee, you can view this in D113135744. (Because this pull request was imported automatically, there will not be any future comments.) |
vegerot
changed the title
|
@vegerot has updated the pull request. You must reimport the pull request before landing. |
vegerot
changed the title
|
@vegerot has updated the pull request. You must reimport the pull request before landing. |
vegerot
changed the title
|
@vegerot has updated the pull request. You must reimport the pull request before landing. |
Summary:
When running under Ghostty (and cmux which uses libghostty) outside of tmux,
eden/scm's internal pager did not register any Shift-modified key.
Root cause:
- `termwiz` `UnixTerminal::set_raw_mode()` enables `modifyOtherKeys=2` (`ESC[>4;2m`).
- Ghostty's `key_encode.zig` correctly implements this: `Shift+G` is sent as
`ESC[27;2;71~` and `Shift+@` as `ESC[27;2;64~`.
- `termwiz` `InputParser` (rev `05343b3870` pinned in `Cargo.toml`) decodes these as
`Char('G')+SHIFT` and `Char('@')+SHIFT`.
- `streampager`'s keymap (`keymaps/default.rs`) stores `'G'` as `Char('G')+NONE` and
`prompt.rs` only accepted `(NONE, Char)`.
Exact modifier match failed, so `G`, `@`, and any capital letter search were
dropped.
tmux masks this because `send-keys -l` sends plain `0x47` (`G+NONE`). Raw hex
injection via tmux reproduces the bug:
```
tmux send-keys -H '1b5b32373b323b37317e' # ESC[27;2;71~ Shift+G
```
Old `sl` stays at top, new `sl` goes to bottom.
Fix:
Normalize input in `Screen::dispatch_key` and `Prompt::dispatch_key` by also
trying SHIFT-stripped variants:
- `Char(c)+SHIFT -> Char(c)+NONE`
- `g+SHIFT -> G+NONE` for case-insensitive handling
This mirrors Windows backend `normalize_shift_to_upper_case` and how kitty
legacy text keys are supposed to work. It handles both `modifyOtherKeys`
(`27;2;71~`) and simple kitty (`71;2u`) forms. The alternate form `103:71;2u`
(`report_alternates`) is still unsupported by current `termwiz` keymap and will
remain broken until `termwiz` is updated.
Test Plan:
- `make oss` - builds with `RUSTC_BOOTSTRAP=1` on stable 1.97
- tmux test:
```bash
tmux new -d -s t; tmux send-keys -t t './out/sl log -l 5000 --pager=always'
# Check plain G goes to bottom, g to top.
tmux send-keys -t t -H '1b5b32373b323b37317e' # Shift+G Ghostty modifyOtherKeys
# => now goes to bottom (was stuck at top)
tmux send-keys -t t -l '/' ; send -H '1b5b32373b323b36347e' # Shift+@ in prompt
# => now shows Search: @ (was empty)
```
Verified old `/usr/local/bin/sl` fails, new `out/sl` passes.
- `cargo test -p sapling-streampager` still 5 passed.
- Manual in Ghostty.app outside tmux: `G` jumps to bottom, `/` `@` search works,
capital letters searchable (previously broken).
Fixes facebook#1363
Co-authored-by: Opencode <opencode@anomalyco.dev>
vegerot
changed the title
|
@vegerot has updated the pull request. You must reimport the pull request before landing. |
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:
When running under Ghostty (and cmux which uses libghostty) outside of tmux,
eden/scm's internal pager did not register any Shift-modified key.
Root cause:
termwizUnixTerminal::set_raw_mode()enablesmodifyOtherKeys=2(ESC[>4;2m).key_encode.zigcorrectly implements this:Shift+Gis sent asESC[27;2;71~andShift+@asESC[27;2;64~.termwizInputParser(rev05343b3870pinned inCargo.toml) decodes these asChar('G')+SHIFTandChar('@')+SHIFT.streampager's keymap (keymaps/default.rs) stores'G'asChar('G')+NONEandprompt.rsonly accepted(NONE, Char).Exact modifier match failed, so
G,@, and any capital letter search weredropped.
tmux masks this because
send-keys -lsends plain0x47(G+NONE). Raw hexinjection via tmux reproduces the bug:
Old
slstays at top, newslgoes to bottom.Fix:
Normalize input in
Screen::dispatch_keyandPrompt::dispatch_keyby alsotrying SHIFT-stripped variants:
Char(c)+SHIFT -> Char(c)+NONEg+SHIFT -> G+NONEfor case-insensitive handlingThis mirrors Windows backend
normalize_shift_to_upper_caseand how kittylegacy text keys are supposed to work. It handles both
modifyOtherKeys(
27;2;71~) and simple kitty (71;2u) forms. The alternate form103:71;2u(
report_alternates) is still unsupported by currenttermwizkeymap and willremain broken until
termwizis updated.Test Plan:
make oss- builds withRUSTC_BOOTSTRAP=1on stable 1.97/usr/local/bin/slfails, newout/slpasses.cargo test -p sapling-streampagerstill 5 passed.Gjumps to bottom,/@search works,capital letters searchable (previously broken).
Fixes #1363
Co-authored-by: Opencode opencode@anomalyco.dev