From dfdbb57e0a084c56c90f764007decc68c4b03429 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Tue, 4 Aug 2026 00:28:36 -0700 Subject: [PATCH] fix(native-chat): stop diff colouring from misreading -- / ++ content lines as file headers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit diffFromText skipped every line starting with --- / +++ as a file header, so a deleted SQL/Lua '-- comment' (git emits '---') or an added '++flag' fell through to gray context with its marker still attached — and when it was the only change, the two-marker gate dropped the coloured diff entirely. Detect real headers structurally instead: an adjacent '--- ' / '+++ ' pair outside any hunk. A hunk header or 'diff --git' line now also proves the text is a diff, so a genuine single-line change renders while prose keeps the guard. Co-authored-by: Orca --- .../native-chat/native-chat-diff.test.ts | 84 +++++++++++++++++++ src/shared/native-chat-diff.ts | 55 ++++++++++-- 2 files changed, 134 insertions(+), 5 deletions(-) diff --git a/src/renderer/src/components/native-chat/native-chat-diff.test.ts b/src/renderer/src/components/native-chat/native-chat-diff.test.ts index 598b6b97d4a..99aa84c4bf1 100644 --- a/src/renderer/src/components/native-chat/native-chat-diff.test.ts +++ b/src/renderer/src/components/native-chat/native-chat-diff.test.ts @@ -60,4 +60,88 @@ describe('diffFromText', () => { expect(diff?.filter((l) => l.kind === 'add').map((l) => l.text)).toEqual(['new']) expect(diff?.filter((l) => l.kind === 'del').map((l) => l.text)).toEqual(['old']) }) + + it('treats a removed line whose content starts with -- as a deletion', () => { + const diff = diffFromText( + '@@ -1,4 +1,1 @@\n SELECT 1;\n--- legacy comment\n-SELECT 2;\n-SELECT 3;' + ) + expect(diff).toEqual([ + { kind: 'meta', text: '@@ -1,4 +1,1 @@' }, + { kind: 'context', text: ' SELECT 1;' }, + { kind: 'del', text: '-- legacy comment' }, + { kind: 'del', text: 'SELECT 2;' }, + { kind: 'del', text: 'SELECT 3;' } + ]) + }) + + it('treats an added line whose content starts with ++ as an addition', () => { + const diff = diffFromText('@@ -1,1 +1,3 @@\n ctx\n+++flag\n+normal') + expect(diff?.filter((l) => l.kind === 'add').map((l) => l.text)).toEqual(['++flag', 'normal']) + }) + + it('classifies a C-style --i deletion inside a hunk', () => { + const diff = diffFromText('@@ -1,3 +1,2 @@\n int i;\n---i;\n-return i;') + expect(diff?.filter((l) => l.kind === 'del').map((l) => l.text)).toEqual(['--i;', 'return i;']) + }) + + it('keeps real file headers as meta in full git diff output', () => { + const diff = diffFromText( + [ + 'diff --git a/a.sql b/a.sql', + 'index 9eff25c..33791bf 100644', + '--- a/a.sql', + '+++ b/a.sql', + '@@ -1,3 +1,2 @@', + ' SELECT 1;', + '--- legacy comment', + ' SELECT 2;' + ].join('\n') + ) + expect(diff?.filter((l) => l.kind === 'meta').map((l) => l.text)).toEqual([ + 'diff --git a/a.sql b/a.sql', + 'index 9eff25c..33791bf 100644', + '--- a/a.sql', + '+++ b/a.sql', + '@@ -1,3 +1,2 @@' + ]) + expect(diff?.filter((l) => l.kind === 'del').map((l) => l.text)).toEqual(['-- legacy comment']) + }) + + it('detects the second file headers in a multi-file diff', () => { + const diff = diffFromText( + [ + 'diff --git a/a.sql b/a.sql', + '--- a/a.sql', + '+++ b/a.sql', + '@@ -1,2 +1,1 @@', + '--- legacy comment', + ' SELECT 2;', + 'diff --git a/b.lua b/b.lua', + '--- a/b.lua', + '+++ b/b.lua', + '@@ -1,2 +1,1 @@', + '--- lua comment', + ' print(1)' + ].join('\n') + ) + expect(diff?.filter((l) => l.kind === 'del').map((l) => l.text)).toEqual([ + '-- legacy comment', + '-- lua comment' + ]) + }) + + it('renders a single-line change once hunk headers prove it is a diff', () => { + expect(diffFromText('@@ -1,2 +1,1 @@\n SELECT 1;\n--- legacy comment')).not.toBeNull() + // Without diff structure the two-marker prose guard still applies. + expect(diffFromText('-- legacy comment')).toBeNull() + }) + + it('handles /dev/null headers for an added file', () => { + const diff = diffFromText('--- /dev/null\n+++ b/x.sql\n+-- new comment\n+SELECT 1;') + expect(diff?.filter((l) => l.kind === 'add').map((l) => l.text)).toEqual([ + '-- new comment', + 'SELECT 1;' + ]) + expect(diff?.filter((l) => l.kind === 'del')).toEqual([]) + }) }) diff --git a/src/shared/native-chat-diff.ts b/src/shared/native-chat-diff.ts index 1e49d1e8969..775d163b133 100644 --- a/src/shared/native-chat-diff.ts +++ b/src/shared/native-chat-diff.ts @@ -13,6 +13,39 @@ const DIFF_TRUNCATED_LINE: NativeChatDiffLine = { text: '… diff truncated …' } +const HUNK_HEADER = /^@@ -\d+(?:,\d+)? \+\d+(?:,\d+)? @@/ +// Lines that open a new file section, so any hunk before them has ended. +const FILE_SECTION_START = + /^(?:diff |index |old mode |new mode |new file mode |deleted file mode |similarity index |dissimilarity index |rename |copy |Binary files )/ + +/** + * Indices of the `--- ` / `+++ ` file headers. A bare `---`/`+++` + * prefix is not enough to spot one: a removed line whose content began with `--` + * (SQL/Lua `-- comment`, C `--i`) is emitted as `---`. Real headers + * always come as an adjacent pair and never appear inside a hunk. + */ +function fileHeaderIndices(lines: string[]): Set { + const indices = new Set() + let inHunk = false + for (let index = 0; index < lines.length; index += 1) { + const line = lines[index] ?? '' + if (line.startsWith('@@')) { + inHunk = true + continue + } + if (FILE_SECTION_START.test(line)) { + inHunk = false + continue + } + if (!inHunk && line.startsWith('--- ') && (lines[index + 1] ?? '').startsWith('+++ ')) { + indices.add(index) + indices.add(index + 1) + index += 1 + } + } + return indices +} + function toLines(value: unknown, maxLines: number): { lines: string[]; truncated: boolean } { if (typeof value !== 'string') { return { lines: [], truncated: false } @@ -62,23 +95,35 @@ export function diffFromText( return null } const bounded = toLines(text, maxLines) + const headers = fileHeaderIndices(bounded.lines) let added = 0 let removed = 0 - const lines = bounded.lines.map((line): NativeChatDiffLine => { - if (line.startsWith('@@') || line.startsWith('diff ') || line.startsWith('index ')) { + const lines = bounded.lines.map((line, index): NativeChatDiffLine => { + if ( + headers.has(index) || + line.startsWith('@@') || + line.startsWith('diff ') || + line.startsWith('index ') + ) { return { kind: 'meta', text: line } } - if (line.startsWith('+') && !line.startsWith('+++')) { + if (line.startsWith('+')) { added += 1 return { kind: 'add', text: line.slice(1) } } - if (line.startsWith('-') && !line.startsWith('---')) { + if (line.startsWith('-')) { removed += 1 return { kind: 'del', text: line.slice(1) } } return { kind: 'context', text: line } }) - if (added + removed < 2) { + // A hunk header or `diff --git` proves this really is diff text, so a + // single-line change renders; without one, two markers guard against + // colouring prose that merely opens a line with `-`. + const isStructuredDiff = bounded.lines.some( + (line) => HUNK_HEADER.test(line) || line.startsWith('diff --git ') + ) + if (added + removed < (isStructuredDiff ? 1 : 2)) { return null } return bounded.truncated ? [...lines.slice(0, maxLines - 1), DIFF_TRUNCATED_LINE] : lines