Skip to content

Write and honour "\ No newline at end of file" in unified and patch - #17

Merged
hellerve merged 1 commit into
masterfrom
claude/no-newline-marker
Nov 5, 2026
Merged

hellerve merged 1 commit into
masterfrom
claude/no-newline-marker

Conversation

@carpentry-agent

Copy link
Copy Markdown

Follow-up to #16: unified, unified-with-context and patch now treat a text's final newline the way diff(1), patch(1) and git do, including the \ No newline at end of file marker.

What changed

Each side of a diff is read the way line-diff splits a text and patch rejoins it. A final "" element is the text's final newline, not a line. A final non-empty element is a last line without a newline. [] and [""] are both the empty text.

Writer. The phantom "" is no longer written as a context, - or + line, and it is no longer counted in the header. A last line without a newline is followed by the marker: after -x for the old side, after +x for the new side, after x for both. When the sides differ only in that newline, the line is written as a change, as diff -u writes it ("a\nb" → "a\nb\n" gives a, -b, \ No newline at end of file, +b). This works whichever order Diff.diff emits the insertion and deletion in, because the sides are normalised before rendering. The trailing-context and zero-count rules from #16 are unchanged.

Applier. After - the marker describes the original's last line, after + the new side's, after both. The result ends with a newline exactly when the new side's last line does. When the last hunk reaches the end of the original, the markers decide that; otherwise the original's untouched tail does. An empty original has zero lines.

Strictness. patch is documented as a safe applier, so a marker that is missing, contradicts the original, or does not follow its side's last line gives Nothing. Here is what the tools do (GNU patch 2.7.6, git 2.39.5):

patch against the original GNU patch git apply patch
-b + marker, original ends with a newline rejects rejects Nothing
-b, no marker, original has no final newline rejects rejects Nothing
b + marker, original ends with a newline applies, keeps the newline applies, drops it Nothing
b, no marker, original has no final newline applies rejects Nothing
marker after -a, more old lines follow rejects (malformed) rejects Nothing
marker after +X, +Y follows in the hunk rejects (malformed) applies, joining them as XY Nothing
marker after +X, original lines follow the hunk applies, ignoring the marker rejects Nothing

The library rejects wherever git does. In the two rows where git still applies, it drops the original's newline in one and joins two lines in the other. A zero-count hunk appended after a last line that lacks a newline is not a marker case, so it still applies: the result is …b\nY\nZ\n, as with GNU patch (git writes …bY\nZ\n).

For Veit

unified also accepts diffs of arrays that did not come from line-diff, and under this convention an array whose last element is not "" reads as a text without a final newline. So (unified (diff &[] &[@"a" @"b"])) now ends with the marker after +b. The two suite rows that pinned this changed: "an empty old side is -0,0" and "an empty new side is +0,0". The alternative would be a text-level entry point that takes two strings. I did not add one; your call.

Changed expectations

Each was checked against diff -u (or GNU patch) for the same two files:

  • Six unified rows gain the marker after their last line, because their texts lack a final newline: "single-hunk diff with correct header", "change at start", "change at end" (two markers), "multiple hunks for distant changes" (two), "unified-with-context 3 matches unified default", "unified-with-context 1 splits hunks". The headers and lines match diff -u, which writes - before +.
  • Three hand-written patch rows run against originals without a final newline. Their patches now carry the marker diff -u writes there; without it they are Nothing. The rows are "no-op patch to a latin-1 document", "skips file headers before the first hunk" and "reads --- and +++ inside a hunk as body lines".
  • "patch appends a zero-count hunk at the end" now gives "a\nb\nc\nd\ne\nY\nZ\n", the same as GNU patch.
  • "patch skips no-newline markers" is renamed "patch honours no-newline markers"; its expectation is unchanged.

Measurements

The kit lives outside the tree. It has 616 pairs: all 16 ordered pairs of "", "\n", "a", "a\n"; 300 random pairs (0–20 lines over a b c d, each side's final newline random); and 300 newline-only pairs (150 texts, newline added and removed). Wrong results, master → branch:

ctx 0 ctx 1 ctx 3
(a) library → GNU patch 519 → 0 525 → 0 525 → 0
(a) library → git apply 365 → 1 601 → 0 609 → 0
(b) tool output → patch diff -U0 diff -U1 diff -U3 git -U0 git -U3
master → branch 454 → 0 454 → 0 454 → 0 454 → 0 454 → 0

On the newline-only pairs, master was wrong for all 300 in (b) and for all 300 in (a), except git at ctx 0 (150).

(c) The round trip (patch old (unified-with-context (line-diff old new) ctx)) was Just new for 1848/1848 on both master and the branch. Master's writer and applier agreed on the phantom line.

The one git ctx-0 failure on the branch is a case where the library writes the same hunks as diff -U0. git apply --unidiff-zero drops the final newline on diff -U0's own output for that pair too; GNU patch applies both correctly.

Tests

The suite has 10 new rows, next to the related unified and patch rows:

  • unified: the newline-only change in both directions, both texts ending with a newline, and "" ↔ "a".
  • patch: two pasted GNU diff -u outputs that carry the marker, one of them against an empty original, plus the contradicting, missing and misplaced marker cases.

Against master's diff.carp the new suite is 105 passed / 19 failed. All 10 new rows fail there, plus 9 of the changed expectations: the 8 unified rows and the zero-count append. The three patch rows that only gained a marker pass on master, because master skips markers.

Mutants of the new logic, each killed by a committed row:

mutant killed by (rows)
old-side marker dropped when the newline-only change is split "unified writes an added final newline as a change of the last line" (2)
old-side marker dropped in deletions "unified reads an empty new text as no lines" (9)
final "" counted as a line by the writer "unified does not count a final newline as a line" (5)
result's final newline always taken from the original "patch adds the final newline a diff -u marker asks for" (3)
no old-side check after a - line "patch returns Nothing on a no-newline marker the original contradicts" (2)
new-side marker position unchecked "patch returns Nothing on a no-newline marker before the last line" (1)
original's final "" counted as a line by the applier "patch reads an empty original as no lines" (1)
newline mismatch not split into a change "unified writes a removed final newline as a change of the last line" (2)
missing old-side marker accepted "patch returns Nothing when the last line of the original lacks its marker" (1)

Suite: 124/0 on armhf and on LP64 (aarch64-linux-gnu-gcc, no -O). Master is 114/0. carp-fmt --check and angler are clean on both files.


Opened by the carpentry-org heartbeat agent (Claude). Veit has not reviewed this yet.

unified and unified-with-context now read each side of the diff the
way line-diff splits a text: a final "" element is the text's final
newline, not a line, and a final non-empty element is a last line
without one. The phantom "" is no longer written as a line or counted
in the hunk header, which made git reject the output at ctx 1 and 3
and let git and GNU patch misplace hunks at ctx 0. A last line without
a newline is followed by the marker, and a line whose newline differs
between the sides is written as a change, as diff -u does.

patch reads the marker: after "-" it describes the original's last
line, after "+" the new side's, after " " both. The result ends with a
newline exactly when the new side's last line does, and an empty
original has no lines. A marker that is missing, contradicts the
original, or does not follow its side's last line makes the patch not
apply. That is git apply's behaviour wherever git rejects; GNU patch
is laxer in some of these cases.

Existing rows whose texts lack a final newline now expect the marker,
checked against diff -u for the same two files.
@carpentry-agent

Copy link
Copy Markdown
Author

Independent check of 2c72fc5. I used my own kit, built before this PR existed and baselined on master ca1cfac. It has 400 pairs with a different seed and mix from the session's: all 56 ordered pairs of "", "\n", "a", "a\n", "a\nb", "a\nb\n", "\n\n" and "b\na\n", plus random texts of 0–12 lines over a b c d, about a third of them newline-only changes. Results, master → head:

master head
library → GNU patch 2.7.6 (400 pairs × ctx 0/1/3) 735 rejected, 185 wrong 0
library → git apply (same 1,200) 969 rejected, 60 wrong 1 wrong (ctx 0)
diff -U0/-U1/-U3, git diff -U0/-U3 → patch (400 each) 237 wrong in each 0
round trip (patch old (unified-with-context (line-diff old new) ctx)) 0 wrong 0 wrong

The one git miss is old "a\na\na\nb\nc\nd\na\nd\nc\nd\na\na" → new "c\nc\nd\nc\nd\na\n" at ctx 0. git apply --unidiff-zero drops the final newline in exactly the same way on GNU diff -U0's output and on git diff --no-index -U0's own output for that pair, and GNU patch applies all three correctly. So the miss is git's, as the body says.

Posted by the carpentry-org heartbeat agent (Claude).

@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

  • Head 2c72fc5 passes carp -x tests/diff.carp 124/0 on armhf. The same main.c rebuilt with aarch64-linux-gnu-gcc (LP64, no -O) also passes 124/0. Master ca1cfac passes 114/0.
  • Teeth: head's tests/diff.carp against master's diff.carp gives 105 passed and 19 failed. The failures are the 10 new rows and the 9 changed expectations (8 unified rows and the zero-count append), which matches the body.
  • CI is green on ubuntu-latest and macos-latest.
  • No leak. A --log-memory probe ran unified-with-context at ctx 0–3 plus patch on six marker pairs and three marker patches. It reads 0 on every row for both head and master. A Debug.leak-array positive control in the same build reads 2. The generated C for patch, line-hunks, side-end and unified-with-context contains no break;.
  • Speed is unchanged. On a 100,000-line pair with 101 changed lines, unified takes 38.8 s on head and 38.6 s on master, and patch takes 54.8 s and 56.1 s (one run each, at a load average of about 3.5). Head's patch of head's own output reproduces the new text byte for byte.

My own differential. It has 1,257 pairs, built with a different generator from the session's and from the verification comment's:

  • 324 ordered pairs of 18 edge texts. These include "", newline-only texts, "a\n\n", "\na", CRLF and lone-CR endings, a text that is only the literal marker line (with and without a final newline), and " ".
  • 700 random pairs of 0–10 lines. Half of them draw from hostile lines: empty, the literal \ No newline at end of file, a lone backslash and \x, -a/+a/ a, --- a, +++ b, @@ -1 +1 @@, a + CR, CR alone, UTF-8 and latin-1 bytes, and -/+. About 15% are newline-only changes.
  • 233 files of 20–80 lines with distant edits, one of them 0–8 lines from the end. The last line is unchanged, and each side's final newline varies.
check, head (master) ctx 0 ctx 1 ctx 3 ctx 2 / 7
library → GNU patch, -F0 0 wrong, 0 rejected (303 wrong, 423 rejected) 0 / 0 (3 / 961) 0 / 0 (0 / 1,037) 0 / 0 (0 / 1,009 and 1,102)
library → git apply 3 wrong (172 wrong, 411 rejected) 0 (3 wrong, 961 rejected) 0 (1,037 rejected) not run
round trip (patch old (unified-with-context …)) 1,257/1,257 (same on master) same same same

diff -U0/-U1/-U3 and git diff -U0/-U3 output fed to patch gives 1,257/1,257 right for each of the five on head. On master each is 541 wrong. The three git misses (pairs 449, 890 and 1229) are the --unidiff-zero bug the body describes. git misapplies GNU diff -U0's output and git diff -U0's own output for the same three pairs, and GNU patch applies all three correctly.

Findings

1. The context-line half of the marker check is not pinned (diff.carp:469). If I delete (when (/= marked (= cursor nonl-end)) (set! failed true)) from the branch, the suite still passes 124/0. Keeping either arm alone also passes. That leaves two rows of the body's strictness table untested:

  • (patch "a\nb\n" "@@ -1,2 +1,2 @@\n-a\n+A\n b\n\\ No newline at end of file"): head gives Nothing, and the mutant gives Just "A\nb", git's answer that drops the newline (table row 3).
  • (patch "a\nb" "@@ -1,2 +1,2 @@\n-a\n+A\n b"): head gives Nothing, and the mutant gives Just "A\nb\n" (table row 4).

The body's mutant table covers only the - branch's copy of this check (:477).

2. Three more applier mutants survive the suite (124/0). Each one restores part of master's phantom line or misreads an empty result:

  • old-line? bounded by (Array.length orig) instead of olen (:422). An empty context line can then match the final newline. (patch "a\n" "@@ -1,2 +1,2 @@\n a\n \n") gives Just "a\n\n" instead of Nothing, and GNU patch and git both reject it.
  • The range check against (Array.length &orig) instead of olen (:497). (patch "a\n" "@@ -2,0 +3 @@\n+X") gives Just "a\n\nX\n" instead of Nothing. GNU patch and git both apply this hunk anyway, at the end of the file, and give "a\nX\n", so pinning Nothing is a choice. The mutant's extra empty line is wrong either way.
  • (if ends …) without (not (Array.empty? &out)) (:522). (patch "a\n" "@@ -1 +0,0 @@\n-a") gives Just "\n" instead of Just "", and both tools give "".

A fourth survivor, (side-end xs nl) for ol at :244, is equivalent: that branch only runs when ol equals nl or the part is empty.

These five rows pin all of the above. They go right after the row "patch returns Nothing on a no-newline marker before the last line". With them the file is carp-fmt --check clean and the suite passes 129/0 on head, and each row fails on its mutant (both arms of finding 1 included):

    (assert-equal test
      &(the (Maybe String) (Maybe.Nothing))
      &(patch "a\nb\n"
              "@@ -1,2 +1,2 @@\n-a\n+A\n b\n\\ No newline at end of file")
      "patch returns Nothing on a context marker the original contradicts")
    (assert-equal test
      &(the (Maybe String) (Maybe.Nothing))
      &(patch "a\nb" "@@ -1,2 +1,2 @@\n-a\n+A\n b")
      "patch returns Nothing when a last context line lacks its marker")
    (assert-equal test
      &(Maybe.Just @"")
      &(patch "a\n" "@@ -1 +0,0 @@\n-a")
      "patch that deletes every line gives the empty text")
    (assert-equal test
      &(the (Maybe String) (Maybe.Nothing))
      &(patch "a\n" "@@ -1,2 +1,2 @@\n a\n \n")
      "patch does not read the final newline as an empty line")
    (assert-equal test
      &(the (Maybe String) (Maybe.Nothing))
      &(patch "a\n" "@@ -2,0 +3 @@\n+X")
      "patch returns Nothing on a zero-count hunk past the last line")

3. For the "your call" on non-line-diff arrays: the library's own string-diff is affected. The doc strings of unified (diff.carp:381-385) and unified-with-context (:276-281) still accept "a diff of string arrays". string-diff produces one whose last element is never "", because String.words drops empty strings. So every word diff whose hunk reaches the last word now ends with a marker. (unified (string-diff "the quick fox" "the slow fox")) now prints:

@@ -1,3 +1,3 @@
 the
+slow
-quick
 fox
\ No newline at end of file

Master printed the same output without the last line. None of the 47 local carpentry clones calls Diff.unified, unified-with-context or Diff.patch outside diff itself, so nothing downstream changes. If the array-level reading stays, the doc could say that word diffs get the marker too. The text-level entry point the body mentions would avoid the marker on word diffs altogether.

4. Minor: a stray marker is skipped, but the doc says it gives Nothing (:486 against the doc at :433-434). A \ line directly after a @@ header, or a second \ line after a marker, is ignored. For example, (patch "a" "@@ -1 +1 @@\n\\ No newline at end of file\n-a\n\\ No newline at end of file\n+b") gives Just "b\n". git agrees and GNU patch rejects both shapes. The result is the same as without the stray line, so only the doc's promise that a marker which "does not follow its side's last line" gives Nothing is off. Either reject a \ line that doesn't follow a body line, or soften the sentence. This one is optional.

5. Body nit: one row of the strictness table. The row "marker after +X, original lines follow the hunk" says git rejects. That holds only for the zero-context form without --unidiff-zero.

  • With --unidiff-zero, @@ -1,0 +2 @@\n+X\n\\ No newline at end of file against "a\nb\n" gives "a\nXb\n".
  • With a context line after the marker (@@ -1,2 +1,3 @@\n a\n+X\n\\ …\n b), git also applies and gives "a\nXb\n", while GNU patch rejects.

So git applies in three rows, not two, and joins lines in two of them. Nothing is the right call there either way.

GNU patch also applies rows 3 and 4 only by dropping the context line: it reports Hunk #1 succeeded at 1 with fuzz 1. With -F0 it rejects both. Those two rows therefore support the library's Nothing as well.

Pre-existing and out of scope: an empty context line from diff --suppress-blank-empty still gives Nothing. For example, (patch "\nb" "@@ -1,2 +1,2 @@\n\n-b\n\\ No newline at end of file\n+c") returns Nothing, where GNU patch and git both give "\nc\n". This is the open item from #16's review, and this PR doesn't touch it.

Verdict: merge

I found no wrong result. GNU patch got 0 wrong in 6,285 runs, five tools' output on 1,257 pairs applied correctly, and all 6,285 round trips held; the only misses were git's own -U0 bug. The PR doesn't leak and costs nothing in speed. The findings are test gaps around behavior that is already correct, plus doc and body precision. Adding the five rows above before merging would pin the strictness the body promises.

@hellerve
hellerve marked this pull request as ready for review November 5, 2026 20:36
@hellerve
hellerve merged commit a632c1b into master Nov 5, 2026
2 checks passed
@hellerve
hellerve deleted the claude/no-newline-marker branch November 5, 2026 20:36
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