fix: preserve trailing newlines in text-based 3-way merges

[jorrite]

what

• Fix atmos scaffold generate --update unconditionally stripping the trailing newline from every file it 3-way-merges, whether or not the file actually changed.
• pkg/generator/merge/text_merger.go: TextMerger.Merge() now appends one newlineSeparator ("\n") to each of ours/base/theirs before handing them to diff3.Merge, so diff3's guaranteed loss of exactly one trailing newline cancels out and the original count survives.
• pkg/generator/merge/text_merger_test.go: consolidated the trailing-newline regression coverage into a single table-driven test, TestTextMerger_TrailingNewlinePreservation, asserting exact byte-for-byte output across a no-op merge (0/1/2/3 trailing newlines, plus an internal blank line) and a genuine template change (theirs with 0/1/2 trailing newlines).
• No change to conflict detection, threshold behavior, or ConflictStrategy handling — out of scope, and unaffected since the appended newline is identical across all three inputs.

why

• TextMerger.Merge() delegates the actual 3-way merge to epiclabs-io/diff3, which reads each of base/ours/theirs line-by-line via bufio.Scanner (ScanLines, Go's standard-library default split function) and rejoins the merged lines with strings.Join(lines, "\n").
• ScanLines strips every line's terminator — including the last — and gives no way to tell afterward whether the original input ended with a trailing newline or not. Concretely: for content ending in N trailing newlines, the round-trip through GetLines + Join always reconstructs exactly N-1 (it loses exactly one, regardless of how many there were; for N = 0 there was nothing to lose in the first place). Verified directly: generating a file with 3 trailing newlines and running --update with nothing changed on the template side reproducibly comes back with 2.
• Appending one newline to each input before the merge bumps every input's count to at least 1, so that guaranteed loss of exactly one cancels out and the original count is preserved — for both the no-op case and genuine changes, since whichever side's content ends up dominating a given region carries its own (now-restored) newline count through, independent of the others.
• This must be applied to all three inputs, not just theirs: appending it only to theirs makes an otherwise-identical ours/theirs pair (a very common no-op shape) differ by one trailing newline as far as diff3 is concerned, which turns a no-op into a spurious detected change/conflict instead of fixing anything.

references

• Closes scaffold generate --update strips trailing newlines (if any) from every merged file #2887.

Summary by CodeRabbit

• Bug Fixes

  • Improved text merging to preserve the exact number of trailing newlines.
  • Prevented unintended changes to blank lines and end-of-file newline states during conflict-free merges.
  • Ensured merged content remains byte-for-byte consistent when no substantive changes are made.

• Tests

  • Added coverage for merges involving varying trailing-newline counts, blank lines, and template-only changes.

[atmos-pro]

Tip

Atmos Pro  

No affected stacks workflow was detected for this pull request.
If this is expected, no action is needed.
Learn More. Ask AI.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: ed3de7e4-bf49-4370-99fd-c3a0bf2dc82f

📥 Commits

Reviewing files that changed from the base of the PR and between ecfc8db and 1e9df24.

📒 Files selected for processing (2)

• pkg/generator/merge/text_merger.go
• pkg/generator/merge/text_merger_test.go

🚧 Files skipped from review as they are similar to previous changes (2)

• pkg/generator/merge/text_merger.go
• pkg/generator/merge/text_merger_test.go

---

📝 Walkthrough

Walkthrough

TextMerger.Merge now preserves trailing-newline counts by appending a newline to each diff3 input. Table-driven tests verify byte-exact output for unchanged and template-only merges.

Changes

Trailing-newline preservation

Layer / File(s)	Summary
Text merge and validation pkg/generator/merge/text_merger.go, pkg/generator/merge/text_merger_test.go	TextMerger.Merge appends a newline to each diff3 input. Tests cover missing, single, and multiple trailing newlines, blank lines, unchanged inputs, and template-only changes.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related issues

• #2886: The merge changes address reported blank-line collapse during scaffold updates.

Suggested labels: patch

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the fix for preserving trailing newlines in text-based three-way merges.
Linked Issues check	✅ Passed	The implementation and regression tests address issue #2887 by preserving trailing newlines during non-YAML text merges.
Out of Scope Changes check	✅ Passed	The code and test changes are limited to trailing-newline preservation in text-based three-way merges.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/generator/merge/text_merger_test.go`:
- Around line 820-839: Extend the merge test cases around the existing
genuine-change scenarios to cover template-only EOF newline changes: keep base
and ours identical, and verify theirs both adds and removes trailing newlines,
including “line 1\n” to “line 1\n\n” and “line 1”. Set each expected result to
preserve theirs’ newline count.
- Around line 756-761: Add a Go doc comment beginning with “Merge” immediately
before the exported TextMerger.Merge method, documenting its purpose and the
newline behavior referenced by TestTextMerger_TrailingNewlinePreservation. Keep
the existing test comment unchanged.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a8cfbfe6-71e9-4064-8143-67c721c93c00

📥 Commits

Reviewing files that changed from the base of the PR and between 3ce4349 and ecfc8db.

📒 Files selected for processing (2)

• pkg/generator/merge/text_merger.go
• pkg/generator/merge/text_merger_test.go