Skip to content

fix: respect Git line-ending normalization in working-tree diffs - #835

Open
mvanhorn wants to merge 1 commit into
modem-dev:mainfrom
mvanhorn:fix/818-git-line-ending-normalization
Open

fix: respect Git line-ending normalization in working-tree diffs#835
mvanhorn wants to merge 1 commit into
modem-dev:mainfrom
mvanhorn:fix/818-git-line-ending-normalization

Conversation

@mvanhorn

Copy link
Copy Markdown
Contributor

Git can keep canonical LF content in its index while materializing CRLF in the working tree according to .gitattributes, but hunk diff currently presents a one-line edit as a whole-file replacement. The same Git-produced patch renders correctly through hunk patch -, isolating the defect to the bundled Git working-tree acquisition path rather than the shared patch parser or terminal renderer. The reported reproduction is concrete and applies to the default working-tree-diff operation, including a single-revision comparison whose new endpoint is still the live worktree. Staged and commit-to-commit reviews must retain their existing byte-sensitive comparison behavior.

Summary

Use the Git endpoint resolution already owned by the bundled adapter to identify operations whose new side is the worktree, and make the patch, numstat, and watch-signature argument builders apply Git's CR-at-EOL comparison tolerance only for those operations. Keep the decision at the adapter/command boundary so patch parsing and renderer logic remain provider-neutral, and ensure the stats query and reload signature use the same comparison policy as the visible patch. Add focused builder coverage proving the flag is present only for worktree-backed inputs, plus a real-Git adapter regression that commits .gitattributes with *.ts text eol=crlf, edits one line in a CRLF checkout, and asserts the loaded patch/stats describe only that line.

Validation

  • A repository with committed *.ts text eol=crlf and a CRLF worktree file changes one line; the adapter patch contains one deletion and one addition rather than every line, and its reported stats agree.
  • Plain hunk diff and hunk diff <single-ref> apply the worktree line-ending policy because their resolved new endpoint is the live worktree.
  • hunk diff --staged and explicit commit-to-commit ranges do not receive worktree-only CR-at-EOL tolerance, preserving exact stored-content comparisons.
  • The watch signature uses the same normalized working-tree diff arguments, so an unchanged CRLF checkout does not create a reload-only full-file delta or disagree with the initially loaded review.

Fixes #818

@vercel

vercel Bot commented Aug 23, 2026

Copy link
Copy Markdown

@mvanhorn is attempting to deploy a commit to the Modem Team on Vercel.

A member of the Team first needs to authorize it.

@greptile-apps

greptile-apps Bot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

PR author is not in the allowed authors list.

@benvinegar

Copy link
Copy Markdown
Member

Blocking: worktree does not mean CR-at-EOL is ignorable for every file.

This change adds --ignore-cr-at-eol to the entire Git command whenever the new endpoint is the worktree. That flag applies globally to every path in the diff, including files where .gitattributes explicitly says line endings must not be normalized.

A concrete example is an HTTP protocol fixture, where CRLF is meaningful:

git init crlf-repro
cd crlf-repro
git config user.email test@example.com
git config user.name Test

mkdir fixtures
printf 'fixtures/*.http -text\n' > .gitattributes
printf 'GET / HTTP/1.1\r\nHost: example.com\r\n\r\n' > fixtures/request.http

git add .
git commit -m "add HTTP fixture"

# Simulate an editor incorrectly converting CRLF to LF.
printf 'GET / HTTP/1.1\nHost: example.com\n\n' > fixtures/request.http

Git correctly reports that the file changed:

$ git status --short
 M fixtures/request.http

$ git diff --numstat
3       3       fixtures/request.http

But the command introduced by this PR suppresses the change completely:

$ git diff --ignore-cr-at-eol --numstat
# no output

$ git diff --ignore-cr-at-eol
# no output

Therefore, after this PR, hunk diff produces an empty tracked patch even though:

  1. Git says the file is modified.
  2. .gitattributes explicitly disables text normalization with -text.
  3. The file's actual protocol bytes changed.

The current regression test only covers:

*.ts text eol=crlf

That is the positive normalization case. On origin/main, the test fixture already produces exactly one addition and one deletion using ordinary git diff, so the new test passes without this PR's implementation. It does not protect against the false-clean review above.

Please add the inverse test using -text and verify that both the loaded patch and watch signature remain non-empty. More importantly, the flag cannot be selected solely from endpoints.new.kind === "worktree": one repository can contain both normalized TypeScript files and byte-sensitive HTTP fixtures in the same diff. The fix needs to preserve Git's per-path attribute semantics rather than globally ignoring CR differences.

This comment was generated by Pi using gpt-5.6-sol

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.

hunk diff doesn't respect Git line-ending normalization

2 participants