~bigbes/sr-ht-spec

d1621dca89d49b047348971fb9a682b232e19a26 — Eugene Blikh 13 days ago f42f81c
fix(prosediff): an equal run's separator comes from whichever side has one (spec-by6.5)

spans() read Span.Space off toks[0], and for an equal run those tokens come
from the old side. An equal run exists in both revisions at once, and the two
sides can disagree about what precedes it: a block that gains words at its head
has nothing before its first old token and the insertion before its first new
one. The script then said Space=false and a renderer joining the spans wrote
"{+in practice+}the storage layer" with the words run together — a defect the
reader would read as the author's, because nothing in the output says a
separator went missing.

Not reachable from the review page today: web/diffrows.go's sideSpans already
carries a dropped span's separator onto the next kept one, and the per-line
merge never joins two spans across that boundary. It was wrong in the data all
the same, and the next consumer of the script would have inherited it.

Passing the flag into emit rather than deriving it also drops the
write-back that reached into out[len(out)-1] to clear a substitution's
separator, which would have edited the wrong span had emit ever skipped an
empty run.
2 files changed, 49 insertions(+), 9 deletions(-)

M prosediff/token.go
M prosediff/token_test.go
M prosediff/token.go => prosediff/token.go +17 -9
@@ 199,7 199,15 @@ func deletesFirst(script []edit) []edit {
// spans walks an edit script and materializes it into renderable runs.
func spans(script []edit, a, b []Token) []Span {
	var out []Span
	emit := func(op Op, toks []Token) {
	// space is passed in rather than read off toks[0] because an equal run
	// exists on both sides at once and the two sides can disagree about the
	// separator in front of it. A block that gains words at its head has
	// Space=false on the old side's first token — nothing precedes it there —
	// and Space=true on the new side's, where the inserted words do. Reading
	// only the old side emitted "{+two words+}the rest" with the words run
	// together; a renderer has no way to recover the separator, because the new
	// side's flag never reached it.
	emit := func(op Op, toks []Token, space bool) {
		if len(toks) == 0 {
			return
		}


@@ 210,7 218,7 @@ func spans(script []edit, a, b []Token) []Span {
			}
			sb.WriteString(t.Text)
		}
		out = append(out, Span{Op: op, Text: sb.String(), Space: toks[0].Space})
		out = append(out, Span{Op: op, Text: sb.String(), Space: space})
	}

	i, j := 0, 0


@@ 220,23 228,23 @@ func spans(script []edit, a, b []Token) []Span {
		// the whitespace the deletion already carried, or every one-word
		// substitution renders as "[-old-] {+new+}".
		if e.op == OpInsert && k > 0 && script[k-1].op == OpDelete {
			emit(OpInsert, b[j:j+e.n])
			if len(out) > 0 {
				out[len(out)-1].Space = false
			}
			emit(OpInsert, b[j:j+e.n], false)
			j += e.n
			continue
		}
		switch e.op {
		case OpEqual:
			emit(OpEqual, a[i:i+e.n])
			// Either side's separator is reason enough to emit one: the run is
			// rendered once, between whatever precedes it on the old side and
			// whatever precedes it on the new.
			emit(OpEqual, a[i:i+e.n], a[i].Space || b[j].Space)
			i += e.n
			j += e.n
		case OpDelete:
			emit(OpDelete, a[i:i+e.n])
			emit(OpDelete, a[i:i+e.n], a[i].Space)
			i += e.n
		case OpInsert:
			emit(OpInsert, b[j:j+e.n])
			emit(OpInsert, b[j:j+e.n], b[j].Space)
			j += e.n
		}
	}

M prosediff/token_test.go => prosediff/token_test.go +32 -0
@@ 128,6 128,38 @@ func TestDiffWordsAbsorbsShredding(t *testing.T) {
	assert.Equal(t, "agents emit, operators approve", spans[1].Text)
}

// TestEqualRunAfterAHeadInsertionKeepsItsSeparator checks the one place the two
// sides of a diff disagree about whitespace. An equal run exists in both
// revisions at once; when words are inserted before it, the old side's first
// token has nothing in front of it and the new side's has the insertion. Reading
// only the old side dropped the separator, and a renderer joining the spans
// wrote "{+two words+}the rest" with no space — a defect the reader would read
// as the author's, since nothing in the output says a space went missing.
func TestEqualRunAfterAHeadInsertionKeepsItsSeparator(t *testing.T) {
	spans := DiffWords(
		"the storage layer keeps every revision",
		"in practice the storage layer keeps every revision",
	)
	require.Len(t, spans, 2)
	assert.Equal(t, OpInsert, spans[0].Op)
	assert.Equal(t, "in practice", spans[0].Text)
	assert.False(t, spans[0].Space, "nothing precedes the first span")
	assert.Equal(t, OpEqual, spans[1].Op)
	assert.True(t, spans[1].Space, "the inserted words are separated from the rest")
}

// TestSubstitutionStillCarriesNoDoubleSeparator guards the rule the fix above
// must not undo: an insertion directly replacing a deletion inherits the
// deletion's separator rather than announcing its own.
func TestSubstitutionStillCarriesNoDoubleSeparator(t *testing.T) {
	spans := DiffWords("the quick brown fox", "the quick red fox")
	require.Len(t, spans, 4)
	assert.Equal(t, OpDelete, spans[1].Op)
	assert.True(t, spans[1].Space)
	assert.Equal(t, OpInsert, spans[2].Op)
	assert.False(t, spans[2].Space)
}

func TestDiffLinesIsWhitespaceSensitive(t *testing.T) {
	old := []string{"func main() {", "\tfmt.Println(1)", "}"}
	nw := []string{"func main() {", "    fmt.Println(1)", "}"}