Skip to content

Commit ce49c77

Browse files
jedudenclaude
andauthored
fix(merge-driver): emit merge=text instead of -merge for ignore-derived exclusions (#761)
* fix(merge-driver): emit merge=text for ignore-derived exclusions The .gitattributes managed block wrote ignore-derived Markdown exclusions as `-merge`, which unsets git's merge attribute. Per git-attributes, an unset merge declares a file binary with no well-defined merge semantics: git takes the current branch and declares a conflict, so two branches editing different sections of an ignored Markdown file conflict and cannot auto-resolve. Markdown has well-defined text-merge semantics, and an ignored path is never linted, so it carries no generated sections for the mdsmith driver to reconcile. Emit `merge=text` (git's built-in 3-way text merge) instead. The mdsmith driver still stays off the path (last-match-wins over the include lines), and ordinary edits merge normally. ExtractGlobs now recognizes both `merge=text` (current) and `-merge` (legacy) as exclusions, so a .gitattributes written by an older mdsmith round-trips unchanged and does not read as drift. The committed .gitattributes is deliberately left in `-merge` form: the merge queue verifies it with the pinned v0.53.0 baseline via `ci-install`, which does not yet parse `merge=text`. Regenerating it is a follow-up gated on bumping that pin, per docs/development/adopt-new-directive-syntax.md. Fixes #755 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W8S3VduddQAbq8FPduzFyf * docs(githooks): sweep last stale -merge reference in scopeExcludeToMarkdown Code-review follow-up: the merge=text change updated every other reference, but scopeExcludeToMarkdown's doc comment still said the already-markdown branch returns "its -merge line". The emitted form is now merge=text; call it an "exclude line" to match. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W8S3VduddQAbq8FPduzFyf --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent d4af5d5 commit ce49c77

5 files changed

Lines changed: 136 additions & 48 deletions

File tree

cmd/mdsmith/mergedriver.go

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,14 +36,18 @@ Subcommands:
3636
exclude lines for each ignore pattern (last-match-wins
3737
overrides). Each ignore pattern is intersected with the
3838
markdown include extensions, so an ignore of demo/** emits
39-
demo/**/*.md -merge and demo/**/*.markdown -merge rather
40-
than a bare demo/** -merge that would also disable git's
41-
merge for non-markdown files in that tree.
39+
demo/**/*.md merge=text and demo/**/*.markdown merge=text
40+
rather than a bare demo/** merge=text that would also change
41+
git's merge for non-markdown files in that tree. The
42+
excludes use merge=text (git's built-in 3-way text merge),
43+
not -merge (merge unset): both take the mdsmith driver off
44+
the path, but -merge leaves these Markdown files binary-
45+
conflicting while merge=text keeps ordinary edits merging.
4246
4347
Optional positional args replace the default include set
4448
when callers want to scope the merge driver to a custom
4549
pattern (e.g. docs/**/*.md); .mdsmith.yml ignore
46-
patterns still apply on top via -merge overrides. Custom
50+
patterns still apply on top via merge=text overrides. Custom
4751
include globs are not compatible with the MDS048
4852
git-hook-sync rule's auto-fix, which restores the
4953
canonical default include set plus ignore-derived
@@ -473,8 +477,8 @@ func hasConflictMarkers(content []byte) bool {
473477
// (`*.md`, `*.markdown`) is used and the project's .mdsmith.yml
474478
// `ignore:` patterns become markdown-scoped exclude overrides —
475479
// each pattern is intersected with the markdown include extensions
476-
// (see githooks.GlobsFromConfig) so a `-merge` line never disables
477-
// git's merge for non-markdown files. Patterns that cannot appear
480+
// (see githooks.GlobsFromConfig) so a `merge=text` line never
481+
// changes git's merge for non-markdown files. Patterns that cannot appear
478482
// verbatim in `.gitattributes` (whitespace, leading `!`) are
479483
// silently dropped by `GlobsFromConfig`. Explicit args replace the
480484
// include set so callers can scope the merge driver to a custom
@@ -604,7 +608,7 @@ func runMergeDriverCIInstall(args []string) int {
604608
// Compute the expected glob set exactly as install would write it,
605609
// so the two commands can never disagree on what "in sync" means.
606610
// nil args means the canonical default include set plus the
607-
// .mdsmith.yml ignore-derived -merge overrides.
611+
// .mdsmith.yml ignore-derived merge=text overrides.
608612
expected, rc := resolveManagedGlobs(repoRoot, nil)
609613
if rc != 0 {
610614
return rc

cmd/mdsmith/mergedriver_test.go

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,7 @@ func TestRunMergeDriverInstall_RejectsWhitespacePath(t *testing.T) {
295295
func TestRunMergeDriverInstall_NoArgsWritesCanonicalGlobs(t *testing.T) {
296296
dir := t.TempDir()
297297
initTestRepo(t, dir)
298-
// .mdsmith.yml ignore patterns become -merge overrides.
298+
// .mdsmith.yml ignore patterns become merge=text overrides.
299299
require.NoError(t, os.WriteFile(filepath.Join(dir, ".mdsmith.yml"),
300300
[]byte("ignore:\n - \"vendor/**\"\n"), 0o644))
301301

@@ -318,12 +318,14 @@ func TestRunMergeDriverInstall_NoArgsWritesCanonicalGlobs(t *testing.T) {
318318
content := string(attrs)
319319
assert.Contains(t, content, "*.md merge=mdsmith")
320320
assert.Contains(t, content, "*.markdown merge=mdsmith")
321-
assert.Contains(t, content, "vendor/**/*.md -merge",
322-
"ignore patterns from .mdsmith.yml must appear as markdown-scoped -merge overrides")
323-
assert.Contains(t, content, "vendor/**/*.markdown -merge",
321+
assert.Contains(t, content, "vendor/**/*.md merge=text",
322+
"ignore patterns from .mdsmith.yml must appear as markdown-scoped merge=text overrides")
323+
assert.Contains(t, content, "vendor/**/*.markdown merge=text",
324324
"ignore patterns are scoped to every markdown include extension")
325-
assert.NotContains(t, content, "vendor/** -merge",
326-
"a bare -merge line would disable git's merge for non-markdown files (issue #750)")
325+
assert.NotContains(t, content, "vendor/** merge=text",
326+
"a bare exclude line would change git's merge for non-markdown files (issue #750)")
327+
assert.NotContains(t, content, "-merge",
328+
"excludes use merge=text, not -merge, so ignored Markdown still gets git's text merge (issue #755)")
327329
}
328330

329331
// --- resolveInstalledBinary ---
@@ -777,9 +779,9 @@ func TestRunMergeDriverInstall_DropsAndWarnsForUnrepresentableIgnore(t *testing.
777779
attrs, err := os.ReadFile(filepath.Join(dir, ".gitattributes"))
778780
require.NoError(t, err)
779781
content := string(attrs)
780-
assert.Contains(t, content, "vendor/**/*.md -merge",
782+
assert.Contains(t, content, "vendor/**/*.md merge=text",
781783
"representable ignore patterns survive, scoped to markdown")
782-
assert.Contains(t, content, "vendor/**/*.markdown -merge",
784+
assert.Contains(t, content, "vendor/**/*.markdown merge=text",
783785
"representable ignore patterns survive, scoped to every markdown extension")
784786
assert.NotContains(t, content, "with space.md",
785787
"unrepresentable ignore patterns are dropped from the managed block")

docs/reference/cli/merge-driver.md

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,25 @@ Register the merge driver in `git config` and ensure
2323
`.gitattributes` assigns it. The managed block is derived
2424
from `.mdsmith.yml`. It lists include patterns first
2525
(default `*.md` and `*.markdown`). Then it lists one
26-
`-merge` exclude line per representable `ignore:` pattern.
27-
Last match wins.
26+
`merge=text` exclude line per representable `ignore:`
27+
pattern. Last match wins.
28+
29+
The exclude lines use `merge=text`, git's built-in 3-way
30+
text merge, not `-merge`. Both take the mdsmith driver off
31+
the path, but `-merge` unsets the `merge` attribute — which
32+
declares the file binary and leaves it whole-file-conflicting.
33+
An ignored path is Markdown that mdsmith never lints, so it
34+
carries no generated sections for the driver to reconcile;
35+
`merge=text` keeps ordinary edits merging and avoids that
36+
class of conflict. A `.gitattributes` written by an older
37+
mdsmith that still lists `-merge` is read the same way, so
38+
it does not report as drift until it is regenerated.
2839

2940
Each `ignore:` pattern is scoped to the markdown include
3041
extensions. So `ignore: ["demo/**"]` emits
31-
`demo/**/*.md -merge` and `demo/**/*.markdown -merge`. A
32-
bare `demo/** -merge` is not used. It would also turn off
33-
git's 3-way merge for the source code in that tree.
42+
`demo/**/*.md merge=text` and `demo/**/*.markdown merge=text`.
43+
A bare `demo/** merge=text` is not used. It would also change
44+
git's merge for the source code in that tree.
3445

3546
Some `ignore:` entries cannot be expressed in
3647
`.gitattributes`: `!` negation patterns and patterns with

internal/githooks/githooks.go

Lines changed: 55 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -170,17 +170,24 @@ const configFileName = ".mdsmith.yml"
170170
// set's markdown extensions before it becomes an exclude line, so a
171171
// coarse directory ignore like `demo/**` emits
172172
//
173-
// demo/**/*.md -merge
174-
// demo/**/*.markdown -merge
173+
// demo/**/*.md merge=text
174+
// demo/**/*.markdown merge=text
175175
//
176-
// rather than a bare `demo/** -merge`. The `ignore:` list scopes the
177-
// markdown linter (files: is *.md / *.markdown), so an ignore pattern
178-
// is only ever meant to affect markdown; a bare `-merge` line, by
179-
// contrast, is not extension-scoped and would disable git's 3-way
180-
// merge for every file in the tree — including source code that
176+
// rather than a bare `demo/** merge=text`. The `ignore:` list scopes
177+
// the markdown linter (files: is *.md / *.markdown), so an ignore
178+
// pattern is only ever meant to affect markdown; a bare exclude line,
179+
// by contrast, is not extension-scoped and would change git's merge
180+
// behaviour for every file in the tree — including source code that
181181
// nobody asked to grandfather (issue #750). scopeExcludeToMarkdown
182182
// guarantees every emitted exclude ends in a markdown extension.
183183
//
184+
// The exclude lines carry `merge=text` (git's built-in 3-way text
185+
// merge), not `-merge` (merge unset). Both take the mdsmith driver out
186+
// of the picture for the path, but `-merge` also forfeits git's text
187+
// merge and leaves the file binary-conflicting; the ignore-derived
188+
// paths are Markdown with no generated sections, so `merge=text` is
189+
// the correct, conflict-avoiding fallback (issue #755).
190+
//
184191
// Patterns that cannot be represented directly in .gitattributes
185192
// are dropped from the exclude set so MDS048's auto-fix never
186193
// produces a broken managed block:
@@ -217,12 +224,14 @@ func GlobsFromConfig(cfg *config.Config) (Globs, []string) {
217224
}
218225

219226
// scopeExcludeToMarkdown rewrites one .mdsmith.yml ignore pattern into
220-
// the .gitattributes exclude patterns that turn off the mdsmith merge
221-
// driver for the Markdown files the pattern grandfathers — and only
222-
// those files (issue #750).
227+
// the .gitattributes exclude patterns that fall back to git's built-in
228+
// text merge (merge=text) for the Markdown files the pattern
229+
// grandfathers — and only those files (issue #750). The returned
230+
// patterns are the path field; RenderManagedBlock appends the
231+
// `merge=text` attribute.
223232
//
224233
// A pattern that already ends in one of exts targets a specific
225-
// Markdown extension, so its -merge line can only affect Markdown; it
234+
// Markdown extension, so its exclude line can only affect Markdown; it
226235
// is returned unchanged. Otherwise the pattern is treated as a path
227236
// scope and one exclude is emitted per extension, keyed on the final
228237
// path segment:
@@ -233,7 +242,7 @@ func GlobsFromConfig(cfg *config.Config) (Globs, []string) {
233242
// - `dir` -> `dir/**/*.md`, `dir/**/*.markdown` (name as a tree)
234243
//
235244
// Every branch appends a Markdown extension to the emitted pattern, so
236-
// the invariant "a derived -merge line never matches a non-Markdown
245+
// the invariant "a derived exclude line never matches a non-Markdown
237246
// file" holds for any input.
238247
func scopeExcludeToMarkdown(pattern string, exts []string) []string {
239248
for _, ext := range exts {
@@ -664,20 +673,26 @@ func findManagedBlockLines(lines []string) (int, int) {
664673
// Globs describes the set of paths the mdsmith merge driver applies
665674
// to. Each Include pattern is written as `<pattern> merge=mdsmith`
666675
// and each Exclude pattern is written after them as `<pattern>
667-
// -merge`. .gitattributes uses last-match-wins, so an exclude line
668-
// after the include lines effectively removes the merge driver from
669-
// any path the include patterns matched.
676+
// merge=text`. .gitattributes uses last-match-wins, so an exclude line
677+
// after the include lines effectively removes the mdsmith merge driver
678+
// from any path the include patterns matched, falling back to git's
679+
// built-in 3-way text merge.
670680
//
671681
// Exclude patterns derived from .mdsmith.yml ignore entries are
672-
// markdown-scoped (see GlobsFromConfig): a `-merge` line only ever
673-
// matches a markdown file, so it disables the mdsmith driver for
682+
// markdown-scoped (see GlobsFromConfig): a `merge=text` line only ever
683+
// matches a markdown file, so it takes the mdsmith driver off
674684
// grandfathered markdown without touching git's default merge for
675-
// non-markdown files in the same tree (issue #750).
685+
// non-markdown files in the same tree (issue #750). Emitting
686+
// `merge=text` rather than `-merge` also keeps git's text merge for
687+
// those files instead of declaring them binary-conflicting (#755).
688+
//
689+
// ExtractGlobs still reads the legacy `-merge` exclude form so a
690+
// `.gitattributes` written by an older mdsmith round-trips unchanged.
676691
//
677692
// `.gitattributes` itself does not support negative patterns (`!*.md`
678-
// is a syntax error there). Order-sensitive override via -merge is the
679-
// supported way to express exclusions, which is why Globs keeps
680-
// Include and Exclude as separate ordered slices.
693+
// is a syntax error there). Order-sensitive override via a trailing
694+
// exclude line is the supported way to express exclusions, which is
695+
// why Globs keeps Include and Exclude as separate ordered slices.
681696
type Globs struct {
682697
Include []string
683698
Exclude []string
@@ -704,7 +719,17 @@ func RenderManagedBlock(globs Globs) string {
704719
fmt.Fprintf(&b, "%s merge=mdsmith\n", p)
705720
}
706721
for _, p := range globs.Exclude {
707-
fmt.Fprintf(&b, "%s -merge\n", p)
722+
// Emit `merge=text`, not `-merge`. Both turn the mdsmith driver
723+
// off for the path (last-match-wins over the include lines), but
724+
// `-merge` also unsets git's built-in merge — declaring the file
725+
// has no well-defined merge semantics, which is only correct for
726+
// binaries and leaves these Markdown files binary-conflicting.
727+
// `merge=text` selects git's built-in 3-way text merge instead.
728+
// The excludes are ignore-derived Markdown paths (see
729+
// GlobsFromConfig): they are never linted, so they carry no
730+
// generated sections for the driver to reconcile, and the text
731+
// merge is strictly safe (issue #755).
732+
fmt.Fprintf(&b, "%s merge=text\n", p)
708733
}
709734
b.WriteString(gitattributesManagedBlockEnd)
710735
b.WriteString("\n")
@@ -740,7 +765,14 @@ func ExtractGlobs(content string) (Globs, bool) {
740765
switch attr {
741766
case "merge=mdsmith":
742767
globs.Include = append(globs.Include, pattern)
743-
case "-merge":
768+
case "merge=text", "-merge":
769+
// `merge=text` is the current exclude form; `-merge` is
770+
// the legacy form still found in blocks written by an
771+
// older mdsmith (or the pinned CI baseline). Both mean
772+
// "turn the mdsmith driver off for this path", so both
773+
// map to an exclude — an unmigrated `-merge` block then
774+
// extracts to the same glob set as a freshly rendered
775+
// `merge=text` one and does not read as drift (#755).
744776
globs.Exclude = append(globs.Exclude, pattern)
745777
default:
746778
continue

internal/githooks/githooks_test.go

Lines changed: 44 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1117,13 +1117,50 @@ func TestRenderManagedBlock_IncludeAndExclude(t *testing.T) {
11171117
Include: []string{"*.md", "*.markdown"},
11181118
Exclude: []string{"demo/**", "vendor/*.md"},
11191119
})
1120+
// Excludes are emitted as `merge=text`, not `-merge`: the ignore-
1121+
// derived paths are Markdown (well-defined text-merge semantics) and
1122+
// carry no generated sections, so falling back to git's built-in
1123+
// 3-way text merge avoids the binary-style conflicts `-merge` forces
1124+
// (issue #755). Last-match-wins still turns the mdsmith driver off.
11201125
expected := "# BEGIN mdsmith merge-driver\n" +
11211126
"*.md merge=mdsmith\n" +
11221127
"*.markdown merge=mdsmith\n" +
1128+
"demo/** merge=text\n" +
1129+
"vendor/*.md merge=text\n" +
1130+
"# END mdsmith merge-driver\n"
1131+
assert.Equal(t, expected, got)
1132+
}
1133+
1134+
func TestExtractGlobs_ParsesMergeTextExcludes(t *testing.T) {
1135+
// The current render form: excludes carry `merge=text`. ExtractGlobs
1136+
// must read them back as excludes so a freshly written block round-
1137+
// trips and drift detection does not report perpetual drift.
1138+
content := "# BEGIN mdsmith merge-driver\n" +
1139+
"*.md merge=mdsmith\n" +
1140+
"demo/** merge=text\n" +
1141+
"vendor/*.md merge=text\n" +
1142+
"# END mdsmith merge-driver\n"
1143+
got, ok := ExtractGlobs(content)
1144+
require.True(t, ok)
1145+
assert.Equal(t, []string{"*.md"}, got.Include)
1146+
assert.Equal(t, []string{"demo/**", "vendor/*.md"}, got.Exclude)
1147+
}
1148+
1149+
func TestExtractGlobs_ParsesLegacyDashMergeExcludes(t *testing.T) {
1150+
// Backward compatibility: a `.gitattributes` written by an older
1151+
// mdsmith (or by the pinned CI baseline) uses `-merge` for excludes.
1152+
// ExtractGlobs must still read those as excludes so an unmigrated
1153+
// committed block extracts to the same glob set as the merge=text
1154+
// form and does not read as drift.
1155+
content := "# BEGIN mdsmith merge-driver\n" +
1156+
"*.md merge=mdsmith\n" +
11231157
"demo/** -merge\n" +
11241158
"vendor/*.md -merge\n" +
11251159
"# END mdsmith merge-driver\n"
1126-
assert.Equal(t, expected, got)
1160+
got, ok := ExtractGlobs(content)
1161+
require.True(t, ok)
1162+
assert.Equal(t, []string{"*.md"}, got.Include)
1163+
assert.Equal(t, []string{"demo/**", "vendor/*.md"}, got.Exclude)
11271164
}
11281165

11291166
func TestRenderManagedBlock_EmptyGlobs(t *testing.T) {
@@ -1165,8 +1202,9 @@ func TestExtractGlobs_IgnoresCommentsAndBlankLinesInBlock(t *testing.T) {
11651202
}
11661203

11671204
func TestExtractGlobs_IgnoresUnknownAttributes(t *testing.T) {
1168-
// A line inside the managed block that is not a merge=mdsmith
1169-
// or -merge assignment must be ignored, not counted as a glob.
1205+
// A line inside the managed block that is not a merge=mdsmith,
1206+
// merge=text, or -merge assignment must be ignored, not counted
1207+
// as a glob.
11701208
content := "# BEGIN mdsmith merge-driver\n" +
11711209
"*.md merge=mdsmith\n" +
11721210
"*.txt text\n" +
@@ -1291,8 +1329,9 @@ func TestGlobsFromConfig_DropsUnrepresentablePatterns(t *testing.T) {
12911329

12921330
func TestExtractGlobs_SkipsSingleFieldLines(t *testing.T) {
12931331
// A managed-block line with only a pattern (no attribute) is
1294-
// not a valid merge=mdsmith or -merge assignment; ExtractGlobs
1295-
// must skip it instead of treating the lone token as a glob.
1332+
// not a valid merge=mdsmith, merge=text, or -merge assignment;
1333+
// ExtractGlobs must skip it instead of treating the lone token
1334+
// as a glob.
12961335
content := "# BEGIN mdsmith merge-driver\n" +
12971336
"orphan-token\n" +
12981337
"*.md merge=mdsmith\n" +

0 commit comments

Comments
 (0)