From 362384112ec765afea0ea2dd32717276ecfd35f9 Mon Sep 17 00:00:00 2001 From: Jacob Richman Date: Tue, 27 Jan 2026 06:19:54 -0800 Subject: [PATCH] paste transform followup (#17624) Co-authored-by: Jack Wotherspoon --- .../src/ui/components/InputPrompt.test.tsx | 83 +++++ .../cli/src/ui/components/InputPrompt.tsx | 58 ++- .../ui/components/shared/text-buffer.test.ts | 125 +++++-- .../src/ui/components/shared/text-buffer.ts | 338 ++++++++++-------- .../shared/vim-buffer-actions.test.ts | 4 +- .../cli/src/ui/contexts/MouseContext.test.tsx | 84 +++++ packages/cli/src/ui/contexts/MouseContext.tsx | 31 ++ packages/cli/src/ui/hooks/useMouseClick.ts | 28 +- .../src/ui/hooks/useMouseDoubleClick.test.ts | 148 -------- .../cli/src/ui/hooks/useMouseDoubleClick.ts | 72 ---- packages/cli/src/ui/hooks/vim.test.tsx | 2 +- packages/cli/src/ui/utils/mouse.ts | 6 +- 12 files changed, 539 insertions(+), 440 deletions(-) delete mode 100644 packages/cli/src/ui/hooks/useMouseDoubleClick.test.ts delete mode 100644 packages/cli/src/ui/hooks/useMouseDoubleClick.ts diff --git a/packages/cli/src/ui/components/InputPrompt.test.tsx b/packages/cli/src/ui/components/InputPrompt.test.tsx index 605d0ac6cb..89b5d30eeb 100644 --- a/packages/cli/src/ui/components/InputPrompt.test.tsx +++ b/packages/cli/src/ui/components/InputPrompt.test.tsx @@ -2987,6 +2987,89 @@ describe('InputPrompt', () => { unmount(); }); + it('should collapse expanded paste on double-click after the end of the line', async () => { + const id = '[Pasted Text: 10 lines]'; + const largeText = + 'line1\nline2\nline3\nline4\nline5\nline6\nline7\nline8\nline9\nline10'; + + const baseProps = props; + const TestWrapper = () => { + const [isExpanded, setIsExpanded] = useState(true); // Start expanded + const currentLines = isExpanded ? largeText.split('\n') : [id]; + const currentText = isExpanded ? largeText : id; + + const buffer = { + ...baseProps.buffer, + text: currentText, + lines: currentLines, + viewportVisualLines: currentLines, + allVisualLines: currentLines, + pastedContent: { [id]: largeText }, + transformationsByLine: isExpanded + ? currentLines.map(() => []) + : [ + [ + { + logStart: 0, + logEnd: id.length, + logicalText: id, + collapsedText: id, + type: 'paste', + id, + }, + ], + ], + visualScrollRow: 0, + visualToLogicalMap: currentLines.map( + (_, i) => [i, 0] as [number, number], + ), + visualToTransformedMap: currentLines.map(() => 0), + getLogicalPositionFromVisual: vi.fn().mockImplementation( + (_vRow, _vCol) => + // Simulate that we are past the end of the line by returning something + // that getTransformUnderCursor won't match, or having the caller handle it. + null, + ), + togglePasteExpansion: vi.fn().mockImplementation(() => { + setIsExpanded(!isExpanded); + }), + getExpandedPasteAtLine: vi + .fn() + .mockImplementation((row) => + isExpanded && row >= 0 && row < 10 ? id : null, + ), + }; + + return ; + }; + + const { stdin, stdout, unmount, simulateClick } = renderWithProviders( + , + { + mouseEventsEnabled: true, + useAlternateBuffer: true, + uiActions, + }, + ); + + // Verify initially expanded + await waitFor(() => { + expect(stdout.lastFrame()).toContain('line1'); + }); + + // Simulate double-click WAY to the right on the first line + await simulateClick(stdin, 100, 2); + await simulateClick(stdin, 100, 2); + + // Verify it is NOW collapsed + await waitFor(() => { + expect(stdout.lastFrame()).toContain(id); + expect(stdout.lastFrame()).not.toContain('line1'); + }); + + unmount(); + }); + it('should move cursor on mouse click with plain borders', async () => { props.config.getUseBackgroundColor = () => false; props.buffer.text = 'hello world'; diff --git a/packages/cli/src/ui/components/InputPrompt.tsx b/packages/cli/src/ui/components/InputPrompt.tsx index 6baae451cb..4f1a383987 100644 --- a/packages/cli/src/ui/components/InputPrompt.tsx +++ b/packages/cli/src/ui/components/InputPrompt.tsx @@ -57,7 +57,6 @@ import { useUIState } from '../contexts/UIStateContext.js'; import { useSettings } from '../contexts/SettingsContext.js'; import { StreamingState } from '../types.js'; import { useMouseClick } from '../hooks/useMouseClick.js'; -import { useMouseDoubleClick } from '../hooks/useMouseDoubleClick.js'; import { useMouse, type MouseEvent } from '../contexts/MouseContext.js'; import { useUIActions } from '../contexts/UIActionsContext.js'; import { useAlternateBuffer } from '../hooks/useAlternateBuffer.js'; @@ -403,35 +402,54 @@ export const InputPrompt: React.FC = ({ const isAlternateBuffer = useAlternateBuffer(); // Double-click to expand/collapse paste placeholders - useMouseDoubleClick( + useMouseClick( innerBoxRef, (_event, relX, relY) => { if (!isAlternateBuffer) return; - const logicalPos = buffer.getLogicalPositionFromVisual( - buffer.visualScrollRow + relY, - relX, - ); - if (!logicalPos) return; + const visualLine = buffer.viewportVisualLines[relY]; + if (!visualLine) return; + + // Even if we click past the end of the line, we might want to collapse an expanded paste + const isPastEndOfLine = relX >= stringWidth(visualLine); + + const logicalPos = isPastEndOfLine + ? null + : buffer.getLogicalPositionFromVisual( + buffer.visualScrollRow + relY, + relX, + ); // Check for paste placeholder (collapsed state) - const transform = getTransformUnderCursor( - logicalPos.row, - logicalPos.col, - buffer.transformationsByLine, - ); - if (transform?.type === 'paste' && transform.id) { - buffer.togglePasteExpansion(transform.id); - return; + if (logicalPos) { + const transform = getTransformUnderCursor( + logicalPos.row, + logicalPos.col, + buffer.transformationsByLine, + ); + if (transform?.type === 'paste' && transform.id) { + buffer.togglePasteExpansion( + transform.id, + logicalPos.row, + logicalPos.col, + ); + return; + } } - // Check for expanded paste region - const expandedId = buffer.getExpandedPasteAtLine(logicalPos.row); + // If we didn't click a placeholder to expand, check if we are inside or after + // an expanded paste region and collapse it. + const row = buffer.visualScrollRow + relY; + const expandedId = buffer.getExpandedPasteAtLine(row); if (expandedId) { - buffer.togglePasteExpansion(expandedId); + buffer.togglePasteExpansion( + expandedId, + row, + logicalPos?.col ?? relX, // Fallback to relX if past end of line + ); } }, - { isActive: focus }, + { isActive: focus, name: 'double-click' }, ); useMouse( @@ -1228,6 +1246,8 @@ export const InputPrompt: React.FC = ({ const absoluteVisualIdx = scrollVisualRow + visualIdxInRenderedSet; const mapEntry = buffer.visualToLogicalMap[absoluteVisualIdx]; + if (!mapEntry) return null; + const cursorVisualRow = cursorVisualRowAbsolute - scrollVisualRow; const isOnCursorLine = diff --git a/packages/cli/src/ui/components/shared/text-buffer.test.ts b/packages/cli/src/ui/components/shared/text-buffer.test.ts index 6242202b76..d8352c662e 100644 --- a/packages/cli/src/ui/components/shared/text-buffer.test.ts +++ b/packages/cli/src/ui/components/shared/text-buffer.test.ts @@ -57,7 +57,7 @@ const initialState: TextBufferState = { transformationsByLine: [[]], visualLayout: defaultVisualLayout, pastedContent: {}, - expandedPasteInfo: new Map(), + expandedPaste: null, }; /** @@ -547,7 +547,7 @@ describe('textBufferReducer', () => { const action: TextBufferAction = { type: 'toggle_paste_expansion', - payload: { id: placeholder }, + payload: { id: placeholder, row: 0, col: 7 }, }; const state = textBufferReducer(stateWithPlaceholder, action); @@ -560,9 +560,10 @@ describe('textBufferReducer', () => { 'line5', 'line6 suffix', ]); - expect(state.expandedPasteInfo.has(placeholder)).toBe(true); - const info = state.expandedPasteInfo.get(placeholder); + expect(state.expandedPaste?.id).toBe(placeholder); + const info = state.expandedPaste; expect(info).toEqual({ + id: placeholder, startLine: 0, lineCount: 6, prefix: 'prefix ', @@ -586,28 +587,24 @@ describe('textBufferReducer', () => { cursorRow: 5, cursorCol: 5, pastedContent: { [placeholder]: content }, - expandedPasteInfo: new Map([ - [ - placeholder, - { - startLine: 0, - lineCount: 6, - prefix: 'prefix ', - suffix: ' suffix', - }, - ], - ]), + expandedPaste: { + id: placeholder, + startLine: 0, + lineCount: 6, + prefix: 'prefix ', + suffix: ' suffix', + }, }); const action: TextBufferAction = { type: 'toggle_paste_expansion', - payload: { id: placeholder }, + payload: { id: placeholder, row: 0, col: 7 }, }; const state = textBufferReducer(expandedState, action); expect(state.lines).toEqual(['prefix ' + placeholder + ' suffix']); - expect(state.expandedPasteInfo.has(placeholder)).toBe(false); + expect(state.expandedPaste).toBeNull(); // Cursor should be at the end of the collapsed placeholder expect(state.cursorRow).toBe(0); expect(state.cursorCol).toBe(('prefix ' + placeholder).length); @@ -625,7 +622,7 @@ describe('textBufferReducer', () => { const state = textBufferReducer(stateWithPlaceholder, { type: 'toggle_paste_expansion', - payload: { id: singleLinePlaceholder }, + payload: { id: singleLinePlaceholder, row: 0, col: 0 }, }); expect(state.lines).toEqual(['some text']); @@ -636,13 +633,13 @@ describe('textBufferReducer', () => { it('should return current state if placeholder ID not found in pastedContent', () => { const action: TextBufferAction = { type: 'toggle_paste_expansion', - payload: { id: 'unknown' }, + payload: { id: 'unknown', row: 0, col: 0 }, }; const state = textBufferReducer(initialState, action); expect(state).toBe(initialState); }); - it('should preserve expandedPasteInfo when lines change from edits outside the region', () => { + it('should preserve expandedPaste when lines change from edits outside the region', () => { // Start with an expanded paste at line 0 (3 lines long) const placeholder = '[Pasted Text: 3 lines]'; const expandedState = createStateWithTransformations({ @@ -650,20 +647,16 @@ describe('textBufferReducer', () => { cursorRow: 3, cursorCol: 0, pastedContent: { [placeholder]: 'line1\nline2\nline3' }, - expandedPasteInfo: new Map([ - [ - placeholder, - { - startLine: 0, - lineCount: 3, - prefix: '', - suffix: '', - }, - ], - ]), + expandedPaste: { + id: placeholder, + startLine: 0, + lineCount: 3, + prefix: '', + suffix: '', + }, }); - expect(expandedState.expandedPasteInfo.size).toBe(1); + expect(expandedState.expandedPaste).not.toBeNull(); // Insert a newline at the end - this changes lines but is OUTSIDE the expanded region const stateAfterInsert = textBufferReducer(expandedState, { @@ -671,9 +664,9 @@ describe('textBufferReducer', () => { payload: '\n', }); - // Lines changed, but expandedPasteInfo should be PRESERVED and optionally shifted (no shift here since edit is after) - expect(stateAfterInsert.expandedPasteInfo.size).toBe(1); - expect(stateAfterInsert.expandedPasteInfo.has(placeholder)).toBe(true); + // Lines changed, but expandedPaste should be PRESERVED and optionally shifted (no shift here since edit is after) + expect(stateAfterInsert.expandedPaste).not.toBeNull(); + expect(stateAfterInsert.expandedPaste?.id).toBe(placeholder); }); }); }); @@ -2914,9 +2907,9 @@ describe('Transformation Utilities', () => { expect(result).toEqual(transformations[0]); }); - it('should find transformation when cursor is at end', () => { + it('should NOT find transformation when cursor is at end', () => { const result = getTransformUnderCursor(0, 14, [transformations]); - expect(result).toEqual(transformations[0]); + expect(result).toBeNull(); }); it('should return null when cursor is not on a transformation', () => { @@ -2928,6 +2921,22 @@ describe('Transformation Utilities', () => { const result = getTransformUnderCursor(0, 5, []); expect(result).toBeNull(); }); + + it('regression: should not find paste transformation when clicking one character after it', () => { + const pasteId = '[Pasted Text: 5 lines]'; + const line = pasteId + ' suffix'; + const transformations = calculateTransformationsForLine(line); + const pasteTransform = transformations.find((t) => t.type === 'paste'); + expect(pasteTransform).toBeDefined(); + + const endPos = pasteTransform!.logEnd; + // Position strictly at end should be null + expect(getTransformUnderCursor(0, endPos, [transformations])).toBeNull(); + // Position inside should be found + expect(getTransformUnderCursor(0, endPos - 1, [transformations])).toEqual( + pasteTransform, + ); + }); }); describe('calculateTransformedLine', () => { @@ -3100,4 +3109,46 @@ describe('Transformation Utilities', () => { expect(result.current.allVisualLines[2]).toBe('line 3'); }); }); + + describe('Scroll Regressions', () => { + const scrollViewport: Viewport = { width: 80, height: 5 }; + + it('should not show empty viewport when collapsing a large paste that was scrolled', () => { + const largeContent = + 'line1\nline2\nline3\nline4\nline5\nline6\nline7\nline8\nline9\nline10'; + const placeholder = '[Pasted Text: 10 lines]'; + + const { result } = renderHook(() => + useTextBuffer({ + initialText: placeholder, + viewport: scrollViewport, + isValidPath: () => false, + }), + ); + + // Setup: paste large content + act(() => { + result.current.setText(''); + result.current.insert(largeContent, { paste: true }); + }); + + // Expand it + act(() => { + result.current.togglePasteExpansion(placeholder, 0, 0); + }); + + // Verify scrolled state + expect(result.current.visualScrollRow).toBe(5); + + // Collapse it + act(() => { + result.current.togglePasteExpansion(placeholder, 9, 0); + }); + + // Verify viewport is NOT empty immediately (clamping in useMemo) + expect(result.current.allVisualLines.length).toBe(1); + expect(result.current.viewportVisualLines.length).toBe(1); + expect(result.current.viewportVisualLines[0]).toBe(placeholder); + }); + }); }); diff --git a/packages/cli/src/ui/components/shared/text-buffer.ts b/packages/cli/src/ui/components/shared/text-buffer.ts index bfa02a4379..90e6b3d71a 100644 --- a/packages/cli/src/ui/components/shared/text-buffer.ts +++ b/packages/cli/src/ui/components/shared/text-buffer.ts @@ -586,7 +586,7 @@ interface UndoHistoryEntry { cursorRow: number; cursorCol: number; pastedContent: Record; - expandedPasteInfo: Map; + expandedPaste: ExpandedPasteInfo | null; } function calculateInitialCursorPosition( @@ -807,7 +807,7 @@ export function getTransformUnderCursor( const spans = spansByLine[row]; if (!spans || spans.length === 0) return null; for (const span of spans) { - if (col >= span.logStart && col <= span.logEnd) { + if (col >= span.logStart && col < span.logEnd) { return span; } if (col < span.logStart) break; @@ -816,6 +816,7 @@ export function getTransformUnderCursor( } export interface ExpandedPasteInfo { + id: string; startLine: number; lineCount: number; prefix: string; @@ -828,15 +829,14 @@ export interface ExpandedPasteInfo { */ export function getExpandedPasteAtLine( lineIndex: number, - expandedPasteInfo: Map, + expandedPaste: ExpandedPasteInfo | null, ): string | null { - for (const [id, info] of expandedPasteInfo) { - if ( - lineIndex >= info.startLine && - lineIndex < info.startLine + info.lineCount - ) { - return id; - } + if ( + expandedPaste && + lineIndex >= expandedPaste.startLine && + lineIndex < expandedPaste.startLine + expandedPaste.lineCount + ) { + return expandedPaste.id; } return null; } @@ -846,55 +846,51 @@ export function getExpandedPasteAtLine( * Adjusts startLine indices and detaches any region that is partially or fully deleted. */ export function shiftExpandedRegions( - expandedPasteInfo: Map, + expandedPaste: ExpandedPasteInfo | null, changeStartLine: number, lineDelta: number, changeEndLine?: number, // Inclusive ): { - newInfo: Map; - detachedIds: Set; + newInfo: ExpandedPasteInfo | null; + isDetached: boolean; } { - const newInfo = new Map(); - const detachedIds = new Set(); - if (expandedPasteInfo.size === 0) return { newInfo, detachedIds }; + if (!expandedPaste) return { newInfo: null, isDetached: false }; const effectiveEndLine = changeEndLine ?? changeStartLine; + const infoEndLine = expandedPaste.startLine + expandedPaste.lineCount - 1; - for (const [id, info] of expandedPasteInfo) { - const infoEndLine = info.startLine + info.lineCount - 1; + // 1. Check for overlap/intersection with the changed range + const isOverlapping = + changeStartLine <= infoEndLine && + effectiveEndLine >= expandedPaste.startLine; - // 1. Check for overlap/intersection with the changed range - const isOverlapping = - changeStartLine <= infoEndLine && effectiveEndLine >= info.startLine; - - if (isOverlapping) { - // If the change is a deletion (lineDelta < 0) that touches this region, we detach. - // If it's an insertion, we only detach if it's a multi-line insertion (lineDelta > 0) - // that isn't at the very start of the region (which would shift it). - // Regular character typing (lineDelta === 0) does NOT detach. - if ( - lineDelta < 0 || - (lineDelta > 0 && - changeStartLine > info.startLine && - changeStartLine <= infoEndLine) - ) { - detachedIds.add(id); - continue; // Detach by not adding to newInfo - } - } - - // 2. Shift regions that start at or after the change point - if (info.startLine >= changeStartLine) { - newInfo.set(id, { - ...info, - startLine: info.startLine + lineDelta, - }); - } else { - newInfo.set(id, info); + if (isOverlapping) { + // If the change is a deletion (lineDelta < 0) that touches this region, we detach. + // If it's an insertion, we only detach if it's a multi-line insertion (lineDelta > 0) + // that isn't at the very start of the region (which would shift it). + // Regular character typing (lineDelta === 0) does NOT detach. + if ( + lineDelta < 0 || + (lineDelta > 0 && + changeStartLine > expandedPaste.startLine && + changeStartLine <= infoEndLine) + ) { + return { newInfo: null, isDetached: true }; } } - return { newInfo, detachedIds }; + // 2. Shift regions that start at or after the change point + if (expandedPaste.startLine >= changeStartLine) { + return { + newInfo: { + ...expandedPaste, + startLine: expandedPaste.startLine + lineDelta, + }, + isDetached: false, + }; + } + + return { newInfo: expandedPaste, isDetached: false }; } /** @@ -905,16 +901,14 @@ export function shiftExpandedRegions( export function detachExpandedPaste(state: TextBufferState): TextBufferState { const expandedId = getExpandedPasteAtLine( state.cursorRow, - state.expandedPasteInfo, + state.expandedPaste, ); if (!expandedId) return state; - const newExpandedInfo = new Map(state.expandedPasteInfo); - newExpandedInfo.delete(expandedId); const { [expandedId]: _, ...newPastedContent } = state.pastedContent; return { ...state, - expandedPasteInfo: newExpandedInfo, + expandedPaste: null, pastedContent: newPastedContent, }; } @@ -1377,7 +1371,7 @@ export interface TextBufferState { viewportHeight: number; visualLayout: VisualLayout; pastedContent: Record; - expandedPasteInfo: Map; + expandedPaste: ExpandedPasteInfo | null; } const historyLimit = 100; @@ -1388,7 +1382,9 @@ export const pushUndo = (currentState: TextBufferState): TextBufferState => { cursorRow: currentState.cursorRow, cursorCol: currentState.cursorCol, pastedContent: { ...currentState.pastedContent }, - expandedPasteInfo: new Map(currentState.expandedPasteInfo), + expandedPaste: currentState.expandedPaste + ? { ...currentState.expandedPaste } + : null, }; const newStack = [...currentState.undoStack, snapshot]; if (newStack.length > historyLimit) { @@ -1491,7 +1487,10 @@ export type TextBufferAction = | { type: 'vim_move_to_last_line' } | { type: 'vim_move_to_line'; payload: { lineNumber: number } } | { type: 'vim_escape_insert_mode' } - | { type: 'toggle_paste_expansion'; payload: { id: string } }; + | { + type: 'toggle_paste_expansion'; + payload: { id: string; row: number; col: number }; + }; export interface TextBufferOptions { inputFilter?: (text: string) => string; @@ -1595,14 +1594,14 @@ function textBufferReducerLogic( newCursorCol = cpLen(before) + cpLen(parts[0]); } - const { newInfo: newExpandedInfo, detachedIds } = shiftExpandedRegions( - nextState.expandedPasteInfo, + const { newInfo: newExpandedPaste, isDetached } = shiftExpandedRegions( + nextState.expandedPaste, nextState.cursorRow, lineDelta, ); - for (const id of detachedIds) { - delete newPastedContent[id]; + if (isDetached && newExpandedPaste === null && nextState.expandedPaste) { + delete newPastedContent[nextState.expandedPaste.id]; } return { @@ -1612,7 +1611,7 @@ function textBufferReducerLogic( cursorCol: newCursorCol, preferredCol: null, pastedContent: newPastedContent, - expandedPasteInfo: newExpandedInfo, + expandedPaste: newExpandedPaste, }; } @@ -1700,16 +1699,16 @@ function textBufferReducerLogic( newCursorCol = newCol; } - const { newInfo: newExpandedInfo, detachedIds } = shiftExpandedRegions( - nextState.expandedPasteInfo, + const { newInfo: newExpandedPaste, isDetached } = shiftExpandedRegions( + nextState.expandedPaste, nextState.cursorRow + lineDelta, // shift based on the line that was removed lineDelta, nextState.cursorRow, ); const newPastedContent = { ...nextState.pastedContent }; - for (const id of detachedIds) { - delete newPastedContent[id]; + if (isDetached && nextState.expandedPaste) { + delete newPastedContent[nextState.expandedPaste.id]; } return { @@ -1719,7 +1718,7 @@ function textBufferReducerLogic( cursorCol: newCursorCol, preferredCol: null, pastedContent: newPastedContent, - expandedPasteInfo: newExpandedInfo, + expandedPaste: newExpandedPaste, }; } @@ -1969,16 +1968,16 @@ function textBufferReducerLogic( return currentState; } - const { newInfo: newExpandedInfo, detachedIds } = shiftExpandedRegions( - nextState.expandedPasteInfo, + const { newInfo: newExpandedPaste, isDetached } = shiftExpandedRegions( + nextState.expandedPaste, nextState.cursorRow, lineDelta, nextState.cursorRow + (lineDelta < 0 ? 1 : 0), ); const newPastedContent = { ...nextState.pastedContent }; - for (const id of detachedIds) { - delete newPastedContent[id]; + if (isDetached && nextState.expandedPaste) { + delete newPastedContent[nextState.expandedPaste.id]; } return { @@ -1986,7 +1985,7 @@ function textBufferReducerLogic( lines: newLines, preferredCol: null, pastedContent: newPastedContent, - expandedPasteInfo: newExpandedInfo, + expandedPaste: newExpandedPaste, }; } @@ -2121,7 +2120,7 @@ function textBufferReducerLogic( cursorRow: state.cursorRow, cursorCol: state.cursorCol, pastedContent: { ...state.pastedContent }, - expandedPasteInfo: new Map(state.expandedPasteInfo), + expandedPaste: state.expandedPaste ? { ...state.expandedPaste } : null, }; return { ...state, @@ -2140,7 +2139,7 @@ function textBufferReducerLogic( cursorRow: state.cursorRow, cursorCol: state.cursorCol, pastedContent: { ...state.pastedContent }, - expandedPasteInfo: new Map(state.expandedPasteInfo), + expandedPaste: state.expandedPaste ? { ...state.expandedPaste } : null, }; return { ...state, @@ -2167,22 +2166,22 @@ function textBufferReducerLogic( newState.lines.length - (nextState.lines.length - oldLineCount); const lineDelta = newLineCount - oldLineCount; - const { newInfo: newExpandedInfo, detachedIds } = shiftExpandedRegions( - nextState.expandedPasteInfo, + const { newInfo: newExpandedPaste, isDetached } = shiftExpandedRegions( + nextState.expandedPaste, startRow, lineDelta, endRow, ); const newPastedContent = { ...newState.pastedContent }; - for (const id of detachedIds) { - delete newPastedContent[id]; + if (isDetached && nextState.expandedPaste) { + delete newPastedContent[nextState.expandedPaste.id]; } return { ...newState, pastedContent: newPastedContent, - expandedPasteInfo: newExpandedInfo, + expandedPaste: newExpandedPaste, }; } @@ -2240,38 +2239,22 @@ function textBufferReducerLogic( return handleVimAction(state, action as VimAction); case 'toggle_paste_expansion': { - const { id } = action.payload; - const info = state.expandedPasteInfo.get(id); + const { id, row, col } = action.payload; + const expandedPaste = state.expandedPaste; - if (info) { + if (expandedPaste && expandedPaste.id === id) { const nextState = pushUndoLocal(state); // COLLAPSE: Restore original line with placeholder const newLines = [...nextState.lines]; newLines.splice( - info.startLine, - info.lineCount, - info.prefix + id + info.suffix, + expandedPaste.startLine, + expandedPaste.lineCount, + expandedPaste.prefix + id + expandedPaste.suffix, ); - const lineDelta = 1 - info.lineCount; - const { newInfo: newExpandedInfo, detachedIds } = shiftExpandedRegions( - nextState.expandedPasteInfo, - info.startLine, - lineDelta, - info.startLine + info.lineCount - 1, - ); - newExpandedInfo.delete(id); // Already shifted, now remove self - - const newPastedContent = { ...nextState.pastedContent }; - for (const detachedId of detachedIds) { - if (detachedId !== id) { - delete newPastedContent[detachedId]; - } - } - // Move cursor to end of collapsed placeholder - const newCursorRow = info.startLine; - const newCursorCol = cpLen(info.prefix) + cpLen(id); + const newCursorRow = expandedPaste.startLine; + const newCursorCol = cpLen(expandedPaste.prefix) + cpLen(id); return { ...nextState, @@ -2279,32 +2262,85 @@ function textBufferReducerLogic( cursorRow: newCursorRow, cursorCol: newCursorCol, preferredCol: null, - pastedContent: newPastedContent, - expandedPasteInfo: newExpandedInfo, + expandedPaste: null, }; } else { // EXPAND: Replace placeholder with content - const content = state.pastedContent[id]; - if (!content) return state; + + // Collapse any existing expanded paste first + let currentState = state; + let targetRow = row; + if (state.expandedPaste) { + const existingInfo = state.expandedPaste; + const lineDelta = 1 - existingInfo.lineCount; + + if (targetRow !== undefined && targetRow > existingInfo.startLine) { + // If we collapsed something above our target, our target row shifted up + targetRow += lineDelta; + } + + currentState = textBufferReducerLogic(state, { + type: 'toggle_paste_expansion', + payload: { + id: existingInfo.id, + row: existingInfo.startLine, + col: 0, + }, + }); + // Update transformations because they are needed for finding the next placeholder + currentState.transformationsByLine = calculateTransformations( + currentState.lines, + ); + } + + const content = currentState.pastedContent[id]; + if (!content) return currentState; // Find line and position containing exactly this placeholder let lineIndex = -1; let placeholderStart = -1; - for (let i = 0; i < state.lines.length; i++) { - const transforms = state.transformationsByLine[i] ?? []; - const transform = transforms.find( - (t) => t.type === 'paste' && t.id === id, + + const tryFindOnLine = (idx: number) => { + const transforms = currentState.transformationsByLine[idx] ?? []; + + // Precise match by col + let transform = transforms.find( + (t) => + t.type === 'paste' && + t.id === id && + col >= t.logStart && + col <= t.logEnd, ); + + if (!transform) { + // Fallback to first match on line + transform = transforms.find( + (t) => t.type === 'paste' && t.id === id, + ); + } + if (transform) { - lineIndex = i; + lineIndex = idx; placeholderStart = transform.logStart; - break; + return true; + } + return false; + }; + + // Try provided row first for precise targeting + if (targetRow >= 0 && targetRow < currentState.lines.length) { + tryFindOnLine(targetRow); + } + + if (lineIndex === -1) { + for (let i = 0; i < currentState.lines.length; i++) { + if (tryFindOnLine(i)) break; } } - if (lineIndex === -1) return state; + if (lineIndex === -1) return currentState; - const nextState = pushUndoLocal(state); + const nextState = pushUndoLocal(currentState); const line = nextState.lines[lineIndex]; const prefix = cpSlice(line, 0, placeholderStart); @@ -2329,26 +2365,6 @@ function textBufferReducerLogic( newLines.splice(lineIndex, 1, ...expandedLines); - const lineDelta = expandedLines.length - 1; - const { newInfo: newExpandedInfo, detachedIds } = shiftExpandedRegions( - nextState.expandedPasteInfo, - lineIndex, - lineDelta, - lineIndex, - ); - - const newPastedContent = { ...nextState.pastedContent }; - for (const detachedId of detachedIds) { - delete newPastedContent[detachedId]; - } - - newExpandedInfo.set(id, { - startLine: lineIndex, - lineCount: expandedLines.length, - prefix, - suffix, - }); - // Move cursor to end of expanded content (before suffix) const newCursorRow = lineIndex + expandedLines.length - 1; const lastExpandedLine = expandedLines[expandedLines.length - 1]; @@ -2360,8 +2376,13 @@ function textBufferReducerLogic( cursorRow: newCursorRow, cursorCol: newCursorCol, preferredCol: null, - pastedContent: newPastedContent, - expandedPasteInfo: newExpandedInfo, + expandedPaste: { + id, + startLine: lineIndex, + lineCount: expandedLines.length, + prefix, + suffix, + }, }; } } @@ -2468,7 +2489,7 @@ export function useTextBuffer({ viewportHeight: viewport.height, visualLayout, pastedContent: {}, - expandedPasteInfo: new Map(), + expandedPaste: null, }; }, [initialText, initialCursorOffset, viewport.width, viewport.height]); @@ -2486,7 +2507,7 @@ export function useTextBuffer({ visualLayout, transformationsByLine, pastedContent, - expandedPasteInfo, + expandedPaste, } = state; const text = useMemo(() => lines.join('\n'), [lines]); @@ -2503,7 +2524,7 @@ export function useTextBuffer({ visualToTransformedMap, } = visualLayout; - const [visualScrollRow, setVisualScrollRow] = useState(0); + const [scrollRowState, setScrollRowState] = useState(0); useEffect(() => { if (onChange) { @@ -2523,11 +2544,11 @@ export function useTextBuffer({ const { height } = viewport; const totalVisualLines = visualLines.length; const maxScrollStart = Math.max(0, totalVisualLines - height); - let newVisualScrollRow = visualScrollRow; + let newVisualScrollRow = scrollRowState; - if (visualCursor[0] < visualScrollRow) { + if (visualCursor[0] < scrollRowState) { newVisualScrollRow = visualCursor[0]; - } else if (visualCursor[0] >= visualScrollRow + height) { + } else if (visualCursor[0] >= scrollRowState + height) { newVisualScrollRow = visualCursor[0] - height + 1; } @@ -2535,10 +2556,10 @@ export function useTextBuffer({ // ensure scroll never starts beyond the last valid start so we can render a full window. newVisualScrollRow = clamp(newVisualScrollRow, 0, maxScrollStart); - if (newVisualScrollRow !== visualScrollRow) { - setVisualScrollRow(newVisualScrollRow); + if (newVisualScrollRow !== scrollRowState) { + setScrollRowState(newVisualScrollRow); } - }, [visualCursor, visualScrollRow, viewport, visualLines.length]); + }, [visualCursor, scrollRowState, viewport, visualLines.length]); const insert = useCallback( (ch: string, { paste = false }: { paste?: boolean } = {}): void => { @@ -2881,6 +2902,14 @@ export function useTextBuffer({ ], ); + const visualScrollRow = useMemo(() => { + const totalVisualLines = visualLines.length; + return Math.min( + scrollRowState, + Math.max(0, totalVisualLines - viewport.height), + ); + }, [visualLines.length, scrollRowState, viewport.height]); + const renderedVisualLines = useMemo( () => visualLines.slice(visualScrollRow, visualScrollRow + viewport.height), [visualLines, visualScrollRow, viewport.height], @@ -3049,14 +3078,17 @@ export function useTextBuffer({ [lines, cursorRow, cursorCol], ); - const togglePasteExpansion = useCallback((id: string): void => { - dispatch({ type: 'toggle_paste_expansion', payload: { id } }); - }, []); + const togglePasteExpansion = useCallback( + (id: string, row: number, col: number): void => { + dispatch({ type: 'toggle_paste_expansion', payload: { id, row, col } }); + }, + [], + ); const getExpandedPasteAtLineCallback = useCallback( (lineIndex: number): string | null => - getExpandedPasteAtLine(lineIndex, expandedPasteInfo), - [expandedPasteInfo], + getExpandedPasteAtLine(lineIndex, expandedPaste), + [expandedPaste], ); const returnValue: TextBuffer = useMemo( @@ -3093,7 +3125,7 @@ export function useTextBuffer({ getLogicalPositionFromVisual, getExpandedPasteAtLine: getExpandedPasteAtLineCallback, togglePasteExpansion, - expandedPasteInfo, + expandedPaste, deleteWordLeft, deleteWordRight, @@ -3168,7 +3200,7 @@ export function useTextBuffer({ getLogicalPositionFromVisual, getExpandedPasteAtLineCallback, togglePasteExpansion, - expandedPasteInfo, + expandedPaste, deleteWordLeft, deleteWordRight, killLineRight, @@ -3356,11 +3388,11 @@ export interface TextBuffer { * If collapsed, expands to show full content inline. * If expanded, collapses back to placeholder. */ - togglePasteExpansion(id: string): void; + togglePasteExpansion(id: string, row: number, col: number): void; /** - * The current expanded paste info map (read-only). + * The current expanded paste info (read-only). */ - expandedPasteInfo: Map; + expandedPaste: ExpandedPasteInfo | null; // Vim-specific operations /** diff --git a/packages/cli/src/ui/components/shared/vim-buffer-actions.test.ts b/packages/cli/src/ui/components/shared/vim-buffer-actions.test.ts index 5323590060..9345a805b0 100644 --- a/packages/cli/src/ui/components/shared/vim-buffer-actions.test.ts +++ b/packages/cli/src/ui/components/shared/vim-buffer-actions.test.ts @@ -35,7 +35,7 @@ const createTestState = ( transformationsByLine: [[]], visualLayout: defaultVisualLayout, pastedContent: {}, - expandedPasteInfo: new Map(), + expandedPaste: null, }); describe('vim-buffer-actions', () => { @@ -912,7 +912,7 @@ describe('vim-buffer-actions', () => { cursorRow: 0, cursorCol: 0, pastedContent: {}, - expandedPasteInfo: new Map(), + expandedPaste: null, }, ]; diff --git a/packages/cli/src/ui/contexts/MouseContext.test.tsx b/packages/cli/src/ui/contexts/MouseContext.test.tsx index a3bf76a146..2f0d9ed1ed 100644 --- a/packages/cli/src/ui/contexts/MouseContext.test.tsx +++ b/packages/cli/src/ui/contexts/MouseContext.test.tsx @@ -229,4 +229,88 @@ describe('MouseContext', () => { }, ); }); + + it('should emit a double-click event when two left-presses occur quickly at the same position', () => { + const handler = vi.fn(); + const { result } = renderHook(() => useMouseContext(), { wrapper }); + + act(() => { + result.current.subscribe(handler); + }); + + // First click + act(() => { + stdin.write('\x1b[<0;10;20M'); + }); + + expect(handler).toHaveBeenCalledTimes(1); + expect(handler).toHaveBeenLastCalledWith( + expect.objectContaining({ name: 'left-press', col: 10, row: 20 }), + ); + + // Second click (within threshold) + act(() => { + stdin.write('\x1b[<0;10;20M'); + }); + + // Should have called for the second left-press AND the double-click + expect(handler).toHaveBeenCalledTimes(3); + expect(handler).toHaveBeenCalledWith( + expect.objectContaining({ name: 'double-click', col: 10, row: 20 }), + ); + }); + + it('should NOT emit a double-click event if clicks are too far apart', () => { + const handler = vi.fn(); + const { result } = renderHook(() => useMouseContext(), { wrapper }); + + act(() => { + result.current.subscribe(handler); + }); + + // First click + act(() => { + stdin.write('\x1b[<0;10;20M'); + }); + + // Second click (too far) + act(() => { + stdin.write('\x1b[<0;15;25M'); + }); + + expect(handler).toHaveBeenCalledTimes(2); + expect(handler).not.toHaveBeenCalledWith( + expect.objectContaining({ name: 'double-click' }), + ); + }); + + it('should NOT emit a double-click event if too much time passes', async () => { + vi.useFakeTimers(); + const handler = vi.fn(); + const { result } = renderHook(() => useMouseContext(), { wrapper }); + + act(() => { + result.current.subscribe(handler); + }); + + // First click + act(() => { + stdin.write('\x1b[<0;10;20M'); + }); + + await act(async () => { + vi.advanceTimersByTime(500); // Threshold is 400ms + }); + + // Second click + act(() => { + stdin.write('\x1b[<0;10;20M'); + }); + + expect(handler).toHaveBeenCalledTimes(2); + expect(handler).not.toHaveBeenCalledWith( + expect.objectContaining({ name: 'double-click' }), + ); + vi.useRealTimers(); + }); }); diff --git a/packages/cli/src/ui/contexts/MouseContext.tsx b/packages/cli/src/ui/contexts/MouseContext.tsx index e8f723975f..d36867bdbf 100644 --- a/packages/cli/src/ui/contexts/MouseContext.tsx +++ b/packages/cli/src/ui/contexts/MouseContext.tsx @@ -22,6 +22,8 @@ import { type MouseEvent, type MouseEventName, type MouseHandler, + DOUBLE_CLICK_THRESHOLD_MS, + DOUBLE_CLICK_DISTANCE_TOLERANCE, } from '../utils/mouse.js'; export type { MouseEvent, MouseEventName, MouseHandler }; @@ -67,6 +69,11 @@ export function MouseProvider({ }) { const { stdin } = useStdin(); const subscribers = useRef>(new Set()).current; + const lastClickRef = useRef<{ + time: number; + col: number; + row: number; + } | null>(null); const subscribe = useCallback( (handler: MouseHandler) => { @@ -96,6 +103,30 @@ export function MouseProvider({ handled = true; } } + + if (event.name === 'left-press') { + const now = Date.now(); + const lastClick = lastClickRef.current; + if ( + lastClick && + now - lastClick.time < DOUBLE_CLICK_THRESHOLD_MS && + Math.abs(event.col - lastClick.col) <= + DOUBLE_CLICK_DISTANCE_TOLERANCE && + Math.abs(event.row - lastClick.row) <= DOUBLE_CLICK_DISTANCE_TOLERANCE + ) { + const doubleClickEvent: MouseEvent = { + ...event, + name: 'double-click', + }; + for (const handler of subscribers) { + handler(doubleClickEvent); + } + lastClickRef.current = null; + } else { + lastClickRef.current = { time: now, col: event.col, row: event.row }; + } + } + if ( !handled && event.name === 'move' && diff --git a/packages/cli/src/ui/hooks/useMouseClick.ts b/packages/cli/src/ui/hooks/useMouseClick.ts index 18ff9ad6f7..5fd7509470 100644 --- a/packages/cli/src/ui/hooks/useMouseClick.ts +++ b/packages/cli/src/ui/hooks/useMouseClick.ts @@ -6,18 +6,30 @@ import { getBoundingBox, type DOMElement } from 'ink'; import type React from 'react'; -import { useMouse, type MouseEvent } from '../contexts/MouseContext.js'; +import { useCallback, useRef } from 'react'; +import { + useMouse, + type MouseEvent, + type MouseEventName, +} from '../contexts/MouseContext.js'; export const useMouseClick = ( containerRef: React.RefObject, handler: (event: MouseEvent, relativeX: number, relativeY: number) => void, - options: { isActive?: boolean; button?: 'left' | 'right' } = {}, + options: { + isActive?: boolean; + button?: 'left' | 'right'; + name?: MouseEventName; + } = {}, ) => { - const { isActive = true, button = 'left' } = options; + const { isActive = true, button = 'left', name } = options; + const handlerRef = useRef(handler); + handlerRef.current = handler; - useMouse( + const onMouse = useCallback( (event: MouseEvent) => { - const eventName = button === 'left' ? 'left-press' : 'right-release'; + const eventName = + name ?? (button === 'left' ? 'left-press' : 'right-release'); if (event.name === eventName && containerRef.current) { const { x, y, width, height } = getBoundingBox(containerRef.current); // Terminal mouse events are 1-based, Ink layout is 0-based. @@ -33,10 +45,12 @@ export const useMouseClick = ( relativeY >= 0 && relativeY < height ) { - handler(event, relativeX, relativeY); + handlerRef.current(event, relativeX, relativeY); } } }, - { isActive }, + [containerRef, button, name], ); + + useMouse(onMouse, { isActive }); }; diff --git a/packages/cli/src/ui/hooks/useMouseDoubleClick.test.ts b/packages/cli/src/ui/hooks/useMouseDoubleClick.test.ts deleted file mode 100644 index 0a58acfe19..0000000000 --- a/packages/cli/src/ui/hooks/useMouseDoubleClick.test.ts +++ /dev/null @@ -1,148 +0,0 @@ -/** - * @license - * Copyright 2026 Google LLC - * SPDX-License-Identifier: Apache-2.0 - */ - -import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -import { act } from 'react'; -import { renderHook } from '../../test-utils/render.js'; -import { useMouseDoubleClick } from './useMouseDoubleClick.js'; -import * as MouseContext from '../contexts/MouseContext.js'; -import type { MouseEvent } from '../contexts/MouseContext.js'; -import type { DOMElement } from 'ink'; - -describe('useMouseDoubleClick', () => { - const mockHandler = vi.fn(); - const mockContainerRef = { - current: {} as DOMElement, - }; - - // Mock getBoundingBox from ink - vi.mock('ink', async () => { - const actual = await vi.importActual('ink'); - return { - ...actual, - getBoundingBox: () => ({ x: 0, y: 0, width: 80, height: 24 }), - }; - }); - - let mouseCallback: (event: MouseEvent) => void; - - beforeEach(() => { - vi.clearAllMocks(); - vi.useFakeTimers(); - - // Mock useMouse to capture the callback - vi.spyOn(MouseContext, 'useMouse').mockImplementation((callback) => { - mouseCallback = callback; - }); - }); - - afterEach(() => { - vi.restoreAllMocks(); - vi.useRealTimers(); - }); - - it('should detect double-click within threshold', async () => { - renderHook(() => useMouseDoubleClick(mockContainerRef, mockHandler)); - - const event1: MouseEvent = { - name: 'left-press', - col: 10, - row: 5, - shift: false, - meta: false, - ctrl: false, - button: 'left', - }; - const event2: MouseEvent = { - name: 'left-press', - col: 10, - row: 5, - shift: false, - meta: false, - ctrl: false, - button: 'left', - }; - await act(async () => { - mouseCallback(event1); - vi.advanceTimersByTime(200); - mouseCallback(event2); - }); - - expect(mockHandler).toHaveBeenCalledWith(event2, 9, 4); - }); - - it('should NOT detect double-click if time exceeds threshold', async () => { - renderHook(() => useMouseDoubleClick(mockContainerRef, mockHandler)); - - const event1: MouseEvent = { - name: 'left-press', - col: 10, - row: 5, - shift: false, - meta: false, - ctrl: false, - button: 'left', - }; - const event2: MouseEvent = { - name: 'left-press', - col: 10, - row: 5, - shift: false, - meta: false, - ctrl: false, - button: 'left', - }; - - await act(async () => { - mouseCallback(event1); - vi.advanceTimersByTime(500); // Threshold is 400ms - mouseCallback(event2); - }); - - expect(mockHandler).not.toHaveBeenCalled(); - }); - - it('should NOT detect double-click if distance exceeds tolerance', async () => { - renderHook(() => useMouseDoubleClick(mockContainerRef, mockHandler)); - - const event1: MouseEvent = { - name: 'left-press', - col: 10, - row: 5, - shift: false, - meta: false, - ctrl: false, - button: 'left', - }; - const event2: MouseEvent = { - name: 'left-press', - col: 15, - row: 10, - shift: false, - meta: false, - ctrl: false, - button: 'left', - }; - - await act(async () => { - mouseCallback(event1); - vi.advanceTimersByTime(200); - mouseCallback(event2); - }); - - expect(mockHandler).not.toHaveBeenCalled(); - }); - - it('should respect isActive option', () => { - renderHook(() => - useMouseDoubleClick(mockContainerRef, mockHandler, { isActive: false }), - ); - - expect(MouseContext.useMouse).toHaveBeenCalledWith(expect.any(Function), { - isActive: false, - }); - }); -}); diff --git a/packages/cli/src/ui/hooks/useMouseDoubleClick.ts b/packages/cli/src/ui/hooks/useMouseDoubleClick.ts deleted file mode 100644 index 46dbce52aa..0000000000 --- a/packages/cli/src/ui/hooks/useMouseDoubleClick.ts +++ /dev/null @@ -1,72 +0,0 @@ -/** - * @license - * Copyright 2026 Google LLC - * SPDX-License-Identifier: Apache-2.0 - */ - -import { getBoundingBox, type DOMElement } from 'ink'; -import type React from 'react'; -import { useRef, useCallback } from 'react'; -import { useMouse, type MouseEvent } from '../contexts/MouseContext.js'; - -const DOUBLE_CLICK_THRESHOLD_MS = 400; -const DOUBLE_CLICK_DISTANCE_TOLERANCE = 2; - -export const useMouseDoubleClick = ( - containerRef: React.RefObject, - handler: (event: MouseEvent, relativeX: number, relativeY: number) => void, - options: { isActive?: boolean } = {}, -) => { - const { isActive = true } = options; - const handlerRef = useRef(handler); - handlerRef.current = handler; - - const lastClickRef = useRef<{ - time: number; - col: number; - row: number; - } | null>(null); - - const onMouse = useCallback( - (event: MouseEvent) => { - if (event.name !== 'left-press' || !containerRef.current) return; - - const now = Date.now(); - const lastClick = lastClickRef.current; - - // Check if this is a valid double-click - if ( - lastClick && - now - lastClick.time < DOUBLE_CLICK_THRESHOLD_MS && - Math.abs(event.col - lastClick.col) <= - DOUBLE_CLICK_DISTANCE_TOLERANCE && - Math.abs(event.row - lastClick.row) <= DOUBLE_CLICK_DISTANCE_TOLERANCE - ) { - // Double-click detected - const { x, y, width, height } = getBoundingBox(containerRef.current); - // Terminal mouse events are 1-based, Ink layout is 0-based. - const mouseX = event.col - 1; - const mouseY = event.row - 1; - - const relativeX = mouseX - x; - const relativeY = mouseY - y; - - if ( - relativeX >= 0 && - relativeX < width && - relativeY >= 0 && - relativeY < height - ) { - handlerRef.current(event, relativeX, relativeY); - } - lastClickRef.current = null; // Reset after double-click - } else { - // First click, record it - lastClickRef.current = { time: now, col: event.col, row: event.row }; - } - }, - [containerRef], - ); - - useMouse(onMouse, { isActive }); -}; diff --git a/packages/cli/src/ui/hooks/vim.test.tsx b/packages/cli/src/ui/hooks/vim.test.tsx index e3f6f34bbe..739e58280d 100644 --- a/packages/cli/src/ui/hooks/vim.test.tsx +++ b/packages/cli/src/ui/hooks/vim.test.tsx @@ -68,7 +68,7 @@ const createMockTextBufferState = ( visualToTransformedMap: [], }, pastedContent: {}, - expandedPasteInfo: new Map(), + expandedPaste: null, ...partial, }; }; diff --git a/packages/cli/src/ui/utils/mouse.ts b/packages/cli/src/ui/utils/mouse.ts index 3485e5a78f..80d1f35330 100644 --- a/packages/cli/src/ui/utils/mouse.ts +++ b/packages/cli/src/ui/utils/mouse.ts @@ -24,7 +24,11 @@ export type MouseEventName = | 'scroll-down' | 'scroll-left' | 'scroll-right' - | 'move'; + | 'move' + | 'double-click'; + +export const DOUBLE_CLICK_THRESHOLD_MS = 400; +export const DOUBLE_CLICK_DISTANCE_TOLERANCE = 2; export interface MouseEvent { name: MouseEventName;