Skip to content

Commit cb6f575

Browse files
committed
lore-revision: Fix conflict markers glued to hunks lacking a trailing newline
When both sides of a conflict end the file without a trailing newline, the merge glues the conflict markers onto the content lines and the file becomes unparseable — markers are only recognizable at the start of a line: ``` <<<<<<< ours This is line 2 changed.||||||| original This is line 2.======= This is line 2 also changed.>>>>>>> theirs ``` git merge-file --diff3 puts every marker on its own line for the same inputs. Repro is easy: commit a file written with no trailing newline on two branches from a common base, then `branch merge start`. With trailing newlines the markers come out fine. The bug was in the diffy crate. It is fixed there now (bmwill/diffy#85), and 0.5.2 makes the behaviour selectable (bmwill/diffy#88). ## Updated per review **No vendoring.** This is now a plain dependency bump to `diffy = "0.5.2"` plus `MergeOptions::set_incomplete_hunk_style(IncompleteHunkStyle::Git)` in `merge3_text`. The bump alone is not enough: 0.5.2 defaults to `IncompleteHunkStyle::Diff3`, which is the old glued behaviour, so the setting is what does the work. **Tests that resolve restores the content unchanged.** `scripts/test/test_merge_resolve.py` gains three cases on a file whose last line has no trailing newline: - every conflict marker occupies a whole line; - `merge resolve mine` restores the committed bytes exactly; - `merge resolve theirs` restores the committed bytes exactly. They compare bytes rather than strings, so an added newline fails the assertion instead of passing unnoticed. The inserted newline belongs to the marker rendering only — resolving through the Lore API reads the `~mine` / `~theirs` sidecars and returns the side as it was committed. `lore-revision/tests/merge.rs` keeps a unit-level regression test for the marker shape. ## Testing - `cargo test -p lore-revision` — 4 merge tests, 364 lib tests - `pytest test_merge_resolve.py test_merge.py test_conflict.py` — 25 passed - `pytest test_diff.py test_diff_git_baseline.py` — 64 passed (`PatchFormatter` is the other diffy consumer, so the diff output is covered too) --- Disclosure: I used Claude to investigate the root cause of this bug and to implement the fix. I reviewed and tested the changes myself. ``` Imported-PR: #119 Imported-From: 70ee9c0 Imported-Base: 715645d Imported-Merge: 627c14d Imported-Merge-Strategy: verbatim Imported-Merged-Paths: 0 Imported-Author: Jochen Hunz (jochenhz) Signed-off-by: Jochen Hunz <j.hunz@anchorpoint.app> GH-URL: #119 ``` Lore-RevId: 864 Lore-Signature: 5fd0a5f2f53be5504b8f1c8c002116d01d80bf35c260225822e787e62478fbc3
1 parent 55eff3a commit cb6f575

5 files changed

Lines changed: 120 additions & 14 deletions

File tree

Cargo.lock

Lines changed: 15 additions & 12 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ config = { version = "0.15.15", features = ["toml"], default-features = false }
6161
ctor = "0.2"
6262
crossbeam = "0.8.4"
6363
dashmap = "6.1.0"
64-
diffy = "0.4.2"
64+
diffy = "0.5.2"
6565
directories = "6.0.0"
6666
enum_dispatch = "0.3.13"
6767
tracing-appender = "0.2.3"

lore-revision/src/merge.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,11 @@ pub fn merge3_text(
3232
mine_marker: Option<&str>,
3333
theirs_marker: Option<&str>,
3434
) -> Result<String, String> {
35-
let merge_result = diffy::merge(base, mine, theirs);
35+
// `Git`, not diffy's `Diff3` default: `Diff3` glues the next marker onto a
36+
// final line that lacks a newline, which is unparsable.
37+
let merge_result = diffy::MergeOptions::new()
38+
.set_incomplete_hunk_style(diffy::IncompleteHunkStyle::Git)
39+
.merge(base, mine, theirs);
3640
let merge_conflicts = merge_result.is_err();
3741
let mut merge_output = match merge_result {
3842
Ok(str) | Err(str) => str,

lore-revision/tests/merge.rs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,36 @@ mod tests {
3535
assert_eq!(result_string, expected_string);
3636
}
3737

38+
/// Fails without `IncompleteHunkStyle::Git`: the default glues the next
39+
/// marker onto the last line when it has no trailing newline.
40+
#[test]
41+
fn test_conflict_without_trailing_newlines() {
42+
let base_string = "This is line 1.
43+
This is line 2.";
44+
let mine_string = "This is line 1.
45+
This is line 2 as I wrote it.";
46+
let theirs_string = "This is line 1.
47+
This is line 2 as they wrote it.";
48+
49+
let result_string =
50+
match merge3_text(base_string, mine_string, theirs_string, None, None, None) {
51+
Err(str) | Ok(str) => str,
52+
};
53+
54+
for marker in [
55+
"<<<<<<< ours",
56+
"||||||| original",
57+
"=======",
58+
">>>>>>> theirs",
59+
] {
60+
assert!(
61+
result_string.lines().any(|line| line == marker),
62+
"`{marker}` must occupy a line of its own, got:
63+
{result_string}"
64+
);
65+
}
66+
}
67+
3868
#[test]
3969
fn test_markers() {
4070
let base_string = "This is line 1.\nThis is line 2.\n";

scripts/test/test_merge_resolve.py

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -584,3 +584,72 @@ def test_merge_resolve_mine_no_paths(new_lore_repo):
584584
)
585585

586586
repo.commit("Resolved all with mine (no paths)", offline=True)
587+
# ---------------------------------------------------------------------------
588+
# Test: a final line without a newline survives resolve unchanged
589+
# ---------------------------------------------------------------------------
590+
591+
# `IncompleteHunkStyle::Git` inserts a newline after an incomplete final line so
592+
# the following conflict marker starts at column 0. These pin that the newline
593+
# belongs to the marker rendering only: resolving restores the side byte for
594+
# byte, exactly as it was committed.
595+
596+
BASE_NO_EOL = b"line 1\nline 2"
597+
MINE_NO_EOL = b"line 1\nline 2 mine"
598+
THEIRS_NO_EOL = b"line 1\nline 2 theirs"
599+
600+
601+
@pytest.mark.smoke
602+
def test_merge_conflict_markers_own_line_without_trailing_newline(new_lore_repo):
603+
"""Every marker starts a line even when the conflicting hunk has no EOL."""
604+
repo: Lore = new_lore_repo()
605+
setup_merge_conflict(
606+
repo, {"a.txt": (BASE_NO_EOL, MINE_NO_EOL, THEIRS_NO_EOL)}
607+
)
608+
609+
with repo.open_file("a.txt", "rb") as f:
610+
conflicted = f.read().decode()
611+
612+
for marker in ["<<<<<<< ours", "||||||| original", "=======", ">>>>>>> theirs"]:
613+
assert any(line == marker for line in conflicted.split("\n")), (
614+
f"{marker!r} must occupy a whole line, got {conflicted!r}"
615+
)
616+
617+
repo.branch_merge_abort(offline=True)
618+
619+
620+
@pytest.mark.smoke
621+
def test_merge_resolve_mine_restores_missing_trailing_newline(new_lore_repo):
622+
"""resolve mine gives back the committed bytes, without the marker newline."""
623+
repo: Lore = new_lore_repo()
624+
setup_merge_conflict(
625+
repo, {"a.txt": (BASE_NO_EOL, MINE_NO_EOL, THEIRS_NO_EOL)}
626+
)
627+
628+
repo.branch_merge_resolve_mine(["a.txt"], offline=True, json=True)
629+
630+
with repo.open_file("a.txt", "rb") as f:
631+
content = f.read()
632+
assert content == MINE_NO_EOL, (
633+
f"resolve mine must restore the exact bytes, got {content!r}"
634+
)
635+
636+
repo.branch_merge_abort(offline=True)
637+
638+
639+
@pytest.mark.smoke
640+
def test_merge_resolve_theirs_restores_missing_trailing_newline(new_lore_repo):
641+
"""resolve theirs gives back the committed bytes, without the marker newline."""
642+
repo: Lore = new_lore_repo()
643+
setup_merge_conflict(
644+
repo, {"a.txt": (BASE_NO_EOL, MINE_NO_EOL, THEIRS_NO_EOL)}
645+
)
646+
647+
repo.branch_merge_resolve_theirs(["a.txt"], offline=True, json=True)
648+
649+
with repo.open_file("a.txt", "rb") as f:
650+
content = f.read()
651+
assert content == THEIRS_NO_EOL, (
652+
f"resolve theirs must restore the exact bytes, got {content!r}"
653+
)
654+
655+
repo.branch_merge_abort(offline=True)

0 commit comments

Comments
 (0)