Skip to content

fix: handle file mode changes (old mode/new mode) in diff parsing#41

Merged
yeonjuan merged 3 commits intoyeonjuan:mainfrom
jkoppel:fix-mode-change-parsing
Feb 14, 2026
Merged

fix: handle file mode changes (old mode/new mode) in diff parsing#41
yeonjuan merged 3 commits intoyeonjuan:mainfrom
jkoppel:fix-mode-change-parsing

Conversation

@jkoppel
Copy link
Contributor

@jkoppel jkoppel commented Feb 11, 2026

Summary

Files with old mode/new mode extended headers (e.g. chmod +x changes) are not parsed correctly, causing two bugs:

  1. new mode lines are not recognized as extended headers. The ExtendedHeader constants include Old: 'old' which matches old mode ..., but only have NewFile: 'new file' for new-prefixed headers. Since 'new mode ...' does not start with 'new file', parseExtendedHeader returns null, breaking the extended header loop prematurely.

  2. All subsequent files are dropped. When parseFileChange returns undefined (due to unrecognized headers and no matching return conditions), parseFileChanges uses break instead of advancing past the problematic entry, causing ALL subsequent files in the diff to be silently dropped.

Reproduction

import parseGitDiff from 'parse-git-diff';

// Mode-only change
const diff1 = `diff --git a/script.sh b/script.sh
old mode 100644
new mode 100755`;

console.log(parseGitDiff(diff1).files); // [] — expected 1 file

// Mode change followed by another file
const diff2 = `diff --git a/script.sh b/script.sh
old mode 100644
new mode 100755
diff --git a/readme.md b/readme.md
index abc1234..def5678 100644
--- a/readme.md
+++ b/readme.md
@@ -1 +1 @@
-old
+new`;

console.log(parseGitDiff(diff2).files); // [] — expected 2 files

Fix

  • Add NewMode: 'new mode' to ExtendedHeader constants so new mode lines are properly recognized
  • Add a fallback return in parseFileChange for files that have only extended headers (like mode changes) but no ---/+++ markers or chunks — returns a ChangedFile with the path from the comparison input line
  • Change parseFileChanges to skip unparseable entries (continue) instead of aborting the entire parse (break), so one unparseable file doesn't cause all subsequent files to be lost

Tests

Added 3 new test cases:

  • Mode-only change (old mode 100644new mode 100755)
  • Mode change with content changes
  • Mode change followed by another file (verifies subsequent files are not dropped)

All existing tests continue to pass.

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>
@yeonjuan yeonjuan self-requested a review February 12, 2026 13:35
@yeonjuan
Copy link
Owner

yeonjuan commented Feb 13, 2026

@jkoppel Hello, thank you. This project has not been properly handling old mode and new mode. I think it would be good to output the changes in old and new mode as part of the parse results. What do you think? I created a PR to address this issue based on your commit.

@jkoppel
Copy link
Contributor Author

jkoppel commented Feb 13, 2026

Sounds like a great idea @yeonjuan !

Copy link
Owner

@yeonjuan yeonjuan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@yeonjuan yeonjuan merged commit 7d5fb6c into yeonjuan:main Feb 14, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants