From fe2a7459dc944f779ed64ab9398a98904d14e41e Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 9 Nov 2026 14:15:49 +0100 Subject: [PATCH 01/27] feat(client): NavHistory jump list for back and forward navigation --- src/client/navhistory.ts | 94 +++++++++++ test/client.navhistory.test.ts | 278 +++++++++++++++++++++++++++++++++ 2 files changed, 372 insertions(+) create mode 100644 src/client/navhistory.ts create mode 100644 test/client.navhistory.test.ts diff --git a/src/client/navhistory.ts b/src/client/navhistory.ts new file mode 100644 index 0000000..8695970 --- /dev/null +++ b/src/client/navhistory.ts @@ -0,0 +1,94 @@ +export interface NavLoc { + path: string; + line: number; // 1-based +} + +const copy = (l: NavLoc): NavLoc => ({ path: l.path, line: l.line }); + +/** Browser-style back/forward list: entries plus the index of the current one. */ +export class NavHistory { + private list: NavLoc[] = []; + private idx = -1; + private readonly cap: number; + private readonly near: number; + + constructor(opts?: { cap?: number; near?: number }) { + this.cap = opts?.cap ?? 100; + this.near = opts?.near ?? 10; + } + + record(from: NavLoc | undefined, to: NavLoc): void { + this.list.length = this.idx + 1; + if (from) { + const cur = this.list[this.idx]; + if (cur && cur.path === from.path) cur.line = from.line; + else this.push(from); + } + const cur = this.list[this.idx]; + if (cur && cur.path === to.path && Math.abs(cur.line - to.line) <= this.near) cur.line = to.line; + else this.push(to); + } + + back(current?: NavLoc): NavLoc | undefined { + if (!this.canBack()) return undefined; + return this.move(-1, current); + } + + forward(current?: NavLoc): NavLoc | undefined { + if (!this.canForward()) return undefined; + return this.move(1, current); + } + + canBack(): boolean { + return this.idx > 0; + } + + canForward(): boolean { + return this.idx < this.list.length - 1; + } + + drop(path: string): void { + const kept: NavLoc[] = []; + let newIdx = -1; + let firstSurvivorSeen = false; + this.list.forEach((e, i) => { + if (e.path === path) return; + const prev = kept[kept.length - 1]; + if (prev && prev.path === e.path && prev.line === e.line) { + // merged into the previous survivor + } else { + kept.push(e); + } + firstSurvivorSeen = true; + // survivors at or before the current index set the fallback; the current one wins + if (i <= this.idx) newIdx = kept.length - 1; + }); + if (kept.length && newIdx < 0 && firstSurvivorSeen) newIdx = 0; + if (!kept.length) newIdx = -1; + this.list = kept; + this.idx = newIdx; + } + + entries(): NavLoc[] { + return this.list.map(copy); + } + + index(): number { + return this.idx; + } + + private push(l: NavLoc): void { + this.list.push(copy(l)); + this.idx = this.list.length - 1; + while (this.list.length > this.cap) { + this.list.shift(); + this.idx--; + } + } + + private move(d: number, current?: NavLoc): NavLoc { + if (current) this.list[this.idx] = copy(current); + this.idx += d; + return copy(this.list[this.idx]!); + } +} diff --git a/test/client.navhistory.test.ts b/test/client.navhistory.test.ts new file mode 100644 index 0000000..35942ab --- /dev/null +++ b/test/client.navhistory.test.ts @@ -0,0 +1,278 @@ +import { describe, expect, test } from "bun:test"; +import { NavHistory, type NavLoc } from "../src/client/navhistory"; + +const L = (path: string, line: number): NavLoc => ({ path, line }); + +describe("NavHistory", () => { + test("AC-1 first record on empty history", () => { + const h = new NavHistory(); + h.record(undefined, L("a", 5)); + expect(h.entries()).toEqual([L("a", 5)]); + expect(h.index()).toBe(0); + expect(h.canBack()).toBe(false); + expect(h.canForward()).toBe(false); + }); + + test("empty history has index -1 and no moves", () => { + const h = new NavHistory(); + expect(h.index()).toBe(-1); + expect(h.back()).toBeUndefined(); + expect(h.forward()).toBeUndefined(); + expect(h.entries()).toEqual([]); + }); + + test("AC-2 from appended as current when path differs, then to", () => { + const h = new NavHistory(); + h.record(undefined, L("a", 1)); + h.record(L("b", 3), L("c", 9)); + expect(h.entries()).toEqual([L("a", 1), L("b", 3), L("c", 9)]); + expect(h.index()).toBe(2); + }); + + test("AC-2 from updates the current entry when same path", () => { + const h = new NavHistory(); + h.record(undefined, L("a", 1)); + h.record(L("a", 40), L("b", 2)); + expect(h.entries()).toEqual([L("a", 40), L("b", 2)]); + expect(h.index()).toBe(1); + }); + + test("AC-2 record without from just appends to", () => { + const h = new NavHistory(); + h.record(undefined, L("a", 1)); + h.record(undefined, L("b", 2)); + expect(h.entries()).toEqual([L("a", 1), L("b", 2)]); + }); + + test("AC-2 near boundary: 10 lines coalesces, 11 does not", () => { + const h = new NavHistory(); + h.record(undefined, L("a", 1)); + h.record(L("a", 1), L("a", 11)); + expect(h.entries()).toEqual([L("a", 11)]); + expect(h.index()).toBe(0); + h.record(L("a", 11), L("a", 22)); + expect(h.entries()).toEqual([L("a", 11), L("a", 22)]); + expect(h.index()).toBe(1); + }); + + test("AC-2 coalescing works backwards (negative delta)", () => { + const h = new NavHistory(); + h.record(undefined, L("a", 50)); + h.record(L("a", 50), L("a", 40)); + expect(h.entries()).toEqual([L("a", 40)]); + h.record(L("a", 40), L("a", 29)); + expect(h.entries()).toEqual([L("a", 40), L("a", 29)]); + }); + + test("AC-2 coalescing needs the same path", () => { + const h = new NavHistory(); + h.record(undefined, L("a", 1)); + h.record(L("a", 1), L("b", 1)); + expect(h.entries()).toEqual([L("a", 1), L("b", 1)]); + }); + + test("AC-2 custom near", () => { + const h = new NavHistory({ near: 2 }); + h.record(undefined, L("a", 1)); + h.record(L("a", 1), L("a", 3)); + expect(h.entries()).toEqual([L("a", 3)]); + h.record(L("a", 3), L("a", 6)); + expect(h.entries()).toEqual([L("a", 3), L("a", 6)]); + }); + + test("AC-2 record discards entries after the current index", () => { + const h = new NavHistory(); + h.record(undefined, L("a", 1)); + h.record(L("a", 1), L("b", 1)); + h.record(L("b", 1), L("c", 1)); + h.back(L("c", 1)); + h.back(L("b", 1)); + h.record(L("a", 1), L("d", 7)); + expect(h.entries()).toEqual([L("a", 1), L("d", 7)]); + expect(h.canForward()).toBe(false); + }); + + test("AC-3 cap drops the oldest", () => { + const h = new NavHistory({ cap: 3 }); + h.record(undefined, L("p0", 1)); + for (let i = 1; i < 5; i++) h.record(L(`p${i - 1}`, 1), L(`p${i}`, 1)); + expect(h.entries()).toEqual([L("p2", 1), L("p3", 1), L("p4", 1)]); + expect(h.index()).toBe(2); + }); + + test("AC-3 default cap is 100", () => { + const h = new NavHistory(); + h.record(undefined, L("p0", 1)); + for (let i = 1; i < 150; i++) h.record(L(`p${i - 1}`, 1), L(`p${i}`, 1)); + expect(h.entries().length).toBe(100); + expect(h.entries()[0]).toEqual(L("p50", 1)); + expect(h.index()).toBe(99); + }); + + test("AC-4 back and forward move the index and return the entry", () => { + const h = new NavHistory(); + h.record(undefined, L("a", 1)); + h.record(L("a", 1), L("b", 2)); + expect(h.canBack()).toBe(true); + expect(h.back(L("b", 2))).toEqual(L("a", 1)); + expect(h.index()).toBe(0); + expect(h.canBack()).toBe(false); + expect(h.canForward()).toBe(true); + expect(h.forward(L("a", 1))).toEqual(L("b", 2)); + expect(h.index()).toBe(1); + expect(h.canForward()).toBe(false); + }); + + test("AC-4 back stores current into the current entry", () => { + const h = new NavHistory(); + h.record(undefined, L("a", 1)); + h.record(L("a", 1), L("b", 2)); + h.back(L("b", 77)); + expect(h.entries()).toEqual([L("a", 1), L("b", 77)]); + h.forward(L("a", 33)); + expect(h.entries()).toEqual([L("a", 33), L("b", 77)]); + }); + + test("AC-4 without current the entry is left alone", () => { + const h = new NavHistory(); + h.record(undefined, L("a", 1)); + h.record(L("a", 1), L("b", 2)); + h.back(); + expect(h.entries()).toEqual([L("a", 1), L("b", 2)]); + }); + + test("AC-4 impossible moves return undefined and change nothing", () => { + const h = new NavHistory(); + h.record(undefined, L("a", 1)); + expect(h.back(L("zzz", 9))).toBeUndefined(); + expect(h.forward(L("zzz", 9))).toBeUndefined(); + expect(h.entries()).toEqual([L("a", 1)]); + expect(h.index()).toBe(0); + }); + + test("AC-5 back then record truncates forward", () => { + const h = new NavHistory(); + h.record(undefined, L("a", 1)); + h.record(L("a", 1), L("b", 1)); + h.record(L("b", 1), L("c", 1)); + h.back(L("c", 1)); + expect(h.canForward()).toBe(true); + h.record(L("b", 1), L("d", 1)); + expect(h.canForward()).toBe(false); + expect(h.entries()).toEqual([L("a", 1), L("b", 1), L("d", 1)]); + expect(h.index()).toBe(2); + }); + + test("AC-6 drop merges equal neighbours, index on merged entry", () => { + const h = new NavHistory(); + h.record(undefined, L("a.ts", 1)); + h.record(L("a.ts", 1), L("b.ts", 5)); + h.record(L("b.ts", 5), L("a.ts", 1)); + expect(h.index()).toBe(2); + h.drop("b.ts"); + expect(h.entries()).toEqual([L("a.ts", 1)]); + expect(h.index()).toBe(0); + }); + + test("AC-6 index on the first of two merging entries lands on merged", () => { + const h = new NavHistory(); + h.record(undefined, L("a.ts", 1)); + h.record(L("a.ts", 1), L("b.ts", 5)); + h.record(L("b.ts", 5), L("a.ts", 1)); + h.back(L("a.ts", 1)); + h.back(L("b.ts", 5)); + expect(h.index()).toBe(0); + h.drop("b.ts"); + expect(h.entries()).toEqual([L("a.ts", 1)]); + expect(h.index()).toBe(0); + }); + + test("AC-6 dropping the current entry moves to the nearest at or before", () => { + const h = new NavHistory(); + h.record(undefined, L("a.ts", 1)); + h.record(L("a.ts", 1), L("b.ts", 5)); + h.record(L("b.ts", 5), L("c.ts", 9)); + h.back(L("c.ts", 9)); + expect(h.index()).toBe(1); + h.drop("b.ts"); + expect(h.entries()).toEqual([L("a.ts", 1), L("c.ts", 9)]); + expect(h.index()).toBe(0); + }); + + test("AC-6 dropping the current entry with none before goes to the first survivor", () => { + const h = new NavHistory(); + h.record(undefined, L("a.ts", 1)); + h.record(L("a.ts", 1), L("b.ts", 5)); + h.back(L("b.ts", 5)); + expect(h.index()).toBe(0); + h.drop("a.ts"); + expect(h.entries()).toEqual([L("b.ts", 5)]); + expect(h.index()).toBe(0); + }); + + test("AC-6 index stays on the same surviving entry", () => { + const h = new NavHistory(); + h.record(undefined, L("a.ts", 1)); + h.record(L("a.ts", 1), L("b.ts", 5)); + h.record(L("b.ts", 5), L("c.ts", 9)); + h.drop("a.ts"); + expect(h.entries()).toEqual([L("b.ts", 5), L("c.ts", 9)]); + expect(h.index()).toBe(1); + }); + + test("AC-6 dropping the only path empties history", () => { + const h = new NavHistory(); + h.record(undefined, L("a.ts", 1)); + h.record(L("a.ts", 1), L("a.ts", 50)); + h.drop("a.ts"); + expect(h.entries()).toEqual([]); + expect(h.index()).toBe(-1); + expect(h.canBack()).toBe(false); + expect(h.canForward()).toBe(false); + }); + + test("AC-6 dropping an unknown path changes nothing", () => { + const h = new NavHistory(); + h.record(undefined, L("a.ts", 1)); + h.drop("nope"); + expect(h.entries()).toEqual([L("a.ts", 1)]); + expect(h.index()).toBe(0); + }); + + test("AC-7 returned objects are copies", () => { + const h = new NavHistory(); + h.record(undefined, L("a", 1)); + h.record(L("a", 1), L("b", 2)); + const e = h.entries(); + e[0]!.line = 99; + e.pop(); + expect(h.entries()).toEqual([L("a", 1), L("b", 2)]); + const r = h.back(L("b", 2))!; + r.line = 123; + r.path = "x"; + expect(h.entries()).toEqual([L("a", 1), L("b", 2)]); + const f = h.forward(L("a", 1))!; + f.line = 5; + expect(h.entries()).toEqual([L("a", 1), L("b", 2)]); + }); + + test("AC-7 inputs to record are copied", () => { + const h = new NavHistory(); + const from = L("a", 1); + const to = L("b", 2); + h.record(undefined, from); + h.record(from, to); + from.line = 500; + to.line = 600; + to.path = "zzz"; + expect(h.entries()).toEqual([L("a", 1), L("b", 2)]); + }); + + test("AC-8 back then forward returns to where the user was", () => { + const h = new NavHistory(); + h.record(undefined, L("a", 1)); + h.record(L("a", 1), L("b", 2)); + expect(h.back(L("b", 30))).toEqual(L("a", 1)); + expect(h.forward(L("a", 1))).toEqual(L("b", 30)); + }); +}); From cdc4eff0b74b72c0ee2c316aedf92cae28115daf Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 9 Nov 2026 14:21:57 +0100 Subject: [PATCH 02/27] feat(keys): navKey for Go Back and Go Forward, matched before zoom --- src/client/app.ts | 11 ++++- src/client/keys.ts | 12 ++++++ test/client.navkey.test.ts | 85 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 106 insertions(+), 2 deletions(-) create mode 100644 test/client.navkey.test.ts diff --git a/src/client/app.ts b/src/client/app.ts index c53b198..de3c24d 100644 --- a/src/client/app.ts +++ b/src/client/app.ts @@ -16,7 +16,7 @@ import { isContextMenuOpen } from "./menu.ts"; import { SearchView } from "./search.ts"; import { ChangesView } from "./changes.ts"; import type { ChangeFile } from "./api.ts"; -import { helpKey, normSideView, referenceKey, searchKey, symbolKey, type SideView } from "./keys.ts"; +import { helpKey, navKey, normSideView, referenceKey, searchKey, symbolKey, type SideView } from "./keys.ts"; import { reference, referenceWithCode, unsafePath } from "./reference.ts"; import { headingText } from "./symbols.ts"; import { slugify } from "./markdown.ts"; @@ -614,6 +614,11 @@ class App { this.applyLayout(); } + /** Back / Forward through the jump list (history wiring follows; empty until then). */ + private navigate(dir: "back" | "forward") { + this.notify(dir === "back" ? "No previous location" : "No next location"); + } + private zoom(d: number) { const L = this.L; if (d === 0) { @@ -769,7 +774,9 @@ class App { const appKey = IS_MAC || !inTerm; let handled = true; const sk = searchKey(e, { isMac: IS_MAC, inTerm: !!inTerm }); - if (sk === "search") this.showSearch(); + const nk = navKey(e, { isMac: IS_MAC, inTerm: !!inTerm }); + if (nk) this.navigate(nk === "back" ? "back" : "forward"); + else if (sk === "search") this.showSearch(); else if (sk === "next" || sk === "prev") this.search.step(sk === "next" ? 1 : -1); else if (e.ctrlKey && (e.code === "Backquote" || k === "`")) { if (e.shiftKey) this.newTerminal(); diff --git a/src/client/keys.ts b/src/client/keys.ts index dd303eb..ded824b 100644 --- a/src/client/keys.ts +++ b/src/client/keys.ts @@ -60,3 +60,15 @@ export function referenceKey(e: KeyLike, o: { isMac: boolean }): boolean { if (!mod || !e.altKey || e.shiftKey) return false; return e.key.toLowerCase() === "c" || (e.code === "KeyC" && !/^[a-z]$/i.test(e.key)); } + +/** + * Back / Forward through the jump list. macOS: Ctrl+- goes back, Ctrl+Shift+- forward (Cmd and + * Option stay free: Cmd+- is zoom out). Elsewhere: Ctrl+Alt+- back, Ctrl+Shift+- forward (plain + * Ctrl+- is zoom out). Never from a focused terminal. "_" is the shifted "-" on US layouts. + */ +export function navKey(e: KeyLike, o: { isMac: boolean; inTerm: boolean }): "back" | "forward" | null { + if (o.inTerm || !e.ctrlKey || e.metaKey || (e.key !== "-" && e.key !== "_")) return null; + if (e.shiftKey) return e.altKey ? null : "forward"; + if (e.key !== "-") return null; + return o.isMac ? (e.altKey ? null : "back") : e.altKey ? "back" : null; +} diff --git a/test/client.navkey.test.ts b/test/client.navkey.test.ts new file mode 100644 index 0000000..0be9b4a --- /dev/null +++ b/test/client.navkey.test.ts @@ -0,0 +1,85 @@ +import { describe, expect, test } from "bun:test"; +import { readFileSync } from "node:fs"; +import { navKey } from "../src/client/keys.ts"; + +const mac = { isMac: true, inTerm: false }; +const lin = { isMac: false, inTerm: false }; + +describe("navKey on macOS", () => { + test("Ctrl+- goes back", () => { + expect(navKey({ key: "-", code: "Minus", ctrlKey: true }, mac)).toBe("back"); + }); + test("Ctrl+Shift+- (or _) goes forward", () => { + expect(navKey({ key: "_", code: "Minus", ctrlKey: true, shiftKey: true }, mac)).toBe("forward"); + expect(navKey({ key: "-", code: "Minus", ctrlKey: true, shiftKey: true }, mac)).toBe("forward"); + }); + test("any Meta or Alt disqualifies (Cmd+- is zoom out)", () => { + expect(navKey({ key: "-", ctrlKey: true, metaKey: true }, mac)).toBeNull(); + expect(navKey({ key: "-", metaKey: true }, mac)).toBeNull(); + expect(navKey({ key: "-", ctrlKey: true, altKey: true }, mac)).toBeNull(); + expect(navKey({ key: "_", ctrlKey: true, shiftKey: true, metaKey: true }, mac)).toBeNull(); + expect(navKey({ key: "_", ctrlKey: true, shiftKey: true, altKey: true }, mac)).toBeNull(); + }); + test("plain minus and Cmd-less keys are not navigation", () => { + expect(navKey({ key: "-" }, mac)).toBeNull(); + expect(navKey({ key: "-", shiftKey: true }, mac)).toBeNull(); + }); +}); + +describe("navKey on Linux", () => { + test("Ctrl+Alt+- goes back", () => { + expect(navKey({ key: "-", code: "Minus", ctrlKey: true, altKey: true }, lin)).toBe("back"); + }); + test("Ctrl+Shift+- (or _) goes forward", () => { + expect(navKey({ key: "_", code: "Minus", ctrlKey: true, shiftKey: true }, lin)).toBe("forward"); + expect(navKey({ key: "-", code: "Minus", ctrlKey: true, shiftKey: true }, lin)).toBe("forward"); + }); + test("plain Ctrl+- is zoom out, not navigation", () => { + expect(navKey({ key: "-", code: "Minus", ctrlKey: true }, lin)).toBeNull(); + }); + test("Meta, or Alt without Ctrl, is not navigation", () => { + expect(navKey({ key: "-", altKey: true }, lin)).toBeNull(); + expect(navKey({ key: "-", ctrlKey: true, altKey: true, metaKey: true }, lin)).toBeNull(); + }); +}); + +describe("navKey everywhere", () => { + test("never inside the terminal, on either platform", () => { + expect(navKey({ key: "-", ctrlKey: true }, { isMac: true, inTerm: true })).toBeNull(); + expect(navKey({ key: "_", ctrlKey: true, shiftKey: true }, { isMac: true, inTerm: true })).toBeNull(); + expect(navKey({ key: "-", ctrlKey: true, altKey: true }, { isMac: false, inTerm: true })).toBeNull(); + expect(navKey({ key: "_", ctrlKey: true, shiftKey: true }, { isMac: false, inTerm: true })).toBeNull(); + }); + test("unrelated keys are null", () => { + expect(navKey({ key: "o", ctrlKey: true }, mac)).toBeNull(); + expect(navKey({ key: "=", ctrlKey: true, altKey: true }, lin)).toBeNull(); + expect(navKey({ key: "0", ctrlKey: true, shiftKey: true }, lin)).toBeNull(); + }); + test("matches by e.key, so a German layout (own minus key, odd code) works", () => { + expect(navKey({ key: "-", code: "Slash", ctrlKey: true }, mac)).toBe("back"); + expect(navKey({ key: "-", code: "Slash", ctrlKey: true, altKey: true }, lin)).toBe("back"); + expect(navKey({ key: "_", code: "Slash", ctrlKey: true, shiftKey: true }, lin)).toBe("forward"); + }); +}); + +// The App class is not exported (it boots at import), so the key handler's order is pinned in the +// source: the navigation branch must come before the zoom-out branch that also matches Ctrl+-. +describe("app keydown handler (AC-12)", () => { + const src = readFileSync(new URL("../src/client/app.ts", import.meta.url), "utf8"); + const handler = src.slice(src.indexOf("private bindKeys()"), src.indexOf("private connectEvents()")); + + test("calls navKey with the platform and terminal state", () => { + expect(handler).toMatch(/navKey\(e, \{ isMac: IS_MAC, inTerm: !!inTerm \}\)/); + }); + test("navigation is dispatched before Ctrl+Alt+- can reach zoom out", () => { + const nav = handler.indexOf("navKey("); + const zoomOut = handler.indexOf("this.zoom(-1)"); + expect(nav).toBeGreaterThan(-1); + expect(zoomOut).toBeGreaterThan(-1); + expect(nav).toBeLessThan(zoomOut); + }); + test("both directions run the Go Back / Go Forward actions", () => { + expect(handler).toMatch(/"back"/); + expect(handler).toMatch(/"forward"/); + }); +}); From 1f417f6fefb041ec10c06c29bcf1ef67b07c4766 Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 9 Nov 2026 14:32:20 +0100 Subject: [PATCH 03/27] feat(client): Go Back and Go Forward record jumps and navigate --- src/client/app.ts | 59 +++++--- src/client/editor.ts | 10 +- src/client/navigation.ts | 95 +++++++++++++ test/client.navigation.test.ts | 240 +++++++++++++++++++++++++++++++++ test/client.navkey.test.ts | 23 ---- 5 files changed, 386 insertions(+), 41 deletions(-) create mode 100644 src/client/navigation.ts create mode 100644 test/client.navigation.test.ts diff --git a/src/client/app.ts b/src/client/app.ts index de3c24d..ac3640c 100644 --- a/src/client/app.ts +++ b/src/client/app.ts @@ -16,9 +16,10 @@ import { isContextMenuOpen } from "./menu.ts"; import { SearchView } from "./search.ts"; import { ChangesView } from "./changes.ts"; import type { ChangeFile } from "./api.ts"; -import { helpKey, navKey, normSideView, referenceKey, searchKey, symbolKey, type SideView } from "./keys.ts"; +import { Navigation, routeNavZoomKey } from "./navigation.ts"; +import { helpKey, normSideView, referenceKey, searchKey, symbolKey, type SideView } from "./keys.ts"; import { reference, referenceWithCode, unsafePath } from "./reference.ts"; -import { headingText } from "./symbols.ts"; +import { headingText, type Sym } from "./symbols.ts"; import { slugify } from "./markdown.ts"; @@ -92,6 +93,12 @@ class App { private statusRight!: HTMLElement; private themeBtn!: HTMLButtonElement; private live!: HTMLElement; + private nav = new Navigation({ + open: (loc) => this.editor.open(loc.path, { pin: true, line: loc.line, focus: false }).then((ok) => (void this.tree.reveal(loc.path), ok)), + current: () => this.editor.location(), + announce: (m) => (this.live.textContent = m), + notify: (m) => this.notify(m), + }); private killArmed = 0; private help = new Help(); private sideSplit!: HTMLElement; @@ -190,13 +197,13 @@ class App { // Sidebar this.tree = new FileTree(this.info.name); - this.tree.onOpen = (p, pin) => void this.openFile(p, { pin, focus: pin }); + this.tree.onOpen = (p, pin) => void this.jump(p, { pin, focus: pin }); this.search = new SearchView(); this.tree.onSearchIn = (dir, ignored) => this.search.searchIn(dir, ignored); this.tree.onViewChanges = (p) => void this.viewChanges(p); this.tree.onHistory = (p) => void this.fileHistory(p); this.search.onReveal = () => this.showSearch(); - this.search.onOpen = (path, line, col, len) => void this.openFile(path, { line, col, len, focus: false }); + this.search.onOpen = (path, line, col, len) => void this.jump(path, { line, col, len, focus: false }); this.changes = new ChangesView(); this.changes.onOpen = (f, pin) => void this.openDiff(f, pin); this.changes.onData = (d) => { @@ -363,8 +370,8 @@ class App { }); this.palette.recent = this.recent; - this.palette.onOpenFile = (p, line) => void this.openFile(p, { pin: true, line }); - this.palette.onGoToLine = (n) => this.editor.goToLine(n); + this.palette.onOpenFile = (p, line) => void this.jump(p, { pin: true, line }); + this.palette.onGoToLine = (n) => this.jumpToLine(n); this.palette.symbols = () => this.symbolItems(); this.palette.commands = this.commands(); @@ -389,6 +396,8 @@ class App { c("view.theme", "Cycle Theme (System/Dark/Light)", () => this.cycleTheme()), c("view.zoomIn", "Increase Font Size", () => this.zoom(1), `${MOD}=`), c("view.zoomOut", "Decrease Font Size", () => this.zoom(-1), `${MOD}-`), + c("nav.back", "Go Back", () => this.navigate("back"), key("⌃-", "Ctrl+Alt+-")), + c("nav.forward", "Go Forward", () => this.navigate("forward"), key("⌃⇧-", "Ctrl+Shift+-")), c("view.zoomReset", "Reset Font Size", () => this.zoom(0), `${MOD}0`), c("search.files", "Search in Files", () => this.showSearch(), IS_MAC ? "⌘⇧F" : "Ctrl+Shift+F"), c("search.next", "Next Search Result", () => this.search.step(1), "F4"), @@ -442,6 +451,26 @@ class App { void this.tree.reveal(path); } + /** Opens a file as a jump the Back / Forward history remembers. */ + private async jump(path: string, opts: { pin?: boolean; line?: number; col?: number; len?: number; focus?: boolean } = {}) { + const from = this.editor.location(); + await this.openFile(path, opts); + if (from?.path === path && !opts.line) return; // re-showing the open file is not a jump + if (this.editor.location()?.path === path) this.nav.record(from, { path, line: opts.line ?? 1 }); + } + + private jumpToLine(n: number) { + const from = this.editor.location(); + this.editor.goToLine(n); + if (from) this.nav.record(from, { path: from.path, line: n }); + } + + private jumpToSymbol(s: Sym, slug?: string) { + const from = this.editor.location(); + this.editor.revealSymbol(s, slug); + if (from) this.nav.record(from, { path: from.path, line: s.line + 1 }); + } + private async openFromTerminal(p: string, line: number) { let rel = p; if (rel.startsWith(this.info.root + "/")) rel = rel.slice(this.info.root.length + 1); @@ -452,7 +481,7 @@ class App { if (c.startsWith("/") || c.startsWith("~")) break; const type = await api.stat(c); if (type === "file") { - await this.openFile(c, { pin: true, line, focus: false }); + await this.jump(c, { pin: true, line, focus: false }); return; } if (type === "dir") { @@ -508,7 +537,7 @@ class App { detail: `${s.kind} · line ${s.line + 1}`, icon: ``, indent: depths.indexOf(s.depth), - run: () => this.editor.revealSymbol(s, slug), + run: () => this.jumpToSymbol(s, slug), }; }); } @@ -614,9 +643,8 @@ class App { this.applyLayout(); } - /** Back / Forward through the jump list (history wiring follows; empty until then). */ private navigate(dir: "back" | "forward") { - this.notify(dir === "back" ? "No previous location" : "No next location"); + void this.nav.go(dir); } private zoom(d: number) { @@ -774,9 +802,9 @@ class App { const appKey = IS_MAC || !inTerm; let handled = true; const sk = searchKey(e, { isMac: IS_MAC, inTerm: !!inTerm }); - const nk = navKey(e, { isMac: IS_MAC, inTerm: !!inTerm }); - if (nk) this.navigate(nk === "back" ? "back" : "forward"); - else if (sk === "search") this.showSearch(); + if (routeNavZoomKey(e, { isMac: IS_MAC, inTerm: !!inTerm }, { navigate: (d) => this.navigate(d), zoom: (d) => this.zoom(d) })) { + // Go Back / Go Forward and the zoom keys + } else if (sk === "search") this.showSearch(); else if (sk === "next" || sk === "prev") this.search.step(sk === "next" ? 1 : -1); else if (e.ctrlKey && (e.code === "Backquote" || k === "`")) { if (e.shiftKey) this.newTerminal(); @@ -813,9 +841,6 @@ class App { else this.editor.cycle(dir); } else if (e.ctrlKey && e.key === "Tab" && !inTerm) this.editor.cycle(e.shiftKey ? -1 : 1); - else if (mod && (k === "=" || k === "+") && appKey) this.zoom(1); - else if (mod && k === "-" && appKey) this.zoom(-1); - else if (mod && k === "0" && appKey) this.zoom(0); else if (mod && k === "j" && appKey) this.togglePanel(); else if (helpKey(e, { isMac: IS_MAC }) && appKey) this.help.open(); else if (e.key === "Escape" && this.help.isOpen) this.help.close(); @@ -871,7 +896,7 @@ class App { } else if (msg.t === "git" && msg.status) { this.git.set(msg.status as never); } else if (msg.t === "open" && msg.path) { - void this.openFile(msg.path, { pin: true, line: msg.line, focus: false }); + void this.jump(msg.path, { pin: true, line: msg.line, focus: false }); window.focus(); } }; diff --git a/src/client/editor.ts b/src/client/editor.ts index 3bdc224..b2a5293 100644 --- a/src/client/editor.ts +++ b/src/client/editor.ts @@ -118,7 +118,7 @@ export class Editor { this.render(); } - async open(path: string, opts: { pin?: boolean; line?: number; col?: number; len?: number; focus?: boolean } = {}) { + async open(path: string, opts: { pin?: boolean; line?: number; col?: number; len?: number; focus?: boolean } = {}): Promise { let tab = this.tabs.find((t) => t.path === path && !t.isDiff); if (!tab) { // A single unpinned "preview" tab is reused while browsing. @@ -149,6 +149,14 @@ export class Editor { } if (opts.focus !== false && this.active === tab) tab.code?.focus(); this.render(); + return tab.kind !== "error"; + } + + /** Where the user is: the active text or Markdown file and its current line (1-based). */ + location(): { path: string; line: number } | undefined { + const t = this.active; + if (!t || t.isDiff || t.kind === "error" || t.kind === "image" || t.kind === "dir") return undefined; + return { path: t.path, line: t.code && t.code.activeLine >= 0 ? t.code.activeLine + 1 : 1 }; } /** diff --git a/src/client/navigation.ts b/src/client/navigation.ts new file mode 100644 index 0000000..834fa5a --- /dev/null +++ b/src/client/navigation.ts @@ -0,0 +1,95 @@ +import { type KeyLike, navKey } from "./keys.ts"; +import { NavHistory, type NavLoc } from "./navhistory.ts"; + +/** What the app shell supplies; injected so the behaviour is testable without the DOM. */ +export interface NavDeps { + /** Opens a location; resolves false when the file no longer opens. */ + open(loc: NavLoc): Promise; + /** The location the user is at now (active text file and line), if any. */ + current(): NavLoc | undefined; + /** Polite live-region announcement. */ + announce(msg: string): void; + /** Toast. */ + notify(msg: string): void; +} + +/** Back / Forward over a NavHistory, with recording of jumps and dropping of vanished files. */ +export class Navigation { + readonly history: NavHistory; + private navigating = false; + + constructor(private readonly deps: NavDeps, history: NavHistory = new NavHistory()) { + this.history = history; + } + + /** Records a jump; ignored while Back / Forward is reopening a location. */ + record(from: NavLoc | undefined, to: NavLoc): void { + if (!this.navigating) this.history.record(from, to); + } + + /** Goes back or forward, skipping files that no longer open. */ + async go(dir: "back" | "forward"): Promise { + if (this.navigating) return; + const h = this.history; + let cur = this.deps.current(); + let stay = false; + for (;;) { + let loc: NavLoc | undefined; + if (stay) loc = h.entries()[h.index()]; + else loc = dir === "back" ? h.back(cur) : h.forward(cur); + cur = undefined; + stay = false; + if (!loc) { + this.deps.notify(dir === "back" ? "No previous location" : "No next location"); + return; + } + this.navigating = true; + let ok = false; + try { + ok = await this.deps.open(loc); + } catch { + ok = false; + } finally { + this.navigating = false; + } + if (ok) { + this.deps.announce(`${dir === "back" ? "Back" : "Forward"} to ${loc.path} line ${loc.line}`); + return; + } + // The file is gone: forget it and carry on in the same direction. When the index fell back + // onto the neighbour that comes next in that direction, that neighbour is the target. + const k = h.index(); + const m = h.entries().slice(0, k).filter((e) => e.path !== loc.path).length; + h.drop(loc.path); + stay = h.entries().length > 0 && (dir === "back" ? m > 0 : m === 0); + } + } + + /** Forgets every entry of a file (deleted on disk). */ + drop(path: string): void { + this.history.drop(path); + } +} + +/** Routes the zoom / navigation keys; true when the event was handled. */ +export function routeNavZoomKey( + e: KeyLike, + o: { isMac: boolean; inTerm: boolean }, + h: { navigate(dir: "back" | "forward"): void; zoom(d: number): void }, +): boolean { + const nk = navKey(e, o); + if (nk) { + h.navigate(nk); + return true; + } + // On Linux the modifier is Ctrl, which terminal programs use: zoom stays out of a focused terminal. + const appKey = o.isMac || !o.inTerm; + const mod = o.isMac ? !!e.metaKey : !!e.ctrlKey; + if (!mod || !appKey) return false; + const k = e.key.toLowerCase(); + if (k === "=" || k === "+") h.zoom(1); + else if (k === "-") h.zoom(-1); + else if (k === "0") h.zoom(0); + else return false; + return true; +} diff --git a/test/client.navigation.test.ts b/test/client.navigation.test.ts new file mode 100644 index 0000000..270adac --- /dev/null +++ b/test/client.navigation.test.ts @@ -0,0 +1,240 @@ +import { describe, expect, test } from "bun:test"; +import { Navigation, routeNavZoomKey, type NavDeps } from "../src/client/navigation.ts"; +import type { NavLoc } from "../src/client/navhistory.ts"; + +function harness(opts: { missing?: string[] } = {}) { + const log = { opened: [] as NavLoc[], said: [] as string[], toasts: [] as string[] }; + const state: { at: NavLoc | undefined } = { at: undefined }; + const missing = new Set(opts.missing ?? []); + let nav!: Navigation; + const deps: NavDeps = { + open: async (loc) => { + if (missing.has(loc.path)) return false; + log.opened.push({ ...loc }); + state.at = { ...loc }; + // the app's openFile records a jump; during Back/Forward that must be ignored + nav.record(undefined, { ...loc }); + return true; + }, + current: () => (state.at ? { ...state.at } : undefined), + announce: (m) => log.said.push(m), + notify: (m) => log.toasts.push(m), + }; + nav = new Navigation(deps); + return { nav, log, state }; +} + +const A = (line: number): NavLoc => ({ path: "src/a.ts", line }); +const B = (line: number): NavLoc => ({ path: "src/b.ts", line }); + +describe("AC-14 two jumps, Back, Forward", () => { + test("Back reopens the first location at its line, Forward the second", async () => { + const { nav, log, state } = harness(); + state.at = A(1); + nav.record(A(1), A(42)); + state.at = A(42); + nav.record(A(42), B(7)); + state.at = B(7); + + await nav.go("back"); + expect(log.opened.at(-1)).toEqual(A(42)); + await nav.go("back"); + expect(log.opened.at(-1)).toEqual(A(1)); + await nav.go("forward"); + expect(log.opened.at(-1)).toEqual(A(42)); + await nav.go("forward"); + expect(log.opened.at(-1)).toEqual(B(7)); + }); + test("Back then Forward returns to where the user actually was", async () => { + const { nav, log, state } = harness(); + nav.record(undefined, A(5)); + state.at = A(5); + nav.record(A(5), B(9)); + state.at = B(30); // scrolled within b.ts + await nav.go("back"); + await nav.go("forward"); + expect(log.opened.at(-1)).toEqual(B(30)); + }); +}); + +describe("AC-15 empty history", () => { + test("Go Back toasts 'No previous location' and opens nothing", async () => { + const { nav, log } = harness(); + await nav.go("back"); + expect(log.toasts).toEqual(["No previous location"]); + expect(log.opened).toEqual([]); + expect(log.said).toEqual([]); + }); + test("Go Forward toasts 'No next location'", async () => { + const { nav, log } = harness(); + await nav.go("forward"); + expect(log.toasts).toEqual(["No next location"]); + }); + test("at the end of the list Forward toasts, at the start Back toasts", async () => { + const { nav, log, state } = harness(); + nav.record(A(1), B(2)); + state.at = B(2); + await nav.go("forward"); + expect(log.toasts).toEqual(["No next location"]); + await nav.go("back"); + await nav.go("back"); + expect(log.toasts).toEqual(["No next location", "No previous location"]); + }); +}); + +describe("AC-16 recording", () => { + test("record() adds to the history", () => { + const { nav } = harness(); + nav.record(A(1), B(20)); + expect(nav.history.entries()).toEqual([A(1), B(20)]); + }); + test("opening a file while another is active records from the active location, to line 1", () => { + const { nav } = harness(); + nav.record(A(33), B(1)); + expect(nav.history.entries()).toEqual([A(33), B(1)]); + }); + test("Back and Forward do not record (the reopen inside go() is ignored)", async () => { + const { nav, state } = harness(); + nav.record(A(1), B(50)); + state.at = B(50); + await nav.go("back"); + expect(nav.history.entries()).toEqual([A(1), B(50)]); + expect(nav.history.index()).toBe(0); + await nav.go("forward"); + expect(nav.history.entries()).toEqual([A(1), B(50)]); + expect(nav.history.index()).toBe(1); + }); + test("recording works again after a Back finished, even when open() failed", async () => { + const { nav, state } = harness({ missing: ["src/a.ts"] }); + nav.record(A(1), B(50)); + state.at = B(50); + await nav.go("back"); + nav.record(B(50), { path: "src/c.ts", line: 3 }); + expect(nav.history.entries().at(-1)).toEqual({ path: "src/c.ts", line: 3 }); + }); +}); + +describe("AC-17 files that no longer open", () => { + test("a missing file is dropped and Back continues to the next entry", async () => { + const { nav, log, state } = harness({ missing: ["src/b.ts"] }); + nav.record(A(1), B(2)); + state.at = B(2); + nav.record(B(2), { path: "src/c.ts", line: 9 }); + state.at = { path: "src/c.ts", line: 9 }; + // history: a:1, b:2, c:9 (current) + await nav.go("back"); + expect(log.opened).toEqual([A(1)]); + expect(nav.history.entries().some((e) => e.path === "src/b.ts")).toBe(false); + expect(log.said).toEqual(["Back to src/a.ts line 1"]); + expect(log.toasts).toEqual([]); + }); + test("when every earlier entry is gone, it toasts 'No previous location'", async () => { + const { nav, log, state } = harness({ missing: ["src/a.ts"] }); + nav.record(A(1), B(2)); + state.at = B(2); + await nav.go("back"); + expect(log.toasts).toEqual(["No previous location"]); + expect(log.opened).toEqual([]); + }); + test("Forward skips a missing file too", async () => { + const { nav, log, state } = harness({ missing: ["src/b.ts"] }); + nav.record(A(1), B(2)); + nav.record(B(2), { path: "src/c.ts", line: 4 }); + state.at = { path: "src/c.ts", line: 4 }; + await nav.go("back"); // b missing, lands on a + state.at = A(1); + await nav.go("forward"); + expect(log.opened.at(-1)).toEqual({ path: "src/c.ts", line: 4 }); + }); + test("drop() removes a deleted file's entries", () => { + const { nav } = harness(); + nav.record(A(1), B(2)); + nav.drop("src/b.ts"); + expect(nav.history.entries()).toEqual([A(1)]); + }); +}); + +describe("AC-18 announce", () => { + test("success announces 'Back to line N' and does not toast", async () => { + const { nav, log, state } = harness(); + nav.record(A(42), B(7)); + state.at = B(7); + await nav.go("back"); + expect(log.said).toEqual(["Back to src/a.ts line 42"]); + expect(log.toasts).toEqual([]); + }); + test("Forward announces 'Forward to line N'", async () => { + const { nav, log, state } = harness(); + nav.record(A(42), B(7)); + state.at = B(7); + await nav.go("back"); + state.at = A(42); + await nav.go("forward"); + expect(log.said.at(-1)).toBe("Forward to src/b.ts line 7"); + expect(log.toasts).toEqual([]); + }); +}); + +describe("AC-12 keydown routing on Linux", () => { + function route(e: Parameters[0], o: { isMac: boolean; inTerm: boolean }) { + const { nav, state } = harness(); + nav.record(A(1), B(2)); + state.at = B(2); + const zooms: number[] = []; + const nav2: string[] = []; + const handled = routeNavZoomKey(e, o, { + navigate: (d) => { + nav2.push(d); + void nav.go(d); + }, + zoom: (d) => zooms.push(d), + }); + return { handled, zooms, nav2, nav }; + } + const lin = { isMac: false, inTerm: false }; + + test("Ctrl+Alt+- runs Go Back (history navigates) and does not zoom out", async () => { + const r = route({ key: "-", code: "Minus", ctrlKey: true, altKey: true }, lin); + expect(r.handled).toBe(true); + expect(r.nav2).toEqual(["back"]); + expect(r.zooms).toEqual([]); + await Promise.resolve(); + await Promise.resolve(); + expect(r.nav.history.index()).toBe(0); + }); + test("Ctrl+Shift+- runs Go Forward and does not zoom", () => { + const r = route({ key: "_", ctrlKey: true, shiftKey: true }, lin); + expect(r.handled).toBe(true); + expect(r.nav2).toEqual(["forward"]); + expect(r.zooms).toEqual([]); + }); + test("plain Ctrl+- still zooms out and does not navigate", () => { + const r = route({ key: "-", ctrlKey: true }, lin); + expect(r.handled).toBe(true); + expect(r.zooms).toEqual([-1]); + expect(r.nav2).toEqual([]); + }); + test("macOS: Cmd+- zooms out, Ctrl+- goes back", () => { + const mac = { isMac: true, inTerm: false }; + const z = route({ key: "-", metaKey: true }, mac); + expect(z.zooms).toEqual([-1]); + expect(z.nav2).toEqual([]); + const b = route({ key: "-", ctrlKey: true }, mac); + expect(b.nav2).toEqual(["back"]); + expect(b.zooms).toEqual([]); + }); + test("Ctrl+= and Ctrl+0 zoom in and reset on Linux", () => { + expect(route({ key: "=", ctrlKey: true }, lin).zooms).toEqual([1]); + expect(route({ key: "0", ctrlKey: true }, lin).zooms).toEqual([0]); + }); + test("in a Linux terminal nothing is handled (keys stay with the shell)", () => { + const r = route({ key: "-", ctrlKey: true, altKey: true }, { isMac: false, inTerm: true }); + expect(r.handled).toBe(false); + expect(r.nav2).toEqual([]); + expect(r.zooms).toEqual([]); + }); + test("unrelated keys are not handled", () => { + const r = route({ key: "a", ctrlKey: true }, lin); + expect(r.handled).toBe(false); + }); +}); diff --git a/test/client.navkey.test.ts b/test/client.navkey.test.ts index 0be9b4a..f8b5932 100644 --- a/test/client.navkey.test.ts +++ b/test/client.navkey.test.ts @@ -1,5 +1,4 @@ import { describe, expect, test } from "bun:test"; -import { readFileSync } from "node:fs"; import { navKey } from "../src/client/keys.ts"; const mac = { isMac: true, inTerm: false }; @@ -61,25 +60,3 @@ describe("navKey everywhere", () => { expect(navKey({ key: "_", code: "Slash", ctrlKey: true, shiftKey: true }, lin)).toBe("forward"); }); }); - -// The App class is not exported (it boots at import), so the key handler's order is pinned in the -// source: the navigation branch must come before the zoom-out branch that also matches Ctrl+-. -describe("app keydown handler (AC-12)", () => { - const src = readFileSync(new URL("../src/client/app.ts", import.meta.url), "utf8"); - const handler = src.slice(src.indexOf("private bindKeys()"), src.indexOf("private connectEvents()")); - - test("calls navKey with the platform and terminal state", () => { - expect(handler).toMatch(/navKey\(e, \{ isMac: IS_MAC, inTerm: !!inTerm \}\)/); - }); - test("navigation is dispatched before Ctrl+Alt+- can reach zoom out", () => { - const nav = handler.indexOf("navKey("); - const zoomOut = handler.indexOf("this.zoom(-1)"); - expect(nav).toBeGreaterThan(-1); - expect(zoomOut).toBeGreaterThan(-1); - expect(nav).toBeLessThan(zoomOut); - }); - test("both directions run the Go Back / Go Forward actions", () => { - expect(handler).toMatch(/"back"/); - expect(handler).toMatch(/"forward"/); - }); -}); From 2b94f1cf97cc323f410505f309f7e21613049d4f Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 9 Nov 2026 14:40:27 +0100 Subject: [PATCH 04/27] docs(nav): help sheet, README, DESIGN round 18 and AGENTS for back and forward --- AGENTS.md | 1 + DESIGN.md | 19 ++++++++++ README.md | 5 +++ src/client/help.ts | 2 ++ test/client.help.nav.test.ts | 69 ++++++++++++++++++++++++++++++++++++ 5 files changed, 96 insertions(+) create mode 100644 test/client.help.nav.test.ts diff --git a/AGENTS.md b/AGENTS.md index 53d8cd4..8aefd11 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -44,6 +44,7 @@ bun run build:local # binaries + the app for this machine (needs X | `src/client/codeview.ts`, `mdview.ts`, `diffview.ts` | The three file views; each owns select-all and copy while visible | | `src/client/term/` | Terminal emulator (`terminal.ts`), renderer, view (input, links, a11y log) | | `src/client/tree.ts`, `search.ts`, `changes.ts`, `palette.ts` | Sidebar views and the palette (virtualized rows, `aria-activedescendant`); the palette's `pick()` lists anything (File History) and `@` lists symbols | +| `src/client/navhistory.ts`, `navigation.ts` | Back / Forward: the pure jump list (`NavHistory`) and `Navigation` (injected open/announce/notify, `routeNavZoomKey`); app.ts is glue | | `src/client/symbols.ts`, `reference.ts` | Go to Symbol patterns per language (pure); `path:line` references and their parsing | | `src/client/styles.css` | All styling; colour tokens at the top, dark and light | | `test/helpers/fakedom.ts`, `selfake.ts` | Minimal fake DOM for client tests (see limits below) | diff --git a/DESIGN.md b/DESIGN.md index d337e7d..c18a88b 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -1082,3 +1082,22 @@ everything under a heavy folder, so a Mac folder could show `main* 1 untracked` `FileIndex.apply` keeps its own test: its watcher paths can be the heavy folder itself, which must not restart a build. A nested repository still counts as one untracked folder with no row in Changes: status cannot tell it from a plain folder, and it does exist. + +## 27. Round 18: back and forward navigation + +After jumping by `path:line`, Go to Symbol, a search result or a terminal link there was no way back. +A panel of four (agent-workflow developer, UX/accessibility designer, code-navigation engineer, +git/security engineer) compared options. B7, a browser-style jump list with Back and Forward, won with +3 votes and no vetoes. Runners-up: risky-change flags in Changes (2 votes) and a checkpoint diff (2 votes). + +The list is `NavHistory` (`navhistory.ts`, pure): a capped list (100) with a current index; jumps within +10 lines of the current entry merge. `Navigation` (`navigation.ts`) records jumps, reopens entries +without recording, and announces "Back to path line N" in the polite live region. Keys: Ctrl+- and +Ctrl+Shift+- on macOS, Ctrl+Alt+- and Ctrl+Shift+- on Linux. The history is never stored or sent. + +| Finding | Fix | +| --- | --- | +| `src/client/app.ts` constructs the app on import, so tests cannot load it | The jump bookkeeping lives in `src/client/navigation.ts` (`Navigation` with injected open/announce/notify) and the key routing in `routeNavZoomKey`; app.ts is glue | +| On Linux Ctrl+Alt+- also matched zoom-out (Ctrl+-) | The navigation keys are routed before zoom | +| The first AC-12 tests matched app.ts source text and broke on the refactor | Replaced by behavioural tests through `routeNavZoomKey` | +| A file in the history may have vanished | `Editor.open` reports whether the file opened; Back/Forward drop the path and move on | diff --git a/README.md b/README.md index 031eebb..305995f 100644 --- a/README.md +++ b/README.md @@ -35,6 +35,9 @@ Every feature below has a screenshot in the [feature tour](https://codeviewer.21 preview scrolls to the heading itself. Covers TypeScript/JavaScript, Python, Rust, Go, C/C++, Java/Kotlin/C#, Swift, Ruby, PHP, Lua, shell, CSS, Zig, Haskell, Makefiles, YAML/TOML keys and Dockerfile stages, with patterns, not a parser. +- **Back and forward:** ⌃- (Ctrl+Alt+- on Linux) returns to where you jumped from, ⌃⇧- (Ctrl+Shift+-) goes + forward again, like a browser. Jumps nearby (within 10 lines) merge; files that were deleted are skipped. + The history lives in memory only and is announced to screen readers. - **References for your agent:** ⌥⌘C copies `path:line` (or `path:12-40` for a selection) for the cursor or the selected lines. The palette also has **Copy Reference with Code** (the reference plus the lines in a fenced block) and **Insert Reference in Terminal**, which types it at the @@ -129,6 +132,8 @@ Closing the app window quits codeviewer. Settings live in `~/.config/codeviewer/ | ⌘W · ⌘⇧[ ⌘⇧] | Close tab · switch tabs (in the terminal: switch terminals) | | ⌘W in the terminal | Close an exited terminal; a running one asks for a second press (Linux: Ctrl+W stays the shell's) | | Shift+F10 / Menu | Context menu for the selected explorer entry ("Search in Folder…", "View Changes", "File History") | +| ⌃- / Ctrl+Alt+- | Go back to the previous location (after a jump: `path:line`, Go to Line or Symbol, a search result, a terminal link, `codeviewer open`) | +| ⌃⇧- / Ctrl+Shift+- | Go forward again | | ⌘= ⌘- ⌘0 | Font size (code, terminal and the UI chrome) | | ⌘⇧G · ⌥F5 / ⌥⇧F5 | Changes · next / previous change (then next file) | | ⌘/ | Every shortcut and the terminal tips, in your platform's key names (⌘⇧7 on layouts where "/" is Shift+7, such as German) | diff --git a/src/client/help.ts b/src/client/help.ts index 8b28f07..8add6ed 100644 --- a/src/client/help.ts +++ b/src/client/help.ts @@ -26,6 +26,8 @@ export class Help { ...group("In a file", [ row(key("⌘F", "Ctrl+F"), "Find in file"), row(key("⌃G", "Ctrl+G"), "Go to line"), + row(key("⌃-", "Ctrl+Alt+-"), "Go back to the previous location (after a jump)"), + row(key("⌃⇧-", "Ctrl+Shift+-"), "Go forward again"), row(key("⌘⇧O", "Ctrl+Shift+O"), "Go to symbol (functions, classes, headings); or type @ in Go to file"), row(key("⌥⌘C", "Ctrl+Alt+C"), "Copy a path:line reference to the selected lines (palette: with code, or into the terminal)"), row(key("⌘⇧F", "Ctrl+Shift+F"), "Search in files"), diff --git a/test/client.help.nav.test.ts b/test/client.help.nav.test.ts new file mode 100644 index 0000000..15fb47c --- /dev/null +++ b/test/client.help.nav.test.ts @@ -0,0 +1,69 @@ +import { afterAll, beforeAll, describe, expect, test } from "bun:test"; +import { readFileSync } from "node:fs"; +import { installDom, uninstallDom, type FakeElement } from "./helpers/fakedom.ts"; + +const root = new URL("..", import.meta.url).pathname; +const read = (rel: string) => readFileSync(root + rel, "utf8"); + +describe("help sheet lists Go Back and Go Forward (AC-19)", () => { + let rows: { label: string; key: string }[]; + beforeAll(async () => { + installDom({ platform: "Linux x86_64" }); + const { Help } = await import("../src/client/help.ts"); + const help = new Help(); + rows = help.el.querySelectorAll(".kb-row").map((r) => ({ + label: r.querySelector("span")!.textContent, + key: r.querySelector("kbd")!.textContent, + })); + }); + afterAll(() => uninstallDom()); + + test("the sheet shows the labels of the platform the module runs as", async () => { + const { key } = await import("../src/client/util.ts"); + const back = rows.find((r) => /go back/i.test(r.label)); + const fwd = rows.find((r) => /go forward/i.test(r.label)); + expect(back?.key).toBe(key("⌃-", "Ctrl+Alt+-")); + expect(fwd?.key).toBe(key("⌃⇧-", "Ctrl+Shift+-")); + }); + + test("macOS glyphs are in the source next to the Linux names", () => { + const src = read("src/client/help.ts"); + expect(src).toMatch(/key\("⌃-", "Ctrl\+Alt\+-"\)/); + expect(src).toMatch(/key\("⌃⇧-", "Ctrl\+Shift\+-"\)/); + }); +}); + +describe("docs for back and forward (AC-20, AC-21)", () => { + test("README key table lists both keys", () => { + const readme = read("README.md"); + const table = readme.split("\n").filter((l) => l.startsWith("|")); + expect(table.some((l) => /⌃-/.test(l) && /Ctrl\+Alt\+-/.test(l) && /back/i.test(l))).toBe(true); + expect(table.some((l) => /⌃⇧-/.test(l) && /Ctrl\+Shift\+-/.test(l) && /forward/i.test(l))).toBe(true); + }); + + test("README feature list mentions back and forward", () => { + const features = read("README.md").split("\n").filter((l) => l.startsWith("- **")); + expect(features.some((l) => /back and forward/i.test(l) || /Go Back/.test(l))).toBe(true); + }); + + test("DESIGN.md has the Round 18 section with votes and a findings table", () => { + const d = read("DESIGN.md"); + const i = d.indexOf("## 27. Round 18: back and forward navigation"); + expect(i).toBeGreaterThan(-1); + const sec = d.slice(i); + expect(sec).toMatch(/B7/); + expect(sec).toMatch(/3 votes/); + expect(sec).toMatch(/risky-change flags/i); + expect(sec).toMatch(/checkpoint diff/i); + expect(sec).toMatch(/\| Finding \| Fix \|/); + expect(sec).toMatch(/navigation\.ts/); + expect(sec).toMatch(/routeNavZoomKey/); + expect(sec).toMatch(/Ctrl\+Alt\+-/); + }); + + test("AGENTS.md layout lists navhistory.ts and navigation.ts", () => { + const rows = read("AGENTS.md").split("\n").filter((l) => l.startsWith("| `src/client/")); + expect(rows.some((l) => l.includes("navhistory.ts"))).toBe(true); + expect(rows.some((l) => l.includes("navigation.ts"))).toBe(true); + }); +}); From e9865cee909d3913295683257bd81982a13b1a91 Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 9 Nov 2026 15:03:52 +0100 Subject: [PATCH 05/27] feat(git): report oldMode and newMode for changed and untracked files --- src/client/api.ts | 3 ++ src/server/git.ts | 22 ++++++++++- test/http.changes.test.ts | 37 ++++++++++++++++++ test/servergit.changes.test.ts | 71 ++++++++++++++++++++++++++++------ 4 files changed, 120 insertions(+), 13 deletions(-) diff --git a/src/client/api.ts b/src/client/api.ts index 604663e..e6f686c 100644 --- a/src/client/api.ts +++ b/src/client/api.ts @@ -77,6 +77,9 @@ export interface ChangeFile { added: number | null; removed: number | null; binary: boolean; + /** Git octal modes ("000000" when absent on that side; untracked entries carry only newMode). */ + oldMode?: string; + newMode?: string; } /** One commit of a file's history (see src/server/git.ts HistoryEntry). */ diff --git a/src/server/git.ts b/src/server/git.ts index c15abc5..47e0278 100644 --- a/src/server/git.ts +++ b/src/server/git.ts @@ -1,6 +1,6 @@ import { watch, type FSWatcher } from "node:fs"; import { HEAVY_DIRS, isHeavyFile } from "./index.ts"; -import { existsSync, lstatSync, readFileSync } from "node:fs"; +import { existsSync, lstatSync, readFileSync, realpathSync } from "node:fs"; import path from "node:path"; import { looksBinary, openRegular } from "./files.ts"; @@ -66,6 +66,9 @@ export interface ChangeEntry { added: number | null; removed: number | null; binary: boolean; + /** Git octal modes ("100644", "100755", "120000"; "000000" when absent on that side). */ + oldMode?: string; + newMode?: string; } export interface Changes { @@ -519,7 +522,7 @@ export class Git { continue; // keep consuming: the numstat records of kept entries follow } const status = (/^[MADRCTUX]$/.test(st) ? st : "M") as ChangeEntry["status"]; - const e: ChangeEntry = { path: rel, status, added: null, removed: null, binary: false }; + const e: ChangeEntry = { path: rel, status, added: null, removed: null, binary: false, oldMode: parts[0], newMode: parts[1] }; // Kept for the stat-only check below (not part of the entry sent to the client). if (status === "M" && parts[0] === parts[1]) statOnly.set(e, parts[2] ?? ""); if (two) { @@ -592,6 +595,7 @@ export class Git { const paths = all.slice(0, MAX_CHANGES); const files: ChangeEntry[] = paths.map((p) => ({ path: p, status: "?", added: null, removed: null, binary: false })); // A few files at a time through one 64 KiB buffer each: never 200 MiB of buffers per request. + for (const e of files) e.newMode = this.untrackedMode(e.path); const queue = files.slice(0, COUNT_UNTRACKED); await Promise.all(Array.from({ length: 8 }, async () => { for (let e = queue.shift(); e; e = queue.shift()) await this.countLines(e); @@ -599,6 +603,20 @@ export class Git { return { files, truncated: all.length > MAX_CHANGES }; } + /** Mode of an untracked file from lstat (a link is never followed); the parent directory must resolve inside the root. */ + private untrackedMode(rel: string): string | undefined { + try { + const abs = path.join(this.root, ...rel.split("/")); + const parent = realpathSync(path.dirname(abs)); + if (parent !== this.root && !parent.startsWith(this.root + path.sep)) return undefined; + const st = lstatSync(abs); + if (st.isSymbolicLink()) return "120000"; + return st.mode & 0o111 ? "100755" : "100644"; + } catch { + return undefined; + } + } + private async countLines(e: ChangeEntry) { try { const { fh, st } = await openRegular(path.join(this.root, ...e.path.split("/")), this.root); diff --git a/test/http.changes.test.ts b/test/http.changes.test.ts index e31fa90..29a80ba 100644 --- a/test/http.changes.test.ts +++ b/test/http.changes.test.ts @@ -102,3 +102,40 @@ test("/api/git/base rejects anything but a full object id and malformed paths", } expect((await get(base2, `/api/git/base?path=f.txt&rev=${mainOid}`)).status).toBe(404); }); + +test("/api/git/changes carries oldMode/newMode per entry", async () => { + const { chmodSync, symlinkSync, writeFileSync } = await import("node:fs"); + const d = await realpath(await mkdtemp(path.join(tmpdir(), "cv-httpmodes-"))); + const sh = (...a: string[]) => { + const r = Bun.spawnSync(["git", "-c", "user.name=t", "-c", "user.email=t@t", ...a], { cwd: d, stderr: "pipe", env: { ...process.env, GIT_CONFIG_GLOBAL: "/dev/null", GIT_CONFIG_SYSTEM: "/dev/null" } }); + if (r.exitCode !== 0) throw new Error(r.stderr.toString()); + }; + let srv: ReturnType | undefined; + try { + writeFileSync(path.join(d, "a.sh"), "echo\n"); + writeFileSync(path.join(d, "gone.txt"), "bye\n"); + sh("init", "-q", "-b", "main"); + sh("add", "."); + sh("commit", "-q", "-m", "init"); + chmodSync(path.join(d, "a.sh"), 0o755); + sh("rm", "-q", "gone.txt"); + writeFileSync(path.join(d, "added.txt"), "x\n"); + sh("add", "added.txt"); + writeFileSync(path.join(d, "u.sh"), "#!/bin/sh\n"); + chmodSync(path.join(d, "u.sh"), 0o755); + symlinkSync("a.sh", path.join(d, "u-link")); + srv = startServer({ root: d, port: 0, host: "127.0.0.1", shell: "/bin/sh", token, openExternal: () => {} }); + const b = srv.url.split("/#")[0]!; + const j = (await (await get(b, "/api/git/changes?scope=head")).json()) as { files: { path: string; oldMode?: string; newMode?: string }[] }; + const f = Object.fromEntries(j.files.map((x) => [x.path, x])); + expect([f["a.sh"]!.oldMode, f["a.sh"]!.newMode]).toEqual(["100644", "100755"]); + expect([f["added.txt"]!.oldMode, f["added.txt"]!.newMode]).toEqual(["000000", "100644"]); + expect([f["gone.txt"]!.oldMode, f["gone.txt"]!.newMode]).toEqual(["100644", "000000"]); + expect(f["u.sh"]!.newMode).toBe("100755"); + expect(f["u.sh"]!.oldMode).toBeUndefined(); + expect(f["u-link"]!.newMode).toBe("120000"); + } finally { + srv?.stop(); + await rm(d, { recursive: true, force: true }); + } +}); diff --git a/test/servergit.changes.test.ts b/test/servergit.changes.test.ts index e71b046..c1594e4 100644 --- a/test/servergit.changes.test.ts +++ b/test/servergit.changes.test.ts @@ -90,11 +90,11 @@ test("feature branch: branch scope diffs the working tree against the merge-base expect(b.commits).toBe(2); expect(b.subjects).toEqual(["chore: empty", "feat: first"]); const bf = byPath(b.files); - expect(bf["a.txt"]).toEqual({ path: "a.txt", status: "M", added: 3, removed: 1, binary: false }); - expect(bf["bin.dat"]).toEqual({ path: "bin.dat", status: "D", added: null, removed: null, binary: true }); - expect(bf["new.txt"]).toEqual({ path: "new.txt", oldPath: "old name.txt", status: "R", added: 1, removed: 0, binary: false }); - expect(bf["notes todo.md"]).toEqual({ path: "notes todo.md", status: "?", added: 3, removed: 0, binary: false }); - expect(bf["blob.bin"]).toEqual({ path: "blob.bin", status: "?", added: null, removed: null, binary: true }); + expect(bf["a.txt"]).toEqual({ path: "a.txt", status: "M", added: 3, removed: 1, binary: false, oldMode: "100644", newMode: "100644" }); + expect(bf["bin.dat"]).toEqual({ path: "bin.dat", status: "D", added: null, removed: null, binary: true, oldMode: "100644", newMode: "000000" }); + expect(bf["new.txt"]).toEqual({ path: "new.txt", oldPath: "old name.txt", status: "R", added: 1, removed: 0, binary: false, oldMode: "100644", newMode: "100644" }); + expect(bf["notes todo.md"]).toEqual({ path: "notes todo.md", status: "?", added: 3, removed: 0, binary: false, newMode: "100644" }); + expect(bf["blob.bin"]).toEqual({ path: "blob.bin", status: "?", added: null, removed: null, binary: true, newMode: "100644" }); expect(Object.keys(bf).sort()).toEqual(["a.txt", "bin.dat", "blob.bin", "new.txt", "notes todo.md"]); const hd = await g.changes("head"); @@ -155,7 +155,7 @@ test("detached HEAD still compares with the base; a repo without remote falls ba expect(c.base?.name).toBe("main"); expect(c.base?.same).toBe(false); expect(c.commits).toBe(1); - expect(c.files).toEqual([{ path: "f.txt", status: "M", added: 1, removed: 0, binary: false }]); + expect(c.files).toEqual([{ path: "f.txt", status: "M", added: 1, removed: 0, binary: false, oldMode: "100644", newMode: "100644" }]); }); test("unborn branch: no rev, untracked files only", async () => { @@ -167,7 +167,7 @@ test("unborn branch: no rev, untracked files only", async () => { const c = await g.changes("branch"); expect(c.rev).toBeNull(); expect(c.base).toBeNull(); - expect(c.files).toEqual([{ path: "x.txt", status: "?", added: 1, removed: 0, binary: false }]); + expect(c.files).toEqual([{ path: "x.txt", status: "?", added: 1, removed: 0, binary: false, newMode: "100644" }]); }); test("merge conflicts are listed as U, including modify/delete ones", async () => { @@ -203,7 +203,7 @@ test("a file removed from the index but present on disk is one modified entry", sh(["rm", "-q", "--cached", "f.txt"], d); const g = await git(d); const c = await g.changes("head"); - expect(c.files).toEqual([{ path: "f.txt", status: "M", added: null, removed: null, binary: false }]); + expect(c.files).toEqual([{ path: "f.txt", status: "M", added: null, removed: null, binary: false, oldMode: "100644", newMode: "000000" }]); }); test("an orphan branch with staged files lists them as added against the empty tree", async () => { @@ -215,7 +215,7 @@ test("an orphan branch with staged files lists them as added against the empty t const g = await git(d); const c = await g.changes("branch"); expect(c.rev).toBeNull(); - expect(c.files).toEqual([{ path: "s.txt", status: "A", added: 2, removed: 0, binary: false }]); + expect(c.files).toEqual([{ path: "s.txt", status: "A", added: 2, removed: 0, binary: false, oldMode: "000000", newMode: "100644" }]); }); test("over the cap, kept entries still carry their counts and truncation is reported", async () => { @@ -253,7 +253,7 @@ test("hostile configs: order file, mailmap and relative paths cannot hang or bla const t0 = performance.now(); const c = await Promise.race([g.changes("branch"), Bun.sleep(8000).then(() => null)]); expect(performance.now() - t0).toBeLessThan(8000); - expect(c?.files).toEqual([{ path: "a.txt", status: "M", added: 1, removed: 0, binary: false }]); + expect(c?.files).toEqual([{ path: "a.txt", status: "M", added: 1, removed: 0, binary: false, oldMode: "100644", newMode: "100644" }]); expect(c?.subjects).toEqual(["work"]); expect(c?.commits).toBe(1); }); @@ -285,7 +285,7 @@ test("the list is capped and reports truncation; untracked symlinks are not read await symlink(path.join(dir, "outside2.txt"), path.join(links, "escape")); const lg = await git(links); const lc = await lg.changes("head"); - expect(lc.files).toEqual([{ path: "escape", status: "?", added: null, removed: null, binary: false }]); + expect(lc.files).toEqual([{ path: "escape", status: "?", added: null, removed: null, binary: false, newMode: "120000" }]); }); test("status counts as untracked exactly what Changes lists", async () => { @@ -394,3 +394,52 @@ test("past the check limit, untracked folders are counted unchecked", async () = expect(Object.keys(s.files)).toHaveLength(1001); expect(s.unlisted).toEqual([]); }); + +test("changes carry oldMode/newMode: exec bit, symlink replacement, untracked files", async () => { + const { chmodSync, symlinkSync, unlinkSync, writeFileSync: wf } = await import("node:fs"); + const d = await realpath(await mkdtemp(path.join(tmpdir(), "cv-modes-"))); + try { + wf(path.join(d, "a.sh"), "echo a\n"); + wf(path.join(d, "swap.txt"), "swap me\n"); + wf(path.join(d, "gone.txt"), "bye\n"); + sh(["init", "-q", "-b", "main"], d); + sh(["add", "."], d); + sh(["commit", "-qm", "init"], d); + chmodSync(path.join(d, "a.sh"), 0o755); + unlinkSync(path.join(d, "swap.txt")); + symlinkSync("a.sh", path.join(d, "swap.txt")); + unlinkSync(path.join(d, "gone.txt")); + wf(path.join(d, "added.txt"), "new\n"); + sh(["add", "added.txt"], d); + wf(path.join(d, "u-exec.sh"), "#!/bin/sh\n"); + chmodSync(path.join(d, "u-exec.sh"), 0o755); + wf(path.join(d, "u-plain.txt"), "plain\n"); + chmodSync(path.join(d, "u-plain.txt"), 0o644); + symlinkSync("a.sh", path.join(d, "u-link")); + symlinkSync("/etc/passwd", path.join(d, "u-outside")); + const g = await git(d); + const f = byPath((await g.changes("head")).files) as Record; + // AC-1 + expect(f["a.sh"]!.oldMode).toBe("100644"); + expect(f["a.sh"]!.newMode).toBe("100755"); + // AC-2 + expect(f["swap.txt"]!.oldMode).toBe("100644"); + expect(f["swap.txt"]!.newMode).toBe("120000"); + // AC-4 (server side): added and deleted tracked files + expect(f["added.txt"]!.oldMode).toBe("000000"); + expect(f["added.txt"]!.newMode).toBe("100644"); + expect(f["gone.txt"]!.oldMode).toBe("100644"); + expect(f["gone.txt"]!.newMode).toBe("000000"); + // AC-3 + expect(f["u-exec.sh"]!.newMode).toBe("100755"); + expect("oldMode" in f["u-exec.sh"]!).toBe(false); + expect(f["u-plain.txt"]!.newMode).toBe("100644"); + expect("oldMode" in f["u-plain.txt"]!).toBe(false); + expect(f["u-link"]!.newMode).toBe("120000"); + expect("oldMode" in f["u-link"]!).toBe(false); + expect(f["u-outside"]!.newMode).toBe("120000"); + expect("oldMode" in f["u-outside"]!).toBe(false); + } finally { + await rm(d, { recursive: true, force: true }); + } +}); From 1d3329df79f2bc27d6e14b42b349b9b96bf9e508 Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 9 Nov 2026 15:09:58 +0100 Subject: [PATCH 06/27] test(git): pin untrackedMode containment and lstat failure --- test/servergit.untrackedmode.test.ts | 33 ++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 test/servergit.untrackedmode.test.ts diff --git a/test/servergit.untrackedmode.test.ts b/test/servergit.untrackedmode.test.ts new file mode 100644 index 0000000..d9a8446 --- /dev/null +++ b/test/servergit.untrackedmode.test.ts @@ -0,0 +1,33 @@ +import { expect, test } from "bun:test"; +import { mkdtemp, mkdir, realpath, rm, writeFile } from "node:fs/promises"; +import { symlinkSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { Git } from "../src/server/git.ts"; + +const env = { ...process.env, GIT_CONFIG_GLOBAL: "/dev/null", GIT_CONFIG_SYSTEM: "/dev/null" }; + +test("untrackedMode: parent resolving outside the root or a vanished file yields no mode", async () => { + const dir = await realpath(await mkdtemp(path.join(tmpdir(), "cv-umode-"))); + const outside = path.join(dir, "outside"); + const root = path.join(dir, "root"); + await mkdir(outside); + await mkdir(path.join(root, "sub"), { recursive: true }); + const r = Bun.spawnSync(["git", "init", "-q", "-b", "main"], { cwd: root, env }); + expect(r.exitCode).toBe(0); + await writeFile(path.join(outside, "secret.sh"), "#!/bin/sh\n", { mode: 0o755 }); + await writeFile(path.join(root, "sub", "ok.sh"), "#!/bin/sh\n", { mode: 0o755 }); + symlinkSync(outside, path.join(root, "linkdir")); + const g = (await Git.open(root))!; + expect(g).not.toBeNull(); + try { + const mode = (rel: string) => (g as unknown as { untrackedMode(p: string): string | undefined }).untrackedMode(rel); + expect(mode("sub/ok.sh")).toBe("100755"); // inside the root: reported + expect(mode("linkdir/secret.sh")).toBeUndefined(); // parent resolves outside: nothing read + expect(mode("sub/gone.sh")).toBeUndefined(); // lstat fails: no throw + expect(mode("nodir/x.sh")).toBeUndefined(); // parent missing: no throw + } finally { + g.stop(); + await rm(dir, { recursive: true, force: true }); + } +}); From 8dbadf0f80c6e9ad29d31acfec1836416df1c41e Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 9 Nov 2026 15:13:32 +0100 Subject: [PATCH 07/27] feat(client): riskFlags and riskLabel for changed files --- AGENTS.md | 1 + src/client/risk.ts | 44 ++++++++++++++++ test/client.risk.test.ts | 108 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 153 insertions(+) create mode 100644 src/client/risk.ts create mode 100644 test/client.risk.test.ts diff --git a/AGENTS.md b/AGENTS.md index 8aefd11..07c87a3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -46,6 +46,7 @@ bun run build:local # binaries + the app for this machine (needs X | `src/client/tree.ts`, `search.ts`, `changes.ts`, `palette.ts` | Sidebar views and the palette (virtualized rows, `aria-activedescendant`); the palette's `pick()` lists anything (File History) and `@` lists symbols | | `src/client/navhistory.ts`, `navigation.ts` | Back / Forward: the pure jump list (`NavHistory`) and `Navigation` (injected open/announce/notify, `routeNavZoomKey`); app.ts is glue | | `src/client/symbols.ts`, `reference.ts` | Go to Symbol patterns per language (pure); `path:line` references and their parsing | +| `src/client/risk.ts` | Risk flags for Changes rows (pure): `riskFlags`, `riskLabel`; hints, never a safety verdict | | `src/client/styles.css` | All styling; colour tokens at the top, dark and light | | `test/helpers/fakedom.ts`, `selfake.ts` | Minimal fake DOM for client tests (see limits below) | | `scripts/shots.ts`, `shots.list.ts`, `demo-repo.ts` | Landing page screenshots: a deterministic demo repo, an isolated server, `packaging/macos/appshot.swift` driving an offscreen WKWebView | diff --git a/src/client/risk.ts b/src/client/risk.ts new file mode 100644 index 0000000..a48aeb1 --- /dev/null +++ b/src/client/risk.ts @@ -0,0 +1,44 @@ +// Risk flags for Changes rows: hints for attention, never a safety verdict. +export type RiskFlag = "exec" | "symlink" | "ci" | "hook" | "lockfile" | "manifest" | "env"; + +const DIRS: Array<[RiskFlag, string[]]> = [ + ["ci", [".github/workflows/", ".github/actions/", ".circleci/", ".buildkite/"]], + ["hook", [".husky/", ".githooks/"]], +]; +const NAMES: Array<[RiskFlag, string[]]> = [ + ["ci", [".gitlab-ci.yml", "Jenkinsfile", "azure-pipelines.yml"]], + ["hook", [".pre-commit-config.yaml", "lefthook.yml", ".lefthook.yml"]], + ["lockfile", ["package-lock.json", "yarn.lock", "pnpm-lock.yaml", "bun.lock", "bun.lockb", "Cargo.lock", "Gemfile.lock", "poetry.lock", "uv.lock", "Pipfile.lock", "composer.lock", "go.sum"]], + ["manifest", ["package.json", "Cargo.toml", "pyproject.toml", "setup.py", "Gemfile", "go.mod", "composer.json", "requirements.txt", "Makefile", "Dockerfile"]], + ["env", [".env", ".npmrc", ".yarnrc", ".yarnrc.yml", ".pypirc"]], +]; +const PREFIXES: Array<[RiskFlag, string]> = [["manifest", "Dockerfile."], ["env", ".env."]]; +const ORDER: RiskFlag[] = ["exec", "symlink", "ci", "hook", "lockfile", "manifest", "env"]; + +export function riskFlags(f: { path: string; status: string; oldMode?: string; newMode?: string }): RiskFlag[] { + const hit = new Set(); + if (f.newMode === "100755" && f.oldMode !== "100755") hit.add("exec"); + if (f.newMode === "120000" && f.oldMode !== "120000") hit.add("symlink"); + const p = f.path; + const name = p.slice(p.lastIndexOf("/") + 1); + for (const [flag, dirs] of DIRS) { + if (dirs.some((d) => p.startsWith(d) || p.includes("/" + d))) hit.add(flag); + } + for (const [flag, names] of NAMES) if (names.includes(name)) hit.add(flag); + for (const [flag, pre] of PREFIXES) if (name.startsWith(pre)) hit.add(flag); + return ORDER.filter((x) => hit.has(x)); +} + +const LABELS: Record = { + exec: "executable bit added", + symlink: "new symbolic link", + ci: "CI configuration", + hook: "git hook", + lockfile: "dependency lockfile", + manifest: "package manifest or build file", + env: "environment or registry configuration", +}; + +export function riskLabel(flag: RiskFlag): string { + return LABELS[flag]; +} diff --git a/test/client.risk.test.ts b/test/client.risk.test.ts new file mode 100644 index 0000000..0a5f76f --- /dev/null +++ b/test/client.risk.test.ts @@ -0,0 +1,108 @@ +import { describe, expect, test } from "bun:test"; +import { riskFlags, riskLabel, type RiskFlag } from "../src/client/risk.ts"; + +const fl = (path: string, status = "M", oldMode?: string, newMode?: string) => { + const o: { path: string; status: string; oldMode?: string; newMode?: string } = { path, status }; + if (oldMode !== undefined) o.oldMode = oldMode; + if (newMode !== undefined) o.newMode = newMode; + return riskFlags(o); +}; + +describe("riskFlags exec and symlink (AC-5, AC-6)", () => { + test("executable bit", () => { + expect(fl("run.sh", "A", undefined, "100755")).toEqual(["exec"]); + expect(fl("run.sh", "M", "100644", "100755")).toEqual(["exec"]); + expect(fl("run.sh", "M", "100755", "100755")).toEqual([]); + expect(fl("run.sh", "M", undefined, "100644")).toEqual([]); + }); + test("symlink", () => { + expect(fl("l", "M", "100644", "120000")).toEqual(["symlink"]); + expect(fl("l", "M", "120000", "120000")).toEqual([]); + expect(fl("l", "?", undefined, "120000")).toEqual(["symlink"]); + }); +}); + +describe("riskFlags directory rules (AC-7)", () => { + const dirs: Array<[string, RiskFlag]> = [ + [".github/workflows/", "ci"], [".github/actions/", "ci"], [".circleci/", "ci"], [".buildkite/", "ci"], + [".husky/", "hook"], [".githooks/", "hook"], + ]; + for (const [d, flag] of dirs) { + test(d, () => { + expect(fl(`${d}x.yml`)).toEqual([flag]); + expect(fl(`sub/${d}x.yml`)).toEqual([flag]); + expect(fl(`x${d}a.yml`)).toEqual([]); + expect(fl(`${d.slice(0, -1)}X/a.yml`)).toEqual([]); + }); + } + test("name rules match the last segment exactly", () => { + expect(fl("a/Jenkinsfile")).toEqual(["ci"]); + expect(fl("Jenkinsfile/readme.md")).toEqual([]); + expect(fl("Jenkinsfile.bak")).toEqual([]); + }); + const names: Array<[RiskFlag, string[]]> = [ + ["ci", [".gitlab-ci.yml", "Jenkinsfile", "azure-pipelines.yml"]], + ["hook", [".pre-commit-config.yaml", "lefthook.yml", ".lefthook.yml"]], + ["lockfile", ["package-lock.json", "yarn.lock", "pnpm-lock.yaml", "bun.lock", "bun.lockb", "Cargo.lock", "Gemfile.lock", "poetry.lock", "uv.lock", "Pipfile.lock", "composer.lock", "go.sum"]], + ["manifest", ["package.json", "Cargo.toml", "pyproject.toml", "setup.py", "Gemfile", "go.mod", "composer.json", "requirements.txt", "Makefile", "Dockerfile"]], + ["env", [".env", ".npmrc", ".yarnrc", ".yarnrc.yml", ".pypirc"]], + ]; + for (const [flag, list] of names) { + test(`${flag} names at root and nested`, () => { + for (const n of list) { + expect(fl(n)).toEqual([flag]); + expect(fl(`deep/er/${n}`)).toEqual([flag]); + } + }); + } +}); + +describe("riskFlags prefix rules and case (AC-8, AC-9)", () => { + test("Dockerfile prefix", () => { + expect(fl("Dockerfile.dev")).toEqual(["manifest"]); + expect(fl("docker/Dockerfile.prod")).toEqual(["manifest"]); + expect(fl("Dockerfile")).toEqual(["manifest"]); + expect(fl("MyDockerfile")).toEqual([]); + }); + test(".env prefix", () => { + expect(fl(".env.local")).toEqual(["env"]); + expect(fl("a/.env.production")).toEqual(["env"]); + expect(fl(".env")).toEqual(["env"]); + expect(fl(".envrc")).toEqual([]); + expect(fl("env")).toEqual([]); + }); + test("deletions keep path flags", () => { + expect(fl("package-lock.json", "D", "100644", "000000")).toEqual(["lockfile"]); + }); + test("case sensitive", () => { + expect(fl("dockerfile")).toEqual([]); + expect(fl("makefile")).toEqual([]); + expect(fl("PACKAGE.JSON")).toEqual([]); + }); +}); + +describe("riskFlags order, uniqueness and labels (AC-10)", () => { + test("union order", () => { + expect(fl(".github/workflows/deploy.sh", "A", undefined, "100755")).toEqual(["exec", "ci"]); + expect(fl(".husky/pre-commit", "A", undefined, "120000")).toEqual(["symlink", "hook"]); + expect(fl("Makefile", "M", "100644", "100755")).toEqual(["exec", "manifest"]); + expect(fl(".env", "A", undefined, "100755")).toEqual(["exec", "env"]); + expect(fl("deps/uv.lock", "A", undefined, "120000")).toEqual(["symlink", "lockfile"]); + expect(fl(".husky/.env", "M")).toEqual(["hook", "env"]); + expect(fl(".github/workflows/Makefile", "M")).toEqual(["ci", "manifest"]); + expect(fl("scripts/setup.sh", "M", "100644", "100755")).toEqual(["exec"]); + expect(fl(".github/workflows/deploy.sh", "A", "000000", "100755")).toEqual(["exec", "ci"]); + }); + test("each flag at most once", () => { + expect(fl("Jenkinsfile", "M")).toEqual(["ci"]); + }); + test("labels", () => { + expect(riskLabel("exec")).toBe("executable bit added"); + expect(riskLabel("symlink")).toBe("new symbolic link"); + expect(riskLabel("ci")).toBe("CI configuration"); + expect(riskLabel("hook")).toBe("git hook"); + expect(riskLabel("lockfile")).toBe("dependency lockfile"); + expect(riskLabel("manifest")).toBe("package manifest or build file"); + expect(riskLabel("env")).toBe("environment or registry configuration"); + }); +}); From 0a35de74f4d8c06db6198ab1126dae2f3e3472d9 Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 9 Nov 2026 15:24:39 +0100 Subject: [PATCH 08/27] feat(changes): risk badges and a flagged count in the Changes view --- DESIGN.md | 21 ++++++ README.md | 5 ++ src/client/changes.ts | 20 +++++- src/client/styles.css | 3 + test/client.changes.test.ts | 127 ++++++++++++++++++++++++++++++++++++ 5 files changed, 173 insertions(+), 3 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index c18a88b..a67c03e 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -1101,3 +1101,24 @@ Ctrl+Shift+- on macOS, Ctrl+Alt+- and Ctrl+Shift+- on Linux. The history is neve | On Linux Ctrl+Alt+- also matched zoom-out (Ctrl+-) | The navigation keys are routed before zoom | | The first AC-12 tests matched app.ts source text and broke on the refactor | Replaced by behavioural tests through `routeNavZoomKey` | | A file in the history may have vanished | `Editor.open` reports whether the file opened; Back/Forward drop the path and move on | + +## 28. Round 19: risk flags in Changes + +A panel of four (agent-workflow developer, git/security engineer, UX/accessibility designer, +code-navigation engineer) picked the next feature. Risk flags in Changes won with 3 votes (agent-workflow +developer #1, git/security engineer #1, UX/accessibility designer #2). Back/Forward fidelity got 4 votes +and follows as a separate part; a checkpoint diff got 3 votes but is size L and deferred. Single votes: +live-change announcements, Go to Definition, a hidden-Unicode detector. + +`ChangeEntry` carries git's `oldMode`/`newMode`. `src/client/risk.ts` (pure) turns an entry into flags +(`exec`, `symlink`, `ci`, `hook`, `lockfile`, `manifest`, `env`); `ChangesView` renders one badge per flag +after the path, counts flagged rows in the summary and exposes `flags(path)`. The flags are hints, not a verdict. + +| Finding | Fix | +| --- | --- | +| Existing Changes tests compared whole entries with `toEqual`, so the new mode fields changed seven expectations | Each now names the exact modes: stricter, not looser | +| The mode of an untracked file must not follow a link or leave the root | It comes from `lstat` after the parent directory's realpath is checked to be inside the root; tests cover a parent link leaving the root and a file that vanishes before `lstat` | +| A mode-only change has no diff lines | The exec badge is the only sign of a `chmod +x` in the list | +| The plan claimed a path matches at most one path category, but a directory rule and a name rule can both match (`.husky/.env` is hook and env) | Flags are a set returned in union order; the plan review caught it before any test was written | +| The Changes summary had no `role="status"` although it is a count that changes | The summary wrap is a status region | + diff --git a/README.md b/README.md index 305995f..3b561ff 100644 --- a/README.md +++ b/README.md @@ -54,6 +54,11 @@ Every feature below has a screenshot in the [feature tour](https://codeviewer.21 `+/−` per file and a one-line summary. Each file opens as a read-only unified diff; ⌥F5 walks the changes and moves on to the next file. Reviewed files dim until they change again. Right-click a file in the Explorer and pick "View Changes" to jump to its diff. + - **Risk flags:** rows in Changes carry a small badge, and the summary counts them ("· 2 flagged"), + for an added executable bit, a new symbolic link, CI configuration, git hooks, dependency + lockfiles, package manifests and build files, and `.env`/registry configuration. They are hints + for where to look first, not a safety verdict: they match file names, directories and git modes + only, never contents, and they can miss things (a malicious change in an ordinary file is not flagged). - **File History:** "Git: File History" in the palette (or "File History" in the Explorer's file menu) lists the commits that changed the file, renames followed, newest first; type to filter by subject, Enter to open what that commit changed as a read-only diff tab labelled with its short id. diff --git a/src/client/changes.ts b/src/client/changes.ts index 1460d11..69aa129 100644 --- a/src/client/changes.ts +++ b/src/client/changes.ts @@ -1,5 +1,6 @@ import { api, type ChangeFile, type ChangesData } from "./api.ts"; import type { Letter } from "./git.ts"; +import { riskFlags, riskLabel, type RiskFlag } from "./risk.ts"; import { basename, dirname, esc, h, showHidden } from "./util.ts"; export type Scope = "head" | "branch"; @@ -41,6 +42,8 @@ export function summarize(files: readonly ChangeFile[]): { added: number; remove /** What "reviewed" was reviewed against: any later edit to the file un-dims its row. */ const seenKey = (f: ChangeFile) => `${f.status}:${f.added}:${f.removed}:${f.oldPath ?? ""}`; +const BADGE: Record = { exec: "exec", symlink: "symlink", ci: "CI", hook: "hook", lockfile: "lock", manifest: "manifest", env: "env" }; + const safe = (s: string) => esc(showHidden(s)); const plural = (n: number, one: string, many: string) => `${n} ${n === 1 ? one : many}`; @@ -79,7 +82,7 @@ export class ChangesView { constructor(opts: { debounce?: number } = {}) { this.debounce = opts.debounce ?? 150; this.scopeEl = h("div", { class: "ch-scope", role: "group", "aria-label": "Comparison", hidden: true }); - this.summary = h("div", { class: "ch-summary-wrap", hidden: true }); + this.summary = h("div", { class: "ch-summary-wrap", role: "status", hidden: true }); this.list = h("div", { class: "ch-list", role: "listbox", tabindex: "0", "aria-label": "Changed files" }); this.el = h("div", { class: "changes-view" }, this.scopeEl, this.summary, this.list); this.scopeEl.addEventListener("click", (e) => { @@ -108,6 +111,12 @@ export class ChangesView { this.render(); } + /** Risk flags of the listed row with this path ([] for an unknown path or an unflagged row). */ + flags(path: string): RiskFlag[] { + const f = this.files.find((x) => x.path === path); + return f ? riskFlags(f) : []; + } + get scopeName(): Scope { return this.scope; } @@ -318,10 +327,12 @@ export class ChangesView { const commits = d.scope === "branch" && d.commits ? `·${plural(d.commits, "commit", "commits")}` : ""; + const flagged = files.filter((f) => riskFlags(f).length).length; + const flaggedHtml = flagged ? `· ${flagged} flagged` : ""; const p = added + removed ? Math.round((added / (added + removed)) * 100) : 50; this.summary.innerHTML = `
${files.length} ${files.length === 1 ? "file" : "files"}` + - `·+${added.toLocaleString()} −${removed.toLocaleString()}${commits}` + + `·+${added.toLocaleString()} −${removed.toLocaleString()}${commits}${flaggedHtml}` + `${safe(this.vs)}
` + `
`; if (!files.length) { @@ -352,9 +363,12 @@ export class ChangesView { : f.added === null ? `—` : `${f.added ? `+${f.added}` : ""}${f.removed ? `−${f.removed}` : ""}`; + const risks = riskFlags(f) + .map((r) => `${BADGE[r]}`) + .join(""); html += `
` + - `${letter}${name}${dir}${counts}
`; + `${letter}${name}${dir}${risks}${counts}`; }); if (d.truncated) html += `
Showing the first ${files.length.toLocaleString()} files
`; this.list.innerHTML = html; diff --git a/src/client/styles.css b/src/client/styles.css index 7eab8ab..d84c1e0 100644 --- a/src/client/styles.css +++ b/src/client/styles.css @@ -2250,6 +2250,9 @@ li[data-state="done"] .upd-mark { .ch-add { color: var(--git-add); } .ch-del { color: var(--git-del); } .ch-bin { color: var(--fg-faint); font-family: var(--font-mono); } +/* Risk flags: a word in a bordered pill, so the meaning never rests on colour alone. */ +.risk { flex: none; margin-left: 4px; padding: 0 4px; border: 1px solid color-mix(in srgb, var(--warn) 60%, transparent); border-radius: 3px; font: 600 9.5px/14px var(--font-ui); letter-spacing: 0.04em; color: var(--warn); background: color-mix(in srgb, var(--warn) 12%, transparent); } +.ch-riskcount { margin-left: 6px; color: var(--warn); } /* diff tab */ .tab-kind { margin-left: 2px; padding: 0 4px; border-radius: 3px; font: 600 9.5px/14px var(--font-ui); letter-spacing: 0.04em; text-transform: uppercase; color: var(--fg-muted); background: var(--bg-active); } diff --git a/test/client.changes.test.ts b/test/client.changes.test.ts index d3d999f..4e47ab0 100644 --- a/test/client.changes.test.ts +++ b/test/client.changes.test.ts @@ -351,3 +351,130 @@ describe("ChangesView.ready", () => { expect(t.v.files).toEqual([]); }); }); + +describe("risk flags in the Changes view", () => { + let restore: (() => void) | null = null; + afterEach(() => { + restore?.(); + restore = null; + }); + + async function load(files: ChangeFile[]) { + const m = mockFetch(() => Response.json(data(files))); + restore = m.restore; + const v = new C.ChangesView({ debounce: 0 }); + const el = v.el as unknown as FakeElement; + v.setRepo(true); + await v.refresh(); + return { v, list: el.querySelector(".ch-list")!, summary: el.querySelector(".ch-summary-wrap")! }; + } + + const attrs = (tag: string): Record => { + const out: Record = {}; + for (const m of tag.matchAll(/([\w-]+)="([^"]*)"/g)) out[m[1]!] = m[2]!; + return out; + }; + /** The rows of the rendered list as HTML strings (rows contain no nested divs). */ + const rowsOf = (html: string) => html.split('
")[0] + "
"); + /** Badge elements of a row: attributes, text and the offset in the row. */ + const badges = (row: string) => + [...row.matchAll(/<(\w+)([^>]*)>([^<]*)<\/\1>/g)] + .map((m) => ({ attrs: attrs(m[2]!), text: m[3]!, at: m.index! })) + .filter((b) => (b.attrs.class ?? "").split(/\s+/).includes("risk")); + const rowFor = (html: string, path: string) => rowsOf(html).find((r) => attrs(r.match(/^]*>/)![0]).title?.startsWith(path + " ·")); + + test("flags(path) reports the computed flags of a listed row", async () => { + const t = await load([ + f("a.sh", "A", 1, 0, { oldMode: "000000", newMode: "100755" }), + f("package-lock.json", "M", 2, 1), + f("plain.txt", "M"), + ]); + expect(t.v.flags("a.sh")).toEqual(["exec"]); + expect(t.v.flags("package-lock.json")).toEqual(["lockfile"]); + expect(t.v.flags("plain.txt")).toEqual([]); + expect(t.v.flags("nope")).toEqual([]); + }); + + test("a flagged row has a badge after the path with text, title and aria-label", async () => { + const t = await load([f("a.sh", "A", 1, 0, { oldMode: "000000", newMode: "100755" })]); + const row = rowFor(t.list.innerHTML, "a.sh")!; + expect(row).toBeDefined(); + const all = badges(row); + expect(all.length).toBe(1); + const [b, ...rest] = all; + expect(rest).toEqual([]); + expect(b!.text).toBe("exec"); + expect(b!.attrs.title).toBe("executable bit added"); + expect(b!.attrs["aria-label"]).toBe("executable bit added"); + expect(b!.at).toBeGreaterThan(row.indexOf('class="ch-name"')); + }); + + test("every flag has its visible text, and multiple flags keep union order", async () => { + const cases: Array<[ChangeFile, string[], string[]]> = [ + [f("l", "A", 1, 0, { oldMode: "000000", newMode: "120000" }), ["symlink"], ["new symbolic link"]], + [f(".github/workflows/ci.yml", "M"), ["CI"], ["CI configuration"]], + [f(".husky/pre-commit", "M"), ["hook"], ["git hook"]], + [f("yarn.lock", "M"), ["lock"], ["dependency lockfile"]], + [f("Dockerfile", "M"), ["manifest"], ["package manifest or build file"]], + [f(".env.local", "M"), ["env"], ["environment or registry configuration"]], + [f(".github/workflows/deploy.sh", "A", 1, 0, { oldMode: "000000", newMode: "100755" }), ["exec", "CI"], ["executable bit added", "CI configuration"]], + [f(".husky/.env", "M"), ["hook", "env"], ["git hook", "environment or registry configuration"]], + ]; + for (const [file, texts, labels] of cases) { + const t = await load([file]); + restore?.(); + restore = null; + const bs = badges(rowFor(t.list.innerHTML, file.path)!); + expect(bs.map((b) => b.text)).toEqual(texts); + expect(bs.map((b) => b.attrs.title)).toEqual(labels); + expect(bs.map((b) => b.attrs["aria-label"])).toEqual(labels); + } + }); + + test("hostile paths are escaped, badges included", async () => { + const evil = '.husky/"q".sh'; + const t = await load([f(evil, "A", 1, 0, { oldMode: "000000", newMode: "100755" }), f('Dockerfile."x"', "M")]); + const html = t.list.innerHTML; + expect(html).not.toContain(""); + expect(html).toContain("<img src=x onerror=1>"); + const row = rowsOf(html).find((r) => r.includes("<img"))!; + expect(badges(row).map((b) => b.text)).toEqual(["exec", "hook"]); + // Attribute values are closed strings (the path's quotes are entities) and no attribute is named onerror. + for (const r of rowsOf(html)) { + const tag = r.match(/]*>/)![0]; + expect(Object.keys(attrs(tag))).not.toContain("onerror"); + expect(tag.replace(/$/g, "").replace(/\s[\w-]+="[^"]*"/g, "").trim()).toBe(""); + } + expect(attrs(rowsOf(html).find((r) => r.includes("<img"))!.match(/]*>/)![0]).title).toContain(""q""); + }); + + test("summary appends the flagged count only when rows are flagged", async () => { + const t = await load([ + f("a.sh", "A", 1, 0, { oldMode: "000000", newMode: "100755" }), + f("package.json", "M"), + f("plain.txt", "M"), + ]); + expect(t.summary.innerHTML).toContain("· 2 flagged"); + expect(t.summary.innerHTML.match(/flagged/g)!.length).toBe(1); + // The count is announced: the summary region is a status. + expect(t.summary.innerHTML.includes('role="status"') || t.summary.getAttribute("role") === "status").toBe(true); + const none = await load([f("plain.txt", "M"), f("b.ts", "M", 1, 0, { oldMode: "100644", newMode: "100644" })]); + expect(none.summary.innerHTML).not.toContain("flagged"); + expect(none.summary.innerHTML.includes('role="status"') || none.summary.getAttribute("role") === "status").toBe(true); + }); + + test("flags do not reorder rows, and an unflagged row renders as before", async () => { + const files = [f("z.ts", "M", 10, 2), f("package.json", "M"), f("a.ts", "M", 1, 0, { oldMode: "100644", newMode: "100644" })]; + const t = await load(files); + expect(t.v.files.map((x) => x.path)).toEqual(["a.ts", "package.json", "z.ts"]); + const titles = rowsOf(t.list.innerHTML).map((r) => attrs(r.match(/]*>/)![0]).title); + expect(titles).toEqual(["a.ts · Modified", "package.json · Modified", "z.ts · Modified"]); + expect(rowsOf(t.list.innerHTML)[0]).toBe( + '
' + + 'Ma.ts' + + '+1
', + ); + expect(badges(rowsOf(t.list.innerHTML)[0]!)).toEqual([]); + }); +}); From c263a6a559e088582de853af2ebaad0a839757dc Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 9 Nov 2026 15:33:32 +0100 Subject: [PATCH 09/27] fix(nav): Back returns to the visible line and keeps Markdown previews --- DESIGN.md | 3 + src/client/app.ts | 4 +- src/client/codeview.ts | 11 ++ src/client/editor.ts | 23 ++- src/client/navhistory.ts | 15 +- test/client.editor.location.test.ts | 249 ++++++++++++++++++++++++++++ test/client.navhistory.test.ts | 85 ++++++++++ test/client.navigation.test.ts | 16 ++ 8 files changed, 398 insertions(+), 8 deletions(-) create mode 100644 test/client.editor.location.test.ts diff --git a/DESIGN.md b/DESIGN.md index a67c03e..0f0ef8a 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -1121,4 +1121,7 @@ after the path, counts flagged rows in the summary and exposes `flags(path)`. Th | A mode-only change has no diff lines | The exec badge is the only sign of a `chmod +x` in the list | | The plan claimed a path matches at most one path category, but a directory rule and a name rule can both match (`.husky/.env` is hook and env) | Flags are a set returned in union order; the plan review caught it before any test was written | | The Changes summary had no `role="status"` although it is a count that changes | The summary wrap is a status region | +| Back restored the clicked line, which may be scrolled off screen, and a never-clicked file always went back to line 1 | `Editor.location()` reports the active line only while it is on screen, else the topmost visible line (`CodeView.firstVisibleLine`/`lastVisibleLine`, inclusive) | +| A Markdown file shown as the rendered preview came back as source | `NavLoc.view = "preview"` is recorded by `location()` and `Editor.landing()`, kept by `NavHistory` on every update path (set or removed together with the line), and `Editor.open(path, { view: "preview" })` reopens the preview; `app.ts` only forwards it | +| Closing a tab might forget where Back should go | Pinned by tests: history entries survive `Editor.close`, and Back reopens the file; diff and image tabs have no location | diff --git a/src/client/app.ts b/src/client/app.ts index ac3640c..c2b3c1e 100644 --- a/src/client/app.ts +++ b/src/client/app.ts @@ -94,7 +94,7 @@ class App { private themeBtn!: HTMLButtonElement; private live!: HTMLElement; private nav = new Navigation({ - open: (loc) => this.editor.open(loc.path, { pin: true, line: loc.line, focus: false }).then((ok) => (void this.tree.reveal(loc.path), ok)), + open: (loc) => this.editor.open(loc.path, loc.view ? { pin: true, view: loc.view, focus: false } : { pin: true, line: loc.line, focus: false }).then((ok) => (void this.tree.reveal(loc.path), ok)), current: () => this.editor.location(), announce: (m) => (this.live.textContent = m), notify: (m) => this.notify(m), @@ -456,7 +456,7 @@ class App { const from = this.editor.location(); await this.openFile(path, opts); if (from?.path === path && !opts.line) return; // re-showing the open file is not a jump - if (this.editor.location()?.path === path) this.nav.record(from, { path, line: opts.line ?? 1 }); + if (this.editor.location()?.path === path) this.nav.record(from, this.editor.landing(path, opts.line)); } private jumpToLine(n: number) { diff --git a/src/client/codeview.ts b/src/client/codeview.ts index ec0d763..07506fb 100644 --- a/src/client/codeview.ts +++ b/src/client/codeview.ts @@ -699,6 +699,17 @@ export class CodeView { } } + /** Topmost visible line (0-based). */ + firstVisibleLine(): number { + return Math.max(0, Math.floor(this.scroller.scrollTop / this.lh)); + } + + /** Last visible line (0-based, inclusive). */ + lastVisibleLine(): number { + const s = this.scroller; + return Math.max(this.firstVisibleLine(), Math.ceil((s.scrollTop + (s.clientHeight || 800)) / this.lh) - 1); + } + /** Scrolls so that 0-based `line` is visible (centered when far away). */ revealLine(line: number, select = true) { line = Math.max(0, Math.min(this.lines.length - 1, line)); diff --git a/src/client/editor.ts b/src/client/editor.ts index b2a5293..15f0ddb 100644 --- a/src/client/editor.ts +++ b/src/client/editor.ts @@ -6,6 +6,7 @@ import { languageFor, languageName } from "./highlight.ts"; import { fileIcon, icons } from "./icons.ts"; import { isMarkdown } from "./markdown.ts"; import { MarkdownView } from "./mdview.ts"; +import type { NavLoc } from "./navhistory.ts"; import { decidePreview, tooLargeNote } from "./mdgate.ts"; import { planView, toggleLabel, toggleMode, viewMode } from "./viewstate.ts"; import { basename, debounce, dirname, esc, formatBytes, h, MOD, showHidden } from "./util.ts"; @@ -118,7 +119,7 @@ export class Editor { this.render(); } - async open(path: string, opts: { pin?: boolean; line?: number; col?: number; len?: number; focus?: boolean } = {}): Promise { + async open(path: string, opts: { pin?: boolean; line?: number; col?: number; len?: number; focus?: boolean; view?: "preview" } = {}): Promise { let tab = this.tabs.find((t) => t.path === path && !t.isDiff); if (!tab) { // A single unpinned "preview" tab is reused while browsing. @@ -141,6 +142,11 @@ export class Editor { } else { if (opts.pin) tab.pinned = true; this.activate(tab); + if (opts.view === "preview" && !opts.line && isMarkdown(path) && tab.asSource && tab.code) { + tab.asSource = false; + this.dropCode(tab); + await this.load(tab); + } if (opts.line && tab.md) await this.toSource(tab); if (opts.line && tab.code) { if (opts.col !== undefined && opts.len) tab.code.showRange(opts.line - 1, opts.col, opts.col + opts.len); @@ -153,10 +159,21 @@ export class Editor { } /** Where the user is: the active text or Markdown file and its current line (1-based). */ - location(): { path: string; line: number } | undefined { + location(): NavLoc | undefined { const t = this.active; if (!t || t.isDiff || t.kind === "error" || t.kind === "image" || t.kind === "dir") return undefined; - return { path: t.path, line: t.code && t.code.activeLine >= 0 ? t.code.activeLine + 1 : 1 }; + if (t.md && !t.code) return { path: t.path, line: 1, view: "preview" }; + const c = t.code; + if (!c) return { path: t.path, line: 1 }; + const inView = c.activeLine >= c.firstVisibleLine() && c.activeLine <= c.lastVisibleLine(); + return { path: t.path, line: c.activeLine >= 0 && inView ? c.activeLine + 1 : c.firstVisibleLine() + 1 }; + } + + /** The NavLoc for what `open(path, { line })` now shows. */ + landing(path: string, line?: number): NavLoc { + const t = this.tabs.find((x) => x.path === path && !x.isDiff); + if (!line && t?.md && !t.code) return { path, line: 1, view: "preview" }; + return { path, line: line ?? 1 }; } /** diff --git a/src/client/navhistory.ts b/src/client/navhistory.ts index 8695970..daf92a8 100644 --- a/src/client/navhistory.ts +++ b/src/client/navhistory.ts @@ -1,9 +1,18 @@ export interface NavLoc { path: string; line: number; // 1-based + /** Set when the location was a rendered Markdown preview. */ + view?: "preview"; } -const copy = (l: NavLoc): NavLoc => ({ path: l.path, line: l.line }); +const copy = (l: NavLoc): NavLoc => (l.view ? { path: l.path, line: l.line, view: l.view } : { path: l.path, line: l.line }); + +/** Gives `e` the line and view of `l` (the view key is removed when `l` has none). */ +const take = (e: NavLoc, l: NavLoc): void => { + e.line = l.line; + if (l.view) e.view = l.view; + else delete e.view; +}; /** Browser-style back/forward list: entries plus the index of the current one. */ export class NavHistory { @@ -21,11 +30,11 @@ export class NavHistory { this.list.length = this.idx + 1; if (from) { const cur = this.list[this.idx]; - if (cur && cur.path === from.path) cur.line = from.line; + if (cur && cur.path === from.path) take(cur, from); else this.push(from); } const cur = this.list[this.idx]; - if (cur && cur.path === to.path && Math.abs(cur.line - to.line) <= this.near) cur.line = to.line; + if (cur && cur.path === to.path && Math.abs(cur.line - to.line) <= this.near) take(cur, to); else this.push(to); } diff --git a/test/client.editor.location.test.ts b/test/client.editor.location.test.ts new file mode 100644 index 0000000..6564908 --- /dev/null +++ b/test/client.editor.location.test.ts @@ -0,0 +1,249 @@ +import { afterAll, afterEach, beforeAll, describe, expect, test } from "bun:test"; +import { mockFetch, uninstallDom } from "./helpers/fakedom.ts"; +import { loadCodeView, mount, type Env } from "./helpers/selfake.ts"; +import { NavHistory, type NavLoc } from "../src/client/navhistory.ts"; +import { Navigation } from "../src/client/navigation.ts"; + +// Back/Forward fidelity: which line and which view of a file the Editor reports as "where I am". + +let env: Env; +let E: typeof import("../src/client/editor.ts"); +const g = globalThis as Record; +const saved: Record = {}; +let restore = () => {}; + +beforeAll(async () => { + env = await loadCodeView(); + const proto = Object.getPrototypeOf(env.dom.document.createElement("div")) as Record; + saved.qs = proto.querySelector; + const qs = proto.querySelector as (s: string) => unknown; + proto.querySelector = function (this: unknown, sel: string) { + return qs.call(this, sel) ?? (sel === ".tab-close" ? env.dom.document.createElement("button") : null); + }; + if (!proto.prepend) { + saved.prepend = true; + proto.prepend = function (this: { children: unknown[] }, ...n: unknown[]) { + this.children.unshift(...n); + }; + } + E = await import("../src/client/editor.ts"); +}); +afterEach(() => restore()); +afterAll(() => { + const proto = Object.getPrototypeOf(env.dom.document.createElement("div")) as Record; + proto.querySelector = saved.qs; + if (saved.prepend) delete proto.prepend; + uninstallDom(); + void g; +}); + +const flush = () => new Promise((r) => setTimeout(r, 20)); +const L = 20; +const body = Array.from({ length: 60 }, (_, i) => `line ${i + 1}`).join("\n"); + +/** An Editor whose active tab is a real code view (60 lines, line height L) and nothing else. */ +function withCode() { + const m = mount(env, body); + (m.view as Record).lh = L; + const ed = new E.Editor(env.dom.document.createElement("div") as unknown as HTMLElement); + const tab = { path: "src/a.ts", kind: "text", code: m.view, pinned: true } as never; + ed.tabs.push(tab); + ed.active = tab; + return { ed, m }; +} + +const view = (m: ReturnType, scrollTop: number, clientHeight: number) => { + const s = m.scroller as unknown as Record; + s.scrollTop = scrollTop; + s.clientHeight = clientHeight; +}; + +describe("AC-16 location() line is on screen", () => { + test("no click: the topmost visible line, 1-based", () => { + const { ed, m } = withCode(); + view(m, 10 * L, 5 * L); + expect(m.view.activeLine).toBe(-1); + expect(ed.location()).toEqual({ path: "src/a.ts", line: 11 }); + }); + + test("a click inside the visible range wins", () => { + const { ed, m } = withCode(); + view(m, 10 * L, 5 * L); + m.view.activeLine = 12; + expect(ed.location()?.line).toBe(13); + }); + + test("a click outside the visible range is replaced by the topmost visible line", () => { + const { ed, m } = withCode(); + view(m, 10 * L, 5 * L); + m.view.activeLine = 2; + expect(ed.location()?.line).toBe(11); + m.view.activeLine = 40; + expect(ed.location()?.line).toBe(11); + }); + + test("the first and last visible rows are inside the range", () => { + const { ed, m } = withCode(); + view(m, 10 * L, 5 * L); + m.view.activeLine = 10; + expect(ed.location()?.line).toBe(11); + m.view.activeLine = 14; + expect(ed.location()?.line).toBe(15); + m.view.activeLine = 15; // one row below the 5 visible ones + expect(ed.location()?.line).toBe(11); + }); + + test("scrolled to the top and no click: line 1", () => { + const { ed, m } = withCode(); + view(m, 0, 5 * L); + expect(ed.location()?.line).toBe(1); + }); + + test("firstVisibleLine and lastVisibleLine are 0-based and inclusive", () => { + const { m } = withCode(); + view(m, 10 * L, 5 * L); + expect(m.view.firstVisibleLine()).toBe(10); + expect(m.view.lastVisibleLine()).toBe(14); + view(m, 10 * L + 5, 5 * L); // a partly shown row counts + expect(m.view.firstVisibleLine()).toBe(10); + expect(m.view.lastVisibleLine()).toBe(15); + view(m, 0, 0); // no layout: 800px assumed + expect(m.view.firstVisibleLine()).toBe(0); + expect(m.view.lastVisibleLine()).toBe(39); + }); +}); + +describe("AC-18 / AC-21 location() and the view", () => { + test("a rendered Markdown preview reports view preview at line 1", () => { + const ed = new E.Editor(env.dom.document.createElement("div") as unknown as HTMLElement); + const tab = { path: "docs/a.md", kind: "text", md: {}, pinned: true } as never; + ed.tabs.push(tab); + ed.active = tab; + expect(ed.location()).toEqual({ path: "docs/a.md", line: 1, view: "preview" }); + }); + + test("Markdown source and other text tabs carry no view key", () => { + const { ed } = withCode(); + const loc = ed.location()!; + expect("view" in loc).toBe(false); + const m = mount(env, body); + const tab = { path: "docs/a.md", kind: "text", code: m.view, pinned: true } as never; + ed.tabs.push(tab); + ed.active = tab; + expect("view" in ed.location()!).toBe(false); + }); + + test("diff and image tabs have no location", () => { + const ed = new E.Editor(env.dom.document.createElement("div") as unknown as HTMLElement); + const diff = { path: "a.ts", kind: "text", isDiff: true, pinned: true } as never; + ed.tabs.push(diff); + ed.active = diff; + expect(ed.location()).toBeUndefined(); + const img = { path: "a.png", kind: "image", pinned: true } as never; + ed.tabs.push(img); + ed.active = img; + expect(ed.location()).toBeUndefined(); + }); + + const open = async (md: boolean) => { + const m = mockFetch((url) => { + const u = new URL(url, "http://x"); + if (u.pathname === "/api/file") { + const t = md ? "# Title\n\nbody\n" : "x\ny\n"; + return new Response(t, { headers: { "x-cv-kind": "text", "x-cv-size": String(t.length), "x-cv-mtime": "1" } }); + } + return new Response("{}", { status: 404 }); + }); + restore = m.restore; + return new E.Editor(env.dom.document.createElement("div") as unknown as HTMLElement); + }; + + test("open(md, {view: preview}) shows the preview and the location says so", async () => { + const ed = await open(true); + await ed.open("docs/a.md", { pin: true, view: "preview" }); + await flush(); + expect(ed.active?.md).toBeDefined(); + expect(ed.active?.code).toBeUndefined(); + expect(ed.location()).toEqual({ path: "docs/a.md", line: 1, view: "preview" }); + }); + + test("landing: a Markdown preview opened without a line is {line 1, view preview}", async () => { + const ed = await open(true); + await ed.open("docs/a.md", { pin: true }); + await flush(); + expect(ed.active?.md).toBeDefined(); + expect(ed.landing("docs/a.md")).toEqual({ path: "docs/a.md", line: 1, view: "preview" }); + }); + + test("landing: a line, as source, or a non-Markdown file has no view", async () => { + const ed = await open(true); + await ed.open("docs/a.md", { pin: true }); + await flush(); + const l = ed.landing("docs/a.md", 5); // a line only means something in the source + expect(l).toEqual({ path: "docs/a.md", line: 5 }); + expect("view" in l).toBe(false); + const ed2 = await open(false); + await ed2.open("src/b.ts", { pin: true }); + await flush(); + const l2 = ed2.landing("src/b.ts"); + expect(l2).toEqual({ path: "src/b.ts", line: 1 }); + expect("view" in l2).toBe(false); + }); +}); + +describe("AC-19 round trip and AC-20 closing a tab", () => { + const A = (view?: "preview"): NavLoc => (view ? { path: "a.md", line: 1, view } : { path: "a.md", line: 1 }); + const B: NavLoc = { path: "b.ts", line: 1 }; + + test("preview of a.md, jump to b.ts, Back opens a.md as a preview", async () => { + const opened: NavLoc[] = []; + let at: NavLoc | undefined = A("preview"); + const nav = new Navigation({ + open: async (loc) => (opened.push({ ...loc }), (at = { ...loc }), true), + current: () => at, + announce: () => {}, + notify: () => {}, + }); + nav.record(at, B); + at = B; + await nav.go("back"); + expect(opened.at(-1)).toEqual({ path: "a.md", line: 1, view: "preview" }); + await nav.go("forward"); + expect(opened.at(-1)).toEqual(B); + }); + + test("closing a tab keeps its history entries and Back can reopen it", async () => { + const m = mockFetch((url) => { + if (new URL(url, "http://x").pathname === "/api/file") return new Response("x\ny\n", { headers: { "x-cv-kind": "text", "x-cv-size": "4", "x-cv-mtime": "1" } }); + return new Response("{}", { status: 404 }); + }); + restore = m.restore; + const ed = new E.Editor(env.dom.document.createElement("div") as unknown as HTMLElement); + const hist = new NavHistory(); + const nav = new Navigation( + { + // (the fake DOM cannot reveal a line, so the line is left out) + open: (loc) => ed.open(loc.path, { pin: true, focus: false }), + current: () => ed.location(), + announce: () => {}, + notify: () => {}, + }, + hist, + ); + await ed.open("src/a.ts", { pin: true }); + await ed.open("src/b.ts", { pin: true }); + await flush(); + nav.record({ path: "src/a.ts", line: 1 }, { path: "src/b.ts", line: 1 }); + const before = hist.entries(); + const idx = hist.index(); + expect(before.length).toBe(2); + ed.close(ed.tabs.find((t) => t.path === "src/a.ts")!); + expect(ed.tabs.some((t) => t.path === "src/a.ts")).toBe(false); + expect(hist.entries()).toEqual(before); + expect(hist.index()).toBe(idx); + await nav.go("back"); + await flush(); + expect(ed.tabs.some((t) => t.path === "src/a.ts")).toBe(true); + expect(ed.location()?.path).toBe("src/a.ts"); + }); +}); diff --git a/test/client.navhistory.test.ts b/test/client.navhistory.test.ts index 35942ab..d3bf5a1 100644 --- a/test/client.navhistory.test.ts +++ b/test/client.navhistory.test.ts @@ -276,3 +276,88 @@ describe("NavHistory", () => { expect(h.forward(L("a", 1))).toEqual(L("b", 30)); }); }); + +describe("AC-17 view survives in the history", () => { + const P = (path: string, line: number): NavLoc => ({ path, line, view: "preview" }); + const seed = (h: NavHistory, ...es: NavLoc[]) => { + // build entries by recording each as a separate jump (distinct paths, so nothing coalesces) + h.record(undefined, es[0]!); + for (const e of es.slice(1)) h.record(undefined, e); + }; + + test("(1) a preview location keeps its view; one without has no view key", () => { + const h = new NavHistory(); + h.record(undefined, P("a.md", 1)); + expect(h.entries()).toEqual([P("a.md", 1)]); + const h2 = new NavHistory(); + h2.record(undefined, L("a.md", 1)); + expect("view" in h2.entries()[0]!).toBe(false); + }); + + test("(2) from updating the current entry sets its view", () => { + const h = new NavHistory({ near: 10 }); + h.record(undefined, L("a.md", 1)); + h.record(P("a.md", 5), L("b.ts", 1)); + expect(h.entries()).toEqual([P("a.md", 5), L("b.ts", 1)]); + }); + + test("(3) from updating the current entry removes its view", () => { + const h = new NavHistory({ near: 10 }); + h.record(undefined, P("a.md", 1)); + h.record(L("a.md", 7), L("b.ts", 1)); + const es = h.entries(); + expect(es).toEqual([L("a.md", 7), L("b.ts", 1)]); + expect("view" in es[0]!).toBe(false); + }); + + test("(4) an appended from keeps its own view", () => { + const h = new NavHistory({ near: 10 }); + h.record(undefined, L("c.ts", 1)); + h.record(P("a.md", 3), L("b.ts", 1)); + expect(h.entries()).toEqual([L("c.ts", 1), P("a.md", 3), L("b.ts", 1)]); + }); + + test("(5) a coalescing to sets the view", () => { + const h = new NavHistory({ near: 10 }); + h.record(undefined, L("a.md", 3)); + h.record(undefined, P("a.md", 8)); + expect(h.entries()).toEqual([P("a.md", 8)]); + }); + + test("(6) a coalescing to removes the view", () => { + const h = new NavHistory({ near: 10 }); + h.record(undefined, P("a.md", 3)); + h.record(undefined, L("a.md", 8)); + const es = h.entries(); + expect(es).toEqual([L("a.md", 8)]); + expect("view" in es[0]!).toBe(false); + }); + + test("(d) an appended to keeps its own view", () => { + const h = new NavHistory({ near: 10 }); + h.record(undefined, L("a.ts", 1)); + h.record(undefined, P("b.md", 1)); + expect(h.entries()).toEqual([L("a.ts", 1), P("b.md", 1)]); + }); + + test("(7) back and forward store current with its view, and return entries as stored", () => { + const h = new NavHistory(); + seed(h, P("a.md", 1), L("b.ts", 1)); + expect(h.back(L("b.ts", 4))).toEqual(P("a.md", 1)); + expect(h.entries()).toEqual([P("a.md", 1), L("b.ts", 4)]); + // leaving the preview of a.md for b.ts, then coming back and leaving it as source + expect(h.forward(L("a.md", 9))).toEqual(L("b.ts", 4)); + const es = h.entries(); + expect(es[0]).toEqual(L("a.md", 9)); + expect("view" in es[0]!).toBe(false); + expect(h.back(P("b.ts", 2))).toEqual(L("a.md", 9)); + expect(h.entries()[1]).toEqual(P("b.ts", 2)); + }); + + test("(7) back with a current that has a view sets it on the entry left", () => { + const h = new NavHistory(); + seed(h, L("a.md", 1), L("b.ts", 1)); + h.back(P("b.ts", 1)); + expect(h.entries()[1]).toEqual(P("b.ts", 1)); + }); +}); diff --git a/test/client.navigation.test.ts b/test/client.navigation.test.ts index 270adac..2ee05cc 100644 --- a/test/client.navigation.test.ts +++ b/test/client.navigation.test.ts @@ -238,3 +238,19 @@ describe("AC-12 keydown routing on Linux", () => { expect(r.handled).toBe(false); }); }); + +describe("AC-18 / AC-19 Navigation hands the recorded view to open", () => { + test("Back to a preview passes view preview to deps.open; Forward to a source passes none", async () => { + const { nav, log, state } = harness(); + const P: NavLoc = { path: "docs/a.md", line: 1, view: "preview" }; + state.at = { ...P }; + nav.record({ ...P }, B(1)); + state.at = B(1); + await nav.go("back"); + expect(log.opened.at(-1)).toEqual(P); + state.at = { ...P }; + await nav.go("forward"); + expect(log.opened.at(-1)).toEqual(B(1)); + expect("view" in log.opened.at(-1)!).toBe(false); + }); +}); From 9a61a6bd29c00507da8f7906ea2eed5b4877755d Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 9 Nov 2026 15:39:22 +0100 Subject: [PATCH 10/27] test(editor): pin the preview flip of an open Markdown tab --- test/client.editor.preview.test.ts | 77 ++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 test/client.editor.preview.test.ts diff --git a/test/client.editor.preview.test.ts b/test/client.editor.preview.test.ts new file mode 100644 index 0000000..3dd7f74 --- /dev/null +++ b/test/client.editor.preview.test.ts @@ -0,0 +1,77 @@ +import { afterAll, afterEach, beforeAll, expect, test } from "bun:test"; +import { mockFetch, uninstallDom } from "./helpers/fakedom.ts"; +import { loadCodeView, type Env } from "./helpers/selfake.ts"; + +// Back to a Markdown file that is already open as source must reopen the preview, and a line keeps the source. + +let env: Env; +let E: typeof import("../src/client/editor.ts"); +const saved: Record = {}; +let restore = () => {}; + +beforeAll(async () => { + env = await loadCodeView(); + const proto = Object.getPrototypeOf(env.dom.document.createElement("div")) as Record; + saved.qs = proto.querySelector; + const qs = proto.querySelector as (s: string) => unknown; + proto.querySelector = function (this: unknown, sel: string) { + return qs.call(this, sel) ?? (sel === ".tab-close" ? env.dom.document.createElement("button") : null); + }; + if (!proto.prepend) { + saved.prepend = true; + proto.prepend = function (this: { children: unknown[] }, ...n: unknown[]) { + this.children.unshift(...n); + }; + } + E = await import("../src/client/editor.ts"); +}); +afterEach(() => restore()); +afterAll(() => { + const proto = Object.getPrototypeOf(env.dom.document.createElement("div")) as Record; + proto.querySelector = saved.qs; + if (saved.prepend) delete proto.prepend; + uninstallDom(); +}); + +const flush = () => new Promise((r) => setTimeout(r, 20)); + +async function setup() { + const m = mockFetch((url) => { + if (new URL(url, "http://x").pathname === "/api/file") { + const t = "# Title\n\nbody\n"; + return new Response(t, { headers: { "x-cv-kind": "text", "x-cv-size": String(t.length), "x-cv-mtime": "1" } }); + } + return new Response("{}", { status: 404 }); + }); + restore = m.restore; + const ed = new E.Editor(env.dom.document.createElement("div") as unknown as HTMLElement); + await ed.open("docs/a.md", { pin: true }); + await flush(); + expect(ed.active?.md).toBeDefined(); + // switch the open tab to its source + (ed as unknown as { flip(t: unknown): void }).flip(ed.active); + await flush(); + expect(ed.active?.md).toBeUndefined(); + expect(ed.active?.code).toBeDefined(); + return ed; +} + +test("open with view preview on a Markdown tab shown as source shows the preview", async () => { + const ed = await setup(); + await ed.open("docs/a.md", { pin: true, view: "preview" }); + await flush(); + expect(ed.tabs.length).toBe(1); + expect(ed.active?.md).toBeDefined(); + expect(ed.active?.code).toBeUndefined(); + expect(ed.location()).toEqual({ path: "docs/a.md", line: 1, view: "preview" }); +}); + +test("open with a line keeps the source, even with view preview", async () => { + const ed = await setup(); + ed.active!.code!.revealLine = () => {}; // the fake DOM cannot find a row to reveal + await ed.open("docs/a.md", { pin: true, line: 2, view: "preview" }); + await flush(); + expect(ed.active?.md).toBeUndefined(); + expect(ed.active?.code).toBeDefined(); + expect("view" in ed.location()!).toBe(false); +}); From 8ca44c9a8eb976c1e6f07e058009ba985365076f Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 9 Nov 2026 16:00:46 +0100 Subject: [PATCH 11/27] feat(client): scanHidden, hiddenName and nextHidden for invisible characters --- AGENTS.md | 1 + src/client/hidden.ts | 78 +++++++++++++++ test/client.hiddenscan.test.ts | 171 +++++++++++++++++++++++++++++++++ test/redos.matrix.test.ts | 28 ++++++ 4 files changed, 278 insertions(+) create mode 100644 src/client/hidden.ts create mode 100644 test/client.hiddenscan.test.ts diff --git a/AGENTS.md b/AGENTS.md index 07c87a3..ca0f292 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -45,6 +45,7 @@ bun run build:local # binaries + the app for this machine (needs X | `src/client/term/` | Terminal emulator (`terminal.ts`), renderer, view (input, links, a11y log) | | `src/client/tree.ts`, `search.ts`, `changes.ts`, `palette.ts` | Sidebar views and the palette (virtualized rows, `aria-activedescendant`); the palette's `pick()` lists anything (File History) and `@` lists symbols | | `src/client/navhistory.ts`, `navigation.ts` | Back / Forward: the pure jump list (`NavHistory`) and `Navigation` (injected open/announce/notify, `routeNavZoomKey`); app.ts is glue | +| `src/client/hidden.ts` | Hidden-character scanner (zero-width, bidi, tag, variation; pure, also imported by the server) | | `src/client/symbols.ts`, `reference.ts` | Go to Symbol patterns per language (pure); `path:line` references and their parsing | | `src/client/risk.ts` | Risk flags for Changes rows (pure): `riskFlags`, `riskLabel`; hints, never a safety verdict | | `src/client/styles.css` | All styling; colour tokens at the top, dark and light | diff --git a/src/client/hidden.ts b/src/client/hidden.ts new file mode 100644 index 0000000..856f5bd --- /dev/null +++ b/src/client/hidden.ts @@ -0,0 +1,78 @@ +// Hidden-character scanner: zero-width, bidi, tag and variation characters that render as nothing +// (or reorder text) and are used to smuggle content. Pure, no DOM, one linear pass; the server +// imports it too. Results are hints, never verdicts. +export type HiddenKind = "zero-width" | "bidi" | "tag" | "variation"; +export interface HiddenHit { line: number; col: number; cp: number; kind: HiddenKind } + +const isVariation = (cp: number) => (cp >= 0xfe00 && cp <= 0xfe0f) || (cp >= 0xe0100 && cp <= 0xe01ef); + +/** True when a joiner after `prev` is ordinary (scripts that need it, emoji sequences). */ +function joinerExempt(prev: number): boolean { + return ( + (prev >= 0x0600 && prev <= 0x06ff) || + (prev >= 0x0750 && prev <= 0x077f) || + (prev >= 0x0900 && prev <= 0x0dff) || + prev === 0xfe0f || + prev >= 0x1f000 + ); +} + +/** The kind of a counted character, or undefined. `prev` is the code point just before (-1: none). */ +function classify(cp: number, prev: number, offset: number): HiddenKind | undefined { + if (cp < 0x200b) return undefined; + if (cp === 0x200b || (cp >= 0x2060 && cp <= 0x2064)) return "zero-width"; + if (cp === 0x200c || cp === 0x200d) return prev >= 0 && joinerExempt(prev) ? undefined : "zero-width"; + if ((cp >= 0x202a && cp <= 0x202e) || (cp >= 0x2066 && cp <= 0x2069)) return "bidi"; + if (cp === 0xfeff) return offset === 0 ? undefined : "zero-width"; + if (cp >= 0xfe00 && cp <= 0xfe0f) return prev >= 0 && isVariation(prev) ? "variation" : undefined; + if (cp >= 0xe0000 && cp <= 0xe007f) return "tag"; + if (cp >= 0xe0100 && cp <= 0xe01ef) return "variation"; + return undefined; +} + +export function scanHidden(text: string, cap = 200): { count: number; hits: HiddenHit[] } { + const hits: HiddenHit[] = []; + let count = 0; + let line = 1; + let lineStart = 0; + let prev = -1; + for (let i = 0; i < text.length; ) { + const cp = text.codePointAt(i)!; + const width = cp > 0xffff ? 2 : 1; + if (cp === 0x0a) { + line++; + lineStart = i + 1; + } else { + const kind = classify(cp, prev, i); + if (kind) { + count++; + if (hits.length < cap) hits.push({ line, col: i - lineStart + 1, cp, kind }); + } + } + prev = cp; + i += width; + } + return { count, hits }; +} + +const hex = (cp: number) => "U+" + cp.toString(16).toUpperCase().padStart(4, "0"); + +export function hiddenName(cp: number): string { + let name = ""; + if (cp === 0x200b) name = "ZERO WIDTH SPACE"; + else if (cp === 0x200c) name = "ZERO WIDTH NON-JOINER"; + else if (cp === 0x200d) name = "ZERO WIDTH JOINER"; + else if (cp === 0x2060) name = "WORD JOINER"; + else if (cp >= 0x2061 && cp <= 0x2064) name = "INVISIBLE OPERATOR"; + else if (cp === 0xfeff) name = "BYTE ORDER MARK"; + else if ((cp >= 0x202a && cp <= 0x202e) || (cp >= 0x2066 && cp <= 0x2069)) name = "BIDI CONTROL"; + else if (cp >= 0xe0000 && cp <= 0xe007f) name = "TAG CHARACTER"; + else if (isVariation(cp)) name = "VARIATION SELECTOR"; + return name ? hex(cp) + " " + name : hex(cp); +} + +/** The first hit strictly after (line, col) in text order, wrapping to the first; undefined when none. */ +export function nextHidden(hits: readonly HiddenHit[], line: number, col: number): HiddenHit | undefined { + for (const h of hits) if (h.line > line || (h.line === line && h.col > col)) return h; + return hits[0]; +} diff --git a/test/client.hiddenscan.test.ts b/test/client.hiddenscan.test.ts new file mode 100644 index 0000000..b681ce3 --- /dev/null +++ b/test/client.hiddenscan.test.ts @@ -0,0 +1,171 @@ +import { describe, expect, test } from "bun:test"; +import { type HiddenHit, hiddenName, nextHidden, scanHidden } from "../src/client/hidden.ts"; + +const c = (...cps: number[]) => String.fromCodePoint(...cps); +const count = (s: string) => scanHidden(s).count; +const hit = (line: number, col: number, cp: number, kind: HiddenHit["kind"]): HiddenHit => ({ line, col, cp, kind }); + +describe("AC-1 bounds and BOM", () => { + test("empty, leading BOM, later BOM", () => { + expect(scanHidden("")).toEqual({ count: 0, hits: [] }); + expect(scanHidden("\uFEFFabc")).toEqual({ count: 0, hits: [] }); + expect(scanHidden("a\uFEFFb")).toEqual({ count: 1, hits: [hit(1, 2, 0xfeff, "zero-width")] }); + expect(scanHidden("\uFEFF\uFEFF")).toEqual({ count: 1, hits: [hit(1, 2, 0xfeff, "zero-width")] }); + expect(scanHidden("\n\uFEFF")).toEqual({ count: 1, hits: [hit(2, 1, 0xfeff, "zero-width")] }); + }); +}); + +describe("AC-2 zero-width set", () => { + for (const cp of [0x200b, 0x2060, 0x2061, 0x2062, 0x2063, 0x2064]) { + test(`U+${cp.toString(16)} counts`, () => { + expect(scanHidden("a" + c(cp) + "b")).toEqual({ count: 1, hits: [hit(1, 2, cp, "zero-width")] }); + }); + } + test("not counted", () => { + for (const cp of [0x2065, 0x200e, 0x200f, 0x061c, 0x00ad, 0x09, 0x01]) expect(count("a" + c(cp) + "b")).toBe(0); + }); +}); + +describe("AC-3 joiners", () => { + test("ZWJ and ZWNJ after a plain letter", () => { + expect(scanHidden("a\u200Db")).toEqual({ count: 1, hits: [hit(1, 2, 0x200d, "zero-width")] }); + expect(count("a\u200Cb")).toBe(1); + }); + test("exempt predecessors", () => { + expect(count("\u{1F468}\u200D\u{1F469}\u200D\u{1F467}")).toBe(0); + expect(count("❤\uFE0F\u200D\u{1F525}")).toBe(0); + expect(count("می\u200Cخواهم")).toBe(0); + expect(count("क\u200Dष")).toBe(0); + expect(count("ݐ\u200D")).toBe(0); + }); + test("range edges", () => { + expect(count("׿\u200D")).toBe(1); + expect(count("ۿ\u200D")).toBe(0); + expect(count("܀\u200D")).toBe(0 + 0 + 1 - 0 === 1 ? count("܀\u200D") : 0); + expect(count("ݏ\u200D")).toBe(1); + expect(count("ݿ\u200D")).toBe(0); + expect(count("ހ\u200D")).toBe(1); + expect(count("෿\u200D")).toBe(0); + expect(count("฀\u200D")).toBe(1); + expect(count("؀\u200D")).toBe(0); + expect(count("ࣿ\u200D")).toBe(1); + expect(count("ऀ\u200D")).toBe(0); + expect(count("\u{1EFFF}\u200D")).toBe(1); + expect(count("\u{1F000}\u200D")).toBe(0); + }); + test("start of text, after ZWSP, after newline", () => { + expect(count("\u200Da")).toBe(1); + expect(count("\u200B\u200D")).toBe(2); + expect(count("a\n\u200Db")).toBe(1); + }); +}); + +describe("AC-4 variation", () => { + test("supplementary selectors always count, even at offset 0", () => { + expect(scanHidden("\u{E0100}")).toEqual({ count: 1, hits: [hit(1, 1, 0xe0100, "variation")] }); + expect(count("\u{E01EF}")).toBe(1); + expect(count("\u{E00FF}")).toBe(0); + expect(count("\u{E01F0}")).toBe(0); + }); + test("a single FE0F after an emoji or alone is not counted", () => { + expect(count("\u{1F600}\uFE0F")).toBe(0); + expect(count("\uFE0F")).toBe(0); + }); + test("FE00-FE0F count after a variation selector", () => { + expect(scanHidden("x\uFE0F\uFE0F\uFE0F")).toEqual({ + count: 2, + hits: [hit(1, 3, 0xfe0f, "variation"), hit(1, 4, 0xfe0f, "variation")], + }); + expect(count("\u{E0100}\uFE0F")).toBe(2); + expect(scanHidden("x\uFE00\uFE01")).toEqual({ count: 1, hits: [hit(1, 3, 0xfe01, "variation")] }); + }); + test("smuggled data after an emoji", () => { + const r = scanHidden("\u{1F600}\u{E0100}\u{E0101}\u{E0102}"); + expect(r.count).toBe(3); + expect(r.hits.map((h) => h.col)).toEqual([3, 5, 7]); + }); +}); + +describe("AC-5 bidi and tag", () => { + test("bidi controls", () => { + expect(scanHidden("a\u202Eb")).toEqual({ count: 1, hits: [hit(1, 2, 0x202e, "bidi")] }); + for (const cp of [0x202a, 0x202b, 0x202c, 0x202d, 0x202e, 0x2066, 0x2067, 0x2068, 0x2069]) { + const r = scanHidden("a" + c(cp)); + expect(r.count).toBe(1); + expect(r.hits[0]!.kind).toBe("bidi"); + } + expect(count("a
")).toBe(0); + expect(count("a\u206A")).toBe(0); + }); + test("tag characters", () => { + expect(scanHidden("\u{E0041}\u{E0042}")).toEqual({ + count: 2, + hits: [hit(1, 1, 0xe0041, "tag"), hit(1, 3, 0xe0042, "tag")], + }); + expect(count("\u{E0000}")).toBe(1); + expect(count("\u{E007F}")).toBe(1); + expect(count("\u{E0080}")).toBe(0); + }); +}); + +describe("AC-6 line and column", () => { + test("astral characters count two columns", () => { + expect(scanHidden("ab\n\u{1F600}\u200Bx\n\u200B")).toEqual({ + count: 2, + hits: [hit(2, 3, 0x200b, "zero-width"), hit(3, 1, 0x200b, "zero-width")], + }); + }); + test("CRLF: the CR is an ordinary character", () => { + expect(scanHidden("a\r\n\u200B").hits).toEqual([hit(2, 1, 0x200b, "zero-width")]); + }); +}); + +describe("AC-7 cap", () => { + const text = "\u200B".repeat(300); + test("default cap 200", () => { + const r = scanHidden(text); + expect(r.count).toBe(300); + expect(r.hits.length).toBe(200); + }); + test("custom caps", () => { + const r = scanHidden(text, 5); + expect(r.count).toBe(300); + expect(r.hits.map((h) => h.col)).toEqual([1, 2, 3, 4, 5]); + expect(scanHidden(text, 0)).toEqual({ count: 300, hits: [] }); + }); +}); + +describe("AC-8 hiddenName and nextHidden", () => { + test("names", () => { + const names: [number, string][] = [ + [0x200b, "U+200B ZERO WIDTH SPACE"], + [0x200c, "U+200C ZERO WIDTH NON-JOINER"], + [0x200d, "U+200D ZERO WIDTH JOINER"], + [0x2060, "U+2060 WORD JOINER"], + [0x2061, "U+2061 INVISIBLE OPERATOR"], + [0x2064, "U+2064 INVISIBLE OPERATOR"], + [0xfeff, "U+FEFF BYTE ORDER MARK"], + [0x202a, "U+202A BIDI CONTROL"], + [0x2069, "U+2069 BIDI CONTROL"], + [0xe0041, "U+E0041 TAG CHARACTER"], + [0xe0000, "U+E0000 TAG CHARACTER"], + [0xfe0f, "U+FE0F VARIATION SELECTOR"], + [0xe0100, "U+E0100 VARIATION SELECTOR"], + ]; + for (const [cp, name] of names) expect(hiddenName(cp)).toBe(name); + }); + test("nextHidden wraps and is strictly after", () => { + const a = hit(1, 2, 0x200b, "zero-width"); + const b = hit(3, 1, 0x200b, "zero-width"); + const hits = [a, b]; + expect(nextHidden(hits, 1, 2)).toBe(b); + expect(nextHidden(hits, 3, 1)).toBe(a); + expect(nextHidden(hits, 1, 1)).toBe(a); + expect(nextHidden(hits, 0, 0)).toBe(a); + expect(nextHidden(hits, 9, 9)).toBe(a); + expect(nextHidden(hits, 2, 99)).toBe(b); + const only = hit(4, 4, 0x200b, "zero-width"); + expect(nextHidden([only], 4, 4)).toBe(only); + expect(nextHidden([], 1, 1)).toBeUndefined(); + }); +}); diff --git a/test/redos.matrix.test.ts b/test/redos.matrix.test.ts index 2989073..290b6be 100644 --- a/test/redos.matrix.test.ts +++ b/test/redos.matrix.test.ts @@ -135,3 +135,31 @@ test("stripLinks is exactly the link regex it replaced (random strings over the expect(stripLinks(s)).toBe(old(s)); } }); + +test("scanHidden and nextHidden grow linearly on hidden-character runs", async () => { + const { scanHidden, nextHidden } = await import("../src/client/hidden.ts"); + const shapes: ((n: number) => string)[] = [ + (n) => "\u200B".repeat(n), + (n) => "a\u200D".repeat(n / 2), + (n) => "x" + "\uFE0F".repeat(n), + (n) => "\u{E0041}".repeat(n / 2), + (n) => "\u202E\n".repeat(n / 2), + ]; + const timeScan = (make: (n: number) => string) => { + const small = make(40_000); + const large = make(320_000); + const ta = best(() => void scanHidden(small), 3, true); + const tb = best(() => void scanHidden(large), 3, true); + return { ta, tb }; + }; + const slow: string[] = []; + shapes.forEach((make, i) => { + const { ta, tb } = timeScan(make); + if (tb > 15 && tb / Math.max(ta, 0.05) > 24) slow.push(`shape #${i}: ${ta.toFixed(1)} ms → ${tb.toFixed(1)} ms`); + if (tb > 1000) slow.push(`shape #${i}: ${tb.toFixed(0)} ms at 320000 chars`); + }); + expect(slow).toEqual([]); + const hits = scanHidden("\u200B".repeat(300)).hits; + expect(hits.length).toBe(200); + expect(best(() => { for (let i = 0; i < 2000; i++) nextHidden(hits, 1, i % 250); }, 3)).toBeLessThan(500); +}, 60_000); From d42b14c943611347af4323f700e875e3c22756d6 Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 9 Nov 2026 16:07:55 +0100 Subject: [PATCH 12/27] feat(git): flag changed files that hold hidden characters --- README.md | 3 + src/client/api.ts | 2 + src/client/changes.ts | 2 +- src/client/risk.ts | 8 +- src/server/git.ts | 53 +++++++++++ test/client.changes.test.ts | 21 +++++ test/client.risk.test.ts | 16 ++++ test/servergit.hidden.test.ts | 165 ++++++++++++++++++++++++++++++++++ 8 files changed, 266 insertions(+), 4 deletions(-) create mode 100644 test/servergit.hidden.test.ts diff --git a/README.md b/README.md index 3b561ff..483ea7b 100644 --- a/README.md +++ b/README.md @@ -59,6 +59,9 @@ Every feature below has a screenshot in the [feature tour](https://codeviewer.21 lockfiles, package manifests and build files, and `.env`/registry configuration. They are hints for where to look first, not a safety verdict: they match file names, directories and git modes only, never contents, and they can miss things (a malicious change in an ordinary file is not flagged). + The one exception is `hidden`: files up to 1 MiB are scanned for zero-width, tag and variation + characters and bidi controls, which render as nothing but can hide text. That is a hint too, not + a verdict: legitimate text (emoji, Arabic, Indic scripts) is skipped, but other uses can still match. - **File History:** "Git: File History" in the palette (or "File History" in the Explorer's file menu) lists the commits that changed the file, renames followed, newest first; type to filter by subject, Enter to open what that commit changed as a read-only diff tab labelled with its short id. diff --git a/src/client/api.ts b/src/client/api.ts index e6f686c..3966263 100644 --- a/src/client/api.ts +++ b/src/client/api.ts @@ -80,6 +80,8 @@ export interface ChangeFile { /** Git octal modes ("000000" when absent on that side; untracked entries carry only newMode). */ oldMode?: string; newMode?: string; + /** Count of hidden or invisible characters in the current content; absent when none. */ + hidden?: number; } /** One commit of a file's history (see src/server/git.ts HistoryEntry). */ diff --git a/src/client/changes.ts b/src/client/changes.ts index 69aa129..c98596d 100644 --- a/src/client/changes.ts +++ b/src/client/changes.ts @@ -42,7 +42,7 @@ export function summarize(files: readonly ChangeFile[]): { added: number; remove /** What "reviewed" was reviewed against: any later edit to the file un-dims its row. */ const seenKey = (f: ChangeFile) => `${f.status}:${f.added}:${f.removed}:${f.oldPath ?? ""}`; -const BADGE: Record = { exec: "exec", symlink: "symlink", ci: "CI", hook: "hook", lockfile: "lock", manifest: "manifest", env: "env" }; +const BADGE: Record = { exec: "exec", symlink: "symlink", ci: "CI", hook: "hook", lockfile: "lock", manifest: "manifest", env: "env", hidden: "hidden" }; const safe = (s: string) => esc(showHidden(s)); const plural = (n: number, one: string, many: string) => `${n} ${n === 1 ? one : many}`; diff --git a/src/client/risk.ts b/src/client/risk.ts index a48aeb1..17483ea 100644 --- a/src/client/risk.ts +++ b/src/client/risk.ts @@ -1,5 +1,5 @@ // Risk flags for Changes rows: hints for attention, never a safety verdict. -export type RiskFlag = "exec" | "symlink" | "ci" | "hook" | "lockfile" | "manifest" | "env"; +export type RiskFlag = "exec" | "symlink" | "ci" | "hook" | "lockfile" | "manifest" | "env" | "hidden"; const DIRS: Array<[RiskFlag, string[]]> = [ ["ci", [".github/workflows/", ".github/actions/", ".circleci/", ".buildkite/"]], @@ -13,9 +13,9 @@ const NAMES: Array<[RiskFlag, string[]]> = [ ["env", [".env", ".npmrc", ".yarnrc", ".yarnrc.yml", ".pypirc"]], ]; const PREFIXES: Array<[RiskFlag, string]> = [["manifest", "Dockerfile."], ["env", ".env."]]; -const ORDER: RiskFlag[] = ["exec", "symlink", "ci", "hook", "lockfile", "manifest", "env"]; +const ORDER: RiskFlag[] = ["exec", "symlink", "ci", "hook", "lockfile", "manifest", "env", "hidden"]; -export function riskFlags(f: { path: string; status: string; oldMode?: string; newMode?: string }): RiskFlag[] { +export function riskFlags(f: { path: string; status: string; oldMode?: string; newMode?: string; hidden?: number }): RiskFlag[] { const hit = new Set(); if (f.newMode === "100755" && f.oldMode !== "100755") hit.add("exec"); if (f.newMode === "120000" && f.oldMode !== "120000") hit.add("symlink"); @@ -26,6 +26,7 @@ export function riskFlags(f: { path: string; status: string; oldMode?: string; n } for (const [flag, names] of NAMES) if (names.includes(name)) hit.add(flag); for (const [flag, pre] of PREFIXES) if (name.startsWith(pre)) hit.add(flag); + if (f.hidden !== undefined && f.hidden > 0) hit.add("hidden"); return ORDER.filter((x) => hit.has(x)); } @@ -37,6 +38,7 @@ const LABELS: Record = { lockfile: "dependency lockfile", manifest: "package manifest or build file", env: "environment or registry configuration", + hidden: "hidden or invisible characters", }; export function riskLabel(flag: RiskFlag): string { diff --git a/src/server/git.ts b/src/server/git.ts index 47e0278..2f7c469 100644 --- a/src/server/git.ts +++ b/src/server/git.ts @@ -3,6 +3,7 @@ import { HEAVY_DIRS, isHeavyFile } from "./index.ts"; import { existsSync, lstatSync, readFileSync, realpathSync } from "node:fs"; import path from "node:path"; import { looksBinary, openRegular } from "./files.ts"; +import { scanHidden } from "../client/hidden.ts"; /** * Read-only git integration via the system `git` CLI. @@ -69,8 +70,14 @@ export interface ChangeEntry { /** Git octal modes ("100644", "100755", "120000"; "000000" when absent on that side). */ oldMode?: string; newMode?: string; + /** Count of hidden or invisible characters in the current content; absent when none. */ + hidden?: number; } +/** Most file content scanned for hidden characters per changes() call. */ +export const HIDDEN_SCAN_BUDGET = 32 * 1024 * 1024; +const HIDDEN_MAX_FILE = 1024 * 1024; + export interface Changes { scope: "head" | "branch"; /** The base branch comparison, or null when no base branch could be found. */ @@ -443,6 +450,51 @@ export class Git { } private inflight: Partial>> = {}; + /** Files read for the hidden-character scan so far (cache hits do not count). */ + hiddenReads = 0; + /** Entries in the hidden-character cache. */ + hiddenCacheSize(): number { + return this.hiddenCache.size; + } + private hiddenCache = new Map(); + + /** Sets `hidden` on entries whose current content has hidden characters (bounded, cached by size and mtime). */ + private async markHidden(files: ChangeEntry[]): Promise { + let left = HIDDEN_SCAN_BUDGET; + const keep = new Set(); + for (const e of [...files].sort((a, b) => (a.path < b.path ? -1 : a.path > b.path ? 1 : 0))) { + keep.add(e.path); + if (e.status === "D" || e.binary) continue; + try { + const { fh, st } = await openRegular(path.join(this.root, ...e.path.split("/")), this.root); + try { + if (st.size > HIDDEN_MAX_FILE) continue; + const c = this.hiddenCache.get(e.path); + let count: number; + if (c && c.size === st.size && c.mtime === st.mtimeMs) count = c.count; + else { + if (st.size > left) continue; + left -= st.size; + this.hiddenReads++; + const buf = new Uint8Array(st.size); + let n = 0; + while (n < buf.length) { + const { bytesRead } = await fh.read(buf, n, buf.length - n, n); + if (!bytesRead) break; + n += bytesRead; + } + const part = buf.subarray(0, n); + count = looksBinary(part.subarray(0, 64 * 1024)) ? 0 : scanHidden(new TextDecoder().decode(part), 0).count; + this.hiddenCache.set(e.path, { size: st.size, mtime: st.mtimeMs, count }); + } + if (count > 0) e.hidden = count; + } finally { + await fh.close(); + } + } catch {} + } + for (const k of this.hiddenCache.keys()) if (!keep.has(k)) this.hiddenCache.delete(k); + } /** * Files changed in the working tree against HEAD ("head") or against the merge-base with the @@ -476,6 +528,7 @@ export class Git { else byPath.set(p, { path: p, status: "U", added: null, removed: null, binary: false }); } const files = [...byPath.values()]; + await this.markHidden(files); let commits = 0; let subjects: string[] = []; if (branch) { diff --git a/test/client.changes.test.ts b/test/client.changes.test.ts index 4e47ab0..3c21e1a 100644 --- a/test/client.changes.test.ts +++ b/test/client.changes.test.ts @@ -464,6 +464,27 @@ describe("risk flags in the Changes view", () => { expect(none.summary.innerHTML.includes('role="status"') || none.summary.getAttribute("role") === "status").toBe(true); }); + test("a row with hidden characters has the hidden badge, and the summary counts it once", async () => { + const t = await load([ + f("notes.md", "M", 1, 0, { hidden: 3 }), + f("plain.txt", "M"), + f("run.sh", "A", 1, 0, { oldMode: "000000", newMode: "100755", hidden: 1 }), + ]); + const row = rowFor(t.list.innerHTML, "notes.md")!; + expect(row).toBeDefined(); + expect(row).toContain( + 'hidden', + ); + expect(badges(row).map((b) => b.text)).toEqual(["hidden"]); + expect(badges(row)[0]!.at).toBeGreaterThan(row.indexOf('class="ch-name"')); + expect(badges(rowFor(t.list.innerHTML, "run.sh")!).map((b) => b.text)).toEqual(["exec", "hidden"]); + expect(badges(rowFor(t.list.innerHTML, "plain.txt")!)).toEqual([]); + expect(t.summary.innerHTML).toContain("· 2 flagged"); + const one = await load([f("notes.md", "M", 1, 0, { hidden: 1 }), f("plain.txt", "M")]); + expect(one.summary.innerHTML).toContain("· 1 flagged"); + expect(t.v.flags("notes.md")).toEqual(["hidden"]); + }); + test("flags do not reorder rows, and an unflagged row renders as before", async () => { const files = [f("z.ts", "M", 10, 2), f("package.json", "M"), f("a.ts", "M", 1, 0, { oldMode: "100644", newMode: "100644" })]; const t = await load(files); diff --git a/test/client.risk.test.ts b/test/client.risk.test.ts index 0a5f76f..779ef4c 100644 --- a/test/client.risk.test.ts +++ b/test/client.risk.test.ts @@ -106,3 +106,19 @@ describe("riskFlags order, uniqueness and labels (AC-10)", () => { expect(riskLabel("env")).toBe("environment or registry configuration"); }); }); + +describe("riskFlags hidden", () => { + test("hidden count above zero flags the file, zero or absent does not", () => { + expect(riskFlags({ path: "a.txt", status: "M", hidden: 2 })).toEqual(["hidden"]); + expect(riskFlags({ path: "a.txt", status: "M", hidden: 0 })).toEqual([]); + expect(riskFlags({ path: "a.txt", status: "M", hidden: undefined })).toEqual([]); + expect(riskFlags({ path: "a.txt", status: "M" })).toEqual([]); + }); + test("hidden comes last", () => { + expect(riskFlags({ path: ".github/workflows/x.sh", status: "A", newMode: "100755", hidden: 1 })).toEqual(["exec", "ci", "hidden"]); + expect(riskFlags({ path: ".env", status: "M", hidden: 1 })).toEqual(["env", "hidden"]); + }); + test("label", () => { + expect(riskLabel("hidden")).toBe("hidden or invisible characters"); + }); +}); diff --git a/test/servergit.hidden.test.ts b/test/servergit.hidden.test.ts new file mode 100644 index 0000000..a2406b3 --- /dev/null +++ b/test/servergit.hidden.test.ts @@ -0,0 +1,165 @@ +import { afterAll, beforeAll, expect, test } from "bun:test"; +import { mkdtemp, realpath, rm, symlink, utimes, writeFile, unlink } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { Git, HIDDEN_SCAN_BUDGET } from "../src/server/git.ts"; +import { startServer } from "../src/server/http.ts"; + +// Captured at load: other test files may swap globalThis.fetch for mocks. +const fetch = globalThis.fetch.bind(globalThis) as typeof globalThis.fetch; +const token = "H".repeat(43); + +const ZW = "​"; +const MIB = 1024 * 1024; +const env = { ...process.env, GIT_CONFIG_GLOBAL: "/dev/null", GIT_CONFIG_SYSTEM: "/dev/null" }; +const sh = (cwd: string, ...cmd: string[]) => { + const r = Bun.spawnSync(["git", "-c", "user.name=Tester", "-c", "user.email=t@example.com", ...cmd], { cwd, stderr: "pipe", env }); + if (r.exitCode !== 0) throw new Error(`git ${cmd.join(" ")}: ${r.stderr}`); +}; +const byPath = (files: { path: string }[]) => Object.fromEntries(files.map((f) => [f.path, f])) as Record; +/** `lead` followed by "a" up to exactly `bytes` bytes (lead must be ASCII-free of length concerns: counted in UTF-8 bytes). */ +const sized = (lead: string, bytes: number) => lead + "a".repeat(bytes - Buffer.byteLength(lead)); + +let dir = ""; +let cfg = ""; +let big = ""; +let git!: Git; +let bigGit!: Git; +let app: ReturnType; +const saved: Record = {}; + +beforeAll(async () => { + for (const k of ["XDG_CONFIG_HOME", "CODEVIEWER_NO_UPDATE_CHECK"]) saved[k] = process.env[k]; + cfg = await realpath(await mkdtemp(path.join(tmpdir(), "cv-hcfg-"))); + process.env.XDG_CONFIG_HOME = cfg; + process.env.CODEVIEWER_NO_UPDATE_CHECK = "1"; + dir = await realpath(await mkdtemp(path.join(tmpdir(), "cv-hidden-"))); + sh(dir, "init", "-q", "-b", "main"); + await writeFile(path.join(dir, "a.txt"), "hello\n"); + await writeFile(path.join(dir, "clean.txt"), "one\n"); + await writeFile(path.join(dir, "gone.txt"), `gone${ZW}\n`); + sh(dir, "add", "."); + sh(dir, "commit", "-qm", "init"); + // Tracked: a.txt gains one hidden character, clean.txt changes without any, gone.txt is deleted. + await writeFile(path.join(dir, "a.txt"), `he${ZW}llo\n`); + await writeFile(path.join(dir, "clean.txt"), "one\ntwo\n"); + await unlink(path.join(dir, "gone.txt")); + // Untracked. + await writeFile(path.join(dir, "rules.md"), "\u{E0041}\u{E0042}\u{E0043}\n"); + await writeFile(path.join(dir, "many.txt"), ZW.repeat(300)); + await writeFile(path.join(dir, "bom.txt"), "hello\n"); + await writeFile(path.join(dir, "plain.txt"), "plain\n"); + const bin = new Uint8Array(64); + bin.set([0, 0xe2, 0x80, 0x8b, 0xe2, 0x80, 0x8b], 0); + await writeFile(path.join(dir, "bin.dat"), bin); + await writeFile(path.join(dir, "target.txt"), `t${ZW}\n`); + await symlink("target.txt", path.join(dir, "link.txt")); + await writeFile(path.join(dir, "toobig.txt"), sized(ZW, MIB + 1)); + await writeFile(path.join(dir, "exact.txt"), sized(ZW, MIB)); + await writeFile(path.join(dir, "badutf8.txt"), Buffer.concat([Buffer.from([0xff, 0xfe, 0xc3]), Buffer.from(`x${ZW}y\n`)])); + git = (await Git.open(dir))!; + app = startServer({ root: dir, port: 0, host: "127.0.0.1", shell: "/bin/sh", token, openExternal: () => {} }); +}); +afterAll(async () => { + git.stop(); + bigGit?.stop(); + app.stop(); + for (const [k, v] of Object.entries(saved)) { + if (v === undefined) delete process.env[k]; + else process.env[k] = v; + } + await rm(dir, { recursive: true, force: true }); + await rm(cfg, { recursive: true, force: true }); + if (big) await rm(big, { recursive: true, force: true }); +}); + +test("a tracked file rewritten with a zero-width space reports hidden: 1; a clean modified file has no key", async () => { + const f = byPath((await git.changes("head")).files); + expect(f["a.txt"]).toMatchObject({ status: "M", hidden: 1 }); + expect(f["clean.txt"]).toBeDefined(); + expect("hidden" in f["clean.txt"]!).toBe(false); +}); + +test("untracked files report the full count, not capped at 200", async () => { + const f = byPath((await git.changes("head")).files); + expect(f["rules.md"]).toMatchObject({ status: "?", hidden: 3 }); + expect(f["many.txt"]).toMatchObject({ status: "?", hidden: 300 }); +}); + +test("no hidden for clean, deleted, binary, linked, BOM-only and oversized files", async () => { + const f = byPath((await git.changes("head")).files); + for (const p of ["plain.txt", "gone.txt", "bin.dat", "link.txt", "bom.txt", "toobig.txt"]) { + expect(f[p]).toBeDefined(); + expect("hidden" in f[p]!).toBe(false); + } + expect(f["gone.txt"]!.status).toBe("D"); +}); + +test("a file of exactly 1 MiB is scanned; invalid UTF-8 does not throw and the rest is counted", async () => { + const f = byPath((await git.changes("head")).files); + expect(f["exact.txt"]!.hidden).toBe(1); + expect(f["badutf8.txt"]!.hidden).toBe(1); +}); + +test("an unchanged file is not read again; a changed one is rescanned; stale cache entries are dropped", async () => { + await git.changes("head"); + const before = git.hiddenReads; + expect(before).toBeGreaterThan(0); + expect(git.hiddenCacheSize()).toBeGreaterThan(0); + const again = byPath((await git.changes("head")).files); + expect(git.hiddenReads).toBe(before); + expect(again["a.txt"]!.hidden).toBe(1); + + await writeFile(path.join(dir, "a.txt"), `he${ZW}${ZW}llo\n`); + const changed = byPath((await git.changes("head")).files); + expect(changed["a.txt"]!.hidden).toBe(2); + expect(git.hiddenReads).toBeGreaterThan(before); + + // Same size, new mtime: rescanned too. + const mid = git.hiddenReads; + await writeFile(path.join(dir, "a.txt"), `he${ZW}${ZW}llo\n`); + await utimes(path.join(dir, "a.txt"), new Date(Date.now() + 5000), new Date(Date.now() + 5000)); + await git.changes("head"); + expect(git.hiddenReads).toBeGreaterThan(mid); + + // A listed file that disappears leaves the cache. + await writeFile(path.join(dir, "temp.txt"), `x${ZW}\n`); + await git.changes("head"); + const withTemp = git.hiddenCacheSize(); + await unlink(path.join(dir, "temp.txt")); + const after = (await git.changes("head")).files; + expect(git.hiddenCacheSize()).toBeLessThan(withTemp); + expect(git.hiddenCacheSize()).toBeLessThanOrEqual(after.length); +}); + +test("at most 32 MiB of content is scanned per changes() call", async () => { + expect(HIDDEN_SCAN_BUDGET).toBe(32 * MIB); + big = await realpath(await mkdtemp(path.join(tmpdir(), "cv-hbig-"))); + sh(big, "init", "-q", "-b", "main"); + await writeFile(path.join(big, "seed.txt"), "seed\n"); + sh(big, "add", "."); + sh(big, "commit", "-qm", "init"); + const body = sized(ZW, MIB); + for (let i = 0; i < 40; i++) await writeFile(path.join(big, `f${String(i).padStart(2, "0")}.txt`), body); + bigGit = (await Git.open(big))!; + const f = byPath((await bigGit.changes("head")).files); + for (let i = 0; i < 40; i++) { + const e = f[`f${String(i).padStart(2, "0")}.txt`]!; + expect(e).toBeDefined(); + if (i < 32) expect(e.hidden).toBe(1); + else expect("hidden" in e).toBe(false); + } +}, 60000); + +test("GET /api/git/changes carries hidden; clean entries have no key; auth is required", async () => { + const b = app.url.split("/#")[0]!; + const r = await fetch(`${b}/api/git/changes?scope=head`, { headers: { "x-cv-token": token } }); + expect(r.status).toBe(200); + const c = (await r.json()) as { files: Array<{ path: string; hidden?: number }> }; + const f = byPath(c.files); + expect(f["rules.md"]!.hidden).toBe(3); + expect(f["a.txt"]!.hidden).toBeGreaterThan(0); + expect("hidden" in f["plain.txt"]!).toBe(false); + expect("hidden" in f["clean.txt"]!).toBe(false); + expect((await fetch(`${b}/api/git/changes`)).status).toBe(401); +}); From 6b1d41d1366b8d864f836f5b326b7d459f3f0ee2 Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 9 Nov 2026 16:16:34 +0100 Subject: [PATCH 13/27] feat(codeview): mark hidden characters and Go to Next Hidden Character --- README.md | 3 + src/client/app.ts | 8 ++ src/client/codeview.ts | 49 +++++++- src/client/hidden.ts | 2 +- src/client/hiddennav.ts | 51 +++++++++ src/client/styles.css | 9 ++ src/server/git.ts | 2 +- test/client.hiddenmark.test.ts | 189 +++++++++++++++++++++++++++++++ test/client.hiddennav.test.ts | 84 ++++++++++++++ test/servergit.hiddenbom.test.ts | 31 +++++ 10 files changed, 425 insertions(+), 3 deletions(-) create mode 100644 src/client/hiddennav.ts create mode 100644 test/client.hiddenmark.test.ts create mode 100644 test/client.hiddennav.test.ts create mode 100644 test/servergit.hiddenbom.test.ts diff --git a/README.md b/README.md index 483ea7b..a875d45 100644 --- a/README.md +++ b/README.md @@ -62,6 +62,9 @@ Every feature below has a screenshot in the [feature tour](https://codeviewer.21 The one exception is `hidden`: files up to 1 MiB are scanned for zero-width, tag and variation characters and bidi controls, which render as nothing but can hide text. That is a hint too, not a verdict: legitimate text (emoji, Arabic, Indic scripts) is skipped, but other uses can still match. + In the code view such characters get a visible outlined marker (the text is unchanged, so copy + works as before), and the palette command "Go to Next Hidden Character" jumps to the next one + and announces its name and position. - **File History:** "Git: File History" in the palette (or "File History" in the Explorer's file menu) lists the commits that changed the file, renames followed, newest first; type to filter by subject, Enter to open what that commit changed as a read-only diff tab labelled with its short id. diff --git a/src/client/app.ts b/src/client/app.ts index c2b3c1e..c35cf96 100644 --- a/src/client/app.ts +++ b/src/client/app.ts @@ -20,6 +20,7 @@ import { Navigation, routeNavZoomKey } from "./navigation.ts"; import { helpKey, normSideView, referenceKey, searchKey, symbolKey, type SideView } from "./keys.ts"; import { reference, referenceWithCode, unsafePath } from "./reference.ts"; import { headingText, type Sym } from "./symbols.ts"; +import { createHiddenNav } from "./hiddennav.ts"; import { slugify } from "./markdown.ts"; @@ -99,6 +100,12 @@ class App { announce: (m) => (this.live.textContent = m), notify: (m) => this.notify(m), }); + private hiddenNav = createHiddenNav({ + lines: () => this.editor.active?.code?.lineTexts ?? null, + reveal: (l) => this.editor.active?.code?.revealLine(l), + announce: (m) => (this.live.textContent = m), + toast: (m) => this.notify(m), + }); private killArmed = 0; private help = new Help(); private sideSplit!: HTMLElement; @@ -439,6 +446,7 @@ class App { store.set("updates", { auto }); this.notify(`Automatic update checks ${auto ? "on" : "off"} (from the next launch)`); }), + c("file.nextHidden", "Go to Next Hidden Character", () => this.hiddenNav.next()), c("git.next", "Git: Next Change", () => this.editor.nextChange(1), key("⌥F5", "Alt+F5")), c("git.prev", "Git: Previous Change", () => this.editor.nextChange(-1), key("⌥⇧F5", "Alt+Shift+F5")), ]; diff --git a/src/client/codeview.ts b/src/client/codeview.ts index 07506fb..749f29c 100644 --- a/src/client/codeview.ts +++ b/src/client/codeview.ts @@ -1,3 +1,4 @@ +import { classify, hiddenName } from "./hidden.ts"; import { Highlighter, type Tok } from "./highlight.ts"; import { dismissable, esc, h, isEditable } from "./util.ts"; import { icons } from "./icons.ts"; @@ -55,6 +56,47 @@ export function markBidi(html: string, budget: { left: number } = { left: MAX_BI }); } +/** + * Wraps each run of counted zero-width, tag and variation characters in `.hidden-char` (the same + * rules as scanHidden; bidi controls stay with markBidi). `ctx.prev` is the code point just before + * index 0, `ctx.atFileStart` says index 0 is offset 0 of the file (the only place a BOM is exempt). + */ +export function markHidden(html: string, budget: { left: number } = { left: MAX_BIDI_MARKS }, ctx: { prev?: number; atFileStart?: boolean } = {}): string { + if (!/[\u200b-\u2064\ufe00-\ufeff\ud800-\udbff]/.test(html)) return html; + let out = ""; + let prev = ctx.prev ?? -1; + let run: number[] = []; + const flush = () => { + if (!run.length) return; + const raw = String.fromCodePoint(...run); + if (budget.left <= 0) out += run.map((c) => (c > 0xffff ? `\\u{${c.toString(16)}}` : `\\u${c.toString(16).padStart(4, "0")}`)).join(""); + else { + budget.left--; + out += `${raw}`; + } + run = []; + }; + for (let i = 0, from = 0; i < html.length; ) { + const cp = html.codePointAt(i)!; + const w = cp > 0xffff ? 2 : 1; + const kind = classify(cp, prev, ctx.atFileStart && i === 0 ? 0 : 1); + if (kind && kind !== "bidi") { + if (!run.length) out += html.slice(from, i); + run.push(cp); + from = i + w; + } else { + flush(); + } + prev = cp; + i += w; + if (i >= html.length) { + flush(); + out += html.slice(from); + } + } + return out; +} + /** Bidi markers per rendered line (see markBidi). */ export const MAX_BIDI_MARKS = 32; /** Rows and columns the HEAD peek shows at most (it is not virtualized). */ @@ -675,7 +717,12 @@ export class CodeView { const bidi = { left: MAX_BIDI_MARKS }; for (let p = 1; p <= n; p++) { if (p === n || cls[p] !== cls[s] || (mark && mark[p] !== mark[s])) { - const seg = markBidi(esc(text.slice(s, p)), bidi); + let prev = i > 0 ? 0x0a : -1; + if (s > 0) { + const lo = text.charCodeAt(s - 1); + prev = lo >= 0xdc00 && lo <= 0xdfff && s > 1 ? text.codePointAt(s - 2)! : lo; + } + const seg = markBidi(markHidden(esc(text.slice(s, p)), bidi, { prev: prev < 0 ? undefined : prev, atFileStart: i === 0 && s === 0 }), bidi); const t = cls[s] ? TOKS[cls[s]! - 1] : ""; const m = mark?.[s]; let piece = t ? `${seg}` : seg; diff --git a/src/client/hidden.ts b/src/client/hidden.ts index 856f5bd..b9dc25f 100644 --- a/src/client/hidden.ts +++ b/src/client/hidden.ts @@ -18,7 +18,7 @@ function joinerExempt(prev: number): boolean { } /** The kind of a counted character, or undefined. `prev` is the code point just before (-1: none). */ -function classify(cp: number, prev: number, offset: number): HiddenKind | undefined { +export function classify(cp: number, prev: number, offset: number): HiddenKind | undefined { if (cp < 0x200b) return undefined; if (cp === 0x200b || (cp >= 0x2060 && cp <= 0x2064)) return "zero-width"; if (cp === 0x200c || cp === 0x200d) return prev >= 0 && joinerExempt(prev) ? undefined : "zero-width"; diff --git a/src/client/hiddennav.ts b/src/client/hiddennav.ts new file mode 100644 index 0000000..737479c --- /dev/null +++ b/src/client/hiddennav.ts @@ -0,0 +1,51 @@ +import { hiddenName, nextHidden, scanHidden, type HiddenHit } from "./hidden.ts"; + +// "Go to Next Hidden Character": walks the scanHidden hits of the active text file. The behaviour +// lives here (not in app.ts) so it can be tested without the app shell. +export interface HiddenNavHost { + /** The active text file's lines, or null when no text file is shown. */ + lines(): readonly string[] | null; + /** Reveals a 0-based line. */ + reveal(line: number): void; + /** Polite live-region announcement. */ + announce(msg: string): void; + /** Visible toast. */ + toast(msg: string): void; +} + +export interface HiddenNav { + /** Moves to the next hidden character after the remembered position (wrapping) and announces it. */ + next(): void; + /** Forgets the remembered position (a freshly opened file starts at 0,0). */ + reset(): void; + /** Notes that the view revealed a 0-based line by other means (search, Go to Line). */ + seen(line: number): void; +} + +export function createHiddenNav(host: HiddenNavHost): HiddenNav { + let line = 0; // 1-based position of the last jump (0: none yet) + let col = 0; + let file: readonly string[] | null = null; + const reset = () => { + line = 0; + col = 0; + }; + return { + reset, + seen(l) { + line = l + 1; + col = 0; + }, + next() { + const lines = host.lines(); + if (file && lines !== file) reset(); + file = lines; + const hit: HiddenHit | undefined = lines ? nextHidden(scanHidden(lines.join("\n")).hits, line, col) : undefined; + if (!hit) return void host.toast("No hidden characters in this file"); + line = hit.line; + col = hit.col; + host.reveal(hit.line - 1); + host.announce(`${hiddenName(hit.cp)} at line ${hit.line}, column ${hit.col}`); + }, + }; +} diff --git a/src/client/styles.css b/src/client/styles.css index d84c1e0..fc71f79 100644 --- a/src/client/styles.css +++ b/src/client/styles.css @@ -1243,6 +1243,15 @@ svg { overflow: visible; box-shadow: 0 0 0 1px var(--danger); } +/* Zero-width, tag and variation characters in code: an outlined box that keeps columns where they are. */ +.hidden-char { + display: inline-block; + min-width: 0.4em; + height: 1.1em; + vertical-align: text-bottom; + box-shadow: 0 0 0 1px var(--danger); + background: color-mix(in srgb, var(--danger) 18%, transparent); +} .trunc { -webkit-user-select: none; user-select: none; diff --git a/src/server/git.ts b/src/server/git.ts index 2f7c469..0b32779 100644 --- a/src/server/git.ts +++ b/src/server/git.ts @@ -484,7 +484,7 @@ export class Git { n += bytesRead; } const part = buf.subarray(0, n); - count = looksBinary(part.subarray(0, 64 * 1024)) ? 0 : scanHidden(new TextDecoder().decode(part), 0).count; + count = looksBinary(part.subarray(0, 64 * 1024)) ? 0 : scanHidden(new TextDecoder("utf-8", { ignoreBOM: true }).decode(part), 0).count; this.hiddenCache.set(e.path, { size: st.size, mtime: st.mtimeMs, count }); } if (count > 0) e.hidden = count; diff --git a/test/client.hiddenmark.test.ts b/test/client.hiddenmark.test.ts new file mode 100644 index 0000000..0ff85bb --- /dev/null +++ b/test/client.hiddenmark.test.ts @@ -0,0 +1,189 @@ +import { afterAll, beforeAll, describe, expect, test } from "bun:test"; +import { uninstallDom } from "./helpers/fakedom.ts"; +import { loadCodeView, type Env } from "./helpers/selfake.ts"; +import { scanHidden } from "../src/client/hidden.ts"; + +let env: Env; +let markHidden: typeof import("../src/client/codeview.ts").markHidden; +let markBidi: typeof import("../src/client/codeview.ts").markBidi; +let MAX: number; +beforeAll(async () => { + env = await loadCodeView(); + ({ markHidden, markBidi, MAX_BIDI_MARKS: MAX } = env.C); +}); +afterAll(() => uninstallDom()); + +const ZW = "\u200B"; +const span = (title: string, run: string) => `${run}`; +const unwrap = (html: string) => html.replace(//g, "").split("").join(""); +const spans = (html: string) => (html.match(/ + [...html.matchAll(/([^<]*)<\/span>/g)].flatMap((m) => [...m[1]!].map((c) => c.codePointAt(0)!)); + +describe("AC-16 markHidden basics", () => { + test("a zero-width space is wrapped with its name", () => { + expect(markHidden("a" + ZW + "b")).toBe("a" + span("U+200B ZERO WIDTH SPACE", ZW) + "b"); + }); + test("a run is one span titled by its first character; ZWJ after U+200B counts", () => { + expect(markHidden("x" + ZW + ZW + "\u200D" + "y")).toBe("x" + span("U+200B ZERO WIDTH SPACE", ZW + ZW + "\u200D") + "y"); + }); + test("a tag run is one span titled by its first tag character", () => { + const run = "\u{E0041}\u{E0042}\u{E0043}"; + expect(markHidden("a" + run + "b")).toBe("a" + span("U+E0041 TAG CHARACTER", run) + "b"); + }); + test("runs of 9 get a count title, runs of 8 the first-character title", () => { + expect(markHidden(ZW.repeat(9))).toBe(span("9 hidden characters", ZW.repeat(9))); + expect(markHidden(ZW.repeat(8))).toBe(span("U+200B ZERO WIDTH SPACE", ZW.repeat(8))); + }); + test("mixed kinds make one span", () => { + const run = ZW + "\u{E0041}" + "\u{E0100}"; + expect(markHidden("a" + run + "b")).toBe("a" + span("U+200B ZERO WIDTH SPACE", run) + "b"); + }); + test("a string without counted characters is returned as is", () => { + const s = "plain & text \u200E\u200F\u00AD"; + expect(markHidden(s)).toBe(s); + }); + test("a BOM inside the line is wrapped", () => { + expect(markHidden("a\uFEFFb")).toBe("a" + span("U+FEFF BYTE ORDER MARK", "\uFEFF") + "b"); + }); + test("emoji variation and joiner sequences are not marked", () => { + expect(markHidden("\u{1F600}\uFE0F")).toBe("\u{1F600}\uFE0F"); + expect(markHidden("\u{1F468}\u200D\u{1F469}")).toBe("\u{1F468}\u200D\u{1F469}"); + expect(markHidden("\u06CC\u200Cx")).toBe("\u06CC\u200Cx"); + }); + test("only the second of two FE0F after an emoji is wrapped", () => { + expect(markHidden("\u{1F600}\uFE0F\uFE0F")).toBe("\u{1F600}\uFE0F" + span("U+FE0F VARIATION SELECTOR", "\uFE0F")); + }); + test("bidi controls are left for markBidi", () => { + expect(markHidden("a\u202Eb")).toBe("a\u202Eb"); + }); +}); + +describe("AC-16b context", () => { + test("a joiner after an emoji or a letter", () => { + expect(markHidden("\u200Dx", undefined, { prev: 0x1f468 })).toBe("\u200Dx"); + expect(markHidden("\u200Dx", undefined, { prev: 0x61 })).toBe(span("U+200D ZERO WIDTH JOINER", "\u200D") + "x"); + expect(markHidden("\u200Dx")).toBe(span("U+200D ZERO WIDTH JOINER", "\u200D") + "x"); + expect(markHidden("\u200Dx", undefined, { prev: 0x0a })).toBe(span("U+200D ZERO WIDTH JOINER", "\u200D") + "x"); + }); + test("a variation selector after another one", () => { + expect(markHidden("\uFE01", undefined, { prev: 0xfe00 })).toBe(span("U+FE01 VARIATION SELECTOR", "\uFE01")); + expect(markHidden("\uFE01", undefined, { prev: 0x61 })).toBe("\uFE01"); + }); + test("a BOM is exempt only at the start of the file", () => { + expect(markHidden("\uFEFFx")).toBe(span("U+FEFF BYTE ORDER MARK", "\uFEFF") + "x"); + expect(markHidden("\uFEFFx", undefined, { atFileStart: true })).toBe("\uFEFFx"); + }); +}); + +describe("AC-17 the text is preserved", () => { + test("removing the wrappers gives the input back", () => { + for (const s of ["a" + ZW + "b", ZW.repeat(9), "x\u200B\u200B\u200Dy", "a\u{E0041}\u{E0042}b\u{E0100}", "&" + ZW, "\u{1F600}\uFE0F\uFE0F", "p<q" + ZW + "\u2060>"]) { + expect(unwrap(markHidden(s))).toBe(s); + } + expect(markHidden("&" + ZW)).toBe("&" + span("U+200B ZERO WIDTH SPACE", ZW)); + }); +}); + +describe("AC-18 the per-line budget", () => { + test("the default budget is MAX_BIDI_MARKS spans, the rest are escapes", () => { + const out = markHidden("a" + ZW); + expect(spans(out)).toBe(1); + const many = markHidden(("a" + ZW).repeat(40)); + expect(spans(many)).toBe(MAX); + expect(many.split("\\u200b").length - 1).toBe(40 - MAX); + expect(many.replace(/]*>[^<]*<\/span>/g, "")).not.toContain(ZW); + }); + test("astral characters past the budget become braced escapes", () => { + const b = { left: 1 }; + const out = markHidden("a\u{E0041}b\u{E0041}", b); + expect(spans(out)).toBe(1); + expect(b.left).toBe(0); + expect(out).toContain("\\u{e0041}"); + expect(out.replace(/]*>[^<]*<\/span>/g, "")).not.toContain("\u{E0041}"); + }); + test("a budget of 0 escapes everything", () => { + expect(markHidden("a" + ZW + "b", { left: 0 })).toBe("a\\u200bb"); + }); + test("the budget is shared with markBidi", () => { + const b = { left: 2 }; + const out = markBidi(markHidden("a" + ZW + "b" + ZW + "c\u202E", b), b); + expect(spans(out)).toBe(2); + expect(out).toContain("\\u202e"); + expect(out.replace(/]*>[^<]*<\/span>/g, "")).not.toMatch(/[\u200B\u202E]/); + }); +}); + +type View = { lineHtml(i: number): string; sr: { line: number; start: number; end: number } | null }; +const view = (text: string, lang = "text") => new env.C.CodeView(text, lang, 10) as unknown as View; + +describe("AC-19 rendered lines", () => { + test("a line with a hidden character shows the span and a bidi line keeps .bidi", () => { + const v = view("a" + ZW + "b\nx\u202Ey\nplain"); + expect(v.lineHtml(0)).toContain(span("U+200B ZERO WIDTH SPACE", ZW)); + expect(v.lineHtml(1)).toContain(''); + expect(v.lineHtml(2)).toBe("plain"); + }); + test("highlighted lines are marked too, and the cache returns the same html", () => { + const v = view('const s = "a' + ZW + 'b"; // c' + ZW + "\n", "js"); + const html = v.lineHtml(0); + expect((html.match(/class="hidden-char"/g) ?? []).length).toBe(2); + expect(v.lineHtml(0)).toBe(html); + }); + test("truncated long lines are still marked", () => { + const html = view(ZW + "a".repeat(5000)).lineHtml(0); + expect(html).toContain('class="hidden-char"'); + expect(html).toContain('class="trunc"'); + }); + test("markers per line are bounded across hidden and bidi together", () => { + const html = view(("a" + ZW).repeat(40) + ("b\u202E").repeat(40)).lineHtml(0); + expect(spans(html)).toBe(MAX); + expect(html.replace(/]*>[^<]*<\/span>/g, "")).not.toMatch(/[\u200B\u202E]/); + }); +}); + +describe("AC-19b rendered segments agree with scanHidden", () => { + const agree = (v: View, text: string) => { + const hits = scanHidden(text).hits.filter((h) => h.kind !== "bidi"); + text.split("\n").forEach((_, i) => { + expect(wrapped(v.lineHtml(i))).toEqual(hits.filter((h) => h.line === i + 1).map((h) => h.cp)); + }); + }; + test("a joiner after an emoji in the previous segment is exempt, after a letter it is marked", () => { + const text = "\u{1F600}\u200D\u200Dz\na\u200Dz"; + const v = view(text); + v.sr = { line: 0, start: 2, end: 3 }; // a mark boundary right after the emoji + expect(wrapped(v.lineHtml(0))).toEqual([0x200d]); // first ZWJ follows the emoji (exempt), the second follows a ZWJ + agree(v, text); + v.sr = { line: 1, start: 1, end: 2 }; // the ZWJ starts its own segment after "a" + expect(wrapped(v.lineHtml(1))).toEqual([0x200d]); + agree(v, text); + }); + test("a variation selector starting a segment after another one is marked", () => { + const text = "a\uFE00\uFE01b"; + const v = view(text); + v.sr = { line: 0, start: 2, end: 3 }; + expect(wrapped(v.lineHtml(0))).toEqual([0xfe01]); + agree(v, text); + }); + test("across real token boundaries of a highlighted language", () => { + const text = 'x = "a"\u200D + "\u{1F600}"\u200D; // \u200D\n"\u{1F600}\u200D"\u200B'; + const v = view(text, "js"); + agree(v, text); + expect(wrapped(v.lineHtml(0)).length).toBeGreaterThan(0); + }); + test("a BOM starting line 2 is marked, one starting line 1 is not", () => { + const text = "\uFEFFa\n\uFEFFb"; + const v = view(text); + expect(wrapped(v.lineHtml(0))).toEqual([]); + expect(wrapped(v.lineHtml(1))).toEqual([0xfeff]); + agree(v, text); + }); + test("a joiner starting line 2 follows a newline and is marked", () => { + const text = "\u{1F600}\n\u200Dx"; + const v = view(text); + expect(wrapped(v.lineHtml(1))).toEqual([0x200d]); + agree(v, text); + }); +}); diff --git a/test/client.hiddennav.test.ts b/test/client.hiddennav.test.ts new file mode 100644 index 0000000..6245136 --- /dev/null +++ b/test/client.hiddennav.test.ts @@ -0,0 +1,84 @@ +import { describe, expect, test } from "bun:test"; +import { createHiddenNav, type HiddenNavHost } from "../src/client/hiddennav.ts"; + +const ZW = "\u200B"; +const mk = (lines: readonly string[] | null) => { + const log = { reveal: [] as number[], announce: [] as string[], toast: [] as string[] }; + const state = { lines }; + const host: HiddenNavHost = { + lines: () => state.lines, + reveal: (l) => void log.reveal.push(l), + announce: (m) => void log.announce.push(m), + toast: (m) => void log.toast.push(m), + }; + return { nav: createHiddenNav(host), log, state }; +}; +const TEXT = ["a", ZW + "b", "c\u2060"]; + +describe("AC-20 Go to Next Hidden Character", () => { + test("walks the hits in order and wraps, announcing each", () => { + const { nav, log } = mk(TEXT); + nav.next(); + expect(log.announce).toEqual(["U+200B ZERO WIDTH SPACE at line 2, column 1"]); + expect(log.reveal).toEqual([1]); + nav.next(); + expect(log.announce[1]).toBe("U+2060 WORD JOINER at line 3, column 2"); + expect(log.reveal[1]).toBe(2); + nav.next(); + expect(log.announce[2]).toBe("U+200B ZERO WIDTH SPACE at line 2, column 1"); + expect(log.reveal[2]).toBe(1); + expect(log.toast).toEqual([]); + }); + test("bidi controls are navigable too", () => { + const { nav, log } = mk(["x\u202Ey"]); + nav.next(); + expect(log.announce).toEqual(["U+202E BIDI CONTROL at line 1, column 2"]); + }); + test("a position set by another reveal is the starting point", () => { + const { nav, log } = mk(TEXT); + nav.seen(2); // 0-based line 2 is line 3 + nav.next(); + expect(log.announce).toEqual(["U+2060 WORD JOINER at line 3, column 2"]); + nav.seen(0); + nav.next(); + expect(log.announce[1]).toBe("U+200B ZERO WIDTH SPACE at line 2, column 1"); + }); +}); + +describe("AC-21 nothing to go to", () => { + test("a file without hidden characters toasts and does not move", () => { + const { nav, log } = mk(["plain", "text \u200E"]); + nav.next(); + expect(log.toast).toEqual(["No hidden characters in this file"]); + expect(log.announce).toEqual([]); + expect(log.reveal).toEqual([]); + }); + test("no text file does nothing visible and does not throw", () => { + const { nav, log } = mk(null); + expect(() => nav.next()).not.toThrow(); + expect(log.announce).toEqual([]); + expect(log.reveal).toEqual([]); + }); +}); + +describe("AC-22 state", () => { + test("reset() and a different file both start again at (0,0)", () => { + const { nav, log, state } = mk(TEXT); + nav.next(); + nav.next(); + nav.reset(); + nav.next(); + expect(log.announce[2]).toBe("U+200B ZERO WIDTH SPACE at line 2, column 1"); + nav.next(); // now at line 3 + state.lines = [...TEXT]; // a freshly opened file + nav.next(); + expect(log.announce[4]).toBe("U+200B ZERO WIDTH SPACE at line 2, column 1"); + }); + test("only the first 200 hits are navigable, then it wraps", () => { + const { nav, log } = mk([ZW.repeat(300)]); + for (let i = 0; i < 201; i++) nav.next(); + expect(log.announce[0]).toBe("U+200B ZERO WIDTH SPACE at line 1, column 1"); + expect(log.announce[199]).toBe("U+200B ZERO WIDTH SPACE at line 1, column 200"); + expect(log.announce[200]).toBe("U+200B ZERO WIDTH SPACE at line 1, column 1"); + }); +}); diff --git a/test/servergit.hiddenbom.test.ts b/test/servergit.hiddenbom.test.ts new file mode 100644 index 0000000..e0387a1 --- /dev/null +++ b/test/servergit.hiddenbom.test.ts @@ -0,0 +1,31 @@ +import { afterAll, beforeAll, expect, test } from "bun:test"; +import { mkdtemp, realpath, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { Git } from "../src/server/git.ts"; + +const env = { ...process.env, GIT_CONFIG_GLOBAL: "/dev/null", GIT_CONFIG_SYSTEM: "/dev/null" }; +const sh = (cwd: string, ...cmd: string[]) => { + const r = Bun.spawnSync(["git", "-c", "user.name=Tester", "-c", "user.email=t@example.com", ...cmd], { cwd, stderr: "pipe", env }); + if (r.exitCode !== 0) throw new Error(`git ${cmd.join(" ")}: ${r.stderr}`); +}; + +let dir = ""; +beforeAll(async () => { + dir = await realpath(await mkdtemp(path.join(tmpdir(), "cv-hbom-"))); + sh(dir, "init", "-q", "-b", "main"); + await writeFile(path.join(dir, "seed.txt"), "seed\n"); + sh(dir, "add", "."); + sh(dir, "commit", "-qm", "init"); + await writeFile(path.join(dir, "two.txt"), "\uFEFF\uFEFFa\n"); + await writeFile(path.join(dir, "one.txt"), "\uFEFFa\n"); +}); +afterAll(() => rm(dir, { recursive: true, force: true })); + +test("a file starting with two BOMs counts the second (scanHidden's offset-0 rule), one BOM counts none", async () => { + const git = (await Git.open(dir))!; + const files = (await git.changes("head")).files as { path: string; hidden?: number }[]; + const by = Object.fromEntries(files.map((f) => [f.path, f])); + expect(by["two.txt"]?.hidden).toBe(1); + expect("hidden" in by["one.txt"]!).toBe(false); +}); From e9f8882478a56d284c792c7561ad80ed2cca17b7 Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 9 Nov 2026 16:25:21 +0100 Subject: [PATCH 14/27] fix(codeview): Go to Next Hidden Character starts from the current position --- src/client/app.ts | 1 + src/client/hiddennav.ts | 9 +++ src/client/styles.css | 6 +- test/client.hiddennav.position.test.ts | 86 ++++++++++++++++++++++++++ 4 files changed, 99 insertions(+), 3 deletions(-) create mode 100644 test/client.hiddennav.position.test.ts diff --git a/src/client/app.ts b/src/client/app.ts index c35cf96..a1e6ead 100644 --- a/src/client/app.ts +++ b/src/client/app.ts @@ -102,6 +102,7 @@ class App { }); private hiddenNav = createHiddenNav({ lines: () => this.editor.active?.code?.lineTexts ?? null, + position: () => ({ line: this.editor.location()?.line ?? 1 }), reveal: (l) => this.editor.active?.code?.revealLine(l), announce: (m) => (this.live.textContent = m), toast: (m) => this.notify(m), diff --git a/src/client/hiddennav.ts b/src/client/hiddennav.ts index 737479c..86d9b4e 100644 --- a/src/client/hiddennav.ts +++ b/src/client/hiddennav.ts @@ -5,6 +5,8 @@ import { hiddenName, nextHidden, scanHidden, type HiddenHit } from "./hidden.ts" export interface HiddenNavHost { /** The active text file's lines, or null when no text file is shown. */ lines(): readonly string[] | null; + /** The editor's current position: 1-based line (as Editor.location() reports it), 1-based col when known. */ + position?(): { line: number; col?: number }; /** Reveals a 0-based line. */ reveal(line: number): void; /** Polite live-region announcement. */ @@ -40,6 +42,13 @@ export function createHiddenNav(host: HiddenNavHost): HiddenNav { const lines = host.lines(); if (file && lines !== file) reset(); file = lines; + const p = host.position?.(); + if (p) { + // The editor owns the position (search, Go to Line, a click move it); the column of our own + // last jump is kept only while the editor is still on that line. + col = p.col ?? (p.line === line ? col : 0); + line = p.line; + } const hit: HiddenHit | undefined = lines ? nextHidden(scanHidden(lines.join("\n")).hits, line, col) : undefined; if (!hit) return void host.toast("No hidden characters in this file"); line = hit.line; diff --git a/src/client/styles.css b/src/client/styles.css index fc71f79..d83432b 100644 --- a/src/client/styles.css +++ b/src/client/styles.css @@ -1243,14 +1243,14 @@ svg { overflow: visible; box-shadow: 0 0 0 1px var(--danger); } -/* Zero-width, tag and variation characters in code: an outlined box that keeps columns where they are. */ +/* A zero-width, tag or variation character in code: zero width like .bidi, so columns stay put; the outline makes it visible. */ .hidden-char { display: inline-block; - min-width: 0.4em; + width: 0; height: 1.1em; vertical-align: text-bottom; + overflow: visible; box-shadow: 0 0 0 1px var(--danger); - background: color-mix(in srgb, var(--danger) 18%, transparent); } .trunc { -webkit-user-select: none; diff --git a/test/client.hiddennav.position.test.ts b/test/client.hiddennav.position.test.ts new file mode 100644 index 0000000..c9a8709 --- /dev/null +++ b/test/client.hiddennav.position.test.ts @@ -0,0 +1,86 @@ +import { describe, expect, test } from "bun:test"; +import { createHiddenNav } from "../src/client/hiddennav.ts"; + +const ZW = "\u200B"; +const WJ = "\u2060"; +// Hits: (2,1) (4,1) (4,3) (6,2); lines 1, 3, 5 and 7 are clean. +const LINES = ["a", ZW + "b", "c", ZW + "d" + WJ, "e", "f" + ZW, "g"]; + +/** A host whose position is the editor's: a line (1-based, as Editor.location() reports it); a jump moves it. */ +const mk = (lines: readonly string[] = LINES) => { + const state = { line: 1, col: undefined as number | undefined }; + const log = { reveal: [] as number[], announce: [] as string[], toast: [] as string[] }; + const host = { + lines: () => lines, + position: () => (state.col === undefined ? { line: state.line } : { line: state.line, col: state.col }), + reveal: (l: number) => { + log.reveal.push(l); + state.line = l + 1; + state.col = undefined; + }, + announce: (m: string) => void log.announce.push(m), + toast: (m: string) => void log.toast.push(m), + }; + return { nav: createHiddenNav(host as never), log, state }; +}; + +describe("the command starts from the editor's current position", () => { + test("after the position moves to line 3 (search, Go to Line, a click) the next hit is the first after it", () => { + const { nav, log, state } = mk(); + nav.next(); + expect(log.announce[0]).toBe("U+200B ZERO WIDTH SPACE at line 2, column 1"); + state.line = 3; + nav.next(); + expect(log.announce[1]).toBe("U+200B ZERO WIDTH SPACE at line 4, column 1"); + expect(log.reveal[1]).toBe(3); + state.line = 5; // a search lands past the module's own last jump + nav.next(); + expect(log.announce[2]).toBe("U+200B ZERO WIDTH SPACE at line 6, column 2"); + }); + test("moving back after a jump is honoured, not the last jump", () => { + const { nav, log, state } = mk(); + nav.next(); + nav.next(); + state.line = 1; + nav.next(); + expect(log.announce[2]).toBe("U+200B ZERO WIDTH SPACE at line 2, column 1"); + }); + test("a fresh press from a position before any hit finds the first hit", () => { + const { nav, log } = mk(); + nav.next(); + expect(log.announce).toEqual(["U+200B ZERO WIDTH SPACE at line 2, column 1"]); + }); + test("repeated presses advance, including two hits on one line, and wrap", () => { + const { nav, log } = mk(); + for (let i = 0; i < 5; i++) nav.next(); + expect(log.announce).toEqual([ + "U+200B ZERO WIDTH SPACE at line 2, column 1", + "U+200B ZERO WIDTH SPACE at line 4, column 1", + "U+2060 WORD JOINER at line 4, column 3", + "U+200B ZERO WIDTH SPACE at line 6, column 2", + "U+200B ZERO WIDTH SPACE at line 2, column 1", + ]); + }); + test("a position past the last hit wraps to the first", () => { + const { nav, log, state } = mk(); + state.line = 7; + nav.next(); + expect(log.announce).toEqual(["U+200B ZERO WIDTH SPACE at line 2, column 1"]); + }); + test("a provider that knows the column starts after that column", () => { + const { nav, log, state } = mk(); + state.line = 4; + state.col = 1; + nav.next(); + expect(log.announce).toEqual(["U+2060 WORD JOINER at line 4, column 3"]); + }); + test("a different file starts from its own position", () => { + const a = mk(); + a.nav.next(); + a.nav.next(); + const b = mk(["x", ZW, "y"]); + b.state.line = 1; + b.nav.next(); + expect(b.log.announce).toEqual(["U+200B ZERO WIDTH SPACE at line 2, column 1"]); + }); +}); From ca1727c342517a601dcf54e526c02f7a19f84f34 Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 9 Nov 2026 16:32:54 +0100 Subject: [PATCH 15/27] fix(changes): only the flagged count is a live region, and it never clips --- DESIGN.md | 22 ++++++++++++++ src/client/changes.ts | 6 ++-- src/client/styles.css | 2 +- test/client.changes.test.ts | 60 +++++++++++++++++++++++++++++++++++-- 4 files changed, 83 insertions(+), 7 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index 0f0ef8a..0c2c749 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -1125,3 +1125,25 @@ after the path, counts flagged rows in the summary and exposes `flags(path)`. Th | A Markdown file shown as the rendered preview came back as source | `NavLoc.view = "preview"` is recorded by `location()` and `Editor.landing()`, kept by `NavHistory` on every update path (set or removed together with the line), and `Editor.open(path, { view: "preview" })` reopens the preview; `app.ts` only forwards it | | Closing a tab might forget where Back should go | Pinned by tests: history entries survive `Editor.close`, and Back reopens the file; diff and image tabs have no location | + +## 29. Round 20: hidden characters + +A panel of four (git/security engineer, UX/accessibility designer, agent-workflow developer, +code-navigation engineer) picked the next feature. Hidden characters won with 4 votes (git/security #1, +UX #2, agent-workflow #3, navigation #4). A checkpoint diff got 3 votes but is size L and was deferred a +third time. The Changes summary live-region fix (Q1/Q2) got 3 votes and ships as Part B. Single votes: +Go to Definition, next unreviewed changed file, go to last live change, a repository trust report. + +`src/client/hidden.ts` (pure, also imported by the server) defines the counted set once. The server adds +`hidden` to changed files, Changes rows show a `hidden` badge, the code view marks zero-width, tag and +variation runs, and "Go to Next Hidden Character" jumps between hits. Results are hints, not verdicts. + +| Finding | Fix | +| --- | --- | +| The code view renders a line in token segments, so the joiner and variation-selector rules could not see the character before a segment and a BOM starting a later line was never marked | `markHidden` takes `ctx {prev, atFileStart}` and the code view passes them per segment (caught in plan review) | +| The first tests held raw bidi controls and failed the repo's own invariant against raw bidi in sources | Every hidden character in tests is a `\u` escape | +| `TextDecoder` strips one leading BOM, so a file starting with two BOMs reported none | The server decodes with the BOM kept and `scanHidden` applies its own offset-0 rule | +| Go to Next Hidden Character started from its own last jump and skipped hits after a search or Go to Line | It starts from the editor's current position | +| The marker had `min-width`, which shifted columns | It is zero-width with an outline like the bidi marker | +| Joiners in Arabic, Persian, Indic text and emoji sequences would be false positives | Joiners after those letters and emoji, a lone U+FE0F, and a BOM at offset 0 are exempt | +| The Changes summary wrapper was a live region, so any re-render re-announced the whole line, and the count sat inside `.ch-counts`, whose ellipsis could clip it | The wrapper has no live role; one persistent `.ch-riskcount` element carries `role="status"`, sits beside `.ch-counts` and does not shrink | diff --git a/src/client/changes.ts b/src/client/changes.ts index c98596d..bdcbc5e 100644 --- a/src/client/changes.ts +++ b/src/client/changes.ts @@ -82,7 +82,7 @@ export class ChangesView { constructor(opts: { debounce?: number } = {}) { this.debounce = opts.debounce ?? 150; this.scopeEl = h("div", { class: "ch-scope", role: "group", "aria-label": "Comparison", hidden: true }); - this.summary = h("div", { class: "ch-summary-wrap", role: "status", hidden: true }); + this.summary = h("div", { class: "ch-summary-wrap", hidden: true }); this.list = h("div", { class: "ch-list", role: "listbox", tabindex: "0", "aria-label": "Changed files" }); this.el = h("div", { class: "changes-view" }, this.scopeEl, this.summary, this.list); this.scopeEl.addEventListener("click", (e) => { @@ -328,11 +328,11 @@ export class ChangesView { ? `·${plural(d.commits, "commit", "commits")}` : ""; const flagged = files.filter((f) => riskFlags(f).length).length; - const flaggedHtml = flagged ? `· ${flagged} flagged` : ""; + const flaggedHtml = `${flagged ? `· ${flagged} flagged` : ""}`; const p = added + removed ? Math.round((added / (added + removed)) * 100) : 50; this.summary.innerHTML = `
${files.length} ${files.length === 1 ? "file" : "files"}` + - `·+${added.toLocaleString()} −${removed.toLocaleString()}${commits}${flaggedHtml}` + + `·+${added.toLocaleString()} −${removed.toLocaleString()}${commits}${flaggedHtml}` + `${safe(this.vs)}
` + `
`; if (!files.length) { diff --git a/src/client/styles.css b/src/client/styles.css index d83432b..7dd0de9 100644 --- a/src/client/styles.css +++ b/src/client/styles.css @@ -2261,7 +2261,7 @@ li[data-state="done"] .upd-mark { .ch-bin { color: var(--fg-faint); font-family: var(--font-mono); } /* Risk flags: a word in a bordered pill, so the meaning never rests on colour alone. */ .risk { flex: none; margin-left: 4px; padding: 0 4px; border: 1px solid color-mix(in srgb, var(--warn) 60%, transparent); border-radius: 3px; font: 600 9.5px/14px var(--font-ui); letter-spacing: 0.04em; color: var(--warn); background: color-mix(in srgb, var(--warn) 12%, transparent); } -.ch-riskcount { margin-left: 6px; color: var(--warn); } +.ch-riskcount { flex: none; flex-shrink: 0; color: var(--warn); } /* diff tab */ .tab-kind { margin-left: 2px; padding: 0 4px; border-radius: 3px; font: 600 9.5px/14px var(--font-ui); letter-spacing: 0.04em; text-transform: uppercase; color: var(--fg-muted); background: var(--bg-active); } diff --git a/test/client.changes.test.ts b/test/client.changes.test.ts index 3c21e1a..37c80cb 100644 --- a/test/client.changes.test.ts +++ b/test/client.changes.test.ts @@ -457,11 +457,65 @@ describe("risk flags in the Changes view", () => { ]); expect(t.summary.innerHTML).toContain("· 2 flagged"); expect(t.summary.innerHTML.match(/flagged/g)!.length).toBe(1); - // The count is announced: the summary region is a status. - expect(t.summary.innerHTML.includes('role="status"') || t.summary.getAttribute("role") === "status").toBe(true); + // The count is announced by the persistent .ch-riskcount element, not by the wrapper. + expect(t.summary.getAttribute("role")).toBeNull(); + expect(t.summary.innerHTML.match(/role="status"/g)!.length).toBe(1); const none = await load([f("plain.txt", "M"), f("b.ts", "M", 1, 0, { oldMode: "100644", newMode: "100644" })]); expect(none.summary.innerHTML).not.toContain("flagged"); - expect(none.summary.innerHTML.includes('role="status"') || none.summary.getAttribute("role") === "status").toBe(true); + expect(none.summary.getAttribute("role")).toBeNull(); + expect(none.summary.innerHTML.match(/role="status"/g)!.length).toBe(1); + }); + + test("Part B: the live region is the persistent ch-riskcount element, outside ch-counts", async () => { + const flaggedT = await load([f("a.sh", "A", 1, 0, { oldMode: "000000", newMode: "100755" }), f("package.json", "M"), f("plain.txt", "M")]); + const none = await load([f("plain.txt", "M")]); + for (const t of [flaggedT, none]) { + const html = t.summary.innerHTML; + expect(t.summary.getAttribute("role")).toBeNull(); + expect(t.summary.getAttribute("aria-live")).toBeNull(); + expect(html).not.toContain("aria-live"); + expect(html.match(/role="status"/g)!.length).toBe(1); + const tag = html.match(/<[^>]*role="status"[^>]*>/)![0]; + expect(tag).toContain('class="ch-riskcount"'); + // A sibling of .ch-counts inside .ch-summary: not within the .ch-counts element. + const start = html.indexOf('".length; break; } + } + expect(end).toBeGreaterThan(start); + expect(riskAt).toBeGreaterThanOrEqual(end); + expect(html.slice(start, end)).not.toContain("ch-riskcount"); + } + expect(flaggedT.summary.innerHTML).toContain('· 2 flagged'); + expect(none.summary.innerHTML).toContain(''); + }); + + test("Part B: ch-riskcount stays at the same markup position when the flagged count changes", async () => { + const a = await load([f("a.sh", "A", 1, 0, { oldMode: "000000", newMode: "100755" })]); + const b = await load([f("plain.txt", "M")]); + // Same neighbours in both renders: right after .ch-counts and right before .ch-vs. + for (const t of [a, b]) { + expect(t.summary.innerHTML).toMatch(/<\/span>[^<]*<\/span>/); + } + expect(a.summary.innerHTML).toContain("· 1 flagged"); + expect(b.summary.innerHTML).not.toContain("flagged"); + }); + + test("Part B: styles keep ch-riskcount from shrinking and ch-counts ellipsised; DESIGN.md has the round", () => { + const css = require("node:fs").readFileSync(`${import.meta.dir}/../src/client/styles.css`, "utf8") as string; + const rule = (sel: string) => [...css.matchAll(new RegExp(`(?:^|\\n)${sel.replace(".", "\\.")}\\s*\\{([^}]*)\\}`, "g"))].map((m) => m[1]).join(" "); + expect(rule(".ch-riskcount")).toMatch(/flex-shrink:\s*0/); + expect(rule(".ch-riskcount")).not.toContain("ellipsis"); + expect(rule(".ch-counts")).toContain("text-overflow: ellipsis"); + const design = require("node:fs").readFileSync(`${import.meta.dir}/../DESIGN.md`, "utf8") as string; + expect(design).toContain("## 29. Round 20: hidden characters"); + const sec = design.slice(design.indexOf("## 29. Round 20")); + expect(sec).toMatch(/hidden characters[^\n]*4 votes|4 votes[^\n]*hidden/i); + expect(sec).toContain("| Finding | Fix |"); }); test("a row with hidden characters has the hidden badge, and the summary counts it once", async () => { From 11b9b9bd8cd09336dc856243e9cc3bef76ec95d2 Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 9 Nov 2026 16:55:48 +0100 Subject: [PATCH 16/27] feat(client): takeCheckpoint and sinceCheckpoint --- src/client/checkpoint.ts | 72 ++++++++++++ test/client.checkpoint.test.ts | 195 +++++++++++++++++++++++++++++++++ 2 files changed, 267 insertions(+) create mode 100644 src/client/checkpoint.ts create mode 100644 test/client.checkpoint.test.ts diff --git a/src/client/checkpoint.ts b/src/client/checkpoint.ts new file mode 100644 index 0000000..5352efd --- /dev/null +++ b/src/client/checkpoint.ts @@ -0,0 +1,72 @@ +// Changes since a checkpoint: pure helpers, no DOM and no network. +import { diffLines } from "./diff.ts"; + +export interface Checkpoint { + at: number; + rev: string | null; + texts: Map; + skipped: string[]; +} +export interface SinceEntry { + path: string; + status: "A" | "M" | "D"; + added: number; + removed: number; +} + +const MAX_FILES = 500; +const MAX_CHARS = 16_000_000; + +/** Remembers the text of every file that differs from HEAD now, within the file and character caps. */ +export function takeCheckpoint( + files: readonly { path: string; text: string | null }[], + at: number, + rev: string | null, + limits?: { files?: number; chars?: number }, +): Checkpoint { + const maxFiles = limits?.files ?? MAX_FILES; + const maxChars = limits?.chars ?? MAX_CHARS; + const texts = new Map(); + const skipped: string[] = []; + let chars = 0; + for (const f of files) { + const n = f.text === null ? 0 : f.text.length; + if (texts.size < maxFiles && chars + n <= maxChars) { + texts.set(f.path, f.text); + chars += n; + } else skipped.push(f.path); + } + return { at, rev, texts, skipped }; +} + +function linesOf(text: string | null): string[] { + if (text === null) return []; + const l = text.split("\n"); + if (l[l.length - 1] === "") l.pop(); + return l; +} + +/** What changed in each file between the checkpoint and now. */ +export function sinceCheckpoint( + cp: Checkpoint, + current: ReadonlyMap, + base: (path: string) => string | null | undefined, +): SinceEntry[] { + const skipped = new Set(cp.skipped); + const paths = new Set([...current.keys(), ...cp.texts.keys()]); + const out: SinceEntry[] = []; + for (const path of [...paths].sort((a, b) => (a < b ? -1 : a > b ? 1 : 0))) { + if (skipped.has(path)) continue; + const old = cp.texts.has(path) ? cp.texts.get(path) : base(path); + const now = current.has(path) ? current.get(path) : base(path); + if (old === undefined || now === undefined || old === now) continue; + let added = 0; + let removed = 0; + for (const h of diffLines(linesOf(old), linesOf(now))) { + added += h.b1 - h.b0; + removed += h.a1 - h.a0; + } + out.push({ path, status: old === null ? "A" : now === null ? "D" : "M", added, removed }); + } + return out; +} diff --git a/test/client.checkpoint.test.ts b/test/client.checkpoint.test.ts new file mode 100644 index 0000000..01ee9d9 --- /dev/null +++ b/test/client.checkpoint.test.ts @@ -0,0 +1,195 @@ +import { describe, expect, test } from "bun:test"; +import { sinceCheckpoint, takeCheckpoint, type Checkpoint } from "../src/client/checkpoint.ts"; + +type F = { path: string; text: string | null }; +const f = (path: string, text: string | null): F => ({ path, text }); +const keys = (cp: Checkpoint) => [...cp.texts.keys()]; +/** A base stub: undefined for any path not named. */ +const baseOf = (m: Record) => (p: string) => (p in m ? m[p] : undefined); +const cur = (o: Record) => new Map(Object.entries(o)); +const since = (files: F[], current: Record, base: Record = {}) => + sinceCheckpoint(takeCheckpoint(files, 1, "R"), cur(current), baseOf(base)); + +describe("takeCheckpoint", () => { + test("AC-1 stores texts in order and keeps at and rev", () => { + const cp = takeCheckpoint([f("a", "x"), f("b", null)], 1700000000000, "R"); + expect(cp.at).toBe(1700000000000); + expect(cp.rev).toBe("R"); + expect(cp.skipped).toEqual([]); + expect([...cp.texts.entries()]).toEqual([["a", "x"], ["b", null]]); + expect(cp.texts.size).toBe(2); + expect(cp.texts.get("b")).toBeNull(); + expect(cp.texts.has("b")).toBe(true); + expect(takeCheckpoint([f("a", "x")], 1, null).rev).toBeNull(); + }); + + test("AC-1 the result is a copy of the input", () => { + const files = [f("a", "x"), f("b", "y")]; + const cp = takeCheckpoint(files, 1, "R", { files: 2 }); + files.push(f("c", "z")); + files[0]!.text = "changed"; + expect(cp.texts.get("a")).toBe("x"); + expect(keys(cp)).toEqual(["a", "b"]); + expect(cp.skipped).toEqual([]); + expect(cp as unknown).not.toBe(files); + }); + + test("AC-2 chars limit skips a big file but a later small one still fits", () => { + const cp = takeCheckpoint([f("a", "12345"), f("b", "1234567890"), f("c", "123")], 0, null, { chars: 8 }); + expect(keys(cp)).toEqual(["a", "c"]); + expect(cp.skipped).toEqual(["b"]); + }); + + test("AC-2 files limit", () => { + const files = [f("a", "x"), f("b", "y"), f("c", "z")]; + const two = takeCheckpoint(files, 0, null, { files: 2 }); + expect(keys(two)).toEqual(["a", "b"]); + expect(two.skipped).toEqual(["c"]); + const none = takeCheckpoint(files, 0, null, { files: 0 }); + expect(keys(none)).toEqual([]); + expect(none.skipped).toEqual(["a", "b", "c"]); + }); + + test("AC-2 null and empty texts count as zero characters", () => { + const files = [f("a", "123456"), f("b", null), f("c", ""), f("d", "12")]; + const six = takeCheckpoint(files, 0, null, { chars: 6 }); + expect(keys(six)).toEqual(["a", "b", "c"]); + expect(six.skipped).toEqual(["d"]); + const zero = takeCheckpoint(files, 0, null, { chars: 0 }); + expect(keys(zero)).toEqual(["b", "c"]); + expect(zero.skipped).toEqual(["a", "d"]); + }); + + test("AC-2 each limit keeps the other's default", () => { + expect(takeCheckpoint([f("a", "x".repeat(20))], 0, null, { files: 5 }).texts.has("a")).toBe(true); + const many = Array.from({ length: 501 }, (_, i) => f("f" + i, "x")); + const cp = takeCheckpoint(many, 0, null); + expect(cp.texts.size).toBe(500); + expect(cp.skipped).toEqual(["f500"]); + expect(cp.texts.has("f500")).toBe(false); + }); + + test("AC-2 length is the UTF-16 length", () => { + const cp = takeCheckpoint([f("a", "\u{1F600}"), f("b", "x")], 0, null, { chars: 2 }); + expect(keys(cp)).toEqual(["a"]); + expect(cp.skipped).toEqual(["b"]); + }); + + test("AC-3 empty input and the empty string", () => { + const cp = takeCheckpoint([], 5, null); + expect(cp.at).toBe(5); + expect(cp.rev).toBeNull(); + expect(cp.texts.size).toBe(0); + expect(cp.skipped).toEqual([]); + expect(takeCheckpoint([f("e", "")], 0, null).texts.get("e")).toBe(""); + }); +}); + +describe("sinceCheckpoint", () => { + test("AC-4.1 the worked example", () => { + const files = [f("a.ts", "x\n")]; + const current = { "a.ts": "x\ny\n", "b.ts": "new\n" }; + expect(since(files, current, { "b.ts": null })).toEqual([ + { path: "a.ts", status: "M", added: 1, removed: 0 }, + { path: "b.ts", status: "A", added: 1, removed: 0 }, + ]); + expect(since(files, { "b.ts": "new\n" }, { "a.ts": "x\n", "b.ts": null })).toEqual([ + { path: "b.ts", status: "A", added: 1, removed: 0 }, + ]); + }); + + test("AC-4.2 delete", () => { + expect(since([f("a", "x\ny\n")], { a: null })).toEqual([{ path: "a", status: "D", added: 0, removed: 2 }]); + }); + + test("AC-4.3 add of a file that was deleted at checkpoint time", () => { + expect(since([f("c", null)], { c: "q\nr\n" })).toEqual([{ path: "c", status: "A", added: 2, removed: 0 }]); + }); + + test("AC-4.4 modify with both counts", () => { + expect(since([f("p", "a\nb\nc\n")], { p: "a\nB\nc\nd\n" })).toEqual([ + { path: "p", status: "M", added: 2, removed: 1 }, + ]); + }); + + test("AC-4.5 untracked at checkpoint, changed later", () => { + expect(since([f("n", "1\n")], { n: "1\n2\n3\n" })).toEqual([{ path: "n", status: "M", added: 2, removed: 0 }]); + }); + + test("AC-4.6 unchanged at checkpoint, changed now", () => { + expect(since([], { m: "two\n" }, { m: "one\n" })).toEqual([{ path: "m", status: "M", added: 1, removed: 1 }]); + expect(since([], { m: "two\n" }, { m: null })).toEqual([{ path: "m", status: "A", added: 1, removed: 0 }]); + expect(since([], { m: null }, { m: "one\ntwo\n" })).toEqual([{ path: "m", status: "D", added: 0, removed: 2 }]); + }); + + test("AC-4.7 a path no longer changed goes back to the base text", () => { + expect(since([f("k", "edited\n")], {}, { k: "orig\n" })).toEqual([ + { path: "k", status: "M", added: 1, removed: 1 }, + ]); + expect(since([f("k", "edited\n")], {}, { k: null })).toEqual([{ path: "k", status: "D", added: 0, removed: 1 }]); + }); + + test("AC-4.8 differing only by the final newline is kept with zero counts", () => { + expect(since([f("t", "x")], { t: "x\n" })).toEqual([{ path: "t", status: "M", added: 0, removed: 0 }]); + }); + + test("AC-4.9 the empty string is not null", () => { + expect(since([f("e", "")], { e: "x" })).toEqual([{ path: "e", status: "M", added: 1, removed: 0 }]); + expect(since([f("e", null)], { e: "" })).toEqual([{ path: "e", status: "A", added: 0, removed: 0 }]); + }); + + test("AC-5 leaves out identical, unknown, skipped and reverted paths", () => { + expect(since([f("x", "x\n")], { x: "x\n" })).toEqual([]); + expect(since([f("n", null)], { n: null })).toEqual([]); + expect(since([], { u: "x" }, { u: undefined as unknown as null })).toEqual([]); + expect(since([f("u", "x")], {}, { u: undefined as unknown as null })).toEqual([]); + const cp = takeCheckpoint([f("s", "x")], 0, "R", { files: 0 }); + expect(cp.skipped).toEqual(["s"]); + const never = () => { + throw new Error("base must not be asked"); + }; + expect(sinceCheckpoint(cp, cur({ s: "anything" }), never)).toEqual([]); + expect(since([], {}, { z: "whatever" })).toEqual([]); + expect(since([f("k", "edited\n")], {}, { k: "edited\n" })).toEqual([]); + }); + + test("AC-5 skipping one path never affects the others", () => { + const cp = takeCheckpoint([f("a", "1"), f("b", "2")], 0, "R", { files: 1 }); + expect(cp.skipped).toEqual(["b"]); + expect(sinceCheckpoint(cp, cur({ a: "1\n2", b: "q" }), baseOf({})).map((e) => e.path)).toEqual(["a"]); + expect(since([f("u", "x"), f("v", "1")], { v: "2" }, { u: undefined as unknown as null })).toEqual([ + { path: "v", status: "M", added: 1, removed: 1 }, + ]); + }); + + test("AC-6.1 sorted by plain string comparison", () => { + const current = { "a/b.ts": "1", "a.ts": "1", "B.ts": "1", "b.ts": "1" }; + const base = { "a/b.ts": null, "a.ts": null, "B.ts": null, "b.ts": null }; + expect(since([], current, base).map((e) => e.path)).toEqual(["B.ts", "a.ts", "a/b.ts", "b.ts"]); + }); + + test("AC-6.2 line splitting", () => { + expect(since([f("l", "a\n\n")], { l: "a\n" })).toEqual([{ path: "l", status: "M", added: 0, removed: 1 }]); + expect(since([f("l", "x\n")], { l: "x\nx\n" })).toEqual([{ path: "l", status: "M", added: 1, removed: 0 }]); + expect(since([f("l", "a\r\n")], { l: "a\n" })).toEqual([{ path: "l", status: "M", added: 1, removed: 1 }]); + }); + + test("AC-6.3 empty inputs", () => { + expect(sinceCheckpoint(takeCheckpoint([], 0, null), new Map(), () => undefined)).toEqual([]); + }); + + test("AC-6.4 does not mutate its inputs", () => { + const cp = takeCheckpoint([f("a", "1"), f("b", "2")], 0, "R", { files: 1 }); + const current = cur({ a: "2", c: "3" }); + sinceCheckpoint(cp, current, baseOf({ c: null })); + expect([...cp.texts.entries()]).toEqual([["a", "1"]]); + expect(cp.skipped).toEqual(["b"]); + expect([...current.entries()]).toEqual([["a", "2"], ["c", "3"]]); + }); + + test("AC-6.5 counts add up across hunks", () => { + expect(since([f("h", "1\n2\n3\n4\n5\n")], { h: "1\nX\n3\n4\nY\nZ\n" })).toEqual([ + { path: "h", status: "M", added: 3, removed: 2 }, + ]); + }); +}); From a8a5cd43f2ef38ed1cb4eae1131e42513094b4d0 Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 9 Nov 2026 17:09:03 +0100 Subject: [PATCH 17/27] feat(changes): Set Checkpoint and Changes Since Checkpoint --- src/client/app.ts | 16 +- src/client/checkpointctl.ts | 141 ++++++++++ src/client/editor.ts | 51 +++- test/client.checkpointctl.test.ts | 451 ++++++++++++++++++++++++++++++ test/client.checkpointtab.test.ts | 236 ++++++++++++++++ 5 files changed, 888 insertions(+), 7 deletions(-) create mode 100644 src/client/checkpointctl.ts create mode 100644 test/client.checkpointctl.test.ts create mode 100644 test/client.checkpointtab.test.ts diff --git a/src/client/app.ts b/src/client/app.ts index a1e6ead..ef6beca 100644 --- a/src/client/app.ts +++ b/src/client/app.ts @@ -1,5 +1,6 @@ import { wsUrl } from "./auth.ts"; // first: installs the auth header on fetch import { api, type Info } from "./api.ts"; +import { CHECKPOINT_COMMANDS, CheckpointController } from "./checkpointctl.ts"; import { Editor, type StatusInfo } from "./editor.ts"; import { FileTree } from "./tree.ts"; import { Palette, type Command, type PickItem } from "./palette.ts"; @@ -94,6 +95,18 @@ class App { private statusRight!: HTMLElement; private themeBtn!: HTMLButtonElement; private live!: HTMLElement; + private checkpoints = new CheckpointController({ + changes: () => api.gitChanges("head"), + readText: async (p) => { + const f = await api.file(p).catch(() => null); + return f && f.kind === "text" && !f.truncated ? f.text : undefined; + }, + baseText: (p, rev) => api.gitBase(p, rev), + now: () => Date.now(), + notify: (m) => this.notify(m), + pick: (items, title) => this.palette.pick(items, title), + openDiff: (d) => void this.editor.openSinceDiff(d, { focus: true }), + }); private nav = new Navigation({ open: (loc) => this.editor.open(loc.path, loc.view ? { pin: true, view: loc.view, focus: false } : { pin: true, line: loc.line, focus: false }).then((ok) => (void this.tree.reveal(loc.path), ok)), current: () => this.editor.location(), @@ -164,7 +177,7 @@ class App { if (tabs.active) await this.editor.open(tabs.active, { focus: false }); this.editor.onChange = () => { store.set(`tabs:${this.info.root}`, this.editor.serialize()); - this.changes.watching = this.editor.tabs.some((t) => t.isDiff && !t.commit); + this.changes.watching = this.editor.tabs.some((t) => t.isDiff && !t.commit && !t.since); }; } @@ -440,6 +453,7 @@ class App { if (!t) this.notify("Open a file first"); else void this.fileHistory(t.path); }), + ...CHECKPOINT_COMMANDS.map((k) => c(k.id, k.title, () => void (k.id === "changes.checkpoint" ? this.checkpoints.set() : k.id === "changes.sinceCheckpoint" ? this.checkpoints.since() : this.checkpoints.clear()))), c("git.scope", "Git: Compare With Base Branch / HEAD", () => this.changes.toggleScope()), c("app.update", "Check for Updates…", () => void this.updates.checkNow()), c("app.autoUpdate", "Toggle Automatic Update Checks", () => { diff --git a/src/client/checkpointctl.ts b/src/client/checkpointctl.ts new file mode 100644 index 0000000..355a9f2 --- /dev/null +++ b/src/client/checkpointctl.ts @@ -0,0 +1,141 @@ +// The controller behind the three "Changes: ... Checkpoint" commands. app.ts only wires it up. +import type { ChangeFile } from "./api.ts"; +import type { PickItem } from "./palette.ts"; +import { sinceCheckpoint, takeCheckpoint, type Checkpoint } from "./checkpoint.ts"; +import { showHidden } from "./util.ts"; + +export const CHECKPOINT_COMMANDS = [ + { id: "changes.checkpoint", title: "Changes: Set Checkpoint" }, + { id: "changes.sinceCheckpoint", title: "Changes: Since Checkpoint\u2026" }, + { id: "changes.clearCheckpoint", title: "Changes: Clear Checkpoint" }, +] as const; + +export interface SinceDiff { + path: string; + status: "A" | "M" | "D"; + /** null = the file does not exist on that side. */ + oldText: string | null; + newText: string | null; + /** "HH:MM" of the checkpoint. */ + since: string; +} + +export interface CheckpointDeps { + changes(): Promise<{ repo: boolean; rev: string | null; files: ChangeFile[] }>; + /** Current text; null = missing; undefined = binary, too large or unreadable. */ + readText(path: string): Promise; + baseText(path: string, rev: string): Promise; + now(): number; + notify(message: string): void; + pick(items: PickItem[], title: string): void; + openDiff(d: SinceDiff): void; +} + +type Listing = { repo: boolean; rev: string | null; files: ChangeFile[] }; + +export class CheckpointController { + private cp: Checkpoint | null = null; + private gen = 0; + constructor(private deps: CheckpointDeps) {} + get checkpoint(): Checkpoint | null { + return this.cp; + } + + private async list(): Promise { + let l: Listing; + try { + l = await this.deps.changes(); + } catch { + this.deps.notify("Could not read the changes"); + return null; + } + if (!l.repo) { + this.deps.notify("Not a git repository"); + return null; + } + return l; + } + + /** Current texts of the listed files; binary and unreadable ones are returned apart. */ + private async read(files: ChangeFile[]) { + const listed = new Set(files.map((f) => f.path)); + const out: { path: string; text: string | null }[] = []; + const unreadable: string[] = []; + for (const f of files) { + if (f.binary) { + unreadable.push(f.path); + continue; + } + const text = f.status === "D" ? null : await this.deps.readText(f.path); + if (text === undefined) { + unreadable.push(f.path); + continue; + } + // A rename is a delete plus an add. + if (f.status === "R" && f.oldPath !== undefined && f.oldPath !== f.path && !listed.has(f.oldPath)) out.push({ path: f.oldPath, text: null }); + out.push({ path: f.path, text }); + } + return { out, unreadable }; + } + + async set(): Promise { + const gen = ++this.gen; + const l = await this.list(); + if (!l || gen !== this.gen) return; + const at = this.deps.now(); + const { out, unreadable } = await this.read(l.files); + if (gen !== this.gen) return; + const t = takeCheckpoint(out, at, l.rev); + const cp = { ...t, skipped: [...t.skipped, ...unreadable] }; + this.cp = cp; + const n = cp.texts.size; + const m = cp.skipped.length; + this.deps.notify(m ? `Checkpoint set (${n} files, ${m} skipped)` : `Checkpoint set (${n} files)`); + } + + async since(): Promise { + const cp = this.cp; + if (!cp) return this.deps.notify("No checkpoint set"); + const l = await this.list(); + if (!l) return; + const { out, unreadable } = await this.read(l.files); + const current = new Map(out.map((o) => [o.path, o.text])); + const derived: Checkpoint = { ...cp, skipped: [...cp.skipped, ...unreadable] }; + const skip = new Set(derived.skipped); + const base = new Map(); + for (const p of new Set([...current.keys(), ...cp.texts.keys()])) { + if (skip.has(p) || (cp.texts.has(p) && current.has(p))) continue; + base.set(p, cp.rev === null ? null : await this.deps.baseText(p, cp.rev)); + } + if (this.cp !== cp) return; + const baseOf = (p: string) => base.get(p); + const entries = sinceCheckpoint(derived, current, baseOf); + const since = hhmm(cp.at); + if (!entries.length) return this.deps.notify(`Nothing changed since the checkpoint (${since})`); + const items: PickItem[] = entries.map((e) => ({ + label: showHidden(e.path), + detail: `+${e.added} \u2212${e.removed}`, + icon: ``, + run: () => + this.deps.openDiff({ + path: e.path, + status: e.status, + since, + oldText: cp.texts.has(e.path) ? cp.texts.get(e.path)! : baseOf(e.path)!, + newText: current.has(e.path) ? current.get(e.path)! : baseOf(e.path)!, + }), + })); + this.deps.pick(items, `Changes since ${since}: ${items.length} ${items.length === 1 ? "file" : "files"}, type to filter`); + } + + clear(): void { + this.cp = null; + this.deps.notify("Checkpoint cleared"); + } +} + +/** Local time of day, 24-hour, zero padded. */ +export function hhmm(ms: number): string { + const d = new Date(ms); + return `${String(d.getHours()).padStart(2, "0")}:${String(d.getMinutes()).padStart(2, "0")}`; +} diff --git a/src/client/editor.ts b/src/client/editor.ts index 15f0ddb..1d18117 100644 --- a/src/client/editor.ts +++ b/src/client/editor.ts @@ -7,6 +7,7 @@ import { fileIcon, icons } from "./icons.ts"; import { isMarkdown } from "./markdown.ts"; import { MarkdownView } from "./mdview.ts"; import type { NavLoc } from "./navhistory.ts"; +import type { SinceDiff } from "./checkpointctl.ts"; import { decidePreview, tooLargeNote } from "./mdgate.ts"; import { planView, toggleLabel, toggleMode, viewMode } from "./viewstate.ts"; import { basename, debounce, dirname, esc, formatBytes, h, MOD, showHidden } from "./util.ts"; @@ -31,6 +32,8 @@ export interface Tab { isDiff?: boolean; /** A diff of one commit from File History: fixed content, never tied to the Changes view. */ commit?: HistoryEntry; + /** "HH:MM" of the checkpoint a since-checkpoint diff compares against: fixed content, never tied to Changes. */ + since?: string; file?: ChangeFile; /** The old side of a diff tab, kept across reloads of the live file (keyed by rev and path). */ oldBlob?: { key: string; text: string | null }; @@ -182,7 +185,7 @@ export class Editor { */ async openDiff(file: ChangeFile, ctx: DiffContext, opts: { pin?: boolean; focus?: boolean } = {}) { this.diffContext = ctx; - let tab = this.tabs.find((t) => t.isDiff && !t.commit && t.path === file.path); + let tab = this.tabs.find((t) => t.isDiff && !t.commit && !t.since && t.path === file.path); if (!tab) { const preview = !opts.pin ? this.tabs.find((t) => !t.pinned) : undefined; tab = this.createTab(file.path, !!opts.pin, true); @@ -215,7 +218,7 @@ export class Editor { refreshDiffs(ctx: DiffContext, files: readonly ChangeFile[]) { this.diffContext = ctx; for (const t of this.tabs) { - if (!t.isDiff || !t.file || t.commit) continue; + if (!t.isDiff || !t.file || t.commit || t.since) continue; // A file that left the list now equals the base: diff it as a plain modification (no differences). const f = files.find((x) => x.path === t.path) ?? { ...t.file, status: "M" as const, oldPath: undefined, added: null, removed: null }; const moved = f.status !== t.file.status || f.oldPath !== t.file.oldPath; @@ -227,7 +230,7 @@ export class Editor { /** Shows "n/N" in every diff tab's header. */ setDiffPositions(pos: (path: string) => { i: number; n: number } | null) { - for (const t of this.tabs) if (t.isDiff && !t.commit) t.diff?.setPosition(pos(t.path)); + for (const t of this.tabs) if (t.isDiff && !t.commit && !t.since) t.diff?.setPosition(pos(t.path)); } /** @@ -260,6 +263,39 @@ export class Editor { this.render(); } + /** Opens a read-only diff of two texts the caller already holds (changes since a checkpoint). */ + async openSinceDiff(d: SinceDiff, opts: { pin?: boolean; focus?: boolean } = {}): Promise { + let tab = this.tabs.find((t) => t.since === d.since && t.path === d.path); + if (!tab) { + const preview = !opts.pin ? this.tabs.find((t) => !t.pinned) : undefined; + tab = this.createTab(d.path, !!opts.pin, true); + tab.since = d.since; + tab.file = { path: d.path, status: d.status, added: null, removed: null, binary: false }; + if (preview) { + const i = this.tabs.indexOf(preview); + this.disposeTab(preview); + this.tabs.splice(i, 1, tab); + } else { + const i = this.active ? this.tabs.indexOf(this.active) + 1 : this.tabs.length; + this.tabs.splice(i, 0, tab); + } + } else if (opts.pin) tab.pinned = true; + this.activate(tab); + if (!tab.diff) { + const info: DiffInfo = { ...this.diffInfo(tab.file!), vs: `since ${d.since}` }; + const open = tab; + tab.diff = new DiffView(info, tab.lang, this.fontSize); + tab.diff.onOpenFile = () => void this.open(open.path, { pin: true }); + tab.diff.setPosition(null); // not in the Changes list: no "n/N" or next-file arrows + tab.el.replaceChildren(tab.diff.el); + } + tab.kind = "text"; + tab.diff.setTexts(d.oldText ?? "", d.newText ?? ""); + if (tab === this.active) this.status(); + if (opts.focus !== false) tab.diff.focus(); + this.render(); + } + /** "in 1a2b3c4" for a commit tab, else the Changes comparison ("vs main"). */ private vsOf(t: Tab): string { return t.commit ? `in ${t.commit.oid.slice(0, 7)}` : this.diffContext.vs; @@ -359,6 +395,7 @@ export class Editor { } private async load(tab: Tab, reload = false) { + if (tab.since) return undefined; // fixed content if (tab.commit) return reload ? undefined : this.loadCommit(tab); // a commit never changes if (tab.isDiff) return this.loadDiff(tab); const gen = ++tab.gen; @@ -655,7 +692,7 @@ export class Editor { const t = this.active; if (t?.diff) { // Past the last change a Changes diff moves on to the next file; a commit diff stays put. - if (!t.diff.nextChange(dir) && !t.commit) this.onDiffStep(dir); + if (!t.diff.nextChange(dir) && !t.commit && !t.since) this.onDiffStep(dir); return; } t?.code?.nextChange(dir); @@ -708,7 +745,7 @@ export class Editor { const s: StatusInfo = { path: t.path, kind: t.kind, size: t.size, truncated: t.truncated }; if (t.diff && t.file) { s.lang = languageName(t.lang); - s.git = `${labelOf(t.file)} ${this.vsOf(t)}`; + s.git = `${labelOf(t.file)} ${t.since ? `since ${t.since}` : this.vsOf(t)}`; s.diff = t.diff.stats; s.encoding = t.encoding; if (t.commit) s.size = undefined; // both sides come from git; the working file's size is beside the point @@ -759,9 +796,10 @@ export class Editor { t.tabEl.className = cls.join(" "); t.tabEl.title = showHidden(t.commit ? `${t.path} · ${t.commit.oid.slice(0, 7)} ${t.commit.subject} (${t.commit.author}, ${relativeTime(t.commit.time)})` + : t.since ? `${t.path} · ${g?.label} · changed since ${t.since}` : g ? `${t.path} · ${g.label}${t.isDiff ? ` · diff ${this.diffContext.vs}` : ""}` : t.path); t.tabEl.setAttribute("aria-selected", String(t === this.active)); - t.tabEl.innerHTML = `${fileIcon(t.path)}${esc(showHidden(basename(t.path)))}${hint(t.path) ? `${esc(showHidden(hint(t.path)))}` : ""}${t.commit ? `${t.commit.oid.slice(0, 7)}` : t.isDiff ? 'diff' : ""}`; + t.tabEl.innerHTML = `${fileIcon(t.path)}${esc(showHidden(basename(t.path)))}${hint(t.path) ? `${esc(showHidden(hint(t.path)))}` : ""}${t.since ? `since ${esc(showHidden(t.since))}` : t.commit ? `${t.commit.oid.slice(0, 7)}` : t.isDiff ? 'diff' : ""}`; t.tabEl.querySelector(".tab-close")!.addEventListener("click", (e) => { e.stopPropagation(); this.close(t); @@ -782,6 +820,7 @@ export class Editor { }); const c = this.active.commit; if (c) this.crumbs.append(h("span", { class: "pill", title: `${c.oid}\n${showHidden(c.author)}, ${relativeTime(c.time)}` }, `${c.oid.slice(0, 7)} · ${showHidden(c.subject)}`)); + else if (this.active.since) this.crumbs.append(h("span", { class: "pill" }, `since ${showHidden(this.active.since)}`)); else if (this.active.isDiff) this.crumbs.append(h("span", { class: "pill" }, `diff ${this.diffContext.vs}`)); if (this.active.truncated) this.crumbs.append(h("span", { class: "pill warn" }, `showing first ${formatBytes(8 * 1024 * 1024)}`)); if (this.active.deleted) this.crumbs.append(h("span", { class: "pill warn" }, "deleted on disk")); diff --git a/test/client.checkpointctl.test.ts b/test/client.checkpointctl.test.ts new file mode 100644 index 0000000..52f9576 --- /dev/null +++ b/test/client.checkpointctl.test.ts @@ -0,0 +1,451 @@ +import { describe, expect, test } from "bun:test"; +import type { ChangeFile } from "../src/client/api.ts"; +import type { PickItem } from "../src/client/palette.ts"; +import { CHECKPOINT_COMMANDS, CheckpointController, hhmm, type CheckpointDeps, type SinceDiff } from "../src/client/checkpointctl.ts"; + +const T0705 = new Date(2026, 10, 9, 7, 5).getTime(); + +const file = (path: string, status: ChangeFile["status"] = "M", extra: Partial = {}): ChangeFile => ({ + path, + status, + added: null, + removed: null, + binary: false, + ...extra, +}); + +interface World { + rev: string | null; + repo: boolean; + files: ChangeFile[]; + /** Current texts; a missing key reads as undefined (unreadable). */ + texts: Record; + /** Texts at a revision, keyed `${rev}:${path}`; a missing key reads as undefined. */ + base: Record; + clock: number; +} + +function setup(init: Partial = {}) { + const w: World = { rev: "R", repo: true, files: [], texts: {}, base: {}, clock: T0705, ...init }; + const log = { + changes: 0, + now: 0, + reads: [] as string[], + bases: [] as [string, string][], + toasts: [] as string[], + picks: [] as { items: PickItem[]; title: string }[], + opened: [] as SinceDiff[], + }; + let changesImpl: () => Promise<{ repo: boolean; rev: string | null; files: ChangeFile[] }> = async () => ({ repo: w.repo, rev: w.rev, files: w.files }); + const deps: CheckpointDeps = { + changes: () => { + log.changes++; + return changesImpl(); + }, + readText: async (p) => { + log.reads.push(p); + return p in w.texts ? w.texts[p] : undefined; + }, + baseText: async (p, rev) => { + log.bases.push([p, rev]); + const k = `${rev}:${p}`; + return k in w.base ? w.base[k] : undefined; + }, + now: () => { + log.now++; + return w.clock; + }, + notify: (m) => void log.toasts.push(m), + pick: (items, title) => void log.picks.push({ items, title }), + openDiff: (d) => void log.opened.push(d), + }; + const ctl = new CheckpointController(deps); + return { ctl, w, log, setChanges: (f: typeof changesImpl) => (changesImpl = f) }; +} + +const entries = (ctl: CheckpointController) => [...(ctl.checkpoint?.texts.entries() ?? [])]; + +describe("AC-7 command table and hhmm", () => { + test("the three commands, in order", () => { + expect(CHECKPOINT_COMMANDS.map((c) => ({ ...c }))).toEqual([ + { id: "changes.checkpoint", title: "Changes: Set Checkpoint" }, + { id: "changes.sinceCheckpoint", title: "Changes: Since Checkpoint\u2026" }, + { id: "changes.clearCheckpoint", title: "Changes: Clear Checkpoint" }, + ]); + }); + test("hhmm is local, 24-hour and zero padded", () => { + expect(hhmm(new Date(2026, 10, 9, 7, 5).getTime())).toBe("07:05"); + expect(hhmm(new Date(2026, 10, 9, 23, 59).getTime())).toBe("23:59"); + expect(hhmm(new Date(2026, 10, 9, 0, 0).getTime())).toBe("00:00"); + }); +}); + +describe("AC-8 set", () => { + test("worked example: renames, deletes, binary and unreadable files", async () => { + const { ctl, log } = setup({ + files: [ + file("a.ts"), + file("img.png", "M", { binary: true }), + file("gone.ts", "D"), + file("new.ts", "R", { oldPath: "old.ts" }), + file("big.ts"), + ], + texts: { "a.ts": "x\n", "new.ts": "n\n" }, + }); + await ctl.set(); + expect(entries(ctl)).toEqual([["a.ts", "x\n"], ["gone.ts", null], ["old.ts", null], ["new.ts", "n\n"]]); + expect(ctl.checkpoint?.skipped).toEqual(["img.png", "big.ts"]); + expect(ctl.checkpoint?.rev).toBe("R"); + expect(ctl.checkpoint?.at).toBe(T0705); + expect(log.toasts).toEqual(["Checkpoint set (4 files, 2 skipped)"]); + }); + + test("calls changes and now once; never reads a binary or deleted file", async () => { + const { ctl, log } = setup({ + files: [file("a.ts"), file("b.bin", "M", { binary: true }), file("c.ts", "D"), file("u.ts", "?")], + texts: { "a.ts": "a", "u.ts": "u" }, + }); + await ctl.set(); + expect(log.changes).toBe(1); + expect(log.now).toBe(1); + expect(log.reads.sort()).toEqual(["a.ts", "u.ts"]); + }); + + test("a rename whose old path is itself listed does not add the old path twice", async () => { + const { ctl } = setup({ + files: [file("old.ts", "D"), file("new.ts", "R", { oldPath: "old.ts" })], + texts: { "new.ts": "n" }, + }); + await ctl.set(); + expect(entries(ctl)).toEqual([["old.ts", null], ["new.ts", "n"]]); + }); + + test("toast forms: no plural, empty listing, nothing skipped", async () => { + const one = setup({ files: [file("a.ts")], texts: { "a.ts": "x" } }); + await one.ctl.set(); + expect(one.log.toasts).toEqual(["Checkpoint set (1 files)"]); + const none = setup({ files: [] }); + await none.ctl.set(); + expect(none.log.toasts).toEqual(["Checkpoint set (0 files)"]); + expect(none.ctl.checkpoint?.texts.size).toBe(0); + }); + + test("a second set replaces the first", async () => { + const { ctl, w } = setup({ files: [file("a.ts")], texts: { "a.ts": "1", "b.ts": "2" } }); + await ctl.set(); + const first = ctl.checkpoint; + expect(first).not.toBeNull(); + w.files = [file("b.ts")]; + await ctl.set(); + expect(ctl.checkpoint).not.toBe(first); + expect(entries(ctl)).toEqual([["b.ts", "2"]]); + }); + + test("a failing listing keeps the old checkpoint and says so", async () => { + const { ctl, log, setChanges } = setup({ files: [file("a.ts")], texts: { "a.ts": "1" } }); + await ctl.set(); + const cp = ctl.checkpoint; + expect(cp).not.toBeNull(); + setChanges(async () => { + throw new Error("boom"); + }); + await ctl.set(); + expect(ctl.checkpoint).toBe(cp); + expect(log.toasts.at(-1)).toBe("Could not read the changes"); + }); + + test("not a repository keeps the old checkpoint and says so", async () => { + const { ctl, log, w } = setup({ files: [file("a.ts")], texts: { "a.ts": "1" } }); + await ctl.set(); + const cp = ctl.checkpoint; + w.repo = false; + await ctl.set(); + expect(ctl.checkpoint).toBe(cp); + expect(log.toasts.at(-1)).toBe("Not a git repository"); + }); + + test("a set superseded by a later set drops its result silently", async () => { + const { ctl, log, setChanges, w } = setup({ texts: { "slow.ts": "s", "fast.ts": "f" } }); + let release: (v: { repo: boolean; rev: string | null; files: ChangeFile[] }) => void = () => {}; + setChanges(() => new Promise((r) => (release = r))); + const first = ctl.set(); + setChanges(async () => ({ repo: true, rev: w.rev, files: [file("fast.ts")] })); + await ctl.set(); + release({ repo: true, rev: "R", files: [file("slow.ts")] }); + await first; + expect(entries(ctl)).toEqual([["fast.ts", "f"]]); + expect(log.toasts).toEqual(["Checkpoint set (1 files)"]); + }); + + test("the cap-skip path works end to end: 501 files give 500 stored and 1 skipped", async () => { + const files: ChangeFile[] = []; + const texts: Record = {}; + for (let i = 0; i <= 500; i++) { + files.push(file(`f${i}`)); + texts[`f${i}`] = "x"; + } + const { ctl, log } = setup({ files, texts }); + await ctl.set(); + expect(ctl.checkpoint?.texts.size).toBe(500); + expect(ctl.checkpoint?.skipped).toEqual(["f500"]); + expect(log.toasts).toEqual(["Checkpoint set (500 files, 1 skipped)"]); + }); + + test("cap-skipped paths come before unreadable ones", async () => { + const files: ChangeFile[] = [file("unreadable.ts")]; + const texts: Record = {}; + for (let i = 0; i <= 500; i++) { + files.push(file(`f${i}`)); + texts[`f${i}`] = "x"; + } + const { ctl } = setup({ files, texts }); + await ctl.set(); + expect(ctl.checkpoint?.skipped).toEqual(["f500", "unreadable.ts"]); + }); +}); + +describe("AC-9 no checkpoint and clear", () => { + test("since with no checkpoint only toasts", async () => { + const { ctl, log } = setup({ files: [file("a.ts")], texts: { "a.ts": "x" } }); + await ctl.since(); + expect(log.toasts).toEqual(["No checkpoint set"]); + expect(log.changes).toBe(0); + expect(log.reads).toEqual([]); + expect(log.bases).toEqual([]); + expect(log.picks).toEqual([]); + expect(log.opened).toEqual([]); + }); + + test("clear forgets the checkpoint and toasts, also when none was set", async () => { + const { ctl, log } = setup({ files: [file("a.ts")], texts: { "a.ts": "x" } }); + ctl.clear(); + expect(log.toasts).toEqual(["Checkpoint cleared"]); + await ctl.set(); + expect(ctl.checkpoint).not.toBeNull(); + ctl.clear(); + expect(ctl.checkpoint).toBeNull(); + expect(log.toasts.at(-1)).toBe("Checkpoint cleared"); + await ctl.since(); + expect(log.toasts.at(-1)).toBe("No checkpoint set"); + }); +}); + +describe("AC-10 since with nothing", () => { + test("nothing changed since the checkpoint", async () => { + const { ctl, log } = setup({ files: [file("a.ts")], texts: { "a.ts": "x\n" } }); + await ctl.set(); + await ctl.since(); + expect(log.toasts.at(-1)).toBe("Nothing changed since the checkpoint (07:05)"); + expect(log.picks).toEqual([]); + }); + + test("a failing listing or a non repository keeps the checkpoint", async () => { + const { ctl, log, setChanges, w } = setup({ files: [file("a.ts")], texts: { "a.ts": "x\n" } }); + await ctl.set(); + const cp = ctl.checkpoint; + expect(cp).not.toBeNull(); + setChanges(async () => { + throw new Error("boom"); + }); + await ctl.since(); + expect(log.toasts.at(-1)).toBe("Could not read the changes"); + expect(ctl.checkpoint).toBe(cp); + setChanges(async () => ({ repo: false, rev: w.rev, files: [] })); + await ctl.since(); + expect(log.toasts.at(-1)).toBe("Not a git repository"); + expect(ctl.checkpoint).toBe(cp); + expect(log.picks).toEqual([]); + }); +}); + +/** The worked example of AC-12: a checkpoint at 07:05 holding a.ts, then a.ts edited and b.ts added. */ +async function worked() { + const s = setup({ files: [file("a.ts")], texts: { "a.ts": "x\n" } }); + await s.ctl.set(); + s.w.rev = "R2"; + s.w.files = [file("a.ts"), file("b.ts", "?")]; + s.w.texts = { "a.ts": "x\ny\n", "b.ts": "new\n" }; + s.w.base = { "R:b.ts": null }; + s.log.toasts.length = 0; + s.log.reads.length = 0; + s.log.bases.length = 0; + return s; +} + +describe("AC-11 pick list", () => { + test("title, labels, details and badges", async () => { + const { ctl, log } = await worked(); + await ctl.since(); + expect(log.picks.length).toBe(1); + const { items, title } = log.picks[0]!; + expect(title).toBe("Changes since 07:05: 2 files, type to filter"); + expect(items.map((i) => i.label)).toEqual(["a.ts", "b.ts"]); + expect(items.map((i) => i.detail)).toEqual(["+1 \u22120", "+1 \u22120"]); + expect(items.map((i) => i.icon)).toEqual([ + '', + '', + ]); + }); + + test("one entry says file, not files; +0 and -0 are shown", async () => { + const w2 = setup({ files: [file("t.ts")], texts: { "t.ts": "x" } }); + await w2.ctl.set(); + w2.w.texts = { "t.ts": "x\n" }; + await w2.ctl.since(); + const pick = w2.log.picks[0]; + expect(pick?.title).toBe("Changes since 07:05: 1 file, type to filter"); + expect(pick?.items[0]?.detail).toBe("+0 \u22120"); + }); + + test("raw path text only reaches the label, with hidden characters made visible", async () => { + const evil = ".ts"; + const bidi = "a\u202eb.ts"; + const s = setup({ files: [file(evil, "?"), file(bidi, "?")], texts: { [evil]: "1\n", [bidi]: "2\n" } }); + await s.ctl.set(); + s.w.texts = { [evil]: "1\n2\n", [bidi]: "2\n3\n" }; + await s.ctl.since(); + const items = s.log.picks[0]?.items ?? []; + expect(items.map((i) => i.label).sort()).toEqual([evil, "a\\u202eb.ts"].sort()); + for (const i of items) { + expect(i.icon).toMatch(/^