-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix(tui): keep a held Up key from crossing into prompt history #3044
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 3 commits
31c7576
72049b6
a2c8686
ec2c3e6
95f5505
087fb7f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@moonshot-ai/kimi-code": patch | ||
| --- | ||
|
|
||
| Keep a held Up key from scrolling the prompt draft into history. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| import type { AutocompleteProvider, AutocompleteSuggestions } from "../autocomplete.ts"; | ||
| import { getKeybindings } from "../keybindings.ts"; | ||
| import { decodePrintableKey, matchesKey } from "../keys.ts"; | ||
| import { decodePrintableKey, isKeyRepeat, isKittyProtocolActive, matchesKey } from "../keys.ts"; | ||
| import { KillRing } from "../kill-ring.ts"; | ||
| import { PasteBurst } from "../paste-burst.ts"; | ||
| import { type Component, CURSOR_MARKER, type Focusable, type TUI } from "../tui.ts"; | ||
|
|
@@ -25,6 +25,20 @@ const PASTE_MARKER_REGEX = /\[paste #(\d+)( (\+\d+ lines|\d+ chars))?\]/g; | |
| /** Non-global version for single-segment testing. */ | ||
| const PASTE_MARKER_SINGLE = /^\[paste #(\d+)( (\+\d+ lines|\d+ chars))?\]$/; | ||
|
|
||
| /** | ||
| * Two ↑ events arriving closer together than this are treated as a held key | ||
| * (key repeat) rather than discrete presses. Terminals emit held-key repeats | ||
| * every ~25-40ms; humans rarely re-press faster than ~100ms. | ||
| */ | ||
| const UP_ARROW_REPEAT_THRESHOLD_MS = 100; | ||
|
|
||
| /** | ||
| * Upper bound of a keyboard's initial repeat delay (how long a key must be | ||
| * held before autorepeat starts) that the history snap-back accounts for. | ||
| * X11 defaults to 660ms; macOS and Windows repeat delays top out around 1s. | ||
| */ | ||
| const UP_ARROW_INITIAL_DELAY_MAX_MS = 1200; | ||
|
|
||
| /** Check if a segment is a paste marker (i.e. was merged by segmentWithMarkers). */ | ||
| function isPasteMarker(segment: string): boolean { | ||
| return segment.length >= 10 && PASTE_MARKER_SINGLE.test(segment); | ||
|
|
@@ -344,6 +358,14 @@ export class Editor implements Component, Focusable { | |
| private historyDraft: EditorState | null = null; | ||
| private hostHistoryDraft: unknown = undefined; | ||
| private historyFilter: ((entry: string) => boolean) | null = null; | ||
| /** Timestamp of the previous ↑ key event, for held-key repeat detection. */ | ||
| private lastUpArrowAt = 0; | ||
| /** | ||
| * Set when ↑ crosses from the draft into history soon after a previous ↑: | ||
| * the crossing may still prove to be a held key's first autorepeat (its | ||
| * initial delay outran the repeat threshold), arming the snap-back. | ||
| */ | ||
| private pendingHeldUpCrossingAt = 0; | ||
|
|
||
| // Kill ring for Emacs-style kill/yank operations | ||
| private killRing = new KillRing(); | ||
|
|
@@ -484,6 +506,25 @@ export class Editor implements Component, Focusable { | |
| return currentVisualLine === visualLines.length - 1; | ||
| } | ||
|
|
||
| /** | ||
| * Classify this ↑ event: a held-key repeat when the terminal reported a | ||
| * repeat event (Kitty keyboard protocol) or — without that protocol — | ||
| * when it arrived faster after the previous ↑ than a human re-presses. | ||
| * A user holding ↑ to reach the top of a long draft expects to stop | ||
| * there, so repeats must not carry the editor from the draft into | ||
| * history browsing; once history was entered by a discrete press, | ||
| * repeats may keep browsing. | ||
| */ | ||
| private upArrowRepeatInfo(data: string): { now: number; gap: number; repeat: boolean } { | ||
| const now = Date.now(); | ||
| const gap = now - this.lastUpArrowAt; | ||
| this.lastUpArrowAt = now; | ||
| const repeat = isKittyProtocolActive() | ||
| ? isKeyRepeat(data) | ||
| : gap < UP_ARROW_REPEAT_THRESHOLD_MS; | ||
|
bj456736 marked this conversation as resolved.
bj456736 marked this conversation as resolved.
Comment on lines
+522
to
+524
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a terminal negotiates only a subset of the Kitty protocol flags, such as flag 1 without flag 2, AGENTS.md reference: packages/pi-tui/AGENTS.md:L17-L17 Useful? React with 👍 / 👎. |
||
| return { now, gap, repeat }; | ||
| } | ||
|
|
||
| private navigateHistory(direction: 1 | -1): void { | ||
| this.lastAction = null; | ||
| if (this.history.length === 0) return; | ||
|
|
@@ -946,10 +987,38 @@ export class Editor implements Component, Focusable { | |
|
|
||
| // Arrow key navigation (with history support) | ||
| if (kb.matches(data, "tui.editor.cursorUp")) { | ||
| const { now, gap, repeat } = this.upArrowRepeatInfo(data); | ||
|
|
||
| // Snap back: a repeat-classified ↑ right after a recent crossing | ||
| // proves the "discrete" press that crossed was really a held key's | ||
| // first autorepeat — the keyboard's initial repeat delay outran | ||
| // the repeat threshold. Return to the draft and stay there. | ||
| if ( | ||
| repeat && | ||
| this.pendingHeldUpCrossingAt > 0 && | ||
| now - this.pendingHeldUpCrossingAt < UP_ARROW_INITIAL_DELAY_MAX_MS && | ||
| this.historyIndex > -1 | ||
| ) { | ||
| this.pendingHeldUpCrossingAt = 0; | ||
| this.navigateHistory(1); | ||
| return; | ||
| } | ||
|
|
||
| if ( | ||
| this.isOnFirstVisualLine() && | ||
| (this.isEditorEmpty() || this.historyIndex > -1 || this.state.cursorCol === 0) | ||
| (this.isEditorEmpty() || this.historyIndex > -1 || this.state.cursorCol === 0) && | ||
| // A held ↑ must not cross from the draft into history; a discrete | ||
| // press still enters, and once browsing, repeats keep browsing. | ||
| !(repeat && this.historyIndex === -1 && this.history.length > 0) | ||
| ) { | ||
| if (this.historyIndex === -1) { | ||
| // A crossing soon after a previous ↑ may still prove to be a | ||
| // held key's first autorepeat — arm the snap-back above. | ||
| this.pendingHeldUpCrossingAt = gap < UP_ARROW_INITIAL_DELAY_MAX_MS ? now : 0; | ||
|
bj456736 marked this conversation as resolved.
Outdated
|
||
| } else { | ||
| // Browsing past the first entry is deliberate navigation. | ||
| this.pendingHeldUpCrossingAt = 0; | ||
| } | ||
| this.navigateHistory(-1); | ||
| } else if (this.isOnFirstVisualLine()) { | ||
| // Already at top - jump to start of line | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.