Skip to content

Commit 760a815

Browse files
committed
refactor(cli): extract keyboard navigation from multiline-input.tsx
(Commit 2.5a) Extract keyboard handling into dedicated hooks and utilities: - use-keyboard-navigation.ts: Arrow keys, home/end, word navigation - use-keyboard-shortcuts.ts: Enter, deletion, emacs bindings - text-navigation.ts: Text boundary helpers (findLineStart, findLineEnd, etc.) - keyboard-event-utils.ts: Keyboard event helpers (isAltModifier, etc.) Reduces multiline-input.tsx from ~1,102 to ~560 lines. All 1,911 CLI tests pass. Reviewed by 4 CLI agents.
1 parent 6566a58 commit 760a815

File tree

6 files changed

+781
-574
lines changed

6 files changed

+781
-574
lines changed

REFACTORING_PLAN.md

Lines changed: 77 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ This document outlines a prioritized refactoring plan for the 51 issues identifi
1515

1616
## Progress Tracker
1717

18-
> **Last Updated:** 2025-01-20 (Commits 2.1–2.4 Complete)
19-
> **Current Status:** Ready for Commit 2.5a (multiline keyboard navigation)
18+
> **Last Updated:** 2025-01-20 (Commits 2.1–2.5a Complete)
19+
> **Current Status:** Ready for Commit 2.5b (multiline editing handlers)
2020
2121
### Phase 1 Progress
2222
| Commit | Description | Status | Completed By |
@@ -34,7 +34,7 @@ This document outlines a prioritized refactoring plan for the 51 issues identifi
3434
| 2.2 | Consolidate block utils + think tags | ✅ Complete | Codebuff |
3535
| 2.3 | Refactor loopAgentSteps | ✅ Complete | Codex CLI |
3636
| 2.4 | Consolidate billing duplication | ✅ Complete | Codex CLI |
37-
| 2.5a | Extract multiline keyboard navigation | ⬜ Not Started | - |
37+
| 2.5a | Extract multiline keyboard navigation | ✅ Complete | Codebuff |
3838
| 2.5b | Extract multiline editing handlers | ⬜ Not Started | - |
3939
| 2.6 | Simplify use-activity-query.ts | ⬜ Not Started | - |
4040
| 2.7 | Consolidate XML parsing | ⬜ Not Started | - |
@@ -395,22 +395,88 @@ During Commit 2.4 review, two pre-existing issues were identified and fixed:
395395

396396
---
397397

398-
### Commit 2.5a: Extract Multiline Input Keyboard Navigation
398+
### Commit 2.5a: Extract Multiline Input Keyboard Navigation ✅ COMPLETE
399399
**Files:** `cli/src/components/multiline-input.tsx`
400400
**Est. Time:** 3-4 hours
401-
**Est. LOC Changed:** ~500-550
401+
**Actual Time:** ~5 hours (including stale closure bug discovery and fix)
402+
**Est. LOC Changed:** ~500-550
403+
**Actual LOC Changed:** 704 insertions, 563 deletions
402404

403405
> ⚠️ **Corrected:** File is 1,102 lines, not 350-450. Split into two commits.
404406
405-
| Task | Description |
406-
|------|-------------|
407-
| Create `useKeyboardNavigation` hook | Arrow keys, home/end |
408-
| Create `useKeyboardShortcuts` hook | Ctrl+C, Ctrl+D, etc. |
409-
| Update multiline-input | Delegate navigation to hooks |
407+
| Task | Description | Status |
408+
|------|-------------|--------|
409+
| Create `useKeyboardNavigation` hook | Arrow keys, home/end, word navigation, emacs bindings ||
410+
| Create `useKeyboardShortcuts` hook | Enter, deletion, Ctrl+C, Ctrl+D, etc. ||
411+
| Create `text-navigation.ts` utilities | findLineStart, findLineEnd, word boundary helpers ||
412+
| Create `keyboard-event-utils.ts` | isAltModifier, keyboard event helpers ||
413+
| Update multiline-input | Delegate navigation to hooks ||
414+
| Fix stale closure bug | Prevent stale state in rapid keypresses ||
415+
416+
**New Files Created:**
417+
- `cli/src/hooks/use-keyboard-navigation.ts` (~210 lines) - Navigation key handling:
418+
- Arrow key navigation (up/down/left/right)
419+
- Word navigation (Alt+Left/Right, Alt+B/F)
420+
- Line navigation (Home/End, Cmd+Left/Right, Ctrl+A/E)
421+
- Document navigation (Cmd+Up/Down, Ctrl+Home/End)
422+
- Emacs bindings (Ctrl+B, Ctrl+F)
423+
- Sticky column handling for vertical navigation
424+
- `cli/src/hooks/use-keyboard-shortcuts.ts` (~280 lines) - Enter/deletion key handling:
425+
- Enter handling (plain, shift, option, backslash)
426+
- Deletion keys (backspace, delete, Ctrl+H, Ctrl+D)
427+
- Word deletion (Alt+Backspace, Ctrl+W, Alt+Delete)
428+
- Line deletion (Ctrl+U, Ctrl+K, Cmd+Delete)
429+
- `cli/src/utils/text-navigation.ts` (~50 lines) - Text boundary helpers:
430+
- `findLineStart`, `findLineEnd`
431+
- `findPreviousWordBoundary`, `findNextWordBoundary`
432+
- `cli/src/utils/keyboard-event-utils.ts` (~30 lines) - Keyboard event helpers:
433+
- `isAltModifier` (handles escape sequences for Alt key)
434+
- `isPrintableCharacterKey`
435+
436+
**Component Size Reduction:**
437+
- `multiline-input.tsx`: ~1,102 → ~560 lines (-542 lines, -49%)
438+
439+
**Stale Closure Bug Fix:**
440+
441+
During tmux testing, a critical stale closure bug was discovered:
442+
443+
| Issue | Problem | Solution |
444+
|-------|---------|----------|
445+
| **Stale state in callbacks** | Hooks captured `value` and `cursorPosition` at render time. Rapid keypresses (e.g., Left arrow then typing) used stale values | Created `stateRef` to hold current state, updated synchronously |
446+
| **React batching delay** | `onChange` updates state, but React may not re-render before next keypress | Created `onChangeWithRef` wrapper that updates `stateRef.current` immediately before calling `onChange` |
447+
448+
**Implementation Pattern:**
449+
```typescript
450+
// State ref for real-time access (avoids stale closures)
451+
const stateRef = useRef({ value, cursorPosition })
452+
stateRef.current = { value, cursorPosition }
453+
454+
// Wrapper that updates ref immediately before React state
455+
const onChangeWithRef = useCallback(
456+
(newValue: string, newCursor: number) => {
457+
stateRef.current = { value: newValue, cursorPosition: newCursor }
458+
onChange(newValue, newCursor)
459+
},
460+
[onChange],
461+
)
462+
```
463+
464+
**Test Results:**
465+
- 1,911 CLI tests pass
466+
- TypeScript compiles cleanly
467+
- Verified via tmux testing with character-by-character input
468+
469+
**Review Findings (from 4 CLI agents):**
470+
- ✅ Extraction boundaries well-chosen with clear responsibilities
471+
- ✅ Keyboard behavior exactly preserved
472+
- ✅ No dead code or unused exports
473+
- ⚠️ Optional: Move `isPrintableCharacterKey` to keyboard-event-utils.ts
474+
- ⚠️ Optional: Remove verbose JSDoc/AI slop comments
410475

411476
**Dependencies:** Commit 2.1 (use-send-message patterns)
412477
**Risk:** Medium - User input handling
413-
**Rollback:** Revert single commit
478+
**Rollback:** Revert single commit
479+
**Commit:** `fc4a66569`
414480

415481
---
416482

0 commit comments

Comments
 (0)