fix: don't treat a literal CR at EOF as a Windows line ending - #701
Conversation
`isWin` and `winToUnix` treated any patch line ending in `\r` as a Windows
line ending. But a line whose content ends in `\r` and is immediately
followed by a `\ No newline at end of file` marker is not a CRLF ending
(those are `\r\n`) - it's a literal carriage return in the final line's
content. A genuine Windows no-newline-at-EOF line never ends in `\r`.
Because of this, `isWin` misclassified such patches as Windows, and when
`applyPatch` auto-converted them to match a Unix source, `winToUnix`
stripped the `\r`, silently corrupting the output. For example
`applyPatch('line1\nline2\n', structuredPatch('f','f','line1\nline2\n',
'line1\nline2\nline3\r', undefined, undefined, {context: 0}))` returned
`line1\nline2\nline3` instead of `line1\nline2\nline3\r`.
`unixToWin` already guards against this case (it won't add `\r` to a
no-newline final line); this makes `isWin` and `winToUnix` symmetric.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>ExplodingCabbage
commented
Aug 17, 2026
It's more specific than that, no? Roughly, the patch also needs to not feature any other lines, in order for |
ExplodingCabbage
left a comment
There was a problem hiding this comment.
LGTM, thank you! Will just see if Copilot offers any useful input before I merge.
There was a problem hiding this comment.
Pull request overview
Fixes applyPatch preserving literal carriage returns at EOF during line-ending conversion.
Changes:
- Guards Windows detection/conversion around no-newline markers.
- Adds unit and regression coverage.
- Documents the fix in release notes.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/patch/line-endings.ts | Preserves literal EOF carriage returns. |
test/patch/line-endings.js | Tests conversion and detection behavior. |
test/patch/apply.js | Adds an applyPatch regression test. |
release-notes.md | Documents the bug fix. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| // A trailing '\r' before a "\ No newline at end of file" marker is a literal carriage | ||
| // return in the final line's content, not a Windows line ending, so it isn't evidence that | ||
| // the patch uses Windows line endings. | ||
| (line, i) => line.endsWith('\r') && !hunk.lines[i + 1]?.startsWith('\\') |
Co-authored-by: ExplodingCabbage <2358339+ExplodingCabbage@users.noreply.github.com>
…n CRLF source; update release notes Co-authored-by: ExplodingCabbage <2358339+ExplodingCabbage@users.noreply.github.com>
…t-literal-cr-at-eof
…on' into fix/autoconvert-literal-cr-at-eof Conflicts: release-notes.md
Bug
applyPatchcorrupts output when a Unix source is patched with a patch whose final line's content ends in a literal carriage return and has no trailing newline. The\ris silently dropped.I found this by fuzzing random source/target pairs through
structuredPatch->applyPatch; 80 of 200k pairs failed to round-trip, all of this shape.Cause
isWinandwinToUnixtreat any patch line ending in\ras a Windows CRLF line ending. But a line ending in\rthat is immediately followed by a\ No newline at end of filemarker is not a CRLF ending (Windows endings are\r\n) - it's a literal carriage return in the final line's content. A genuine Windows no-newline-at-EOF line never ends in\r.So
isWinmisclassifies such a patch as Windows, and whenapplyPatch(withautoConvertLineEndingson) converts it to match the Unix source,winToUnixstrips the\r.unixToWinalready guards this exact case (hunk.lines[i + 1]?.startsWith('\\')), so it won't add a\rto a no-newline final line. This change adds the symmetric guard toisWinandwinToUnix.Fix
Ignore a trailing
\rthat sits immediately before a\ No newline at end of filemarker when deciding whether a patch is Windows / when stripping CRs.Added tests: a
winToUnixunit test, anisWinunit test, and anapplyPatchround-trip regression test. Full suite passes with 100% coverage.