Skip to content

merge: keep conflict markers on their own lines when hunks lack a trailing newline - #85

Merged
bmwill merged 1 commit into
bmwill:masterfrom
jochenhz:fix/merge-conflict-marker-newlines
Jul 19, 2026
Merged

merge: keep conflict markers on their own lines when hunks lack a trailing newline#85
bmwill merged 1 commit into
bmwill:masterfrom
jochenhz:fix/merge-conflict-marker-newlines

Conversation

@jochenhz

Copy link
Copy Markdown
Contributor

When a conflicting hunk sits at end-of-file without a trailing newline, merge/merge_bytes
glue the next conflict marker onto the content line, producing unparseable output:

    base:   "This is line 1.\nThis is line 2."
    ours:   "This is line 1.\nThis is line 2 changed."
    theirs: "This is line 1.\nThis is line 2 also changed."

Actual:

    This is line 1.
    <<<<<<< ours
    This is line 2 changed.||||||| original
    This is line 2.=======
    This is line 2 also changed.>>>>>>> theirs

Expected (matches git merge-file --diff3 on identical inputs):

    This is line 1.
    <<<<<<< ours
    This is line 2 changed.
    ||||||| original
    This is line 2.
    =======
    This is line 2 also changed.
    >>>>>>> theirs

add_conflict_marker/add_conflict_marker_bytes now prefix a newline when the output
doesn't already end with one. Only reachable for file-final hunks, so all existing merge
output is unchanged — the full suite passes untouched. One new test covers both the str
and bytes paths via assert_merge!.

A conflicting hunk at end-of-file without a trailing newline glued the
next marker onto its last content line, producing unparseable output.
Matches git merge-file --diff3 behavior.

Signed-off-by: Jochen Hunz <j.hunz@anchorpoint.app>
jochenhz added a commit to Anchorpoint-Software/lore that referenced this pull request Jul 18, 2026
Vendor diffy 0.4.2 with a newline guard in both conflict-marker
builders until a release contains the upstream fix
(bmwill/diffy#85).

Signed-off-by: Jochen Hunz <j.hunz@anchorpoint.app>
@bmwill

bmwill commented Jul 19, 2026

Copy link
Copy Markdown
Owner

Thanks for the fix! Looks like there are some issues with CI unrelated to your PR (newer git on the CI machines changing an expected output string and some new clippy lint). I'll fix those and get this merged in.

@bmwill
bmwill merged commit 31de940 into bmwill:master Jul 19, 2026
21 of 26 checks passed
@bmwill

bmwill commented Jul 19, 2026

Copy link
Copy Markdown
Owner

Alright merged and just published version 0.5.1 with your fix.

jochenhz added a commit to Anchorpoint-Software/lore that referenced this pull request Aug 20, 2026
Vendor diffy 0.4.2 with a newline guard in both conflict-marker
builders until a release contains the upstream fix
(bmwill/diffy#85).

Signed-off-by: Jochen Hunz <j.hunz@anchorpoint.app>
bmwill added a commit that referenced this pull request Aug 31, 2026
Commit 31de940 (merge: keep conflict markers on their own lines, #85)
changed how conflict markers are rendered when a conflicting hunk ends in
an incomplete line (one without a trailing newline), inserting a newline
so that every marker starts at the beginning of a line. That matched the
behavior of `git merge-file`, but silently diverged from GNU `diff3 -m`,
which the diffutils manual documents as appending the succeeding markers
directly to the incomplete line.

With this commit, the behavior is now selectable via a new two-variant
enum, `IncompleteHunkStyle`, on `MergeOptions`:

* `Diff3` (the default) appends markers directly to the incomplete line,
  matching GNU `diff3 -m` and restoring the pre-#85 output.
* `Git` inserts a newline after the incomplete line, matching
  `git merge-file`.

Also add a table-driven test covering all eight permutations of the
three inputs having or lacking a trailing newline, for both styles and
for both the str and bytes paths. The expected outputs were verified
against GNU diff3 3.12 and git 2.55.0: git produces byte-identical
output for every permutation, while GNU diff3 glues each side's
succeeding marker independently.
epic-lore-bot Bot pushed a commit to EpicGames/lore that referenced this pull request Sep 1, 2026
… 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
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.

2 participants