From 76a8c17ce14c32b267b965595b585ae880b28c3c Mon Sep 17 00:00:00 2001 From: Anton Arnaudov Date: Thu, 15 Oct 2026 00:45:32 +0300 Subject: [PATCH] fix(staging): typing no longer stalls the extension host re-diffing the gutter (#115) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The staging gutter re-diffs the open file against HEAD and the index on every pause in typing. vscode-diff checks its 5 s time budget with Date.now() once per cell of its n×m table for texts under 1,700 lines together: 643,765 clock reads for one keystroke in an 800-line file. Where reading the clock is a system call (Linux on the hpet clocksource, ~1.2 µs) that is ~820 ms blocked per refresh; measured with a 1.2 µs clock: 823 ms before, 14 ms after. - lineDiff: 256 of vscode-diff's budget checks share one clock read (2,516 reads for the same edit). A spent budget stays spent; same results. - StagedGutter: an edit re-diffs the buffer against the HEAD/index text and conflict state git gave last time, instead of `git status` + two `git show`s per pause. Focus, save, ref/index moves and toggles still read git. A refresh waits for the running one for the same document, and an unchanged version is not re-diffed. - Blame: an untracked file is no longer blamed (and logged as a failure) on every edit. BlameProvider asks git whether the path is in HEAD (cat-file -e, not the translated message) and throws BlameNotCommittedError; the controller stops asking until git reports a repository change. Co-Authored-By: Claude Opus 5.5 --- apps/extension/src/blame/blameController.ts | 26 ++++- apps/extension/src/changes/stagedGutter.ts | 109 ++++++++++++++++-- .../test/blameController.surface.test.ts | 56 ++++++++- apps/extension/test/stagedGutterState.test.ts | 50 +++++++- packages/engine/src/lineDiff.ts | 34 ++++++ packages/engine/test/diffClock.test.ts | 64 ++++++++++ packages/git-service/src/BlameProvider.ts | 29 ++++- packages/git-service/src/blockStaging.ts | 19 ++- packages/git-service/test/indexWrites.test.ts | 24 ++++ 9 files changed, 386 insertions(+), 25 deletions(-) create mode 100644 packages/engine/test/diffClock.test.ts diff --git a/apps/extension/src/blame/blameController.ts b/apps/extension/src/blame/blameController.ts index 66b43854..ee4f24cb 100644 --- a/apps/extension/src/blame/blameController.ts +++ b/apps/extension/src/blame/blameController.ts @@ -2,6 +2,7 @@ import * as vscode from "vscode"; import { relative } from "node:path"; import type { BlameResult, BlameCommit } from "@gitstudio/git-service/index"; import { UNCOMMITTED_SHA } from "@gitstudio/git-service/index"; +import { BlameNotCommittedError } from "@gitstudio/git-service/BlameProvider"; import type { RepoManager, RepoEntry } from "../git/repoManager"; import { touches } from "../git/repoChange"; import { notice, notifyCopied } from "../ui/notify"; @@ -49,6 +50,14 @@ export class BlameController implements vscode.Disposable { string, { version: number; result: Promise } >(); + /** + * Documents git cannot blame because they are not in HEAD (untracked). + * Not asked again — every edit used to spawn a blame that failed and + * logged it (#115) — until git reports a change in the repository. + */ + private readonly notInHead = new Set(); + /** Bumped on every repository change, so an answer from before it is not kept. */ + private gitEpoch = 0; // Full-file annotation state, per editor (by document uri string). private readonly annotated = new Map(); @@ -146,6 +155,7 @@ export class BlameController implements vscode.Disposable { vscode.workspace.onDidCloseTextDocument((doc) => { const key = doc.uri.toString(); this.blameCache.delete(key); + this.notInHead.delete(key); // Annotate mode creates a decoration type per opened file; without this // the map (and its types) grow for every file visited in the session. const type = this.annotated.get(key); @@ -215,6 +225,13 @@ export class BlameController implements vscode.Disposable { // checkout, a rebase), not when a file is saved — a file's own edits // already drop its entry (keyed by document version, above). It used // to drop EVERY file's blame on each save and window focus. + // Any change git reports — a commit, or a `git add`, after which git + // blames the file from the index — may have put a file that was not in + // HEAD where blame can see it: ask again once, not on every keystroke. + this.repos.onDidChange(() => { + this.notInHead.clear(); + this.gitEpoch++; + }), this.repos.onDidChange((e) => { if (!touches(e, "refs", "operation", "repos")) { return; @@ -802,6 +819,9 @@ export class BlameController implements vscode.Disposable { return undefined; } const key = document.uri.toString(); + if (this.notInHead.has(key)) { + return undefined; + } const cached = this.blameCache.get(key); if (cached && cached.version === document.version) { return cached.result; @@ -814,11 +834,15 @@ export class BlameController implements vscode.Disposable { } // Feed the live (possibly dirty) buffer so blame matches what's on screen. const contents = document.isDirty ? document.getText() : undefined; + const epoch = this.gitEpoch; const promise = ctx.ctx.blame .blameFile(relPath, { contents, signal: controller.signal }) .catch((e: unknown) => { - if (!controller.signal.aborted) { + if (e instanceof BlameNotCommittedError) { + // Expected, not a failure: there is no history to show yet. + if (epoch === this.gitEpoch) this.notInHead.add(key); + } else if (!controller.signal.aborted) { const msg = e instanceof Error ? e.message : String(e); this.log?.(l10n.t("blame failed for {0}: {1}", relPath, msg)); console.error("[GitStudio] blame failed", e); diff --git a/apps/extension/src/changes/stagedGutter.ts b/apps/extension/src/changes/stagedGutter.ts index 4ddda9bb..d479b419 100644 --- a/apps/extension/src/changes/stagedGutter.ts +++ b/apps/extension/src/changes/stagedGutter.ts @@ -1,7 +1,12 @@ import * as vscode from "vscode"; import { stat } from "node:fs/promises"; -import { listChangeBlocks, setBlockStaged } from "@gitstudio/git-service/blockStaging"; -import type { ChangeBlock } from "@gitstudio/git-service/blockStaging"; +import { + changeBlocksFrom, + isStageableText, + readBaselines, + setBlockStaged, +} from "@gitstudio/git-service/blockStaging"; +import type { Baselines, ChangeBlock } from "@gitstudio/git-service/blockStaging"; import type { RepoManager, RepoEntry } from "../git/repoManager"; import { relativePath } from "./changesView"; import { blockAtLine } from "./blockAtLine"; @@ -59,6 +64,21 @@ export class StagedGutter implements vscode.Disposable { * codebase keeps hitting. */ private readonly generation = new Map(); + /** + * What git said about each document when it was last read: whether it is + * conflicted, and its HEAD and index text. An EDIT moves neither, so a + * refresh caused only by typing diffs the buffer against these instead of + * spawning `git status` and two `git show`s per pause in typing (#115). + * Every other cause — focus, save, a ref or index move, a toggle — reads git + * again and replaces the entry. + */ + private readonly baselines = new Map(); + /** Documents whose next scheduled refresh must read git, not reuse `baselines`. */ + private readonly readGit = new Set(); + /** Documents with a scheduled refresh running now; a timer that fires meanwhile waits for it. */ + private readonly running = new Map>(); + /** The document version each document's marks were last painted for. */ + private readonly paintedVersion = new Map(); private enabled: boolean; constructor( @@ -99,8 +119,10 @@ export class StagedGutter implements vscode.Disposable { if (editor) void this.schedule(editor.document, 0); }), vscode.workspace.onDidChangeTextDocument((event) => { - void this.schedule(event.document, REFRESH_DEBOUNCE_MS); + // Typing: same HEAD, same index — re-diff, do not re-read git. + void this.schedule(event.document, REFRESH_DEBOUNCE_MS, false); }), + vscode.workspace.onDidCloseTextDocument((doc) => this.forget(doc.uri.toString())), vscode.workspace.onDidSaveTextDocument((doc) => void this.schedule(doc, 0)), vscode.window.onDidChangeTextEditorSelection((event) => this.onPossibleGutterClick(event), @@ -181,21 +203,49 @@ export class StagedGutter implements vscode.Disposable { } } - private schedule(document: vscode.TextDocument, delay: number): void { + /** + * Refreshes `document` after `delay`, coalescing: a newer call replaces a + * pending one, and a refresh never starts while the last one for the same + * document is still running — it runs once that one is done. `readGit` + * false means only the buffer changed; any true call before the refresh + * runs makes it read git. + */ + private schedule(document: vscode.TextDocument, delay: number, readGit = true): void { if (document.uri.scheme !== "file") return; const key = document.uri.toString(); + if (readGit) this.readGit.add(key); const existing = this.timers.get(key); if (existing) clearTimeout(existing); this.timers.set( key, setTimeout(() => { this.timers.delete(key); - void this.refresh(document); + void this.runScheduled(document); }, delay), ); } - private async refresh(document: vscode.TextDocument): Promise { + private async runScheduled(document: vscode.TextDocument): Promise { + const key = document.uri.toString(); + const inFlight = this.running.get(key); + if (inFlight) { + // Run once it is done — unless something newer has been scheduled by then. + await inFlight; + if (this.timers.has(key) || this.running.has(key)) return; + } + const readGit = this.readGit.delete(key) || !this.baselines.has(key); + // Only the buffer could have changed, and it has not: nothing to redo. + if (!readGit && typeof document.version === "number" && this.paintedVersion.get(key) === document.version) { + return; + } + const run = this.refresh(document, !readGit).finally(() => { + if (this.running.get(key) === run) this.running.delete(key); + }); + this.running.set(key, run); + await run; + } + + private async refresh(document: vscode.TextDocument, reuseBaselines = false): Promise { const key = document.uri.toString(); const gen = (this.generation.get(key) ?? 0) + 1; this.generation.set(key, gen); @@ -203,18 +253,34 @@ export class StagedGutter implements vscode.Disposable { const target = this.resolve(document.uri); if (!target || !this.enabled) { this.blocksByDoc.delete(key); + this.baselines.delete(key); + this.paintedVersion.delete(key); this.paint(document, []); return; } - const blocks = await this.readBlocks(target, document); + const version = document.version; + const blocks = await this.readBlocks(target, document, reuseBaselines); // A newer refresh started while git was answering; its answer wins. if (this.generation.get(key) !== gen) return; this.blocksByDoc.set(key, blocks); + if (typeof version === "number") this.paintedVersion.set(key, version); + else this.paintedVersion.delete(key); this.paint(document, blocks); } + private forget(key: string): void { + const timer = this.timers.get(key); + if (timer) clearTimeout(timer); + this.timers.delete(key); + this.blocksByDoc.delete(key); + this.baselines.delete(key); + this.readGit.delete(key); + this.paintedVersion.delete(key); + this.generation.delete(key); + } + private paint(document: vscode.TextDocument, blocks: ChangeBlock[]): void { const editors = vscode.window.visibleTextEditors.filter( (e) => e.document.uri.toString() === document.uri.toString(), @@ -345,17 +411,35 @@ export class StagedGutter implements vscode.Disposable { await this.toggleAtLine(document, line); } - /** The document's change blocks as git has them NOW; none for a conflicted file. */ + /** + * The document's change blocks as git has them NOW; none for a conflicted + * file. `reuseBaselines`: git was read for this document since anything but + * its text changed, so diff the text against what it said then. + */ private async readBlocks( target: { entry: RepoEntry; rel: string }, document: vscode.TextDocument, + reuseBaselines = false, ): Promise { + const key = document.uri.toString(); + // Binary or huge: no ticks, and no reason to ask git anything. + if (!isStageableText(document.getText())) return []; try { - // A conflicted file has no stage-0 index entry, so the tick model would - // read every change as unstaged. Say nothing rather than something wrong. - const conflicted = await target.entry.ctx.conflict.isConflicted(target.rel); - return conflicted ? [] : await listChangeBlocks(target.entry.ctx, target.rel, document.getText()); + let known = reuseBaselines ? this.baselines.get(key) : undefined; + if (!known) { + // A conflicted file has no stage-0 index entry, so the tick model would + // read every change as unstaged. Say nothing rather than something wrong. + const conflicted = await target.entry.ctx.conflict.isConflicted(target.rel); + const baselines = conflicted + ? { head: "", index: "" } + : await readBaselines(target.entry.ctx, target.rel); + known = { conflicted, baselines }; + this.baselines.set(key, known); + } + // The text as it is now, after git answered — not as it was when asked. + return known.conflicted ? [] : changeBlocksFrom(known.baselines, document.getText()); } catch { + this.baselines.delete(key); return []; } } @@ -377,6 +461,7 @@ export class StagedGutter implements vscode.Disposable { dispose(): void { for (const timer of this.timers.values()) clearTimeout(timer); this.timers.clear(); + this.baselines.clear(); for (const decoration of this.decorations.values()) decoration.dispose(); this.decorations.clear(); for (const disposable of this.disposables) disposable.dispose(); diff --git a/apps/extension/test/blameController.surface.test.ts b/apps/extension/test/blameController.surface.test.ts index c349aaa4..a7548fa6 100644 --- a/apps/extension/test/blameController.surface.test.ts +++ b/apps/extension/test/blameController.surface.test.ts @@ -213,16 +213,66 @@ test("a file outside every repository, or not on disk, or too long to blame, get test("a blame git cannot run is logged, and the line is left bare", async () => { const s = setup(); const logged: string[] = []; - const e = editor(doc(join(s.r.dir, "never-committed.txt"), { text: "x\n" }), 0); - e.document.isDirty = false; + const blame = s.r.ctx.blame as unknown as { blameFile: (...a: unknown[]) => Promise }; + blame.blameFile = () => Promise.reject(new Error("git blame failed for f.txt (exit 128): fatal: bad object")); + const e = editor(doc(s.file), 0); focus(e); const ui = start(s, (m) => logged.push(m)); await kit.until(() => logged.length, "logged"); - assert.match(logged[0], /^blame failed for never-committed\.txt: /); + assert.match(logged[0], /^blame failed for f\.txt: /); await kit.until(() => e.painted.has(ui.inline.id), "cleared"); assert.deepEqual(e.painted.get(ui.inline.id), []); }); +test("a file not in HEAD is blamed once, quietly — not on every edit — until a commit adds it (#115)", async () => { + const s = setup(); + const logged: string[] = []; + const blame = s.r.ctx.blame as unknown as { blameFile: (...a: unknown[]) => Promise }; + const real = blame.blameFile.bind(blame); + let asked = 0; + blame.blameFile = (...a: unknown[]) => { + asked++; + return real(...a); + }; + const path = join(s.r.dir, "never-committed.txt"); + s.r.write("never-committed.txt", "x\n"); + let d = doc(path, { text: "x\n" }); + const e = editor(d, 0); + focus(e); + const ui = start(s, (m) => logged.push(m)); + await kit.until(() => asked === 1 && e.painted.has(ui.inline.id), "the first look"); + // Typing: every edit is a new version, and each moves the caret. + for (let v = 2; v <= 6; v++) { + d = doc(path, { text: "x".repeat(v) + "\n" }); + d.version = v; + e.document = d; + kit.editorEvents.docChanged.fire({ document: d }); + kit.editorEvents.selection.fire({ textEditor: e, kind: 1, selections: [e.selection] }); + // The hover asks too. + await ui.c.getBlame(d as never, s.entry as never); + } + // Let the caret's debounced re-render run. + e.painted.delete(ui.inline.id); + e.selection = new kit.Selection(new kit.Position(0, 1), new kit.Position(0, 1)); + kit.editorEvents.selection.fire({ textEditor: e, kind: 1, selections: [e.selection] }); + await kit.until(() => e.painted.has(ui.inline.id), "re-rendered"); + assert.equal(asked, 1, "git blame is not asked again for a file it said is not in HEAD"); + assert.deepEqual(logged, [], "and nothing is logged: an untracked file is not a failure"); + assert.deepEqual(e.painted.get(ui.inline.id), []); + + // A commit adds it: the next look blames it. + commitAt(s.r, "never-committed.txt", "x\n", "add it", "2022-01-01T00:00:00Z", "Grace Hopper "); + s.repos.fire(repoChange(["refs"])); + d = doc(path); + d.version = 7; + e.document = d; + e.selection = new kit.Selection(new kit.Position(0, 0), new kit.Position(0, 0)); + kit.editorEvents.selection.fire({ textEditor: e, kind: 1, selections: [e.selection] }); + await kit.until(() => /Grace Hopper/.test(ui.bar.text), "blamed after the commit"); + assert.equal(asked, 2); + assert.deepEqual(logged, []); +}); + test("a commit elsewhere drops the cached blame and repaints the line", async () => { const s = setup(); const e = editor(doc(s.file), 0); diff --git a/apps/extension/test/stagedGutterState.test.ts b/apps/extension/test/stagedGutterState.test.ts index cfa9d191..b3c97723 100644 --- a/apps/extension/test/stagedGutterState.test.ts +++ b/apps/extension/test/stagedGutterState.test.ts @@ -21,7 +21,7 @@ const { StagedGutter } = require("../src/changes/stagedGutter") as typeof import const { GitContext } = require("@gitstudio/git-service/GitContext") as typeof import("@gitstudio/git-service/GitContext"); /* eslint-enable @typescript-eslint/no-require-imports */ -const vs = vscode as unknown as { window: Record; Range: unknown }; +const vs = vscode as unknown as { window: Record; workspace: Record; Range: unknown }; // A real Range: the stand-in's answers anything, including `start.line`. vs.Range = class { @@ -38,6 +38,13 @@ const STATES = ["staged", "unstaged", "partial"] as const; let made = 0; vs.window.createTextEditorDecorationType = () => ({ state: STATES[made++ % 3], dispose() {} }); +// The editor's text-change event, so a test can type. +let onEdit: ((e: { document: unknown }) => void) | undefined; +vs.workspace.onDidChangeTextDocument = (listener: (e: { document: unknown }) => void) => { + onEdit = listener; + return { dispose() {} }; +}; + const repos: { done: () => void }[] = []; after(() => repos.forEach((r) => r.done())); @@ -51,7 +58,7 @@ function setup(name: string) { writeFileSync(file, "a\nB\nc\nd\nE\n"); // two one-line changes: lines 1 and 4 (0-based) const uri = { scheme: "file", fsPath: file, toString: () => `file://${file}` }; - const document = { uri, getText: () => readFileSync(file, "utf8"), lineCount: 6 }; + const document = { uri, getText: () => readFileSync(file, "utf8"), lineCount: 6, version: 1 }; // What each change's mark was last painted as, by 0-based line. const painted = new Map(); const editor = { @@ -67,6 +74,7 @@ function setup(name: string) { const entry = { root: r.dir, ctx: new GitContext({ root: r.dir }) }; // Every read of git the marks make. let reads = 0; + let conflictChecks = 0; const staging = entry.ctx.staging as unknown as Record Promise>; for (const name of ["indexContent", "headContent"]) { const real = staging[name].bind(staging); @@ -75,6 +83,12 @@ function setup(name: string) { return real(...a); }; } + const conflict = entry.ctx.conflict as unknown as Record Promise>; + const isConflicted = conflict.isConflicted.bind(conflict); + conflict.isConflicted = (...a: unknown[]) => { + conflictChecks++; + return isConflicted(...a); + }; const manager = { getAll: () => [entry], onDidChange: (listener: (e: RepoChangeEvent) => void) => { @@ -90,6 +104,13 @@ function setup(name: string) { painted, index: () => r.git("show", ":f.txt"), reads: () => reads, + conflictChecks: () => conflictChecks, + /** Types `text` into the buffer: a new document version and the editor's change event. */ + type: (text: string) => { + writeFileSync(file, text); + document.version++; + onEdit?.({ document }); + }, fireRepoChange: (kind: "workingTree" | "refs" = "workingTree") => onRepoChange?.({ kinds: new Set([kind]) }), }; } @@ -164,3 +185,28 @@ test("a status run that left the index alone reads no git; a moved index or HEAD await until(() => t.reads() > afterAdd, "a ref change always repaints"); t.gutter.dispose(); }); + +test("typing re-diffs the buffer against what git said, without asking git again (#115)", async () => { + const t = setup("gutter-typing"); + await until(() => t.painted.get(1) === "unstaged" && t.painted.get(4) === "unstaged", "painted from git at the start"); + await quiet(t.reads); + const reads = t.reads(); + const checks = t.conflictChecks(); + // A burst of keystrokes, then a pause. + t.type("a\nB\nc\nd\nE\nx"); + t.type("a\nB\nc\nd\nE\nxy"); + t.type("a\nB\nc\nD\nE\n"); + await until(() => t.painted.get(3) === "unstaged", "the typed change is marked"); + // Typed after a pause — line 1 back as committed: still nothing asked of git. + t.type("a\nb\nc\nD\nE\n"); + await until(() => !t.painted.has(1) && t.painted.get(3) === "unstaged", "the undone change loses its mark"); + await quiet(t.reads); + assert.equal(t.reads(), reads, "no `git show` for HEAD or the index while typing"); + assert.equal(t.conflictChecks(), checks, "no `git status` while typing"); + // The repository moved: the next paint reads git again. + t.r.git("add", "f.txt"); + t.fireRepoChange("refs"); + await until(() => t.reads() > reads && t.painted.get(3) === "staged", "a repository change reads git"); + assert.ok(t.conflictChecks() > checks); + t.gutter.dispose(); +}); diff --git a/packages/engine/src/lineDiff.ts b/packages/engine/src/lineDiff.ts index 633d2d6c..46e8be2a 100644 --- a/packages/engine/src/lineDiff.ts +++ b/packages/engine/src/lineDiff.ts @@ -1,4 +1,5 @@ import { linesDiffComputers } from "vscode-diff"; +import { DateTimeout } from "vscode-diff/dist/vs/editor/common/diff/defaultLinesDiffComputer/algorithms/diffAlgorithm"; import type { InnerRange, LineEnding, @@ -30,6 +31,39 @@ export interface DiffOptions { whitespaceOnlyChanges?: boolean; } +/** + * How many of vscode-diff's "still within the time budget?" questions share + * one look at the clock. + * + * vscode-diff asks that question (`DateTimeout.isValid`, a `Date.now()`) in + * its innermost loops: for a text under 1,700 lines on both sides together, once + * per CELL of an n×m table. Editing an 800-line file is 640,000 clock reads per + * diff. Where the clock is cheap that hides inside the diff; where reading it + * is a system call (Linux on the hpet clocksource, ~1.2 µs) it is ~0.8 s of + * blocked extension host per refresh of the staging gutter, i.e. per pause in + * typing (#115). + * + * The answer between two reads is the same answer: the budget is 5 s, and 256 + * cells or Myers steps are microseconds to a few milliseconds of work. A + * budget that has run out stays out, as before. + */ +export const CLOCK_STRIDE = 256; + +type StridedTimeout = { valid: boolean; gitstudioAsks?: number; isValid(): boolean }; + +const timeoutProto = DateTimeout.prototype as unknown as StridedTimeout & { gitstudioStrided?: true }; +if (!timeoutProto.gitstudioStrided) { + const readClock = timeoutProto.isValid; + timeoutProto.isValid = function (this: StridedTimeout): boolean { + if (!this.valid) return false; + const asked = (this.gitstudioAsks ?? 0) + 1; + this.gitstudioAsks = asked; + // The first question reads the clock, then every CLOCK_STRIDE-th. + return (asked - 1) % CLOCK_STRIDE === 0 ? readClock.call(this) : true; + }; + timeoutProto.gitstudioStrided = true; +} + const BASE_DIFF_OPTIONS = { ignoreTrimWhitespace: false, maxComputationTimeMs: 5000, diff --git a/packages/engine/test/diffClock.test.ts b/packages/engine/test/diffClock.test.ts new file mode 100644 index 00000000..8ae7dca0 --- /dev/null +++ b/packages/engine/test/diffClock.test.ts @@ -0,0 +1,64 @@ +// How often a diff reads the clock (#115). +// +// vscode-diff checks its time budget with Date.now() in its innermost loops — +// for texts under 1,700 lines together, once per cell of an n×m table. On a +// machine whose clock read is a system call (Linux on the hpet clocksource, +// ~1.2 µs) that alone blocked the extension host ~0.8 s every time the editor +// gutter re-diffed an 800-line file. lineDiff.ts makes 256 of those checks +// share one read; these pin that, and that a budget that runs out still does. + +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { computeChangeBlocks } from "../src/staging/blockStaging"; +import { computeLineChanges, splitLines } from "../src/lineDiff"; + +function source(lines: number): string { + const out: string[] = []; + for (let i = 0; i < lines; i++) { + out.push(i % 4 === 0 ? `function f${i}(a, b) {` : i % 4 === 3 ? "}" : ` const v${i} = a * ${i} + b;`); + } + return out.join("\n") + "\n"; +} + +function countingClock(run: () => T, step = 0): { result: T; reads: number } { + const real = Date.now; + let reads = 0; + let fake = real(); + Date.now = () => { + reads++; + fake += step; + return step ? fake : real(); + }; + try { + return { result: run(), reads }; + } finally { + Date.now = real; + } +} + +test("re-diffing an 800-line file after a keystroke reads the clock thousands of times, not 640,000", () => { + const head = source(800); + const lines = splitLines(head); + lines[400] += " // typed"; + const working = lines.join("\n"); + const { result, reads } = countingClock(() => computeChangeBlocks(head, head, working)); + assert.deepEqual( + result.map((b) => [b.working.start, b.working.end, b.state]), + [[400, 400, "unstaged"]], + ); + // One read per cell was 643,765 for this edit. + assert.ok(reads < 10_000, `${reads} clock reads`); +}); + +test("a diff that runs out of time still says so", () => { + const a = splitLines(source(600)); + const b = a.map((l, i) => (i % 3 === 0 ? `${l} // changed` : l)); + // Every read of the clock is ten seconds later: the 5 s budget is spent at + // the first look after the start. + const { result, reads } = countingClock( + () => computeLineChanges(a, b, { ignoreTrimWhitespace: false, maxComputationTimeMs: 5000, computeMoves: false }), + 10_000, + ); + assert.equal(result.hitTimeout, true); + assert.ok(reads >= 2); +}); diff --git a/packages/git-service/src/BlameProvider.ts b/packages/git-service/src/BlameProvider.ts index f3c9db81..63a244d5 100644 --- a/packages/git-service/src/BlameProvider.ts +++ b/packages/git-service/src/BlameProvider.ts @@ -1,6 +1,7 @@ import type { BlameResult } from "@gitstudio/host-bridge/blame"; import { parseIncrementalBlame } from "@gitstudio/engine/blame/parse"; import type { GitProcess } from "./GitProcess"; +import { sep } from "node:path"; import * as l10n from "@vscode/l10n"; export interface BlameFileOptions { @@ -15,6 +16,18 @@ export interface BlameFileOptions { signal?: AbortSignal; } +/** + * Blame refused because the file is not in HEAD: it is untracked. (A file that + * is added but not committed git blames from the index.) Nothing is wrong — + * there is no history to show yet. Thrown so a + * caller that blames on every edit can stop asking (and stop logging it, + * #115) until a commit could have changed the answer. The message is the same + * as any other refusal's. + */ +export class BlameNotCommittedError extends Error { + readonly notCommitted = true; +} + /** * Reads `git blame --incremental` for a single file and parses it into a * BlameResult (commits keyed by sha + one entry per source line). Uses the @@ -49,9 +62,21 @@ export class BlameProvider { }); if (result.code !== 0) { - throw new Error( - l10n.t("git blame failed for {0} (exit {1}): {2}", relPath, result.code, result.stderr.trim()), + const message = l10n.t( + "git blame failed for {0} (exit {1}): {2}", + relPath, + result.code, + result.stderr.trim(), ); + // Asked of git rather than read from its message, which is translated. + // `HEAD:` is relative to the repository root, as relPath is, and + // spelled with "/" — a Windows relative path arrives with backslashes. + if (!opts?.rev && !opts?.signal?.aborted) { + const treePath = sep === "\\" ? relPath.replace(/\\/g, "/") : relPath; + const inHead = await this.proc.run(["cat-file", "-e", `HEAD:${treePath}`], { signal: opts?.signal }); + if (inHead.code !== 0) throw new BlameNotCommittedError(message); + } + throw new Error(message); } return parseIncrementalBlame(result.stdout); diff --git a/packages/git-service/src/blockStaging.ts b/packages/git-service/src/blockStaging.ts index 7559a66d..0cc2232b 100644 --- a/packages/git-service/src/blockStaging.ts +++ b/packages/git-service/src/blockStaging.ts @@ -60,7 +60,7 @@ export function isStageableText(text: string): boolean { } /** What the two sides of the tick model are computed from. */ -interface Baselines { +export interface Baselines { head: string; index: string; } @@ -70,8 +70,12 @@ interface Baselines { * * A path with no index entry reads as "" — which is correct for a brand-new * file, where the whole thing is one unstaged block against an empty baseline. + * + * Exported for a caller that repaints on every pause in typing (the editor + * gutter, #115): typing moves neither HEAD nor the index, so it reads these + * once and hands them to `changeBlocksFrom` until git says otherwise. */ -async function baselines(host: BlockStagingHost, rel: string): Promise { +export async function readBaselines(host: BlockStagingHost, rel: string): Promise { const [head, index] = await Promise.all([ host.staging.headContent(rel), host.staging.indexContent(rel), @@ -95,8 +99,13 @@ export async function listChangeBlocks( if (!isStageableText(workingText)) { return []; } - const { head, index } = await baselines(host, rel); - if (!isStageableText(head) || !isStageableText(index)) { + return changeBlocksFrom(await readBaselines(host, rel), workingText); +} + +/** listChangeBlocks over baselines already read (readBaselines). */ +export function changeBlocksFrom(baselines: Baselines, workingText: string): ChangeBlock[] { + const { head, index } = baselines; + if (!isStageableText(workingText) || !isStageableText(head) || !isStageableText(index)) { return []; } return computeChangeBlocks(head, index, workingText); @@ -136,7 +145,7 @@ export async function setBlockStaged( if (!isStageableText(workingText)) { return { ok: false, expected: true, stderr: NOT_STAGEABLE }; } - const { head, index } = await baselines(host, rel); + const { head, index } = await readBaselines(host, rel); if (!isStageableText(head) || !isStageableText(index)) { return { ok: false, expected: true, stderr: NOT_STAGEABLE }; } diff --git a/packages/git-service/test/indexWrites.test.ts b/packages/git-service/test/indexWrites.test.ts index b8ec3eab..28b37b90 100644 --- a/packages/git-service/test/indexWrites.test.ts +++ b/packages/git-service/test/indexWrites.test.ts @@ -7,6 +7,7 @@ import { serializeIndexWrite } from "../src/indexWrites"; import { stoppedIn } from "../src/stoppedOperation"; import { checkedOutElsewhere } from "../src/branchElsewhere"; import { GitProcess } from "../src/GitProcess"; +import { BlameNotCommittedError } from "../src/BlameProvider"; import { makeRepo, type Repo } from "./opRepo"; import { removeTempRepo } from "./tmpRepo"; @@ -99,6 +100,29 @@ test("blame refuses loudly — naming the file and git's exit — for a path git await assert.rejects(ctx.blame.blameFile("f.txt", { rev: "no-such-rev" }), /git blame failed for f\.txt/); }); +test("a file not in HEAD is refused as not committed yet — the one refusal a caller may expect (#115)", async () => { + const repo = makeRepo("blame-not-in-head"); + cleanup.push(() => repo.cleanup()); + repo.write("f.txt", "x\n"); + repo.commitAll("base"); + repo.write("sub/new.txt", "fresh\n"); + const ctx = repo.ctx(); + // Untracked, on disk and as an editor buffer. + for (const contents of [undefined, "fresh\ntyped\n"]) { + await assert.rejects(ctx.blame.blameFile(join("sub", "new.txt"), { contents }), (err: Error) => { + assert.ok(err instanceof BlameNotCommittedError, `not committed, for ${String(contents)}`); + assert.match(err.message, /^git blame failed for sub[\\/]new\.txt \(exit \d+\): /); + return true; + }); + } + // Added: git blames it from the index, every line not committed yet. + repo.git("add", "sub/new.txt"); + const added = await ctx.blame.blameFile("sub/new.txt"); + assert.ok(added.lines.length > 0); + // Every other refusal stays a plain one. + await assert.rejects(ctx.blame.blameFile("f.txt", { rev: "no-such-rev" }), (err: Error) => !(err instanceof BlameNotCommittedError)); +}); + test("stoppedIn reports conflicted files with no operation (a stash pop that conflicted)", async () => { const repo: Repo = makeRepo("stopped-pop"); cleanup.push(() => repo.cleanup());