fix(wiki): canonicalize whitespace across a whole section, not just its ends
Review of PR #129 found MergeReport's disagreement check compared strings.TrimSpace(sA.content) against strings.TrimSpace(sB.content) -- only leading/trailing whitespace is tolerated. Every other content comparison in this file (appendNew, mergeBullets) already canonicalizes internal whitespace runs too, because a re-extraction's exact spacing isn't byte-stable (#103). A mid-line double-space would false-positive as a "disagreement" here even though the file's own established tolerance says it shouldn't. canonicalBlock applies the existing canonicalLine per line of a multi-line section body. A genuine content difference is still caught (TestMerge_ReportsSectionsWhereTheFreshVersionWasDiscarded stays green); only internal-whitespace-only variants stop false-positiving. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -84,7 +84,7 @@ func MergeReport(a, b Page) (Page, []string) {
|
|||||||
secsA[i].content = appendNew(sA.content, sB.content)
|
secsA[i].content = appendNew(sA.content, sB.content)
|
||||||
default:
|
default:
|
||||||
// a wins — but say that b disagreed, rather than dropping it mute.
|
// a wins — but say that b disagreed, rather than dropping it mute.
|
||||||
if strings.TrimSpace(sA.content) != strings.TrimSpace(sB.content) {
|
if canonicalBlock(sA.content) != canonicalBlock(sB.content) {
|
||||||
conflicts = append(conflicts, sB.heading)
|
conflicts = append(conflicts, sB.heading)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -329,6 +329,21 @@ func canonicalLine(line string) string {
|
|||||||
return strings.Join(strings.Fields(line), " ")
|
return strings.Join(strings.Fields(line), " ")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// canonicalBlock applies canonicalLine per line of a multi-line section body
|
||||||
|
// — the same whitespace-insensitivity appendNew/mergeBullets already give a
|
||||||
|
// single line, extended to a whole section for MergeReport's disagreement
|
||||||
|
// check (found in review, PR #129: comparing raw strings.TrimSpace only
|
||||||
|
// tolerated leading/trailing whitespace, not a re-extraction's mid-line
|
||||||
|
// spacing, which every other comparison in this file already tolerates).
|
||||||
|
func canonicalBlock(block string) string {
|
||||||
|
lines := strings.Split(block, "\n")
|
||||||
|
out := make([]string, len(lines))
|
||||||
|
for i, l := range lines {
|
||||||
|
out[i] = canonicalLine(l)
|
||||||
|
}
|
||||||
|
return strings.Join(out, "\n")
|
||||||
|
}
|
||||||
|
|
||||||
// appendNew concatenates the lines of b that a does not already carry.
|
// appendNew concatenates the lines of b that a does not already carry.
|
||||||
//
|
//
|
||||||
// Append sections used to concatenate wholesale, which is correct exactly once.
|
// Append sections used to concatenate wholesale, which is correct exactly once.
|
||||||
|
|||||||
@@ -385,3 +385,32 @@ Appetite is fixed, scope is variable.
|
|||||||
_, conflicts := MergeReport(page, page)
|
_, conflicts := MergeReport(page, page)
|
||||||
assert.Empty(t, conflicts, "identical content is not a disagreement")
|
assert.Empty(t, conflicts, "identical content is not a disagreement")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestMerge_ReportsNothingForInternalWhitespaceVariantsOnly(t *testing.T) {
|
||||||
|
// Found in review of PR #129: the conflict comparison used raw
|
||||||
|
// strings.TrimSpace, which only tolerates LEADING/TRAILING whitespace —
|
||||||
|
// unlike appendNew/mergeBullets, which already canonicalize internal
|
||||||
|
// whitespace runs too (#103: a re-extraction's spacing isn't guaranteed
|
||||||
|
// byte-stable). A mid-line double-space from re-extraction would
|
||||||
|
// false-positive as a "disagreement" here even though every other
|
||||||
|
// whitespace-sensitive comparison in this file already tolerates it.
|
||||||
|
existing := Page{Path: "wiki/sources/x.md", Content: `---
|
||||||
|
title: 'X'
|
||||||
|
---
|
||||||
|
|
||||||
|
## Summary
|
||||||
|
|
||||||
|
Appetite is fixed, scope is variable.
|
||||||
|
`}
|
||||||
|
fresh := Page{Path: "wiki/sources/x.md", Content: `---
|
||||||
|
title: 'X'
|
||||||
|
---
|
||||||
|
|
||||||
|
## Summary
|
||||||
|
|
||||||
|
Appetite is fixed, scope is variable.
|
||||||
|
`}
|
||||||
|
|
||||||
|
_, conflicts := MergeReport(existing, fresh)
|
||||||
|
assert.Empty(t, conflicts, "internal whitespace differences alone are not a disagreement")
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user