fix: render added blocks via markdown-safe sentinel path emitting cw-ins-* (heading/list structure preserved)
Defect: an ADDED block was colored via `render(wordDiffByAuthor("", raw, ...))`, which
wrapped the whole raw markdown in an inline `<ins>` BEFORE markdown-it parsed it — so
`## X` rendered as literal text, not `<h2>`. Same structural bug for list, blockquote,
and thematic-break blocks.
Fix (three-part, pure / vscode-free):
1. `blockContentStart(raw)`: new helper returning the offset past block-level markers
(ATX headings `## `, unordered/ordered list markers, blockquotes). Open sentinels must
not precede these markers or markdown-it cannot recognise the block construct.
2. `injectSentinels`: clamp the open-sentinel lower-bound to `blockContentStart(raw)`,
so for `## New Section` the sentinel lands AFTER `## ` (at position 3) instead of at
position 0, letting the heading parse correctly.
3. `sentinelsToSpans` / `colorByAuthor`: add optional `kind: "by" | "ins" = "by"`
parameter so the sentinel path can emit `cw-ins-{author}` for added blocks while all
existing `colorByAuthor`/`cw-by-*` call-sites are unchanged.
4. `renderReview` ADDED block path: `render(wordDiffByAuthor("", raw, ...))` →
`colorByAuthor(raw, blk.start, landedSpans, render, "ins")`. The CHANGED non-atomic
prose path (`wordDiffByAuthor(before, after, ...)`) is untouched — it correctly keeps
the block markers at column 0 of the unchanged prefix.
5. `renderReviewOp`: removed the now-vestigial `colored` parameter (was always `render`
after the added-block path moved to `changedHtml`); unchanged blocks now call
`render(op.block.raw)` directly.
Regression test added: "renderReview: an added heading block renders as a heading AND
is author-colored as an insertion" — RED on old code, GREEN after fix. All 259 tests
pass; typecheck clean.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
+40
-12
@@ -671,11 +671,38 @@ function clampOffDelimiterRun(raw: string, at: number): number {
|
|||||||
return p;
|
return p;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Returns the offset within `raw` after block-level markdown markers (ATX
|
||||||
|
* headings, list markers, blockquote markers). Open sentinels must not be
|
||||||
|
* injected before these markers — a PUA char at position 0 would prevent
|
||||||
|
* markdown-it from recognising the block construct (e.g. `##` is only a
|
||||||
|
* heading marker when it is the very first char on the line).
|
||||||
|
*/
|
||||||
|
function blockContentStart(raw: string): number {
|
||||||
|
// ATX headings: "#{1,6} " at the start of the line.
|
||||||
|
const heading = raw.match(/^#{1,6} /);
|
||||||
|
if (heading) return heading[0].length;
|
||||||
|
// Unordered list markers: "- ", "* ", "+ ".
|
||||||
|
const ul = raw.match(/^[-*+] /);
|
||||||
|
if (ul) return ul[0].length;
|
||||||
|
// Ordered list markers: "N. " or "N) ".
|
||||||
|
const ol = raw.match(/^\d+[.)]\s+/);
|
||||||
|
if (ol) return ol[0].length;
|
||||||
|
// Blockquotes: "> " or ">".
|
||||||
|
const bq = raw.match(/^> ?/);
|
||||||
|
if (bq) return bq[0].length;
|
||||||
|
return 0;
|
||||||
|
}
|
||||||
|
|
||||||
/** Inject paired sentinels into a prose block's raw text for the spans clipped to it. */
|
/** Inject paired sentinels into a prose block's raw text for the spans clipped to it. */
|
||||||
function injectSentinels(raw: string, blockStart: number, spans: AuthorSpan[]): string {
|
function injectSentinels(raw: string, blockStart: number, spans: AuthorSpan[]): string {
|
||||||
const inserts: { at: number; marker: string }[] = [];
|
const inserts: { at: number; marker: string }[] = [];
|
||||||
|
// Open sentinels must not precede block-level markdown markers (heading/list/
|
||||||
|
// blockquote syntax at position 0) — a PUA char before "## " prevents markdown-it
|
||||||
|
// from recognising the heading. Clamp to the content-start offset.
|
||||||
|
const minOpen = blockContentStart(raw);
|
||||||
for (const s of spans) {
|
for (const s of spans) {
|
||||||
const lo = clampOffDelimiterRun(raw, Math.max(0, s.start - blockStart));
|
const lo = Math.max(minOpen, clampOffDelimiterRun(raw, Math.max(0, s.start - blockStart)));
|
||||||
const hi = clampOffDelimiterRun(raw, Math.min(raw.length, s.end - blockStart));
|
const hi = clampOffDelimiterRun(raw, Math.min(raw.length, s.end - blockStart));
|
||||||
if (hi <= lo) continue; // empty (or clamped to empty) span contributes nothing
|
if (hi <= lo) continue; // empty (or clamped to empty) span contributes nothing
|
||||||
inserts.push({ at: lo, marker: SENT[s.author].open });
|
inserts.push({ at: lo, marker: SENT[s.author].open });
|
||||||
@@ -711,13 +738,13 @@ const ALL_SENTINELS = new RegExp(
|
|||||||
* (one `<span>` segment per text run). Tags are copied verbatim (with any stray
|
* (one `<span>` segment per text run). Tags are copied verbatim (with any stray
|
||||||
* sentinel stripped, so no Private-Use-Area char ever leaks). Pure, deterministic.
|
* sentinel stripped, so no Private-Use-Area char ever leaks). Pure, deterministic.
|
||||||
*/
|
*/
|
||||||
function sentinelsToSpans(html: string): string {
|
function sentinelsToSpans(html: string, kind: "by" | "ins" = "by"): string {
|
||||||
const out: string[] = [];
|
const out: string[] = [];
|
||||||
let current: AuthorKind | null = null; // which author region we're inside
|
let current: AuthorKind | null = null; // which author region we're inside
|
||||||
let spanOpen = false; // whether a <span> is currently open in `out`
|
let spanOpen = false; // whether a <span> is currently open in `out`
|
||||||
const openSpan = () => {
|
const openSpan = () => {
|
||||||
if (current && !spanOpen) {
|
if (current && !spanOpen) {
|
||||||
out.push(`<span class="cw-by-${current}">`);
|
out.push(`<span class="cw-${kind}-${current}">`);
|
||||||
spanOpen = true;
|
spanOpen = true;
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
@@ -765,10 +792,11 @@ export function colorByAuthor(
|
|||||||
blockStart: number,
|
blockStart: number,
|
||||||
spans: AuthorSpan[],
|
spans: AuthorSpan[],
|
||||||
render: (src: string) => string,
|
render: (src: string) => string,
|
||||||
|
kind: "by" | "ins" = "by",
|
||||||
): string {
|
): string {
|
||||||
const overlapping = spans.filter((s) => s.end > blockStart && s.start < blockStart + raw.length);
|
const overlapping = spans.filter((s) => s.end > blockStart && s.start < blockStart + raw.length);
|
||||||
const injected = injectSentinels(raw, blockStart, overlapping);
|
const injected = injectSentinels(raw, blockStart, overlapping);
|
||||||
return sentinelsToSpans(render(injected));
|
return sentinelsToSpans(render(injected), kind);
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -839,7 +867,6 @@ function proposalBlockHtml(p: ProposalView, render: (src: string) => string): st
|
|||||||
function renderReviewOp(
|
function renderReviewOp(
|
||||||
op: BlockOp,
|
op: BlockOp,
|
||||||
render: (src: string) => string,
|
render: (src: string) => string,
|
||||||
colored: (raw: string) => string,
|
|
||||||
src: string,
|
src: string,
|
||||||
changedHtml?: string,
|
changedHtml?: string,
|
||||||
): string {
|
): string {
|
||||||
@@ -851,9 +878,9 @@ function renderReviewOp(
|
|||||||
return `<div class="cw-blk cw-changed"${src}>${changedHtml}</div>`;
|
return `<div class="cw-blk cw-changed"${src}>${changedHtml}</div>`;
|
||||||
}
|
}
|
||||||
if (op.kind === "removed" || op.kind === "changed") return renderOp(op, render, src);
|
if (op.kind === "removed" || op.kind === "changed") return renderOp(op, render, src);
|
||||||
// "added": use changedHtml (author-colored word-diff from empty before, all insertions).
|
// "added": changedHtml is colorByAuthor sentinel output (markdown-safe, cw-ins-*).
|
||||||
// "unchanged": colored() returns plain render(raw) — color means "changed since baseline".
|
// "unchanged": render(raw) — plain, no author coloring (color = "changed since baseline").
|
||||||
return `<div class="cw-blk ${op.kind === "added" ? "cw-added" : "cw-unchanged"}"${src}>${changedHtml ?? colored(op.block.raw)}</div>`;
|
return `<div class="cw-blk ${op.kind === "added" ? "cw-added" : "cw-unchanged"}"${src}>${changedHtml ?? render(op.block.raw)}</div>`;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -967,19 +994,20 @@ export function renderReview(
|
|||||||
const blk = op.kind === "removed" ? undefined : ranges[ci++];
|
const blk = op.kind === "removed" ? undefined : ranges[ci++];
|
||||||
// Only ADDED and CHANGED prose blocks are author-colored (they differ since baseline);
|
// Only ADDED and CHANGED prose blocks are author-colored (they differ since baseline);
|
||||||
// UNCHANGED blocks render plain (color means "changed since baseline").
|
// UNCHANGED blocks render plain (color means "changed since baseline").
|
||||||
const colored = (raw: string): string => render(raw);
|
|
||||||
// A non-atomic changed prose block: author-colored word diff (ins by author,
|
// A non-atomic changed prose block: author-colored word diff (ins by author,
|
||||||
// del by adjacency). An added block: word diff from empty before — all insertions.
|
// del by adjacency). An added block: sentinel-safe colorByAuthor with kind="ins"
|
||||||
|
// (markdown-it parses the block first, so ## → <h2> before sentinels become spans;
|
||||||
|
// the old wordDiffByAuthor path wrapped raw in <ins> BEFORE parsing — structural bug).
|
||||||
// `blk.start` is the landed offset of the after-side text.
|
// `blk.start` is the landed offset of the after-side text.
|
||||||
const changedHtml =
|
const changedHtml =
|
||||||
blk && !clean
|
blk && !clean
|
||||||
? op.kind === "changed" && !op.atomic
|
? op.kind === "changed" && !op.atomic
|
||||||
? render(wordDiffByAuthor(op.before.raw, op.block.raw, blk.start, landedSpans))
|
? render(wordDiffByAuthor(op.before.raw, op.block.raw, blk.start, landedSpans))
|
||||||
: op.kind === "added"
|
: op.kind === "added"
|
||||||
? render(wordDiffByAuthor("", op.block.raw, blk.start, landedSpans))
|
? colorByAuthor(op.block.raw, blk.start, landedSpans, render, "ins")
|
||||||
: undefined
|
: undefined
|
||||||
: undefined;
|
: undefined;
|
||||||
bodyParts.push(renderReviewOp(op, render, colored, srcAttr(blk), changedHtml));
|
bodyParts.push(renderReviewOp(op, render, srcAttr(blk), changedHtml));
|
||||||
const here = blockIndex >= 0 ? byBlock.get(blockIndex) : undefined;
|
const here = blockIndex >= 0 ? byBlock.get(blockIndex) : undefined;
|
||||||
if (here) for (const p of here) bodyParts.push(proposalBlockHtml(p, render));
|
if (here) for (const p of here) bodyParts.push(proposalBlockHtml(p, render));
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -491,6 +491,18 @@ describe("renderReview", () => {
|
|||||||
const html = renderReview("Keep.", "Keep.\n\nFresh line.", [{ start: 6, end: 17, author: "human" }], []);
|
const html = renderReview("Keep.", "Keep.\n\nFresh line.", [{ start: 6, end: 17, author: "human" }], []);
|
||||||
expect(html).toContain("cw-ins-human");
|
expect(html).toContain("cw-ins-human");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("renderReview: an added heading block renders as a heading AND is author-colored as an insertion", () => {
|
||||||
|
// "Intro.\n\n" = 8 chars; "## New Section" = 14 chars → offset 8..22 in currentText.
|
||||||
|
// Before fix: wordDiffByAuthor wrapped raw in <ins> BEFORE markdown-it parsed it,
|
||||||
|
// so "## New Section" rendered as literal text (structural bug). After fix:
|
||||||
|
// colorByAuthor uses the sentinel technique — markdown-it parses the block first,
|
||||||
|
// so ## becomes a real <h2>, THEN sentinels are replaced with cw-ins-* spans.
|
||||||
|
const html = renderReview("Intro.", "Intro.\n\n## New Section", [{ start: 8, end: 22, author: "human" }], []);
|
||||||
|
expect(html).toMatch(/<h[12][^>]*>/); // real heading element, not literal "## "
|
||||||
|
expect(html).toContain("cw-ins-human"); // author-colored insertion
|
||||||
|
expect(html).not.toContain(">>"); // sanity: markers not escaped as text
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
import { renderTrackChanges as rtc2 } from "../src/trackChangesModel";
|
import { renderTrackChanges as rtc2 } from "../src/trackChangesModel";
|
||||||
|
|||||||
Reference in New Issue
Block a user