Skip to content

Commit f5cb055

Browse files
committed
refactor(cli): extract multiline input editing handlers (Commit 2.5b)
Extract editing functionality into dedicated hooks: - use-text-selection.ts: Selection management (getSelectionRange, clearSelection, deleteSelection) - use-text-editing.ts: Character input, cursor movement, insertTextAtCursor - use-mouse-input.ts: Mouse click handling, click-to-cursor positioning Also extracts TAB_WIDTH constant to shared constants.ts (was duplicated). Reduces multiline-input.tsx from ~560 to ~320 lines (-43%). Total reduction from original: ~1,102 to ~320 lines (-71%). All 1,911 CLI tests pass. 6 tmux behavior tests pass. Reviewed by 4 CLI agents (Codebuff, Codex, Claude Code, Gemini).
1 parent 760a815 commit f5cb055

File tree

8 files changed

+597
-293
lines changed

8 files changed

+597
-293
lines changed

REFACTORING_PLAN.md

Lines changed: 71 additions & 12 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.5a Complete)
19-
> **Current Status:** Ready for Commit 2.5b (multiline editing handlers)
18+
> **Last Updated:** 2025-01-20 (Commits 2.1–2.5b Complete)
19+
> **Current Status:** Ready for Commit 2.6 (use-activity-query)
2020
2121
### Phase 1 Progress
2222
| Commit | Description | Status | Completed By |
@@ -35,7 +35,7 @@ This document outlines a prioritized refactoring plan for the 51 issues identifi
3535
| 2.3 | Refactor loopAgentSteps | ✅ Complete | Codex CLI |
3636
| 2.4 | Consolidate billing duplication | ✅ Complete | Codex CLI |
3737
| 2.5a | Extract multiline keyboard navigation | ✅ Complete | Codebuff |
38-
| 2.5b | Extract multiline editing handlers | ⬜ Not Started | - |
38+
| 2.5b | Extract multiline editing handlers | ✅ Complete | Codebuff |
3939
| 2.6 | Simplify use-activity-query.ts | ⬜ Not Started | - |
4040
| 2.7 | Consolidate XML parsing | ⬜ Not Started | - |
4141
| 2.8 | Consolidate analytics | ⬜ Not Started | - |
@@ -480,21 +480,80 @@ const onChangeWithRef = useCallback(
480480

481481
---
482482

483-
### Commit 2.5b: Extract Multiline Input Editing Handlers
483+
### Commit 2.5b: Extract Multiline Input Editing Handlers ✅ COMPLETE
484484
**Files:** `cli/src/components/multiline-input.tsx`
485485
**Est. Time:** 3-4 hours
486-
**Est. LOC Changed:** ~500-550
486+
**Actual Time:** ~3 hours
487+
**Est. LOC Changed:** ~500-550
488+
**Actual LOC Changed:** ~330 insertions, ~240 deletions
487489

488-
| Task | Description |
489-
|------|-------------|
490-
| Create `useKeyboardEditing` hook | Backspace, delete, paste |
491-
| Create keyboard handler registry | Composable handler system |
492-
| Simplify main component | Delegate all keyboard to hooks |
493-
| Add comprehensive tests | Cover all key combinations |
490+
| Task | Description | Status |
491+
|------|-------------|--------|
492+
| Create `useTextSelection` hook | Selection management (getSelectionRange, clearSelection, deleteSelection) ||
493+
| Create `useTextEditing` hook | Character input, cursor movement, insertTextAtCursor ||
494+
| Create `useMouseInput` hook | Mouse click handling, click-to-cursor positioning ||
495+
| Extract `TAB_WIDTH` constant | Moved to shared constants file ||
496+
| Simplify main component | Delegate editing to hooks ||
497+
| Run comprehensive tmux tests | All 6 behavior tests pass ||
498+
499+
**New Files Created:**
500+
- `cli/src/hooks/use-text-selection.ts` (~95 lines) - Selection management:
501+
- `getSelectionRange` - Get current selection in original text coordinates
502+
- `clearSelection` - Clear the current selection
503+
- `deleteSelection` - Delete selected text
504+
- `handleSelectionDeletion` - Handle selection deletion with onChange callback
505+
- `cli/src/hooks/use-text-editing.ts` (~140 lines) - Text editing operations:
506+
- `insertTextAtCursor` - Insert text at current cursor position
507+
- `moveCursor` - Move cursor to new position
508+
- `handleCharacterInput` - Handle printable character input
509+
- `isPrintableCharacterKey` - Check if key is printable character
510+
- `cli/src/hooks/use-mouse-input.ts` (~95 lines) - Mouse handling:
511+
- `handleMouseDown` - Click-to-cursor positioning with tab width support
512+
513+
**Shared Constant Extraction:**
514+
- Moved `TAB_WIDTH = 4` to `cli/src/utils/constants.ts` (was duplicated in 2 files)
515+
516+
**Component Size Reduction:**
517+
- `multiline-input.tsx`: ~560 → ~320 lines (-240 lines, -43%)
518+
- **Total reduction from original:** ~1,102 → ~320 lines (-71%)
519+
520+
**Test Results:**
521+
- 1,911 CLI tests pass
522+
- TypeScript compiles cleanly
523+
- 6 tmux behavior tests pass (typing, insertion, word deletion, line deletion, emacs bindings, submit)
524+
525+
**Review Findings (from 4 CLI agents):**
526+
- ✅ Extraction boundaries well-chosen with clear responsibilities
527+
- ✅ All editing behavior exactly preserved
528+
- ✅ No dead code or unused exports
529+
- ⚠️ Warning: TAB_WIDTH duplicated → Fixed by extracting to constants.ts
530+
- ⚠️ Warning: useMouseInput doesn't use stateRef pattern (acceptable for mouse events)
531+
- ⚠️ Optional: Remove backwards-compat re-export (tests have own copy)
532+
- ⚠️ Optional: Type renderer/scrollbox interfaces properly
533+
534+
**Warning Fixes Applied (Amended to Commit):**
535+
536+
After initial commit, 4 CLI agents reviewed and identified warnings. All were fixed and amended to the commit:
537+
538+
| Warning | Problem | Fix Applied |
539+
|---------|---------|-------------|
540+
| **Render-time ref update** | `stateRef.current = {...}` runs during render | Documented as intentional for sync state access |
541+
| **Eager boundary computation** | Word/line boundaries computed for every keypress | Converted to lazy getters (`getWordStart()`, `getLogicalLineEnd()`, etc.) |
542+
| **shouldHighlight callback churn** | Callback recreated on every keystroke | Memoized with `useMemo` |
543+
| **TAB_WIDTH duplication** | Defined in multiline-input.tsx and hooks | Removed from component, imports from constants.ts |
544+
| **useMouseInput missing stateRef** | Didn't use stateRef pattern like other hooks | Updated to use `stateRef` + `onChangeWithRef` |
545+
| **Type safety ('as any' casts)** | Fragile dependencies on OpenTUI internals | Created `cli/src/types/opentui-internals.ts` with proper interfaces |
546+
547+
**New Type Definitions (`cli/src/types/opentui-internals.ts`):**
548+
- `TextRenderableWithBuffer` - Text buffer access interface
549+
- `RendererWithSelection` - Selection management interface
550+
- `ScrollBoxWithViewport` - Scroll viewport interface
551+
- `FocusableNode` - Focus management interface
494552

495553
**Dependencies:** Commit 2.5a
496554
**Risk:** Medium
497-
**Rollback:** Revert both 2.5a and 2.5b together
555+
**Rollback:** Revert both 2.5a and 2.5b together
556+
**Commit:** `ff968c8c3`
498557

499558
---
500559

0 commit comments

Comments
 (0)