Repository navigation
patch_set: accept CRLF line endings in all patch syntax - #90
Merged
Merged
Conversation
added 3 commits
October 8, 2026 12:05
Previously, only the `---`/`+++` headers accepted CRLF line endings (#87). The git extended headers kept the `\r` in their values, so in a CRLF patch mode lines failed with `InvalidFileMode("100644\r")`, renames and copies failed with `InvalidCharInUnquotedFilename`, and mode-only changes failed with `InvalidDiffGitPath`. A `git format-patch` email with CRLF line endings also kept its commit message, because the separator was matched as `\n---\n`, so a message line starting with `diff --git` began a bogus patch. The same bug was reported in #89. With this commit, `Text::strip_line_ending` strips either `\n` or `\r\n`, and every parser that reads patch syntax uses it: the git extended headers, the `---`/`+++` filenames, the binary patch parser (which had two hand-written copies), and the email separator. A `\r` that no `\n` follows is not a line ending, as in #87. Hunk lines are unchanged: they keep their `\r` as content, which matches `git apply`, and the docs for `Line` and `PatchSet` now say so. GNU patch instead strips the `\r` from every line of a CRLF patch. Also make the gitdiff test helper check that the CRLF form of each input parses the same as the LF form, and add tests for format-patch emails, hunk content, the bytes API, and a lone `\r`.
snapbox treats `\r\n` and `\n` as equal when it compares text files, so
the compat harness could not tell whether a patched line kept its `\r`.
Add an opt-in `Case::exact_line_endings` that also compares the output
files byte for byte, against both the `out/` snapshot and the external
tool's output. The `out/` files of these cases are written by hand,
because `SNAPSHOTS=overwrite` normalizes their line endings, and
`.gitattributes` marks the `crlf_*` fixtures `-text` so that a checkout
with `core.autocrlf` cannot convert them.
Add CRLF cases against `git apply` (create, modify, rename with a mode
change, and mode-only change) and GNU patch (a CRLF patch on an LF file
and on a CRLF file). They record where diffy diverges on purpose:
* `git apply` rejects a CRLF mode-only change ("git diff header lacks
filename information"); diffy accepts it, like GNU patch.
* GNU patch strips the `\r` from every line of a CRLF patch, so it
patches LF files and rejects CRLF ones. diffy keeps the `\r`, like
`git apply`, so the results are the opposite.
The expected outputs were verified against git 2.55.0 and GNU patch 2.8.
Previously, the formatter wrote two spaces before the function context that `diff -p` and `git diff` add after the closing `@@` of a hunk header, and then a newline. The parsed function context already ends with the header's line ending, so formatting `@@ -4,6 +4,6 @@ int main(void)` produced `@@ -4,6 +4,6 @@ int main(void)` followed by a blank line. Both GNU patch and `git apply` reject that patch, because the blank line counts as an extra context line, and each round trip added another leading space. With this commit, the formatter writes a single space before the function context, matching GNU diff 3.12 and git 2.55, and adds a newline only when the function context doesn't already end in one. Formatting a parsed patch now reproduces its hunk headers byte for byte, including a `\r\n` ending. `Hunk::function_context` still includes the line ending, and its docs now say so. Also add tests for the round trip on both the str and bytes paths, a CRLF hunk header, absent and empty function contexts, a hunk header at the end of the input, and colored output.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Using #89 as inspiration to do a bit more tightening around CRLF handling.