fix: stop a partial selection at a no-newline EOF from merging two lines

[ashproto]

Stacked on #26 — base is feat/diff-hunk-affordance, so this shows only its own commit. discard_lines and require_contiguous do not exist on next yet; both arrive with #26. Merge order: #25 → #26 → this.

Found by a review pass on #26 and deliberately kept out of it — this is pre-existing and reproduces byte-for-byte against HEAD.

The bug

When the last line of a file with no trailing newline changes, git emits a -/+ pair where both sides carry \ No newline at end of file:

@@ -1,2 +1,2 @@
 a
-t
\ No newline at end of file
+T2
\ No newline at end of file

The marker claims the line before it ends the image it belongs to. build_partial_hunk kept it whenever the preceding line had been emitted — including when a demoted-to-context line was emitted after it, which makes the claim false. git apply accepts the contradiction and resolves it by concatenating.

With base "a\nt" and worktree "a\nT2", discarding just the deletion produced "a\ntT2" — the two lines merged onto one — returning Ok(()), on the path with no reflog and no backup bundle.

The two directions are not symmetric

	discard / unstage (reverse)	stage (forward)
Before	Ok + "a\ntT2"	Ok + "a\ntT2"
Drop the marker	Ok + "a\nt\nT2" — correct	Err "patch does not apply"
Correct patch needs	just dropping the marker	re-emitting t as a -/+ pair

Reversing keeps the other half as context on the side that still ends there, so dropping the stale marker expresses the intent exactly — verified against real git.

Going forward would need the demoted line newline-terminated on the new side and not on the old. One context line cannot say that, so no patch expresses it. It is now refused, in the same spirit as require_contiguous. Whole-hunk ops replay git's own hunk verbatim and are unaffected, so staging the whole hunk remains available as the escape hatch.

Implementation

Deciding a marker's fate needs to know what follows it, so the emitter now tracks what the previous line was emitted as (context / - / +) rather than merely that it was, plus the first ordinal after it. build_partial_hunk returns Result<Option<String>, String> to carry the refusal; callers keep their own "no lines selected" messages.

That is ~30 lines rather than the ~8 a marker-only tweak would need. The extra buys one source of truth: a separate guard function would have to re-simulate the emitter's drop/demote rules and could drift from them.

A test that asserted the bug

partial_hunk_no_newline_marker asserted the corrupt output as correct, and its comment narrated the buggy mechanism approvingly:

// The no-newline after -old becomes context so last_emitted=true → marker kept

It was written against the implementation rather than the behavior, so it passed happily while the code merged two lines. I reconstructed the exact patch that shape emits and ran it through real git before touching it — it stages oldnew instead of old\nnew. Only then did I flip the assertion, with the proof recorded in the test comment.

Tests (TDD — all four watched fail first)

test	before	after
discard_lines_at_no_newline_eof_keeps_lines_separate	Ok + "a\ntT2"	Ok + "a\nt\nT2"
unstage_lines_at_no_newline_eof_keeps_lines_separate	Ok + "a\ntT2"	Ok + "a\nt\nT2"
stage_lines_at_no_newline_eof_refuses_rather_than_corrupting	Ok + corrupt	Err, index and worktree untouched
whole_hunk_ops_at_no_newline_eof_are_unaffected	passed	passed (escape hatch holds)

require_contiguous is untouched and remains the independent defence against a gapped selection reordering the file.

Gates: cargo test -p git-core 224 passed · npm run check 530 files / 0 errors · npm test 386 passed.

🤖 Generated with Claude Code