f6d0679 · 827b4fe
@@ -8,7 +8,14 @@
import type { Diff } from "../git/diff.ts";
import { blobForRender } from "../git/read.ts";
import { detectLanguage, highlightLines } from "./highlight.ts";
import {
  INITIAL_SCAN,
  detectLanguage,
  highlightLines,
  scanOneLine,
  type ScanState,
  type Token,
} from "./highlight.ts";
import {
  MAX_DIFF_FILES,
  stubFor,
@@ -53,3 +60,64 @@
export async function diffSyntax(
  }
  return map;
}
/**
 * The SyntaxMap for just the lines a diff draws: each hunk's old side
 * (context and deletions) and new side (context and additions) is tokenised
 * from the diff itself, in order, and placed at its line numbers in a sparse
 * array. No blob is read and nothing outside a hunk is scanned, so the cost
 * follows the size of the change rather than of the files.
 *
 * The trade: a string or comment opened above a hunk starts the hunk in the
 * plain state, so its first lines may colour differently from a whole-file
 * pass. `diffSyntax` keeps that exactness for the pull page.
 */
export function hunkSyntax(diff: Diff): SyntaxMap {
  const map: SyntaxMap = new Map();
  for (const file of diff.files.slice(0, MAX_DIFF_FILES)) {
    if (file.binary || stubFor(file) !== null) continue;
    const entry: FileTokens = {};
    const oldLang = file.oldPath ? detectLanguage(file.oldPath) : null;
    const newLang = file.newPath ? detectLanguage(file.newPath) : null;
    for (const hunk of file.hunks) {
      let oldState = INITIAL_SCAN;
      let newState = INITIAL_SCAN;
      for (const line of hunk.lines) {
        let oldTokens: Token[] | undefined;
        let newTokens: Token[] | undefined;
        // A context line both sides scan from the plain state, in one
        // language, tokenises the same on both: scan it once.
        const shared =
          line.kind === "context" &&
          oldLang === newLang &&
          isPlain(oldState) &&
          isPlain(newState);
        if (line.oldNumber !== null && oldLang) {
          const scanned = scanOneLine(line.content, oldLang, oldState);
      
        junipercrew
        19d ago
        
      
      Context lines are shared only when both states are plain. Add a test where a context line opens a block comment, so the newState = oldState branch is covered.
    
          oldTokens = scanned.tokens;
          oldState = scanned.state;
          if (shared) newTokens = oldTokens;
        }
        if (line.newNumber !== null && newLang) {
          if (newTokens) {
            newState = oldState;
          } else {
            const scanned = scanOneLine(line.content, newLang, newState);
            newTokens = scanned.tokens;
            newState = scanned.state;
          }
        }
        if (oldTokens) (entry.old ??= [])[line.oldNumber! - 1] = oldTokens;
        if (newTokens) (entry.new ??= [])[line.newNumber! - 1] = newTokens;
      }
    }
    map.set(file.path, entry);
  }
  return map;
}
function isPlain(s: ScanState): boolean {
  return (
    !s.block && !s.str && !s.heredoc && !s.embed && !s.scalar && !s.tag && s.depth === 0
  );
}
@@ -18,7 +18,7 @@
import { notePhase, takeSessionTime, timePhase, timePhaseAsync } from "../../cor
import type { Ctx } from "../../http/context.ts";
import type { Commit } from "../../view/source.ts";
import { loadRepo, notFoundPage, scopeHeader } from "../chrome.ts";
import { diffSyntax } from "../diff-syntax.ts";
import { hunkSyntax } from "../diff-syntax.ts";
import { renderDiff, type SyntaxMap } from "../diff-view.ts";
import { html, when, type Html } from "../html.ts";
import { page } from "../layout.ts";
@@ -269,11 +269,8 @@
export async function commitPage(ctx: Ctx): Promise<Response> {
  const diff = diffResult.ok
    ? diffResult.value
    : { files: [], additions: 0, deletions: 0 };
  // Old side is the first parent (or the empty tree for a root commit, where
  // every file is an addition and no old blob is read).
  const parent = commit.parents[0] ?? commit.oid;
  const highlighting = performance.now();
  const syntax = await diffSyntax(dir, diff, parent, commit.oid);
  const syntax = hunkSyntax(diff);
  notePhase(ctx.request, "highlight", performance.now() - highlighting);
  const rendering = performance.now();
@@ -0,0 +1,77 @@
/** A commit page highlights the lines its hunks draw, not whole files (#6183). */
import { expect, test } from "bun:test";
      
        junipercrew
        19d ago
        
      
      The header cites #6183, but this is #6187. Point it at the right issue.
    
import { parseUnifiedDiff } from "../src/git/diff.ts";
import { hunkSyntax } from "../src/web/diff-syntax.ts";
import { detectLanguage, highlightLines } from "../src/web/highlight.ts";
const OLD = ['const a = "x";', "// note", "let b = 1;", "return a;"];
const NEW = ['const a = "x";', "// note", "let b = 2; // two", "return a;"];
function patch(path: string, oldLines: string[], newLines: string[]): string {
  return [
    `diff --git a/${path} b/${path}`,
    "index 1111111..2222222 100644",
    `--- a/${path}`,
    `+++ b/${path}`,
    `@@ -1,${oldLines.length} +1,${newLines.length} @@`,
    " " + oldLines[0],
    " " + oldLines[1],
    "-" + oldLines[2],
    "+" + newLines[2],
    " " + oldLines[3],
    "",
  ].join("\n");
}
test("a hunk's tokens equal the whole-file tokens at its line numbers", () => {
  const diff = parseUnifiedDiff(patch("a.ts", OLD, NEW));
  const got = hunkSyntax(diff).get("a.ts")!;
  const lang = detectLanguage("a.ts");
  expect(got.old).toEqual(highlightLines(OLD.join("\n"), lang));
  expect(got.new).toEqual(highlightLines(NEW.join("\n"), lang));
});
test("only the hunk's lines are tokenised", () => {
  const text = patch("a.ts", OLD, NEW).replace("@@ -1,4 +1,4 @@", "@@ -101,4 +101,4 @@");
  const got = hunkSyntax(parseUnifiedDiff(text)).get("a.ts")!;
  expect(got.new![0]).toBeUndefined();
  expect(got.new![100]).toBeDefined();
  expect(got.new![103]).toBeDefined();
});
test("20 files of 800 lines: only the 7-line hunks are tokenised", () => {
  const body = Array.from({ length: 800 }, (_, i) => `const v${i} = foo("s${i}", ${i}); // c`);
  const parts: string[] = [];
  for (let f = 0; f < 20; f++) {
    const path = `f${f}.ts`;
    const lines: string[] = [];
    // Ten edits 80 lines apart, each with the usual three lines of context.
    for (let e = 0; e < 10; e++) {
      const at = e * 80 + 40;
      lines.push(
        `@@ -${at - 2},7 +${at - 2},7 @@`,
        ...body.slice(at - 3, at).map((l) => " " + l),
        "-" + body[at],
        "+" + body[at] + "!",
        ...body.slice(at + 1, at + 4).map((l) => " " + l),
      );
    }
    parts.push(
      [`diff --git a/${path} b/${path}`, "index 1111111..2222222 100644", `--- a/${path}`, `+++ b/${path}`, ...lines, ""].join("\n"),
    );
  }
  const diff = parseUnifiedDiff(parts.join("\n"));
  expect(diff.files.length).toBe(20);
  const map = hunkSyntax(diff);
  expect(map.size).toBe(20);
  // Work done, not wall time: one token row per drawn line per side. Each
  // hunk draws 7 old and 7 new lines (6 shared context + 1 changed each),
  // so 20 files x 10 hunks x 7 per side — not the 800 lines of each file.
  let rows = 0;
  for (const entry of map.values()) {
    for (const side of [entry.old!, entry.new!]) {
      rows += Object.keys(side).length;
    }
  }
      
        junipercrew
        19d ago
        outdated
      
      Wall-clock assertion: flaky on shared CI (author saw 10–23 ms on a single run). Assert the amount of work instead, such as lines tokenised or scan calls, which fails without the change and never flakes.
    
  expect(rows).toBe(20 * 10 * 7 * 2);
});
		@@ -8,7 +8,14 @@
8	8
9	9	import type { Diff } from "../git/diff.ts";
10	10	import { blobForRender } from "../git/read.ts";
11		import { detectLanguage, highlightLines } from "./highlight.ts";
	11	import {
	12	INITIAL_SCAN,
	13	detectLanguage,
	14	highlightLines,
	15	scanOneLine,
	16	type ScanState,
	17	type Token,
	18	} from "./highlight.ts";
12	19	import {
13	20	MAX_DIFF_FILES,
14	21	stubFor,
		@@ -53,3 +60,64 @@ export async function diffSyntax(
53	60	}
54	61	return map;
55	62	}
	63
	64	/**
	65	* The SyntaxMap for just the lines a diff draws: each hunk's old side
	66	* (context and deletions) and new side (context and additions) is tokenised
	67	* from the diff itself, in order, and placed at its line numbers in a sparse
	68	* array. No blob is read and nothing outside a hunk is scanned, so the cost
	69	* follows the size of the change rather than of the files.
	70	*
	71	* The trade: a string or comment opened above a hunk starts the hunk in the
	72	* plain state, so its first lines may colour differently from a whole-file
	73	* pass. `diffSyntax` keeps that exactness for the pull page.
	74	*/
	75	export function hunkSyntax(diff: Diff): SyntaxMap {
	76	const map: SyntaxMap = new Map();
	77	for (const file of diff.files.slice(0, MAX_DIFF_FILES)) {
	78	if (file.binary \|\| stubFor(file) !== null) continue;
	79	const entry: FileTokens = {};
	80	const oldLang = file.oldPath ? detectLanguage(file.oldPath) : null;
	81	const newLang = file.newPath ? detectLanguage(file.newPath) : null;
	82	for (const hunk of file.hunks) {
	83	let oldState = INITIAL_SCAN;
	84	let newState = INITIAL_SCAN;
	85	for (const line of hunk.lines) {
	86	let oldTokens: Token[] \| undefined;
	87	let newTokens: Token[] \| undefined;
	88	// A context line both sides scan from the plain state, in one
	89	// language, tokenises the same on both: scan it once.
	90	const shared =
	91	line.kind === "context" &&
	92	oldLang === newLang &&
	93	isPlain(oldState) &&
	94	isPlain(newState);
	95	if (line.oldNumber !== null && oldLang) {
	96	const scanned = scanOneLine(line.content, oldLang, oldState);
		junipercrew 19d ago Context lines are shared only when both states are plain. Add a test where a context line opens a block comment, so the `newState = oldState` branch is covered.
	97	oldTokens = scanned.tokens;
	98	oldState = scanned.state;
	99	if (shared) newTokens = oldTokens;
	100	}
	101	if (line.newNumber !== null && newLang) {
	102	if (newTokens) {
	103	newState = oldState;
	104	} else {
	105	const scanned = scanOneLine(line.content, newLang, newState);
	106	newTokens = scanned.tokens;
	107	newState = scanned.state;
	108	}
	109	}
	110	if (oldTokens) (entry.old ??= [])[line.oldNumber! - 1] = oldTokens;
	111	if (newTokens) (entry.new ??= [])[line.newNumber! - 1] = newTokens;
	112	}
	113	}
	114	map.set(file.path, entry);
	115	}
	116	return map;
	117	}
	118
	119	function isPlain(s: ScanState): boolean {
	120	return (
	121	!s.block && !s.str && !s.heredoc && !s.embed && !s.scalar && !s.tag && s.depth === 0
	122	);
	123	}
		@@ -18,7 +18,7 @@ import { notePhase, takeSessionTime, timePhase, timePhaseAsync } from "../../cor
18	18	import type { Ctx } from "../../http/context.ts";
19	19	import type { Commit } from "../../view/source.ts";
20	20	import { loadRepo, notFoundPage, scopeHeader } from "../chrome.ts";
21		import { diffSyntax } from "../diff-syntax.ts";
	21	import { hunkSyntax } from "../diff-syntax.ts";
22	22	import { renderDiff, type SyntaxMap } from "../diff-view.ts";
23	23	import { html, when, type Html } from "../html.ts";
24	24	import { page } from "../layout.ts";
		@@ -269,11 +269,8 @@ export async function commitPage(ctx: Ctx): Promise<Response> {
269	269	const diff = diffResult.ok
270	270	? diffResult.value
271	271	: { files: [], additions: 0, deletions: 0 };
272		// Old side is the first parent (or the empty tree for a root commit, where
273		// every file is an addition and no old blob is read).
274		const parent = commit.parents[0] ?? commit.oid;
275	272	const highlighting = performance.now();
276		const syntax = await diffSyntax(dir, diff, parent, commit.oid);
	273	const syntax = hunkSyntax(diff);
277	274	notePhase(ctx.request, "highlight", performance.now() - highlighting);
278	275
279	276	const rendering = performance.now();
		@@ -0,0 +1,77 @@
	1	/** A commit page highlights the lines its hunks draw, not whole files (#6183). */
	2	import { expect, test } from "bun:test";
		junipercrew 19d ago The header cites #6183, but this is #6187. Point it at the right issue.
	3	import { parseUnifiedDiff } from "../src/git/diff.ts";
	4	import { hunkSyntax } from "../src/web/diff-syntax.ts";
	5	import { detectLanguage, highlightLines } from "../src/web/highlight.ts";
	6
	7	const OLD = ['const a = "x";', "// note", "let b = 1;", "return a;"];
	8	const NEW = ['const a = "x";', "// note", "let b = 2; // two", "return a;"];
	9
	10	function patch(path: string, oldLines: string[], newLines: string[]): string {
	11	return [
	12	`diff --git a/${path} b/${path}`,
	13	"index 1111111..2222222 100644",
	14	`--- a/${path}`,
	15	`+++ b/${path}`,
	16	`@@ -1,${oldLines.length} +1,${newLines.length} @@`,
	17	" " + oldLines[0],
	18	" " + oldLines[1],
	19	"-" + oldLines[2],
	20	"+" + newLines[2],
	21	" " + oldLines[3],
	22	"",
	23	].join("\n");
	24	}
	25
	26	test("a hunk's tokens equal the whole-file tokens at its line numbers", () => {
	27	const diff = parseUnifiedDiff(patch("a.ts", OLD, NEW));
	28	const got = hunkSyntax(diff).get("a.ts")!;
	29	const lang = detectLanguage("a.ts");
	30	expect(got.old).toEqual(highlightLines(OLD.join("\n"), lang));
	31	expect(got.new).toEqual(highlightLines(NEW.join("\n"), lang));
	32	});
	33
	34	test("only the hunk's lines are tokenised", () => {
	35	const text = patch("a.ts", OLD, NEW).replace("@@ -1,4 +1,4 @@", "@@ -101,4 +101,4 @@");
	36	const got = hunkSyntax(parseUnifiedDiff(text)).get("a.ts")!;
	37	expect(got.new![0]).toBeUndefined();
	38	expect(got.new![100]).toBeDefined();
	39	expect(got.new![103]).toBeDefined();
	40	});
	41
	42	test("20 files of 800 lines: only the 7-line hunks are tokenised", () => {
	43	const body = Array.from({ length: 800 }, (_, i) => `const v${i} = foo("s${i}", ${i}); // c`);
	44	const parts: string[] = [];
	45	for (let f = 0; f < 20; f++) {
	46	const path = `f${f}.ts`;
	47	const lines: string[] = [];
	48	// Ten edits 80 lines apart, each with the usual three lines of context.
	49	for (let e = 0; e < 10; e++) {
	50	const at = e * 80 + 40;
	51	lines.push(
	52	`@@ -${at - 2},7 +${at - 2},7 @@`,
	53	...body.slice(at - 3, at).map((l) => " " + l),
	54	"-" + body[at],
	55	"+" + body[at] + "!",
	56	...body.slice(at + 1, at + 4).map((l) => " " + l),
	57	);
	58	}
	59	parts.push(
	60	[`diff --git a/${path} b/${path}`, "index 1111111..2222222 100644", `--- a/${path}`, `+++ b/${path}`, ...lines, ""].join("\n"),
	61	);
	62	}
	63	const diff = parseUnifiedDiff(parts.join("\n"));
	64	expect(diff.files.length).toBe(20);
	65	const map = hunkSyntax(diff);
	66	expect(map.size).toBe(20);
	67	// Work done, not wall time: one token row per drawn line per side. Each
	68	// hunk draws 7 old and 7 new lines (6 shared context + 1 changed each),
	69	// so 20 files x 10 hunks x 7 per side — not the 800 lines of each file.
	70	let rows = 0;
	71	for (const entry of map.values()) {
	72	for (const side of [entry.old!, entry.new!]) {
	73	rows += Object.keys(side).length;
	74	}
	75	}
		junipercrew 19d ago outdated Wall-clock assertion: flaky on shared CI (author saw 10–23 ms on a single run). Assert the amount of work instead, such as lines tokenised or scan calls, which fails without the change and never flakes.
	76	expect(rows).toBe(20 * 10 * 7 * 2);
	77	});