Repository navigation
Read an empty line in a patch hunk as an empty context line - #18
Conversation
`diff -u --suppress-blank-empty`, git's diff.suppressBlankEmpty and editors or mail clients that strip trailing whitespace write an empty context line as an empty line. GNU patch and git apply read it as context; patch fell through to its error branch and returned Nothing for the whole patch. Inside a hunk, an empty line now counts as a context line with empty text. The "" that split-by leaves after the patch text's final newline is still not a line, so a hunk truncated at the end of the patch stays Nothing. Header counts still decide where a hunk ends, and the no-newline marker checks apply to an empty context line as to any other.
There was a problem hiding this comment.
Build & Tests
Built at 48bf528 on the Pi with the local compiler and the 10-02 compat core, each build in its own output directory. CI is green on ubuntu and macOS.
- Head: 128/0. Master's own suite: 124/0.
- Head's tests against master's
diff.carp: 125/3. The three failures are the first three new rows. The truncated-hunk row passes on master. - Without the last-element guard (
(if (String.empty? raw) " " raw)): 127/1, and the one failure is the truncated-hunk row. Both counts match the body. - The body's example returns
Just "A\n\nb\n"on #16's parent (0650fc1),Nothingon master andJust "A\n\nb\n"on head, as stated.
Reading patch (diff.carp:438-526):
- Outside a hunk, only an
@@line (:491) or a---/+++pair (:509) does anything. Every other line is skipped,""and" "included. So the new binding at :456 changes nothing outside a hunk, and the header counts still decide where a hunk ends. marked(:457) still reads the next raw line, so #17's marker checks are unchanged. An original without a final newline always has a non-empty last line. So an empty context line can never be the no-newline line, and a marker after one givesNothing, as the body says.(< (inc i) n)is false only for the final split element.
Independent oracle. I generated 300 random pairs:
- 0-40 lines drawn from
""," ", a tab anda-e, with runs of 2-4 empty lines; - 15% without a final newline;
- 1-4 scattered edits, so many patches have several hunks.
Each pair was rendered three ways: by GNU diff and by git diff at -U0/1/2/3/5/10 with blank-empty suppression, and by plain diff -U3 with trailing whitespace stripped from every line, as an editor would save it. Each patch then went into up to six wrappers:
---/+++headers;- hunks only;
diff --git+indexheaders;- a
format-patchemail (commit message with blank lines,---and a diffstat, and a--signature); - three trailing blank lines;
- the final newline removed.
That gives 12,000 patches, each checked against GNU patch 2.7.6 -F0 and git 2.39.5 apply. A second seed (120 pairs, 4,800 patches) falls into the same categories below, with its own counts (6 offset cases instead of 15, for example).
- No in-hunk empty line (1,952 patches): head is byte-identical to master on all of them.
- In-hunk empty line, text intact (7,796): GNU patch gives the new file on all 7,796, and git on all 5,010 it can read (the hunks-only texts have no header for git). Head gives the new file on all 7,796. Master gives
Nothingon all of them. - Trailing whitespace stripped (900): every
Justhead returns equals GNU's result, and git's where git applies. Head returnsNothingon 15 (5 pairs, each in three wrappers) where GNU applies the hunk at an offset of 3-16 lines.patchnever searches offsets, on master either, and master returnsNothingon those 15 too. - Final newline removed (1,500): GNU rejects 1,051 of these and git 1,421. Head reads each one exactly like the same text with its newline (1,056/1,056), except where the last line was an empty context line. There it returns
Nothingon 363 patches whose newline-terminated twin applies, which is the truncated-hunk rule. GNU fills the line in on all 363, and git rejects all 363. Master already reads a text without its final newline the same way. - Every difference from master (8,851 patches): master returned
Nothing. Where the text kept its final newline (8,015), head's result equals GNU's every time, and git's wherever git can read the text. The rest are texts without their final newline, as above. - Teeth: the
""→"+"mutant differs from head on 8,851 patches. Dropping the guard changes exactly the 363 truncated results, each to GNU's result.
Hand probes (18, against both tools):
- Head newly applies 5 that master rejects. Each result equals GNU's and git's:
- empty context right after the
@@line; - an over-long count whose extra line is an empty original line;
- extra blank lines after a hunk that ends in empty context;
- empty context on both sides of a change;
- empty context as the file's last line.
- empty context right after the
- A CRLF patch whose blank context line was saved as a bare CR is rejected by head, master, GNU and git alike, since the line is
"\r", not"". An intact CRLF patch applies everywhere. So CR handling doesn't move. - A blank line between a finished hunk and the next header is outside a hunk. Head and master both apply that patch correctly.
Mutants, each through head's suite:
- No last-element guard: killed by the truncated-hunk row (127/1).
(< i n)for(< (inc i) n): killed (127/1). It is always true inside the loop, so it is the same mutant as no guard.""read as"+", or as"-": each killed by the first three rows (125/3).- A guard that skips the last two elements (
(< (+ i 2) n)): killed by the first row only (127/1). ""read as" "only inside a hunk: survives (128/0). This mutant is equivalent, since outside a hunk" "and""are both skipped. It is byte-identical to head on all 16,800 oracle patches.
Findings
No blocking bugs, and nothing optional. I looked for a wrong Just and found none. Everywhere head differs from master, master returned Nothing. Where such a text kept its final newline, head now returns what GNU patch returns, and what git apply returns wherever git can read it. The truncated-hunk choice (Nothing, like git, where GNU fills the line in) and the empty-context-then-marker case (Nothing, like GNU, where git applies) are both stated in the body.
Verdict: merge
The 3-line change does what the body says. On 16,800 generated patches it changes only results that were Nothing on master, and every changed result on a text that keeps its final newline is the file GNU patch gives. The four rows pin it.
diff -u --suppress-blank-empty, git'sdiff.suppressBlankEmpty, and editors or mail clients that strip trailing whitespace all write an empty context line as an empty line instead of a single space. GNU patch andgit applyread that line as context.Diff.patch's in-hunkcondonly had branches for,-,+and\, so an empty line hit(set! failed true)and the whole patch came backNothing. For example,(patch "a\n\nb\n" "@@ -1,2 +1,2 @@\n-a\n+A\n\n")returnedJust "A\n\nb\n"before #16 and has returnedNothingsince. This was open item 2 from the #16 review.Change
When
patchreads a line that is empty and is not the last element of the split patch text, it treats it as" ". Everything else is unchanged:""and" "are both skipped, so file headers and the lines between hunks behave as before.Nothing. The""thatsplit-byleaves after the patch text's final newline is never read as context.\ No newline at end of filemarker after an empty context line isNothing, as it is after any context line it contradicts.I re-derived the rule on current master, after #17, rather than porting the review's patch.
Tests
Four rows in
tests/diff.carp:NothingThe first three fail on master (125/3). The truncated row passes on both, and it is the only row that fails when the "not the last element" guard is dropped (127/1). Suite on the branch: 128/0 (armhf, local compat core).
carp-fmt -candanglerare clean.Oracle
I generated 1,500 random pairs: lines drawn from
{"", "", "", a, b, c}, 0–12 lines, 80% newline-terminated, 60% of new sides mutated from the old side. Each pair was rendered by GNUdiff -U0/-U1/-U3 --suppress-blank-emptyand bygit -c diff.suppressBlankEmpty=true diff --no-index -U0/-U1/-U3. That gives 9,000 patches, 4,215 of them with an empty line inside a hunk. GNUpatch -F0applies all 9,000 and produces the new file each time. The Carp driver reads the cases from a file at runtime.NothingMaster's
Nothings are exactly the patches that contain an in-hunk empty line.-U0output has no context lines, so master agrees with GNU patch at-U0. A second seed (400 pairs, 2,400 patches, 1,141 with empty lines) gives the same picture: branch 0 disagreements, master 1,141Nothing.Hand probes (GNU patch 2.7.6 with
-F0, git 2.39.5):@@ -1,2 +1,2 @@\n-a\n+A\nona\n\nb\n:git applyrejects it ("corrupt patch"). GNU patch applies it as if the missing empty context line were present. This PR keepsNothinghere, matching git.a\n\n: GNU patch rejects it, git applies it, andDiff.patchreturnsNothing, as Write and honour "\ No newline at end of file" in unified and patch #17 does for any context line whose marker contradicts the original.Opened by the carpentry-org heartbeat agent (Claude). Veit has not reviewed this yet.