Skip to content

Commit dd8a33c

Browse files
jkoppelclaude
andcommitted
fix: handle file mode changes (old mode/new mode) in diff parsing
Files with `old mode`/`new mode` extended headers were not parsed correctly, causing two bugs: 1. `new mode` lines were not recognized as extended headers. The ExtendedHeader constants included `Old: 'old'` which matches `old mode ...`, but only had `NewFile: 'new file'` for new-prefixed headers. Since `'new mode ...'` does not start with `'new file'`, `parseExtendedHeader` returned null, breaking the extended header loop prematurely. 2. When `parseFileChange` returned undefined (due to unrecognized headers and no matching return conditions), `parseFileChanges` used `break` instead of `continue`, causing ALL subsequent files in the diff to be silently dropped. Fixes: - Add `NewMode: 'new mode'` to ExtendedHeader constants - Handle mode-only changes (no --- /+++ markers, no chunks) by falling back to the comparison line path - Change `parseFileChanges` to skip unparseable entries instead of aborting the entire parse Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 9740e08 commit dd8a33c

File tree

7 files changed

+182
-1
lines changed

7 files changed

+182
-1
lines changed

src/__fixtures__/mode-change

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
diff --git a/script.sh b/script.sh
2+
old mode 100644
3+
new mode 100755
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
diff --git a/script.sh b/script.sh
2+
old mode 100644
3+
new mode 100755
4+
diff --git a/readme.md b/readme.md
5+
index abc1234..def5678 100644
6+
--- a/readme.md
7+
+++ b/readme.md
8+
@@ -1,3 +1,3 @@
9+
# Title
10+
-old content
11+
+new content
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
diff --git a/script.sh b/script.sh
2+
old mode 100644
3+
new mode 100755
4+
index abc1234..def5678
5+
--- a/script.sh
6+
+++ b/script.sh
7+
@@ -1,3 +1,3 @@
8+
#!/bin/bash
9+
-echo "hello"
10+
+echo "world"
Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing
2+
3+
exports[`mode-change parse mode change followed by another file 1`] = `
4+
{
5+
"files": [
6+
{
7+
"chunks": [],
8+
"path": "script.sh",
9+
"type": "ChangedFile",
10+
},
11+
{
12+
"chunks": [
13+
{
14+
"changes": [
15+
{
16+
"content": "# Title",
17+
"lineAfter": 1,
18+
"lineBefore": 1,
19+
"type": "UnchangedLine",
20+
},
21+
{
22+
"content": "old content",
23+
"lineBefore": 2,
24+
"type": "DeletedLine",
25+
},
26+
{
27+
"content": "new content",
28+
"lineAfter": 2,
29+
"type": "AddedLine",
30+
},
31+
],
32+
"context": undefined,
33+
"fromFileRange": {
34+
"lines": 3,
35+
"start": 1,
36+
},
37+
"toFileRange": {
38+
"lines": 3,
39+
"start": 1,
40+
},
41+
"type": "Chunk",
42+
},
43+
],
44+
"path": "readme.md",
45+
"type": "ChangedFile",
46+
},
47+
],
48+
"type": "GitDiff",
49+
}
50+
`;
51+
52+
exports[`mode-change parse mode change with content changes 1`] = `
53+
{
54+
"files": [
55+
{
56+
"chunks": [
57+
{
58+
"changes": [
59+
{
60+
"content": "#!/bin/bash",
61+
"lineAfter": 1,
62+
"lineBefore": 1,
63+
"type": "UnchangedLine",
64+
},
65+
{
66+
"content": "echo "hello"",
67+
"lineBefore": 2,
68+
"type": "DeletedLine",
69+
},
70+
{
71+
"content": "echo "world"",
72+
"lineAfter": 2,
73+
"type": "AddedLine",
74+
},
75+
],
76+
"context": undefined,
77+
"fromFileRange": {
78+
"lines": 3,
79+
"start": 1,
80+
},
81+
"toFileRange": {
82+
"lines": 3,
83+
"start": 1,
84+
},
85+
"type": "Chunk",
86+
},
87+
],
88+
"path": "script.sh",
89+
"type": "ChangedFile",
90+
},
91+
],
92+
"type": "GitDiff",
93+
}
94+
`;
95+
96+
exports[`mode-change parse mode-only change 1`] = `
97+
{
98+
"files": [
99+
{
100+
"chunks": [],
101+
"path": "script.sh",
102+
"type": "ChangedFile",
103+
},
104+
],
105+
"type": "GitDiff",
106+
}
107+
`;

src/__tests__/mode-change.test.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
import { getFixture } from './test-utils';
2+
import parseGitDiff from '../parse-git-diff';
3+
4+
describe('mode-change', () => {
5+
it('parse mode-only change', () => {
6+
const fixture = getFixture('mode-change');
7+
const result = parseGitDiff(fixture);
8+
expect(result.files).toHaveLength(1);
9+
expect(result.files[0]).toMatchObject({
10+
type: 'ChangedFile',
11+
path: 'script.sh',
12+
});
13+
expect(result).toMatchSnapshot();
14+
});
15+
16+
it('parse mode change with content changes', () => {
17+
const fixture = getFixture('mode-change-with-content');
18+
const result = parseGitDiff(fixture);
19+
expect(result.files).toHaveLength(1);
20+
expect(result.files[0]).toMatchObject({
21+
type: 'ChangedFile',
22+
path: 'script.sh',
23+
});
24+
expect(result.files[0].chunks).toHaveLength(1);
25+
expect(result).toMatchSnapshot();
26+
});
27+
28+
it('parse mode change followed by another file', () => {
29+
const fixture = getFixture('mode-change-followed-by-file');
30+
const result = parseGitDiff(fixture);
31+
expect(result.files).toHaveLength(2);
32+
expect(result.files[0]).toMatchObject({
33+
type: 'ChangedFile',
34+
path: 'script.sh',
35+
});
36+
expect(result.files[1]).toMatchObject({
37+
type: 'ChangedFile',
38+
path: 'readme.md',
39+
});
40+
expect(result).toMatchSnapshot();
41+
});
42+
});

src/constants.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ export const FileType = {
1515
export const ExtendedHeader = {
1616
Index: 'index',
1717
Old: 'old',
18+
NewMode: 'new mode',
1819
Copy: 'copy',
1920
Similarity: 'similarity',
2021
Dissimilarity: 'dissimilarity',

src/parse-git-diff.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,8 @@ function parseFileChanges(ctx: Context): AnyFileChange[] {
3232
while (!ctx.isEof()) {
3333
const changed = parseFileChange(ctx);
3434
if (!changed) {
35-
break;
35+
ctx.nextLine();
36+
continue;
3637
}
3738
changedFiles.push(changed);
3839
}
@@ -123,6 +124,12 @@ function parseFileChange(ctx: Context): AnyFileChange | undefined {
123124
chunks,
124125
path: chunks[0].pathAfter,
125126
};
127+
} else if (comparisonLineParsed?.to) {
128+
return {
129+
type: FileType.Changed,
130+
chunks,
131+
path: comparisonLineParsed.to,
132+
};
126133
}
127134
return;
128135
}

0 commit comments

Comments
 (0)