From 56017af779125b386d5a810cefcff6908d684a04 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 7 Oct 2026 19:22:22 -0700 Subject: [PATCH] Avoid repeatedly extracting source text for the copy-context hint (#26115) --- .../components/editor/selection-copy.test.ts | 30 +++++++++++++- .../src/components/editor/selection-copy.ts | 18 ++++---- .../editor/setup-contextual-copy.test.ts | 41 ++++++++++++++++--- .../editor/setup-contextual-copy.ts | 21 ++++------ 4 files changed, 84 insertions(+), 26 deletions(-) diff --git a/src/renderer/src/components/editor/selection-copy.test.ts b/src/renderer/src/components/editor/selection-copy.test.ts index edc750dbdf3..d658cfa7980 100644 --- a/src/renderer/src/components/editor/selection-copy.test.ts +++ b/src/renderer/src/components/editor/selection-copy.test.ts @@ -1,5 +1,33 @@ import { describe, expect, it } from 'vitest' -import { formatCopiedSelectionWithContext, getContextualCopyLineRange } from './selection-copy' +import { + canCopySelectionWithContext, + formatCopiedSelectionWithContext, + getContextualCopyLineRange +} from './selection-copy' + +describe('contextual copy eligibility', () => { + it('matches formatting eligibility for line boundaries and whitespace-only selections', () => { + for (const startLineNumber of [1, 2, 44]) { + for (const endLineNumber of [1, 2, 3, 44, 45]) { + for (const endColumn of [1, 2, 15]) { + const selection = { startLineNumber, startColumn: 1, endLineNumber, endColumn } + const expected = + endLineNumber !== startLineNumber && + (endColumn === 1 ? endLineNumber - 1 : endLineNumber) >= startLineNumber + expect(canCopySelectionWithContext(selection)).toBe(expected) + expect( + formatCopiedSelectionWithContext({ + relativePath: '', + language: 'plaintext', + selection, + selectedText: ' \n' + }) !== null + ).toBe(expected) + } + } + } + }) +}) describe('formatCopiedSelectionWithContext', () => { it('formats multi-line selections with file and line context', () => { diff --git a/src/renderer/src/components/editor/selection-copy.ts b/src/renderer/src/components/editor/selection-copy.ts index 689fe7bcd02..9630a09487d 100644 --- a/src/renderer/src/components/editor/selection-copy.ts +++ b/src/renderer/src/components/editor/selection-copy.ts @@ -13,15 +13,10 @@ export function formatCopiedSelectionWithContext({ selection, selectedText }: FormatCopiedSelectionArgs): string | null { + if (!canCopySelectionWithContext(selection)) { + return null + } const { startLine, endLine } = getContextualCopyLineRange(selection) - const isSingleLineSelection = selection.startLineNumber === selection.endLineNumber - if (isSingleLineSelection) { - return null - } - - if (endLine < startLine) { - return null - } const codeFenceLanguage = getCodeFenceLanguage(language) const codeBlock = selectedText.endsWith('\n') ? selectedText : `${selectedText}\n` @@ -30,6 +25,13 @@ export function formatCopiedSelectionWithContext({ return `File: ${relativePath}\n${lineLabel}\n\n\`\`\`${codeFenceLanguage}\n${codeBlock}\`\`\`` } +export function canCopySelectionWithContext(selection: IRange): boolean { + return ( + selection.startLineNumber !== selection.endLineNumber && + getInclusiveEndLine(selection) >= selection.startLineNumber + ) +} + export function getContextualCopyLineRange(selection: IRange): { startLine: number endLine: number diff --git a/src/renderer/src/components/editor/setup-contextual-copy.test.ts b/src/renderer/src/components/editor/setup-contextual-copy.test.ts index 8141a2ff3d3..d42bd0d39f5 100644 --- a/src/renderer/src/components/editor/setup-contextual-copy.test.ts +++ b/src/renderer/src/components/editor/setup-contextual-copy.test.ts @@ -29,6 +29,8 @@ vi.mock('@/lib/primary-selection', () => ({ setPrimarySelectionText: () => {} })) +vi.mock('./editor-shortcuts', () => ({ editorShortcutMatches: () => true })) + describe('setupContextualCopy', () => { afterEach(() => { vi.unstubAllGlobals() @@ -85,9 +87,16 @@ describe('setupContextualCopy', () => { expect(setInterval).not.toHaveBeenCalled() }) - it('polls a focused editor while a contextual copy hint is visible', () => { - const setInterval = vi.fn(() => 1) + it('refreshes a visible hint without reading selection text and extracts it only on copy', async () => { + let refreshHint = (): void => {} + let keydown = (_event: KeyboardEvent): void => {} + const setInterval = vi.fn((callback: () => void) => { + refreshHint = callback + return 1 + }) + const writeClipboardText = vi.fn(async (_text: string) => {}) vi.stubGlobal('window', { + api: { ui: { writeClipboardText } }, clearInterval: vi.fn(), clearTimeout: vi.fn(), setInterval, @@ -111,16 +120,24 @@ describe('setupContextualCopy', () => { getStartPosition: () => ({ lineNumber: 1, column: 1 }), getEndPosition: () => ({ lineNumber: 2, column: 4 }) } + const selectedText = `${'x'.repeat(4 * 1024 * 1024)}\ntwo` + const getValueInRange = vi.fn(() => selectedText) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: The editor stub implements every method setupContextualCopy invokes in this scenario. const editorInstance = { addContentWidget: vi.fn(), getContainerDomNode: () => ({ - addEventListener: vi.fn(), - removeEventListener: vi.fn() + addEventListener: (type: string, callback: (event: KeyboardEvent) => void) => { + if (type === 'keydown') { + keydown = callback + } + }, + removeEventListener: vi.fn(), + getBoundingClientRect: () => ({ left: 0, top: 0, width: 500 }) }), getLayoutInfo: () => ({ height: 500 }), getModel: () => ({ getLineMaxColumn: () => 4, - getValueInRange: () => 'one\ntwo' + getValueInRange }), getScrolledVisiblePosition: () => ({ top: 20, left: 8, height: 16 }), getSelection: () => selection, @@ -148,6 +165,20 @@ describe('setupContextualCopy', () => { }) expect(setInterval).toHaveBeenCalledTimes(1) + for (let tick = 0; tick < 20; tick += 1) { + refreshHint() + } + expect(getValueInRange).not.toHaveBeenCalled() + + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Shortcut matching is mocked; the handler reads only these event methods. + keydown({ preventDefault: vi.fn(), stopPropagation: vi.fn() } as unknown as KeyboardEvent) + await Promise.resolve() + expect(getValueInRange).toHaveBeenCalledTimes(1) + expect(writeClipboardText).toHaveBeenCalledWith( + `File: src/example.ts\nLines: 1-2\n\n\`\`\`ts\n${selectedText}\n\`\`\`` + ) + refreshHint() + expect(getValueInRange).toHaveBeenCalledTimes(1) }) it('clears editor-scoped contextual copy cleanup on dispose', () => { diff --git a/src/renderer/src/components/editor/setup-contextual-copy.ts b/src/renderer/src/components/editor/setup-contextual-copy.ts index b69236617b8..27458492474 100644 --- a/src/renderer/src/components/editor/setup-contextual-copy.ts +++ b/src/renderer/src/components/editor/setup-contextual-copy.ts @@ -3,7 +3,11 @@ import { formatShortcutLabel } from '@/hooks/useShortcutLabel' import { monaco } from '@/lib/monaco-setup' import { useAppStore } from '@/store' import { editorShortcutMatches } from './editor-shortcuts' -import { formatCopiedSelectionWithContext, getContextualCopyLineRange } from './selection-copy' +import { + canCopySelectionWithContext, + formatCopiedSelectionWithContext, + getContextualCopyLineRange +} from './selection-copy' import { PRIMARY_SELECTION_MAX_LENGTH, isPrimarySelectionEnabled, @@ -87,8 +91,10 @@ export function setupContextualCopy({ const updateCopyHint = (): void => { updateCopyHintLabel() - const contextualCopyText = getContextualCopyText() - if (!contextualCopyText) { + const model = editorInstance.getModel() + const selection = editorInstance.getSelection() + // Why: hint ticks need selection bounds; extracting the selected text scales with file size. + if (!model || !selection || selection.isEmpty() || !canCopySelectionWithContext(selection)) { copyHintNode.style.display = 'none' copyHintWidgetPosition = null editorInstance.layoutContentWidget(copyHintWidget) @@ -102,15 +108,6 @@ export function setupContextualCopy({ return } - const model = editorInstance.getModel() - const selection = editorInstance.getSelection() - if (!model || !selection) { - copyHintNode.style.display = 'none' - copyHintWidgetPosition = null - editorInstance.layoutContentWidget(copyHintWidget) - return - } - const { startLine, endLine } = getContextualCopyLineRange(selection) const startVisiblePosition = editorInstance.getScrolledVisiblePosition( selection.getStartPosition()