fix: editor renders standalone committed deletions neutral; doc/comment cleanups (final review)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -7,8 +7,9 @@
|
|||||||
* for deletions (INV-52, decorationPlan/INV-49), and (3) provides a CodeLens
|
* for deletions (INV-52, decorationPlan/INV-49), and (3) provides a CodeLens
|
||||||
* `Accept ▾ / Reject ▾` above each block whose ▾ opens a QuickPick (this / all).
|
* `Accept ▾ / Reject ▾` above each block whose ▾ opens a QuickPick (this / all).
|
||||||
* Owns no proposal STATE — it is a view over ProposalController (which stays the
|
* Owns no proposal STATE — it is a view over ProposalController (which stays the
|
||||||
* pure F4 owner). A document with no pending proposals shows nothing (INV-32's
|
* pure F4 owner). Phase 2 decorates ALL committed changes-since-baseline by author
|
||||||
* spirit when nothing is pending).
|
* (human green / Claude blue + struck hints for deletions), plus pending proposals;
|
||||||
|
* a pinned baseline suppresses committed marks entirely (pin→clean, INV-48).
|
||||||
*/
|
*/
|
||||||
import * as vscode from "vscode";
|
import * as vscode from "vscode";
|
||||||
import type { ProposalController } from "./proposalController";
|
import type { ProposalController } from "./proposalController";
|
||||||
@@ -29,9 +30,10 @@ export class EditorProposalController implements vscode.Disposable, vscode.CodeL
|
|||||||
border: "0 0 2px 0", borderStyle: "solid",
|
border: "0 0 2px 0", borderStyle: "solid",
|
||||||
}),
|
}),
|
||||||
};
|
};
|
||||||
private readonly delDeco: Record<"human" | "claude", vscode.TextEditorDecorationType> = {
|
private readonly delDeco: Record<"human" | "claude" | "none", vscode.TextEditorDecorationType> = {
|
||||||
human: vscode.window.createTextEditorDecorationType({ after: { color: "#f85149" } }),
|
human: vscode.window.createTextEditorDecorationType({ after: { color: "#f85149" } }),
|
||||||
claude: vscode.window.createTextEditorDecorationType({ after: { color: "#bc8cff" } }),
|
claude: vscode.window.createTextEditorDecorationType({ after: { color: "#bc8cff" } }),
|
||||||
|
none: vscode.window.createTextEditorDecorationType({ after: { color: new vscode.ThemeColor("descriptionForeground") } }),
|
||||||
};
|
};
|
||||||
private readonly lensEmitter = new vscode.EventEmitter<void>();
|
private readonly lensEmitter = new vscode.EventEmitter<void>();
|
||||||
readonly onDidChangeCodeLenses = this.lensEmitter.event;
|
readonly onDidChangeCodeLenses = this.lensEmitter.event;
|
||||||
@@ -51,7 +53,7 @@ export class EditorProposalController implements vscode.Disposable, vscode.CodeL
|
|||||||
private readonly attribution: AttributionController,
|
private readonly attribution: AttributionController,
|
||||||
) {
|
) {
|
||||||
this.disposables.push(
|
this.disposables.push(
|
||||||
this.insDeco.human, this.insDeco.claude, this.delDeco.human, this.delDeco.claude, this.lensEmitter,
|
this.insDeco.human, this.insDeco.claude, this.delDeco.human, this.delDeco.claude, this.delDeco.none, this.lensEmitter,
|
||||||
vscode.languages.registerCodeLensProvider({ language: "markdown" }, this),
|
vscode.languages.registerCodeLensProvider({ language: "markdown" }, this),
|
||||||
this.proposals.onDidChangeProposals(({ uri }) => this.scheduleApply(uri)),
|
this.proposals.onDidChangeProposals(({ uri }) => this.scheduleApply(uri)),
|
||||||
vscode.window.onDidChangeActiveTextEditor((ed) => ed && this.renderEditor(ed)),
|
vscode.window.onDidChangeActiveTextEditor((ed) => ed && this.renderEditor(ed)),
|
||||||
@@ -119,15 +121,13 @@ export class EditorProposalController implements vscode.Disposable, vscode.CodeL
|
|||||||
private renderEditor(editor: vscode.TextEditor): void {
|
private renderEditor(editor: vscode.TextEditor): void {
|
||||||
const doc = editor.document;
|
const doc = editor.document;
|
||||||
const clearAll = () => {
|
const clearAll = () => {
|
||||||
for (const a of ["human", "claude"] as const) {
|
for (const a of ["human", "claude"] as const) editor.setDecorations(this.insDeco[a], []);
|
||||||
editor.setDecorations(this.insDeco[a], []);
|
for (const a of ["human", "claude", "none"] as const) editor.setDecorations(this.delDeco[a], []);
|
||||||
editor.setDecorations(this.delDeco[a], []);
|
|
||||||
}
|
|
||||||
};
|
};
|
||||||
if (doc.languageId !== "markdown") return clearAll();
|
if (doc.languageId !== "markdown") return clearAll();
|
||||||
const key = this.proposals.keyFor(doc);
|
const key = this.proposals.keyFor(doc);
|
||||||
const ins: Record<"human" | "claude", vscode.Range[]> = { human: [], claude: [] };
|
const ins: Record<"human" | "claude", vscode.Range[]> = { human: [], claude: [] };
|
||||||
const del: Record<"human" | "claude", vscode.DecorationOptions[]> = { human: [], claude: [] };
|
const del: Record<"human" | "claude" | "none", vscode.DecorationOptions[]> = { human: [], claude: [], none: [] };
|
||||||
// Pending proposals — always Claude (INV: proposals are agent-authored).
|
// Pending proposals — always Claude (INV: proposals are agent-authored).
|
||||||
for (const v of this.proposals.listProposals(doc)) {
|
for (const v of this.proposals.listProposals(doc)) {
|
||||||
if (v.anchorStart === null || v.original === undefined || !this.proposals.isApplied(key, v.id)) continue;
|
if (v.anchorStart === null || v.original === undefined || !this.proposals.isApplied(key, v.id)) continue;
|
||||||
@@ -140,12 +140,10 @@ export class EditorProposalController implements vscode.Disposable, vscode.CodeL
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
// Committed changes-since-baseline are added in Task 7 (pushes into ins/del[author]).
|
// Committed changes-since-baseline are added by decorateCommitted (pushes into ins/del[author]).
|
||||||
this.decorateCommitted(editor, ins, del); // no-op stub until Task 7
|
this.decorateCommitted(editor, ins, del);
|
||||||
for (const a of ["human", "claude"] as const) {
|
for (const a of ["human", "claude"] as const) editor.setDecorations(this.insDeco[a], ins[a]);
|
||||||
editor.setDecorations(this.insDeco[a], ins[a]);
|
for (const a of ["human", "claude", "none"] as const) editor.setDecorations(this.delDeco[a], del[a]);
|
||||||
editor.setDecorations(this.delDeco[a], del[a]);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -153,11 +151,14 @@ export class EditorProposalController implements vscode.Disposable, vscode.CodeL
|
|||||||
* Diffs current buffer against the F6 baseline; attributes each changed run to its author;
|
* Diffs current buffer against the F6 baseline; attributes each changed run to its author;
|
||||||
* skips any hunk overlapping an applied pending proposal (the proposal loop owns those).
|
* skips any hunk overlapping an applied pending proposal (the proposal loop owns those).
|
||||||
* pin→clean: returns without marking when baseline.reason === "pinned".
|
* pin→clean: returns without marking when baseline.reason === "pinned".
|
||||||
|
* Adjacency heuristic: a deletion in a hunk that has insertions inherits the hunk author;
|
||||||
|
* a standalone deletion (hunk with no insertions) renders neutral ("none") — matching the
|
||||||
|
* preview's cw-del-none treatment.
|
||||||
*/
|
*/
|
||||||
private decorateCommitted(
|
private decorateCommitted(
|
||||||
editor: vscode.TextEditor,
|
editor: vscode.TextEditor,
|
||||||
ins: Record<"human" | "claude", vscode.Range[]>,
|
ins: Record<"human" | "claude", vscode.Range[]>,
|
||||||
del: Record<"human" | "claude", vscode.DecorationOptions[]>,
|
del: Record<"human" | "claude" | "none", vscode.DecorationOptions[]>,
|
||||||
): void {
|
): void {
|
||||||
const doc = editor.document;
|
const doc = editor.document;
|
||||||
const baseline = this.diffView.getBaseline(doc.uri.toString());
|
const baseline = this.diffView.getBaseline(doc.uri.toString());
|
||||||
@@ -180,6 +181,8 @@ export class EditorProposalController implements vscode.Disposable, vscode.CodeL
|
|||||||
const hunks = wordEditHunks(current, baseline.text);
|
const hunks = wordEditHunks(current, baseline.text);
|
||||||
for (const h of hunks) {
|
for (const h of hunks) {
|
||||||
// Skip hunks whose current-buffer range overlaps an applied pending proposal.
|
// Skip hunks whose current-buffer range overlaps an applied pending proposal.
|
||||||
|
// The skip is whole-hunk and therefore conservative: a committed edit word-merged
|
||||||
|
// into a proposal's hunk is skipped entirely — rare at word granularity.
|
||||||
if (appliedRanges.some((r) => h.start < r.end && h.end > r.start)) continue;
|
if (appliedRanges.some((r) => h.start < r.end && h.end > r.start)) continue;
|
||||||
const author = authorAt(h.start, spans) ?? "human";
|
const author = authorAt(h.start, spans) ?? "human";
|
||||||
// h.replacement = baseline text for this span; current.slice(h.start, h.end) = applied text.
|
// h.replacement = baseline text for this span; current.slice(h.start, h.end) = applied text.
|
||||||
@@ -187,8 +190,11 @@ export class EditorProposalController implements vscode.Disposable, vscode.CodeL
|
|||||||
for (const i of plan.insertions) {
|
for (const i of plan.insertions) {
|
||||||
ins[author].push(new vscode.Range(doc.positionAt(i.start), doc.positionAt(i.end)));
|
ins[author].push(new vscode.Range(doc.positionAt(i.start), doc.positionAt(i.end)));
|
||||||
}
|
}
|
||||||
|
// Adjacency heuristic: a deletion paired with an insertion in THIS hunk inherits the
|
||||||
|
// insertion author; a standalone deletion (no insertion in the hunk) is neutral.
|
||||||
|
const delAuthor = plan.insertions.length > 0 ? author : "none";
|
||||||
for (const d of plan.deletions) {
|
for (const d of plan.deletions) {
|
||||||
del[author].push({
|
del[delAuthor].push({
|
||||||
range: new vscode.Range(doc.positionAt(d.at), doc.positionAt(d.at)),
|
range: new vscode.Range(doc.positionAt(d.at), doc.positionAt(d.at)),
|
||||||
renderOptions: { after: { contentText: ` ${d.text} `, textDecoration: "line-through" } },
|
renderOptions: { after: { contentText: ` ${d.text} `, textDecoration: "line-through" } },
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -720,7 +720,7 @@ const ALL_SENTINELS = new RegExp(
|
|||||||
);
|
);
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* #33: token-aware replacement of the rendered author sentinels with `cw-by-*`
|
* #33: token-aware replacement of the rendered author sentinels with `cw-ins-*`
|
||||||
* spans. A naive global string-replace (the old approach) could emit a span that
|
* spans. A naive global string-replace (the old approach) could emit a span that
|
||||||
* CROSSES an element boundary — `<span><strong>bo</span>ld</strong>` — when a
|
* CROSSES an element boundary — `<span><strong>bo</span>ld</strong>` — when a
|
||||||
* boundary fell inside an emphasis run (CASE3). This walks the rendered HTML and
|
* boundary fell inside an emphasis run (CASE3). This walks the rendered HTML and
|
||||||
|
|||||||
@@ -286,6 +286,21 @@ test("committed change with NO shared prefix: current offsets + deleted text bot
|
|||||||
expect(plan.deletions.some((d) => d.text.includes("alpha"))).toBe(true); // deletion not lost
|
expect(plan.deletions.some((d) => d.text.includes("alpha"))).toBe(true); // deletion not lost
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("standalone committed deletion → plan has no insertions (controller routes del to neutral)", () => {
|
||||||
|
// current = "keep word" (baseline had "this " between "keep " and "word" — standalone deletion)
|
||||||
|
const [h] = wordEditHunks("keep word", "keep this word");
|
||||||
|
const plan = decorationPlan(h.start, h.replacement, "keep word".slice(h.start, h.end));
|
||||||
|
expect(plan.insertions.length).toBe(0);
|
||||||
|
expect(plan.deletions.some((d) => d.text.includes("this"))).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("paired committed replace → plan has insertions (deletion inherits author, not neutral)", () => {
|
||||||
|
// current = "the dark mode", baseline = "the light mode" — replacement, so paired
|
||||||
|
const [h] = wordEditHunks("the dark mode", "the light mode");
|
||||||
|
const plan = decorationPlan(h.start, h.replacement, "the dark mode".slice(h.start, h.end));
|
||||||
|
expect(plan.insertions.length).toBeGreaterThan(0);
|
||||||
|
});
|
||||||
|
|
||||||
describe("renderPlain", () => {
|
describe("renderPlain", () => {
|
||||||
test("renderPlain renders current buffer as plain markdown (no marks)", () => {
|
test("renderPlain renders current buffer as plain markdown (no marks)", () => {
|
||||||
const html = renderPlain("# Title\n\nhello");
|
const html = renderPlain("# Title\n\nhello");
|
||||||
|
|||||||
Reference in New Issue
Block a user