From 658bae75f9714af212b463dbba6bb2de565112f9 Mon Sep 17 00:00:00 2001 From: Eugene Blikh Date: Sat, 25 Jul 2026 14:43:46 +0300 Subject: [PATCH] feat(prosediff): recover the source line each word edit sits on (spec-by6.3.5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The review UI is moving to a line-numbered unified diff, which needs to know which line a word-level change happened on. The differ does not keep that. Tokenize drops whitespace — "\n" and " " both collapse to Token.Space — and that is precisely what makes a rewrapped paragraph produce a byte-identical token stream and therefore no diff at all. The property is load-bearing, so the line is recovered here rather than retained there. It is recoverable because the script is ordered: the equal and deleted runs reproduce the old block's tokens in sequence, and the equal and inserted ones the new block's. Walking each side in step with that side's re-tokenized lines says which line every token belongs to, and a run crossing a line break is cut at the boundary. The script supplies only the operation per token; text and spacing come from re-tokenizing the source line. Taking text from the spans instead drops separators — Span.Space is false on an insertion that directly replaces a deletion, because in a combined rendering the deletion before it carried the space, and split onto one side that deletion is gone. Caught by a test: "delta CHANGED zeta" rendered as "deltaCHANGED zeta". Reports ok=false rather than guessing when a block's Lines and Text disagree about token count. A caller that cannot split falls back to rendering the block as one old/new pair labelled by line range: a wrong line number is worse than an honest range, because it invites a comment onto text that was never there. spec-by6.3.5 --- prosediff/wordlines.go | 116 ++++++++++++++++++++++ prosediff/wordlines_test.go | 192 ++++++++++++++++++++++++++++++++++++ 2 files changed, 308 insertions(+) create mode 100644 prosediff/wordlines.go create mode 100644 prosediff/wordlines_test.go diff --git a/prosediff/wordlines.go b/prosediff/wordlines.go new file mode 100644 index 0000000000000000000000000000000000000000..3ea9d259915cab335bb9fcd5dbb91e2caf3877c1 --- /dev/null +++ b/prosediff/wordlines.go @@ -0,0 +1,116 @@ +package prosediff + +import "strings" + +// LineWords is one source line of a modified block, with the part of the +// block's word edit script that falls on it. +type LineWords struct { + // Line is the 1-based source line number, in the revision this side of the + // diff came from. + Line int + // Spans is the slice of the block's edit script covering this line. A span + // that straddled a line break is split, so every span here belongs wholly + // to this line. + Spans []Span +} + +// WordsByLine attributes a modified block's word-level edit script to the +// source lines it came from: the old side keeps equal and deleted runs, the new +// side keeps equal and inserted ones. +// +// A word diff has no line information in it, and that is deliberate. [Tokenize] +// drops whitespace — "\n" and " " both collapse to Token.Space — which is what +// makes a rewrapped paragraph produce a byte-identical token stream and +// therefore no diff at all. The cost of that property is this function: to draw +// a line-numbered diff, the line each word sat on has to be recovered rather +// than read off. +// +// It is recoverable because the script is ordered. The spans carrying equal and +// deleted text reproduce the old block's tokens in sequence, and the equal and +// inserted ones reproduce the new block's; walking each side in step with that +// side's re-tokenized lines says which line every token belongs to. +// +// ok is false when a side's lines do not tokenize to the same sequence length +// the script consumed. That means the block's Lines and Text disagree, and the +// caller should fall back to rendering the block as one old/new pair labelled +// by line range — a wrong line number is worse than an honest range, because it +// invites a comment onto text that was never there. +func WordsByLine(c BlockChange) (old, nw []LineWords, ok bool) { + if c.Kind != ChangeModify || len(c.Words) == 0 || c.Old == nil || c.New == nil { + return nil, nil, false + } + old, ok = spread(c.Old, c.Words, OpDelete) + if !ok { + return nil, nil, false + } + nw, ok = spread(c.New, c.Words, OpInsert) + if !ok { + return nil, nil, false + } + return old, nw, true +} + +// spread walks one side of the edit script — the equal runs plus the runs of +// side (OpDelete for the old side, OpInsert for the new) — and re-cuts it along +// the block's own lines. +// +// The script supplies only the operation per token; the text and its spacing +// come from re-tokenizing each source line. Taking the text from the spans +// instead would drop separators: Span.Space is false on an insertion that +// directly replaces a deletion, because in a combined rendering the deletion +// before it already carried the space. Split onto one side that deletion is +// gone, and "delta CHANGED zeta" renders as "deltaCHANGED zeta". The line's own +// tokens carry the spacing that was actually written, so they are the authority. +func spread(blk *Block, script []Span, side Op) ([]LineWords, bool) { + // One op per token of this side, in order. + var ops []Op + for _, sp := range script { + if sp.Op != OpEqual && sp.Op != side { + continue + } + for range Tokenize(sp.Text) { + ops = append(ops, sp.Op) + } + } + + out := make([]LineWords, len(blk.Lines)) + at := 0 + for i, ln := range blk.Lines { + out[i] = LineWords{Line: blk.StartLine + i} + toks := Tokenize(ln) + for j := 0; j < len(toks); { + if at >= len(ops) { + return nil, false // the lines hold more tokens than the script + } + // Group the run of following tokens sharing this token's op. + op, k := ops[at], j + for k < len(toks) && at+(k-j) < len(ops) && ops[at+(k-j)] == op { + k++ + } + out[i].Spans = append(out[i].Spans, Span{ + Op: op, + Text: joinTokens(toks[j:k]), + Space: toks[j].Space, + }) + at += k - j + j = k + } + } + if at != len(ops) { + return nil, false // the script holds more tokens than the lines + } + return out, true +} + +// joinTokens rebuilds text from a run of tokens, restoring the single space +// each token records as having preceded it. +func joinTokens(toks []Token) string { + var b strings.Builder + for i, t := range toks { + if i > 0 && t.Space { + b.WriteByte(' ') + } + b.WriteString(t.Text) + } + return b.String() +} diff --git a/prosediff/wordlines_test.go b/prosediff/wordlines_test.go new file mode 100644 index 0000000000000000000000000000000000000000..925ea5b892ddb769b5e76115802dbf9dbf639c1a --- /dev/null +++ b/prosediff/wordlines_test.go @@ -0,0 +1,192 @@ +package prosediff + +import ( + "strings" + "testing" +) + +// modifyOf returns the single ChangeModify in a diff of two documents, failing +// when the alignment did not produce one — the tests below are about splitting +// a word script, not about whether the aligner paired the blocks. +func modifyOf(t *testing.T, oldSrc, newSrc string) BlockChange { + t.Helper() + for _, c := range Compare([]byte(oldSrc), []byte(newSrc)).Changes { + if c.Kind == ChangeModify && len(c.Words) > 0 { + return c + } + } + t.Fatalf("no modified prose block in the diff of:\n%s\n---\n%s", oldSrc, newSrc) + return BlockChange{} +} + +// renderLines renders one side back to plain strings, per line, so a test can assert +// on what a reader would see. +func renderLines(rows []LineWords) []string { + out := make([]string, len(rows)) + for i, r := range rows { + var b strings.Builder + for j, s := range r.Spans { + if s.Space && j > 0 { + b.WriteByte(' ') + } + b.WriteString(s.Text) + } + out[i] = b.String() + } + return out +} + +// The words of each line must come back on that line, with the source line +// numbers the block actually occupies. Without this the gutter would number +// rows that hold someone else's text. +func TestWordsByLineKeepsEachLinesOwnWords(t *testing.T) { + const oldSrc = "# H\n\nalpha beta gamma\ndelta epsilon zeta\neta theta iota\n" + const newSrc = "# H\n\nalpha beta gamma\ndelta CHANGED zeta\neta theta iota\n" + + old, nw, ok := WordsByLine(modifyOf(t, oldSrc, newSrc)) + if !ok { + t.Fatal("WordsByLine reported the block unsplittable") + } + if len(old) != 3 || len(nw) != 3 { + t.Fatalf("rows = %d old / %d new, want 3 each", len(old), len(nw)) + } + + wantOld := []string{"alpha beta gamma", "delta epsilon zeta", "eta theta iota"} + wantNew := []string{"alpha beta gamma", "delta CHANGED zeta", "eta theta iota"} + for i := range wantOld { + if got := renderLines(old)[i]; got != wantOld[i] { + t.Errorf("old line %d = %q, want %q", i, got, wantOld[i]) + } + if got := renderLines(nw)[i]; got != wantNew[i] { + t.Errorf("new line %d = %q, want %q", i, got, wantNew[i]) + } + } + + // Line numbers are the block's own, not indices into the slice. + for i, r := range old { + if r.Line != old[0].Line+i { + t.Errorf("old row %d numbered %d, want %d", i, r.Line, old[0].Line+i) + } + } +} + +// The change must be marked on the line that carries it and nowhere else, +// which is the whole point of splitting rather than marking the block. +func TestTheChangeLandsOnOneLine(t *testing.T) { + const oldSrc = "# H\n\nalpha beta gamma\ndelta epsilon zeta\neta theta iota\n" + const newSrc = "# H\n\nalpha beta gamma\ndelta CHANGED zeta\neta theta iota\n" + + old, nw, ok := WordsByLine(modifyOf(t, oldSrc, newSrc)) + if !ok { + t.Fatal("unsplittable") + } + marked := func(rows []LineWords, op Op) []int { + var hit []int + for i, r := range rows { + for _, s := range r.Spans { + if s.Op == op { + hit = append(hit, i) + break + } + } + } + return hit + } + if got := marked(old, OpDelete); len(got) != 1 || got[0] != 1 { + t.Errorf("deletions on old rows %v, want only row 1", got) + } + if got := marked(nw, OpInsert); len(got) != 1 || got[0] != 1 { + t.Errorf("insertions on new rows %v, want only row 1", got) + } +} + +// An unchanged run spanning a line break is cut at the boundary rather than +// dumped whole onto the line it started on. This is the common case in real +// prose: one long equal run covers most of a paragraph, and if it were not +// split every wrapped paragraph would collapse onto its first line. +func TestARunIsSplitAtTheLineBreak(t *testing.T) { + const oldSrc = "# H\n\none two three four\nfive six seven eight\nnine ten\n" + const newSrc = "# H\n\none two three four\nfive six seven eight\nnine ELEVEN\n" + + old, nw, ok := WordsByLine(modifyOf(t, oldSrc, newSrc)) + if !ok { + t.Fatal("unsplittable") + } + for name, side := range map[string][]LineWords{"old": old, "new": nw} { + if len(side) != 3 { + t.Fatalf("%s rows = %d, want 3", name, len(side)) + } + for i, r := range side { + if len(r.Spans) == 0 { + t.Errorf("%s row %d got no spans; the equal run was not split", name, i) + } + } + } + if got := renderLines(old); got[0] != "one two three four" || got[2] != "nine ten" { + t.Errorf("old = %q", got) + } + if got := renderLines(nw); got[0] != "one two three four" || got[2] != "nine ELEVEN" { + t.Errorf("new = %q", got) + } +} + +// Reflow is invisible to the word differ by design, so a rewrapped paragraph +// with a real edit still splits — onto the NEW line structure, which is what a +// reader of the new revision sees. +func TestRewrappedParagraphStillSplits(t *testing.T) { + const oldSrc = "# H\n\nthe quick brown fox\njumps over the lazy dog\n" + const newSrc = "# H\n\nthe quick brown fox jumps\nover the SLEEPY dog\n" + + old, nw, ok := WordsByLine(modifyOf(t, oldSrc, newSrc)) + if !ok { + t.Fatal("unsplittable") + } + if got := strings.Join(renderLines(old), "|"); got != "the quick brown fox|jumps over the lazy dog" { + t.Errorf("old = %q, want the old wrapping", got) + } + if got := strings.Join(renderLines(nw), "|"); got != "the quick brown fox jumps|over the SLEEPY dog" { + t.Errorf("new = %q, want the new wrapping", got) + } +} + +// Anything that is not a modified prose block has no word script to spread, +// and says so rather than returning empty rows a caller might render. +func TestWordsByLineRefusesWhatItCannotSplit(t *testing.T) { + d := Compare([]byte("# H\n\nkept\n"), []byte("# H\n\nkept\n\nadded paragraph\n")) + for _, c := range d.Changes { + if c.Kind == ChangeModify && len(c.Words) > 0 { + continue + } + if _, _, ok := WordsByLine(c); ok { + t.Errorf("%s block reported splittable", c.Kind) + } + } +} + +// Every token of every line is accounted for, on both sides. A dropped token +// would silently delete text from the rendered diff. +func TestNoTokenIsLost(t *testing.T) { + const oldSrc = "# H\n\nalpha beta gamma\ndelta epsilon zeta\n" + const newSrc = "# H\n\nalpha BETA gamma\ndelta epsilon ZETA\n" + + c := modifyOf(t, oldSrc, newSrc) + old, nw, ok := WordsByLine(c) + if !ok { + t.Fatal("unsplittable") + } + count := func(rows []LineWords) int { + n := 0 + for _, r := range rows { + for _, s := range r.Spans { + n += len(Tokenize(s.Text)) + } + } + return n + } + if got, want := count(old), len(Tokenize(c.Old.Text)); got != want { + t.Errorf("old side carries %d tokens, the block has %d", got, want) + } + if got, want := count(nw), len(Tokenize(c.New.Text)); got != want { + t.Errorf("new side carries %d tokens, the block has %d", got, want) + } +}