diff --git a/src/renderer/src/components/GitHubItemDialog.tsx b/src/renderer/src/components/GitHubItemDialog.tsx index dfc2fe90afe..bb2e374afbd 100644 --- a/src/renderer/src/components/GitHubItemDialog.tsx +++ b/src/renderer/src/components/GitHubItemDialog.tsx @@ -76,6 +76,7 @@ import { } from '@/components/ui/dropdown-menu' import CommentMarkdown from '@/components/sidebar/CommentMarkdown' import { detectLanguage } from '@/lib/language-detect' +import { isImeCompositionKeyDown } from '@/lib/ime-composition-keyboard-event' import { cn } from '@/lib/utils' import { DiffSectionItem } from '@/components/editor/DiffSectionItem' import type { DecoratedDiffComment } from '@/components/diff-comments/useDiffCommentDecorator' @@ -1261,6 +1262,12 @@ function PRReviewersPanel({ aria-haspopup="listbox" className="h-8 min-w-0 cursor-text rounded-md border-border/50 bg-background text-xs" onKeyDown={(event) => { + // Above the dispatch: while a candidate window is open the arrows move the + // candidate, Enter commits it and Escape dismisses it — none of the three may + // also drive the reviewer picker. + if (isImeCompositionKeyDown(event)) { + return + } if (event.key === 'ArrowDown' && actionableReviewerRows.length > 0) { event.preventDefault() setActiveReviewerIndex( @@ -5940,6 +5947,10 @@ function GHEditSection({ setDuplicateError(null) }} onKeyDown={(event) => { + // An Enter that only commits a CJK candidate must not also run the search. + if (isImeCompositionKeyDown(event)) { + return + } if (event.key === 'Enter') { event.preventDefault() handleDuplicateSearchSubmit() diff --git a/src/renderer/src/components/PullRequestPage.tsx b/src/renderer/src/components/PullRequestPage.tsx index c00abfad692..1a5e95a33a2 100644 --- a/src/renderer/src/components/PullRequestPage.tsx +++ b/src/renderer/src/components/PullRequestPage.tsx @@ -69,6 +69,7 @@ import { } from '@/components/ui/dropdown-menu' import CommentMarkdown from '@/components/sidebar/CommentMarkdown' import { detectLanguage } from '@/lib/language-detect' +import { isImeCompositionKeyDown } from '@/lib/ime-composition-keyboard-event' import { cn } from '@/lib/utils' import { setWithLRU } from '@/lib/scroll-cache' import { isScreenSubmitShortcut } from '@/lib/screen-submit-shortcut' @@ -1304,6 +1305,12 @@ function PRReviewersPanel({ aria-haspopup="listbox" className="h-8 min-w-0 cursor-text rounded-md border-border/50 bg-background text-xs" onKeyDown={(event) => { + // Above the dispatch: while a candidate window is open the arrows move the + // candidate, Enter commits it and Escape dismisses it — none of the three may + // also drive the reviewer picker. + if (isImeCompositionKeyDown(event)) { + return + } if (event.key === 'ArrowDown' && actionableReviewerRows.length > 0) { event.preventDefault() setActiveReviewerIndex( @@ -5424,6 +5431,12 @@ function MentionTextarea({ }} onBlur={() => setMentionQuery(null)} onKeyDown={(event) => { + // Above the dispatch, and above the `onKeyDown` delegate below: this is the only DOM + // handler on the textarea, so refusing here is what keeps a composing Escape from + // reaching a caller that would discard the draft. + if (isImeCompositionKeyDown(event)) { + return + } if (showSuggestions) { if (event.key === 'ArrowDown') { event.preventDefault() diff --git a/src/renderer/src/lib/ime-escape-guard-ordering.test.ts b/src/renderer/src/lib/ime-escape-guard-ordering.test.ts index 69c2bb010e4..2d82d9a7988 100644 --- a/src/renderer/src/lib/ime-escape-guard-ordering.test.ts +++ b/src/renderer/src/lib/ime-escape-guard-ordering.test.ts @@ -1,5 +1,5 @@ import { readdirSync, readFileSync } from 'node:fs' -import { join } from 'node:path' +import { join, sep } from 'node:path' import { describe, expect, it } from 'vitest' // The marker sweep that found the dropped-`isComposing` adapters is blind to this defect: the @@ -13,6 +13,11 @@ import { describe, expect, it } from 'vitest' const ROOTS = ['src/renderer/src', 'src/main', 'src/shared'] const GUARD = 'isImeCompositionKeyDown' const ESCAPE = "=== 'Escape'" +const MUST_SCAN = [ + 'src/renderer/src/components/native-chat/use-native-chat-composer-keydown.ts', + 'src/renderer/src/components/new-workspace/SmartWorkspaceNameField.tsx', + 'src/renderer/src/components/settings/ShortcutRecorderButton.tsx' +] function sourceFiles(dir: string): string[] { const found: string[] = [] @@ -32,12 +37,40 @@ function sourceFiles(dir: string): string[] { return found } +// A body brace never follows `(`, `,`, `:` or `=`; those introduce a destructured parameter list, +// a JSX expression container, or an object type — all of which open with `{` and would otherwise +// be brace-matched in place of the body. Returns -1 for a handler passed by name, where the +// expression closes before any body opens. +function bodyBrace(source: string, from: number): number { + let skipped = 0 + for (let i = from; i < source.length; i++) { + if (source[i] === '{') { + let before = i - 1 + while (before >= 0 && /\s/.test(source[before])) { + before-- + } + if (!'(,:='.includes(source[before])) { + return i + } + skipped++ + continue + } + if (source[i] === '}') { + if (skipped === 0) { + return -1 + } + skipped-- + } + } + return -1 +} + // Returns the body of every keydown handler in `source`, brace-matched from its opening `{`. function keydownHandlerBodies(source: string): string[] { const bodies: string[] = [] const starts = /onKeyDown(?:Capture)?=\{|(?:function|const)\s+\w*[Kk]ey[Dd]own\w*\s*[=(]/g for (let match = starts.exec(source); match !== null; match = starts.exec(source)) { - const open = source.indexOf('{', match.index + match[0].length - 1) + const open = bodyBrace(source, match.index + match[0].length) if (open === -1) { continue } @@ -79,13 +112,19 @@ describe('composition guards sit above the Escape branch', () => { expect(offenders).toEqual([]) }) - // Guards against the check silently matching nothing — a scanner that parses no handlers - // would report zero offenders forever. - it('actually scanned guarded keydown handlers', () => { - const scanned = ROOTS.flatMap(sourceFiles).filter((file) => { - const source = readFileSync(file, 'utf8') - return source.includes(GUARD) && keydownHandlerBodies(source).some((b) => b.includes(GUARD)) - }) + // A scanner that parses nothing reports zero offenders forever, so the coverage it claims is + // asserted by name. MUST_SCAN holds the handler shapes the brace matcher has actually got wrong: + // a hook whose destructured parameter list opens before its body, an inline JSX arrow, and a + // named callback. A parser regression fails here naming the file it stopped seeing. + it('actually scanned the guarded keydown handlers it claims to cover', () => { + const scanned = ROOTS.flatMap(sourceFiles) + .filter((file) => { + const source = readFileSync(file, 'utf8') + return source.includes(GUARD) && keydownHandlerBodies(source).some((b) => b.includes(GUARD)) + }) + .map((file) => file.split(sep).join('/')) + + expect(scanned).toEqual(expect.arrayContaining(MUST_SCAN)) expect(scanned.length).toBeGreaterThan(8) }) })