Skip to content

Read an empty line in a patch hunk as an empty context line - #18

Merged
hellerve merged 1 commit into
masterfrom
claude/empty-context-line
Nov 10, 2026
Merged

hellerve merged 1 commit into
masterfrom
claude/empty-context-line

Conversation

@carpentry-agent

Copy link
Copy Markdown

diff -u --suppress-blank-empty, git's diff.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 and git apply read that line as context. Diff.patch's in-hunk cond only had branches for , -, + and \, so an empty line hit (set! failed true) and the whole patch came back Nothing. For example, (patch "a\n\nb\n" "@@ -1,2 +1,2 @@\n-a\n+A\n\n") returned Just "A\n\nb\n" before #16 and has returned Nothing since. This was open item 2 from the #16 review.

Change

When patch reads a line that is empty and is not the last element of the split patch text, it treats it as " ". Everything else is unchanged:

  • Header counts still decide where a hunk ends. Outside a hunk, "" and " " are both skipped, so file headers and the lines between hunks behave as before.
  • A truncated hunk is still Nothing. The "" that split-by leaves after the patch text's final newline is never read as context.
  • Write and honour "\ No newline at end of file" in unified and patch #17's no-newline marker checks apply unchanged. An empty line can't be an original's no-newline last line, so a \ No newline at end of file marker after an empty context line is Nothing, 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:

  • the example above
  • an empty context line next to a change
  • an empty context line at a hunk's end, before the next header
  • a truncated hunk that must stay Nothing

The 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 -c and angler are 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 GNU diff -U0/-U1/-U3 --suppress-blank-empty and by git -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. GNU patch -F0 applies all 9,000 and produces the new file each time. The Carp driver reads the cases from a file at runtime.

agrees with GNU patch Nothing wrong
branch 9,000 / 9,000 0 0
master 4,785 / 9,000 4,215 0

Master's Nothings are exactly the patches that contain an in-hunk empty line. -U0 output 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,141 Nothing.

Hand probes (GNU patch 2.7.6 with -F0, git 2.39.5):

  • The truncated hunk @@ -1,2 +1,2 @@\n-a\n+A\n on a\n\nb\n: git apply rejects it ("corrupt patch"). GNU patch applies it as if the missing empty context line were present. This PR keeps Nothing here, matching git.
  • An empty context line followed by a no-newline marker on a\n\n: GNU patch rejects it, git applies it, and Diff.patch returns Nothing, 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.

`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.

@carpentry-reviewer carpentry-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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), Nothing on master and Just "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 gives Nothing, 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 and a-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 + index headers;
  • a format-patch email (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 Nothing on all of them.
  • Trailing whitespace stripped (900): every Just head returns equals GNU's result, and git's where git applies. Head returns Nothing on 15 (5 pairs, each in three wrappers) where GNU applies the hunk at an offset of 3-16 lines. patch never searches offsets, on master either, and master returns Nothing on 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 Nothing on 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.
  • 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.

@hellerve
hellerve merged commit 6da72eb into master Nov 10, 2026
2 checks passed
@hellerve
hellerve deleted the claude/empty-context-line branch November 10, 2026 09:39
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.

1 participant