fix(promptinput): keep bash-mode ! out of the local mirror (#1179)#1182
fix(promptinput): keep bash-mode ! out of the local mirror (#1179)#11820xghost42 wants to merge 1 commit into
! out of the local mirror (#1179)#1182Conversation
…#1179) Typing `!` into empty input is meant to enter bash mode and leave the prompt buffer empty (the `!` shows in the mode prefix only). The useTextInput special case at the default keystroke handler was `cursor.insert(text).left()`, which placed `!` into the cursor text with the offset at 0, then called `onChange("!")`. PromptInput's `detectModeEntry` then stripped the controlled parent value back to "" with cursor 0 — values it numerically already held. Because the parent's controlled props ended up identical to what they were before the keystroke, React did not re-render PromptInput, the useLayoutEffect in useTextInput never re-ran, and the local mirror retained `!` at offset 0. Subsequent keystrokes inserted before the retained `!`, producing "!git status" with the cursor wedged before the `!` instead of a clean "git status" buffer. Fix is in useTextInput's default handler: when the keystroke is the input-mode character at the start of an empty buffer, emit `onChange` as a one-shot mode-entry notification but return `undefined` so `setValue` is not called. The local mirror stays at "" / offset 0, the parent strip remains a no-op on the controlled state, and the next character is inserted into a clean buffer. Test: - New regression in TextInput.test.tsx that mounts a controlled TextInput with a parent `onChange` mirroring PromptInput's strip (return early with `setValue('')` when the new value starts with `!`), types `!` then `git`, and asserts the rendered frame contains `git` and does NOT contain `!`, `!git`, or `git!`. - 4/4 in TextInput.test.tsx, 24/24 across PromptInput + hooks suites, build clean.
jatmn
left a comment
There was a problem hiding this comment.
Findings
- [P2] Preserve the existing buffer when
!is typed at offset 0
src/hooks/useTextInput.ts:488
This branch now runs whenever the cursor is at offset 0, not only when the buffer is empty. If the prompt already containsgit status, the user moves the cursor to the beginning, and then types!to switch that command into bash mode,useTextInputnow callsonChange('!')instead of sending the full!git statusvalue.PromptInput.detectModeEntrythen seesprevInputLength > 0with a one-character value, does not enter bash mode, and the existing prompt is replaced with just!. That path is already documented as supported by thedetectModeEntrytest for prepending!to non-empty existing text, so please restrict the one-shot empty-mirror path to an empty buffer or otherwise preserve the full inserted value for non-empty buffers.
Code ReviewPR: #1182 — fix(promptinput): keep bash-mode Blockerssrc/hooks/useTextInput.ts:488 — The fix breaks The condition The previous reviewer (@jatmn) already flagged this — needs a Non-Blocking
Looks Good
Verdict: Changes Requested — the non-empty buffer regression is a real blocker. |
|
Please rebase before making changes, that should fix the smoke issues currently. |
|
Took an independent look — @jatmn's finding holds. The new branch in |
Summary
Closes #1179. Typing
!into empty input is meant to enter bash mode and leave the prompt buffer empty (the!only shows in the mode prefix). Before this patch, the!stayed in the buffer and the cursor was wedged before it: e.g.!git statuswith the caret at position 0 instead of an empty buffer.Root cause
useTextInput's default keystroke handler had a special case at the start of an empty buffer for the input-mode character:That produced a cursor with
text="!"andoffset=0, whichsetValue("!", 0)then committed into the local mirror (liveValueRef="!",liveOffsetRef=0) and emitted asonChange("!").PromptInput.detectModeEntrystripped the controlled parent value back to""with cursor0— values the controlled props already held. Because the parent props ended up numerically identical to what they were before the keystroke, React did not re-render PromptInput, theuseLayoutEffectinuseTextInputnever re-ran, and the local mirror kept!at offset 0.The next keystroke read
liveValueRef="!"/liveOffsetRef=0, so inserts landed before the retained!, producing!git statuswith the cursor pinned at 0.Fix
src/hooks/useTextInput.ts— at the same special case, emitonChange(text)as a one-shot mode-entry notification and returnundefinedsosetValueis not called. The local mirror stays at""/ offset 0, the parent's strip remains a no-op on the controlled state, and the next character types into a clean buffer.Test
New regression in
src/components/TextInput.test.tsx:TextInputwith a parentonChangethat mirrorsPromptInput's strip (setValue('')when the new value starts with!).!thengthenithent.git, and does NOT contain!,!git, orgit!.Before the fix the test fails (
Received: "git!"). After the fix all 4 tests in the file pass.Validation
bun test src/components/TextInput.test.tsx→ 4/4 pass.bun test src/components/PromptInput/ src/hooks/→ 24/24 pass.bun run buildclean.Test plan
!git statuson Ubuntu Linux 24.04 — buffer rendersgit status, mode prefix shows!, cursor stays at end of typed text.!ls -lainto empty input — buffer rendersls -la, mode prefix shows!. (This path goes throughdetectModeEntry's multi-char branch and was already working.)