Repository navigation
Conversation
|
The following comment was made by an LLM, it may be inaccurate: |
There was a problem hiding this comment.
Looks good. I reproduced the problem on the base in a session prompt driven with opencode-drive: NumPad Home and End (kitty CSI 57423 u / CSI 57424 u) were ignored. With this change they move the caret to the start and end of the buffer. The new test fails on the base and passes here; config-v2, keymap, config and plugin-keymap tests pass, and tsc -b is clean.
The keys are bound through the existing managed-textarea layer, so they only apply while a textarea has focus. That's the right place, and #55870 doesn't touch these commands.
Public API: no keys are added or removed. Only the default bindings change: input.buffer.home none → kphome and input.buffer.end none → kpend. This is backwards compatible: user configs that set either command still win.
One point: the new test file can be dropped so the change stays minimal (see the inline comment).
| @@ -0,0 +1,139 @@ | |||
| import { expect, test } from "bun:test" | |||
There was a problem hiding this comment.
This adds about 140 lines (a mock session server, transcript scroll checks, and a second test that rebinds the transcript jumps) for a two-value default change. The updated assertion in config-v2.test.tsx already pins the new defaults, so this file can be dropped to keep the change minimal.
f43aec3 to
05bdc4d
Compare
|
Trimmed in I kept one test instead of dropping the file, because Also rebased onto |
05bdc4d to
ecbb8d2
Compare
|
Rebased onto current Link corrected. This PR said The reported bug, reproduced on current
The red line is the issue's own "Observed" text character for character: both keypad keys are dropped and the characters land at the caret. The one assertion is
which fails with Gates on this head: The change itself is still two default values in |
There was a problem hiding this comment.
The prompt fix still works on the rebased head. In a session prompt driven with opencode-drive, NumPad Home and End are ignored on the base and move the caret to the start and end of the buffer here (before and after screenshots attached). The new test and config-v2 pass, and tsc -b is clean.
The test file was cut from two tests to one, but my earlier point was to drop it, so that thread stays open.
New: the description now says Closes #55533, but this change doesn't fix that issue. #55533 is about NumPad End doing nothing in the transcript, plus the other keypad navigation keys. I ran its reproduction against this branch: after Home, sending CSI 57424 u still leaves the transcript at the top with "Jump to latest ↓" showing, the same as on the base. That's expected, because kpend/kphome are only bound in the prompt's text box, not on session.first/session.last or any other keypad key. Please change the line to Refs #55533 so merging this doesn't close the issue while it's still broken.
Public API: no keys are added or removed. Only the default bindings change: input.buffer.home none → kphome and input.buffer.end none → kpend. This is backwards compatible: user configs that set either command still win.
ecbb8d2 to
1ae10ff
Compare
|
Both points taken, and you were right on the second one — I made that link worse, not better.
For the record on how I got it wrong: I had originally written Test file dropped in The red-before-green gate survives the drop, which is why I am comfortable removing it: with the two defaults reverted to I also left the reproduction I measured earlier in the thread — reverted defaults produce |
There was a problem hiding this comment.
Both earlier points are addressed: the extra test file is gone, and #55533 is now referenced instead of closed. I re-ran the session prompt check: NumPad Home and End move the caret to the start and end of the buffer here and are ignored on the base (screenshots attached). config-v2 passes and tsc -b is clean. No further concerns.
Public API: no keys are added or removed. Only the default bindings change: input.buffer.home none → kphome and input.buffer.end none → kpend. This is backwards compatible: user configs that set either command still win.
NumPad Home, End, PageUp and PageDown arrive as `kphome`, `kpend`, `kppageup` and `kppagedown` (kitty `CSI 57423 u`, `CSI 57424 u`, and `CSI 57421 u` / `CSI 57422 u`), while the session bindings list only the plain `home`, `end`, `pageup` and `pagedown`. The keypad key beside a working key therefore does nothing: pressing NumPad End in the transcript does not jump to the last message. List the keypad key beside the key it mirrors on `session.first`, `session.last`, `session.page.up` and `session.page.down`. Plain Home and End stay reserved for transcript navigation, so `input.buffer.home` and `input.buffer.end` keep their empty defaults.
1ae10ff to
4c1cfb9
Compare
There was a problem hiding this comment.
This fixes the transcript case in #55533. I ran the issue's reproduction with opencode-drive on the base and on this branch. On the base, NumPad End (CSI 57424 u) does nothing after scrolling to the top. Here it jumps to the latest message, NumPad Home goes back to the first message, and NumPad PageDown scrolls down a page (screenshots attached). The config-v2, config and keybind tests pass, and tsc -b prints no errors.
The test file I asked to drop earlier is gone, and the new config-v2 test is small. No further concerns.
Public API: no keys are added or removed. Four default bindings change, each gaining its keypad key:
session.first:ctrl+g,home,alt+home→ctrl+g,home,alt+home,kphomesession.last:ctrl+alt+g,end→ctrl+alt+g,end,kpendsession.page.up:pageup,ctrl+alt+b→pageup,ctrl+alt+b,kppageupsession.page.down:pagedown,ctrl+alt+f→pagedown,ctrl+alt+f,kppagedown
This is backwards compatible: user configs that set these commands still replace the defaults.
|
@kitlangton — this is ready for a human look. Merging it closes #55533, because the transcript half of that report is now covered: NumPad End jumps to the last message, NumPad Home to the first, and NumPad PageUp/PageDown scroll a page. Do you want it as-is, or is there anything you would rather see reworked before it lands? |







Issue for this PR
Closes #55533. The same bug was reported separately as #57113, already closed as a duplicate of #55533.
Type of change
What does this PR do?
Under the kitty keyboard protocol a terminal reports NumPad navigation keys as their own functional key codes.
@opentui/core0.6.8 decodes them askppageup(CSI 57421 u),kppagedown(CSI 57422 u),kphome(CSI 57423 u) andkpend(CSI 57424 u), and@opentui/keymapaccepts all four as named single-stroke keys — but no default binding claimed any of them. So the keypad key next to a working key was silently dropped: in a session, plainEndjumps to the last message and NumPadEnddoes nothing.This PR lists each keypad key beside the key it mirrors, in
packages/tui/src/config/keybind.ts:session.firstctrl+g,home,alt+homectrl+g,home,alt+home,kphomesession.lastctrl+alt+g,endctrl+alt+g,end,kpendsession.page.uppageup,ctrl+alt+bpageup,ctrl+alt+b,kppageupsession.page.downpagedown,ctrl+alt+fpagedown,ctrl+alt+f,kppagedownWhy the session navigation commands and not the prompt. These are the navigation keys whose plain counterpart already has exactly one meaning in the session view, and that meaning is transcript navigation. Plain
HomeandEndare deliberately reserved for the transcript:input.buffer.homeandinput.buffer.enddefault tonone, andconfig-v2.test.tsxpins that withreserves home and end for navigation. Binding the keypad keys to the buffer commands instead would give NumPad Home a different meaning from the plain Home key beside it, which is the split #55533 asks to be rid of. Users who want the prompt behaviour still get it from thekeybindsoverride the issue itself shows.Scope. The NumPad arrow keys are not included:
upanddownare shared by the prompt, history, dialogs and the session, so mirroring them changes far more than the reported End/Home/PageUp/PageDown case.alt+end(session.messages_last_user) is not mirrored either, since there is no natural keypad chord for it.An earlier PR of mine, #55870, proposed the same keypad mirrors; I closed it when the prompt half looked like the target. This PR is that direction, aimed at the transcript half #55533 actually reports.
How did you verify your code works?
mirrors plain navigation keys on the keypadinconfig-v2.test.tsxasserts, for each of the four commands, that the resolved bindings contain both the plain key and the keypad key. It runs againstresolve()fromsrc/config, the table the TUI loads (src/app.tsx).config.test.tsxpins the legacysrc/config/v1table, which only config migration still reads (packages/cli/src/config/migrate.ts), so it is unchanged by design.1 fail, 35 pass); with the change,config,config-v2andkeybindpass (37 pass, 0 fail).@opentui/core0.6.8 (functional-key table) and@opentui/keymap0.6.8 (named single-stroke key set), not from memory. The branch installs the versions the workspace catalog pins.tsc -bclean forpackages/tui;oxlintandprettier --checkclean on both changed files.^[[57424uwhile the binding never fires.Checklist
Added by the opencode bot (AI-generated).
Related: #56890