17fbcbb fix(diff): cap one-file loads at 50,000 changed lines and harden the web splice · ermite

M docs/merge-requests.md +1 -1


@@ -67,7 +67,7 @@ In the web merge box a mode that is not possible is disabled with its reason, an
67	67
68	68		The merge request, commit and compare pages list every changed file. Generated files (lockfiles, `vendor/`, `node_modules/`, minified or generated code) and large files (over 2,000 changed lines) are collapsed: the header and the counts show, the lines do not. "load diff" fetches that one file in place, in full, and you can comment on its lines. Without scripts the link reloads the page with that file loaded. After the first 25 files of a merge request, the other files load as you scroll.
69	69
70		-	A file over 8 MiB of text cannot be shown in the browser: the page links to its raw blob. A commit page lists at most 300 files; clone the repository to read the rest.
	70	+	A commit page reads the hunks of its first 300 files with the page; the other files are listed collapsed and load one at a time. A file that cannot be shown in the browser, over 8 MiB of text or over 50,000 changed lines when loaded alone, has no "load diff": the page links to its raw blob instead.
71	71
72	72		## Notes
73	73


@@ -67,7 +67,7 @@ In the web merge box a mode that is not possible is disabled with its reason, an
67			67
68		The merge request, commit and compare pages list every changed file. Generated files (lockfiles, `vendor/`, `node_modules/`, minified or generated code) and large files (over 2,000 changed lines) are collapsed: the header and the counts show, the lines do not. "load diff" fetches that one file in place, in full, and you can comment on its lines. Without scripts the link reloads the page with that file loaded. After the first 25 files of a merge request, the other files load as you scroll.	68		The merge request, commit and compare pages list every changed file. Generated files (lockfiles, `vendor/`, `node_modules/`, minified or generated code) and large files (over 2,000 changed lines) are collapsed: the header and the counts show, the lines do not. "load diff" fetches that one file in place, in full, and you can comment on its lines. Without scripts the link reloads the page with that file loaded. After the first 25 files of a merge request, the other files load as you scroll.
69			69
70	-	A file over 8 MiB of text cannot be shown in the browser: the page links to its raw blob. A commit page lists at most 300 files; clone the repository to read the rest.	70	+	A commit page reads the hunks of its first 300 files with the page; the other files are listed collapsed and load one at a time. A file that cannot be shown in the browser, over 8 MiB of text or over 50,000 changed lines when loaded alone, has no "load diff": the page links to its raw blob instead.
71			71
72		## Notes	72		## Notes
73			73

M internal/adapters/web/commit.go +7 -14


@@ -147,7 +147,7 @@ func (s Server) commit(ctx context.Context, r http.Request) (templ.Component,
147	147		p.Refs = append(p.Refs, "tag: "+t.ShortName)
148	148		}
149	149		}
150		-	p.Note = diffNote(res.Diff, len(p.Files), p.NFiles)
	150	+	p.Note = diffNote(res.Diff)
151	151		p.Checks = checksSummary(d.repo.Name, cid, s.checksFor(ctx, d.repo.Name, cid))
152	152
153	153		lines, invisible := diffLines(p.Files)

@@ -190,26 +190,19 @@ func commitBody(c domain.Commit) string {
190	190		}
191	191
192	192		// diffNote says what is missing from an incomplete diff, "" for a complete one:
193		-	// files past the limit of the listing, files too large for the browser at all.
194		-	// Collapsed files say so on their own line and load on their own.
195		-	func diffNote(d domain.Diff, shown, total int) string {
	193	+	// files too large for the browser at all. Collapsed files say so on their own
	194	+	// line and load on their own; every changed file is listed.
	195	+	func diffNote(d domain.Diff) string {
196	196		gone := 0
197	197		for _, f := range d.Files {
198	198		if f.Omitted == domain.OmitUnavailable {
199	199		gone++
200	200		}
201	201		}
202		-	var parts []string
203		-	if d.Truncated && shown < total {
204		-	parts = append(parts, plural(total-shown, "file is", "files are")+" not listed")
205		-	}
206		-	if gone > 0 {
207		-	parts = append(parts, plural(gone, "file is", "files are")+" too large to show")
208		-	}
209		-	if len(parts) == 0 {
	202	+	if gone == 0 {
210	203		return ""
211	204		}
212		-	return "this diff is incomplete: " + strings.Join(parts, ", ") + ". the totals above count everything; clone the repository to read the rest."
	205	+	return "this diff is incomplete: " + plural(gone, "file is", "files are") + " too large to show. the totals above count everything; clone the repository to read the rest."
213	206		}
214	207
215	208		// commitWindow is how many files of a commit get their hunks read with the

@@ -272,7 +265,7 @@ func (s Server) commitFiles(ctx context.Context, r http.Request) (templ.Compon
272	265		files := s.hl.files(ctx, raw)
273	266		files[0].ID = "f" + itoa(i+1)
274	267		contextGaps(files, raw, repoHref(d.name(), "context", cid))
275		-	collapseFiles(files, raw, commitLinks(d.name(), cid))
	268	+	files[0].Raw = rawLink(raw[0], commitLinks(d.name(), cid))
276	269		lines, _ := diffLines(files)
277	270		return MRFiles(MRFilesPage{Files: files, DiffView: fragmentView(r, lines)}), http.StatusOK, nil
278	271		}

 			p.Refs = append(p.Refs, "tag: "+t.ShortName)
 		}
 	}
-	p.Note = diffNote(res.Diff, len(p.Files), p.NFiles)
+	p.Note = diffNote(res.Diff)
 	p.Checks = checksSummary(d.repo.Name, cid, s.checksFor(ctx, d.repo.Name, cid))
 	lines, invisible := diffLines(p.Files)
 }
 // diffNote says what is missing from an incomplete diff, "" for a complete one:
-// files past the limit of the listing, files too large for the browser at all.
+// files too large for the browser at all. Collapsed files say so on their own
-// Collapsed files say so on their own line and load on their own.
+// line and load on their own; every changed file is listed.
-func diffNote(d domain.Diff, shown, total int) string {
+func diffNote(d domain.Diff) string {
 	gone := 0
 	for _, f := range d.Files {
 		if f.Omitted == domain.OmitUnavailable {
 			gone++
 		}
 	}
-	var parts []string
+	if gone == 0 {
-	if d.Truncated && shown < total {
-		parts = append(parts, plural(total-shown, "file is", "files are")+" not listed")
-	}
-	if gone > 0 {
-		parts = append(parts, plural(gone, "file is", "files are")+" too large to show")
-	}
-	if len(parts) == 0 {
 		return ""
 	}
-	return "this diff is incomplete: " + strings.Join(parts, ", ") + ". the totals above count everything; clone the repository to read the rest."
+	return "this diff is incomplete: " + plural(gone, "file is", "files are") + " too large to show. the totals above count everything; clone the repository to read the rest."
 }
 // commitWindow is how many files of a commit get their hunks read with the
 	files := s.hl.files(ctx, raw)
 	files[0].ID = "f" + itoa(i+1)
 	contextGaps(files, raw, repoHref(d.name(), "context", cid))
-	collapseFiles(files, raw, commitLinks(d.name(), cid))
+	files[0].Raw = rawLink(raw[0], commitLinks(d.name(), cid))
 	lines, _ := diffLines(files)
 	return MRFiles(MRFilesPage{Files: files, DiffView: fragmentView(r, lines)}), http.StatusOK, nil
 }

M internal/adapters/web/compare.go +1 -1


@@ -85,7 +85,7 @@ func (s Server) compare(ctx context.Context, r http.Request) (templ.Component,
85	85		contextGaps(p.Files, res.Diff.Files, repoHref(d.name(), "context", res.Commit.ID.String()))
86	86		collapseFiles(p.Files, res.Diff.Files, commitLinks(name, res.Commit.ID.String()))
87	87		p.NFiles, p.Adds, p.Dels = res.Stats.FilesChanged, res.Stats.Additions, res.Stats.Deletions
88		-	if n := diffNote(res.Diff, len(p.Files), p.NFiles); n != "" {
	88	+	if n := diffNote(res.Diff); n != "" {
89	89		p.Note = n
90	90		}
91	91		lines, invisible := diffLines(p.Files)


@@ -85,7 +85,7 @@ func (s Server) compare(ctx context.Context, r http.Request) (templ.Component,
85		contextGaps(p.Files, res.Diff.Files, repoHref(d.name(), "context", res.Commit.ID.String()))	85		contextGaps(p.Files, res.Diff.Files, repoHref(d.name(), "context", res.Commit.ID.String()))
86		collapseFiles(p.Files, res.Diff.Files, commitLinks(name, res.Commit.ID.String()))	86		collapseFiles(p.Files, res.Diff.Files, commitLinks(name, res.Commit.ID.String()))
87		p.NFiles, p.Adds, p.Dels = res.Stats.FilesChanged, res.Stats.Additions, res.Stats.Deletions	87		p.NFiles, p.Adds, p.Dels = res.Stats.FilesChanged, res.Stats.Additions, res.Stats.Deletions
88	-	if n := diffNote(res.Diff, len(p.Files), p.NFiles); n != "" {	88	+	if n := diffNote(res.Diff); n != "" {
89		p.Note = n	89		p.Note = n
90		}	90		}
91		lines, invisible := diffLines(p.Files)	91		lines, invisible := diffLines(p.Files)

M internal/adapters/web/diff.go +12 -3


@@ -424,7 +424,7 @@ func fileNote(f domain.FileDiff) string {
424	424		case f.Omitted == domain.OmitBudget:
425	425		parts = append(parts, "collapsed to keep the page light")
426	426		case f.Omitted == domain.OmitUnavailable:
427		-	parts = append(parts, "too large to show in the browser (over 8 MiB of text)")
	427	+	parts = append(parts, "too large to show in the browser")
428	428		}
429	429		if f.OldMode != 0 && f.NewMode != 0 && f.OldMode != f.NewMode {
430	430		parts = append(parts, fmt.Sprintf("mode %06o → %06o", f.OldMode, f.NewMode))

@@ -505,12 +505,21 @@ func collapseFiles(out []DiffFile, files []domain.FileDiff, l fileLinks) {
505	505		q = "?from=" + itoa(l.from) + "&file=" + itoa(i)
506	506		}
507	507		out[i].Href = q + "#" + out[i].ID
508		-	case f.Omitted == domain.OmitUnavailable && f.Status != domain.FileDeleted && l.rev != "":
509		-	out[i].Raw = repoHref(l.repo, "raw", l.rev, f.NewPath)
	508	+	default:
	509	+	out[i].Raw = rawLink(f, l)
510	510		}
511	511		}
512	512		}
513	513
	514	+	// rawLink is the address of the raw blob of a file too large to show, "" for
	515	+	// any other file.
	516	+	func rawLink(f domain.FileDiff, l fileLinks) string {
	517	+	if f.Omitted != domain.OmitUnavailable \|\| f.Status == domain.FileDeleted \|\| l.rev == "" {
	518	+	return ""
	519	+	}
	520	+	return repoHref(l.repo, "raw", l.rev, f.NewPath)
	521	+	}
	522	+
514	523		// fileParam reads ?file= of a page without scripts: the index of a collapsed
515	524		// file to read in full, in [0, n).
516	525		func fileParam(r *http.Request, n int) (int, bool) {

 	case f.Omitted == domain.OmitBudget:
 		parts = append(parts, "collapsed to keep the page light")
 	case f.Omitted == domain.OmitUnavailable:
-		parts = append(parts, "too large to show in the browser (over 8 MiB of text)")
+		parts = append(parts, "too large to show in the browser")
 	}
 	if f.OldMode != 0 && f.NewMode != 0 && f.OldMode != f.NewMode {
 		parts = append(parts, fmt.Sprintf("mode %06o → %06o", f.OldMode, f.NewMode))
 				q = "?from=" + itoa(l.from) + "&file=" + itoa(i)
 			}
 			out[i].Href = q + "#" + out[i].ID
-		case f.Omitted == domain.OmitUnavailable && f.Status != domain.FileDeleted && l.rev != "":
+		default:
-			out[i].Raw = repoHref(l.repo, "raw", l.rev, f.NewPath)
+			out[i].Raw = rawLink(f, l)
 		}
 	}
 }
+// rawLink is the address of the raw blob of a file too large to show, "" for
+// any other file.
+func rawLink(f domain.FileDiff, l fileLinks) string {
+	if f.Omitted != domain.OmitUnavailable || f.Status == domain.FileDeleted || l.rev == "" {
+		return ""
+	}
+	return repoHref(l.repo, "raw", l.rev, f.NewPath)
+}
 // fileParam reads ?file= of a page without scripts: the index of a collapsed
 // file to read in full, in [0, n).
 func fileParam(r *http.Request, n int) (int, bool) {

M internal/adapters/web/diff_test.go +5 -10


@@ -102,7 +102,7 @@ func TestFileNotes(t *testing.T) {
102	102		{"generated", domain.FileDiff{Omitted: domain.OmitGenerated, Additions: 5}, "generated file: collapsed", true},
103	103		{"large", domain.FileDiff{Omitted: domain.OmitLarge, Additions: 2000, Deletions: 500}, "large diff (2500 lines changed): collapsed", true},
104	104		{"budget", domain.FileDiff{Omitted: domain.OmitBudget, Additions: 40}, "collapsed to keep the page light", true},
105		-	{"unavailable", domain.FileDiff{Omitted: domain.OmitUnavailable, Additions: 40}, "too large to show in the browser (over 8 MiB of text)", true},
	105	+	{"unavailable", domain.FileDiff{Omitted: domain.OmitUnavailable, Additions: 40}, "too large to show in the browser", true},
106	106		}
107	107		for _, tt := range tests {
108	108		t.Run(tt.name, func(t *testing.T) {

@@ -204,15 +204,9 @@ func TestDiffNote(t *testing.T) {
204	204		}
205	205		return d
206	206		}
207		-	assert.Empty(t, diffNote(files(domain.OmitGenerated, domain.OmitLarge, domain.OmitBudget), 3, 3), "collapsed files have no global note")
208		-	d := files(domain.OmitNone)
209		-	d.Truncated = true
210		-	assert.Equal(t, "this diff is incomplete: 299 files are not listed. the totals above count everything; clone the repository to read the rest.", diffNote(d, 1, 300))
	207	+	assert.Empty(t, diffNote(files(domain.OmitGenerated, domain.OmitLarge, domain.OmitBudget)), "collapsed files have no global note")
211	208		assert.Equal(t, "this diff is incomplete: 2 files are too large to show. the totals above count everything; clone the repository to read the rest.",
212		-	diffNote(files(domain.OmitUnavailable, domain.OmitLarge, domain.OmitUnavailable), 3, 3))
213		-	d = files(domain.OmitUnavailable)
214		-	d.Truncated = true
215		-	assert.Contains(t, diffNote(d, 1, 5), "4 files are not listed, 1 file is too large to show")
	209	+	diffNote(files(domain.OmitUnavailable, domain.OmitLarge, domain.OmitUnavailable)))
216	210		}
217	211
218	212		func TestCollapsedFileMarkup(t *testing.T) {

@@ -224,7 +218,8 @@ func TestCollapsedFileMarkup(t *testing.T) {
224	218		fs := diffFiles(raw)
225	219		collapseFiles(fs, raw, fileLinks{src: src, repo: "demo", rev: "abc1234"})
226	220		out := renderToString(t, DiffFileView(fs[0], view))
227		-	assert.NotContains(t, out, " open", "omission %d", why)
	221	+	assert.NotRegexp(t, `<details[^>]*\sopen[\s>]`, out, "omission %d", why)
	222	+	assert.Regexp(t, `<details `, out, "the file is a details element")
228	223		assert.Contains(t, out, `<div class="dlazy" data-src="/demo/commit/abc1234/files?one=1&from=0" data-manual>`)
229	224		assert.Contains(t, out, `<a href="?file=0#f1">load diff</a>`)
230	225		assert.NotContains(t, out, "<table")

 		{"generated", domain.FileDiff{Omitted: domain.OmitGenerated, Additions: 5}, "generated file: collapsed", true},
 		{"large", domain.FileDiff{Omitted: domain.OmitLarge, Additions: 2000, Deletions: 500}, "large diff (2500 lines changed): collapsed", true},
 		{"budget", domain.FileDiff{Omitted: domain.OmitBudget, Additions: 40}, "collapsed to keep the page light", true},
-		{"unavailable", domain.FileDiff{Omitted: domain.OmitUnavailable, Additions: 40}, "too large to show in the browser (over 8 MiB of text)", true},
+		{"unavailable", domain.FileDiff{Omitted: domain.OmitUnavailable, Additions: 40}, "too large to show in the browser", true},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 		}
 		return d
 	}
-	assert.Empty(t, diffNote(files(domain.OmitGenerated, domain.OmitLarge, domain.OmitBudget), 3, 3), "collapsed files have no global note")
+	assert.Empty(t, diffNote(files(domain.OmitGenerated, domain.OmitLarge, domain.OmitBudget)), "collapsed files have no global note")
-	d := files(domain.OmitNone)
-	d.Truncated = true
-	assert.Equal(t, "this diff is incomplete: 299 files are not listed. the totals above count everything; clone the repository to read the rest.", diffNote(d, 1, 300))
 	assert.Equal(t, "this diff is incomplete: 2 files are too large to show. the totals above count everything; clone the repository to read the rest.",
-		diffNote(files(domain.OmitUnavailable, domain.OmitLarge, domain.OmitUnavailable), 3, 3))
+		diffNote(files(domain.OmitUnavailable, domain.OmitLarge, domain.OmitUnavailable)))
-	d = files(domain.OmitUnavailable)
-	d.Truncated = true
-	assert.Contains(t, diffNote(d, 1, 5), "4 files are not listed, 1 file is too large to show")
 }
 func TestCollapsedFileMarkup(t *testing.T) {
 		fs := diffFiles(raw)
 		collapseFiles(fs, raw, fileLinks{src: src, repo: "demo", rev: "abc1234"})
 		out := renderToString(t, DiffFileView(fs[0], view))
-		assert.NotContains(t, out, " open", "omission %d", why)
+		assert.NotRegexp(t, `<details[^>]*\sopen[\s>]`, out, "omission %d", why)
+		assert.Regexp(t, `<details `, out, "the file is a details element")
 		assert.Contains(t, out, `<div class="dlazy" data-src="/demo/commit/abc1234/files?one=1&amp;from=0" data-manual>`)
 		assert.Contains(t, out, `<a href="?file=0#f1">load diff</a>`)
 		assert.NotContains(t, out, "<table")

M internal/adapters/web/jstest/lazy.test.mjs +95 -4


@@ -17,18 +17,19 @@ function placeholder(src, manual) {
17	17		return { ph, link, details };
18	18		}
19	19
20		-	async function setup(phs, io = true) {
	20	+	async function setup(phs, io = true, opts = {}) {
21	21		const observed = [];
22	22		const urls = [];
23	23		if (io) globalThis.IntersectionObserver = class { observe(p) { observed.push(p); } unobserve() {} };
24	24		else delete globalThis.IntersectionObserver;
25		-	globalThis.fetch = async (u) => { urls.push(u); return { status: 200, ok: true, text: async () => '' }; };
	25	+	const reply = opts.reply \|\| { status: 200, ok: true, body: '' };
	26	+	globalThis.fetch = async (u) => { urls.push(u); return { status: reply.status, ok: reply.ok, text: async () => reply.body }; };
26	27		globalThis.document = {
27	28		readyState: 'complete',
28	29		querySelector: (s) => (s === '#diffs' ? { dataset: {} } : null),
29	30		querySelectorAll: (s) => (s.includes(':not([data-manual])') ? phs.filter((p) => !('manual' in p.dataset)) : phs),
30		-	getElementById: () => null,
31		-	createElement: () => ({ content: { querySelectorAll: () => [] } }),
	31	+	getElementById: opts.getElementById \|\| (() => null),
	32	+	createElement: () => ({ content: { querySelectorAll: () => opts.files \|\| [] } }),
32	33		addEventListener() {},
33	34		};
34	35		delete globalThis.ermiteLazy;

@@ -37,6 +38,7 @@ async function setup(phs, io = true) {
37	38		}
38	39		const settle = () => new Promise((r) => setTimeout(r, 0));
39	40		const restore = () => {
	41	+	delete globalThis.ermiteLazy;
40	42		for (const [k, v] of [['document', saved.document], ['fetch', saved.fetch], ['IntersectionObserver', saved.IO]]) {
41	43		if (v === undefined) delete globalThis[k]; else globalThis[k] = v;
42	44		}

@@ -96,3 +98,92 @@ test('a manual placeholder without an address is a plain link', async () => {
96	98		assert.equal(window.ermiteLazy.onClick({ target: a.link, button: 0, preventDefault() { assert.fail(); } }), false);
97	99		} finally { await settle(); restore(); }
98	100		});
	101	+
	102	+	// A file of the page (old) or of a fragment (new), with the queries swap makes.
	103	+	function fileEl(id, { lazy = false, manual = false, open = false } = {}) {
	104	+	const f = { id, open, queried: [] };
	105	+	f.querySelector = (sel) => {
	106	+	if (sel === '.dlazy[data-manual]') return manual ? {} : null;
	107	+	if (sel === '.dlazy') return lazy \|\| manual ? {} : null;
	108	+	if (sel === '.ln a') return f.line \|\| null;
	109	+	return null;
	110	+	};
	111	+	f.querySelectorAll = () => [];
	112	+	return f;
	113	+	}
	114	+
	115	+	test('a loaded manual file opens and takes the focus without scrolling', async () => {
	116	+	const a = placeholder('/f?one=1&from=0', true);
	117	+	const old = fileEl('f1', { manual: true });
	118	+	const fresh = fileEl('f1');
	119	+	const focused = [];
	120	+	fresh.line = { focus: (o) => focused.push(o) };
	121	+	let current = old;
	122	+	old.replaceWith = (n) => { current = n; };
	123	+	try {
	124	+	await setup([a.ph], true, { reply: { status: 200, ok: true, body: '<x>' }, files: [fresh], getElementById: () => current });
	125	+	window.ermiteLazy.onClick({ target: a.link, button: 0, preventDefault() {} });
	126	+	await settle(); await settle();
	127	+	assert.equal(current, fresh);
	128	+	assert.equal(fresh.open, true);
	129	+	assert.deepEqual(focused, [{ preventScroll: true }]);
	130	+	} finally { await settle(); restore(); }
	131	+	});
	132	+
	133	+	test('a window that still holds a collapsed file keeps it closed', async () => {
	134	+	const w = placeholder('/w?from=25', false);
	135	+	const old = fileEl('f26', { lazy: true, open: true });
	136	+	const fresh = fileEl('f26', { manual: true, open: true });
	137	+	let replaced = null;
	138	+	old.replaceWith = (n) => { replaced = n; };
	139	+	try {
	140	+	await setup([w.ph], true, { reply: { status: 200, ok: true, body: '<x>' }, files: [fresh], getElementById: () => old });
	141	+	await window.ermiteLazy.load(w.ph);
	142	+	assert.equal(replaced, fresh);
	143	+	assert.equal(fresh.open, false);
	144	+	} finally { await settle(); restore(); }
	145	+	});
	146	+
	147	+	test('a 409 makes the page stale: no later load fetches', async () => {
	148	+	const a = placeholder('/f?one=1&from=0', true);
	149	+	const b = placeholder('/f?one=1&from=1', true);
	150	+	try {
	151	+	const { urls } = await setup([a.ph, b.ph], true, { reply: { status: 409, ok: false, body: '<p>moved</p>' } });
	152	+	await window.ermiteLazy.load(a.ph);
	153	+	assert.equal(a.ph.innerHTML, '<p>moved</p>');
	154	+	assert.equal(urls.length, 1);
	155	+	await window.ermiteLazy.load(b.ph);
	156	+	assert.equal(urls.length, 1, 'stale: nothing is fetched');
	157	+	} finally { await settle(); restore(); }
	158	+	});
	159	+
	160	+	test('a failed load clears the loading mark and can be retried', async () => {
	161	+	const a = placeholder('/f?one=1&from=0', true);
	162	+	const marks = new Set();
	163	+	a.ph.setAttribute = (n) => marks.add(n);
	164	+	a.ph.removeAttribute = (n) => marks.delete(n);
	165	+	try {
	166	+	const { urls } = await setup([a.ph], true, { reply: { status: 500, ok: false, body: 'boom' } });
	167	+	const p = window.ermiteLazy.load(a.ph);
	168	+	assert.ok(marks.has('data-loading'));
	169	+	await p;
	170	+	assert.equal(marks.has('data-loading'), false);
	171	+	await window.ermiteLazy.load(a.ph);
	172	+	assert.equal(urls.length, 2, 'a retry fetches again');
	173	+	} finally { await settle(); restore(); }
	174	+	});
	175	+
	176	+	test('Space is prevented on the link; a held key is ignored', async () => {
	177	+	const a = placeholder('/f?one=1&from=1', true);
	178	+	try {
	179	+	const { urls } = await setup([a.ph]);
	180	+	let prevented = 0;
	181	+	const ev = (extra) => ({ target: a.link, key: ' ', preventDefault() { prevented++; }, ...extra });
	182	+	assert.equal(window.ermiteLazy.onKey(ev({ repeat: true })), false);
	183	+	assert.equal(prevented, 0);
	184	+	assert.equal(urls.length, 0);
	185	+	assert.equal(window.ermiteLazy.onKey(ev({})), true);
	186	+	assert.equal(prevented, 1, 'Space does not scroll the page');
	187	+	assert.equal(urls.length, 1);
	188	+	} finally { await settle(); restore(); }
	189	+	});


@@ -17,18 +17,19 @@ function placeholder(src, manual) {
17		return { ph, link, details };	17		return { ph, link, details };
18		}	18		}
19			19
20	-	async function setup(phs, io = true) {	20	+	async function setup(phs, io = true, opts = {}) {
21		const observed = [];	21		const observed = [];
22		const urls = [];	22		const urls = [];
23		if (io) globalThis.IntersectionObserver = class { observe(p) { observed.push(p); } unobserve() {} };	23		if (io) globalThis.IntersectionObserver = class { observe(p) { observed.push(p); } unobserve() {} };
24		else delete globalThis.IntersectionObserver;	24		else delete globalThis.IntersectionObserver;
25	-	globalThis.fetch = async (u) => { urls.push(u); return { status: 200, ok: true, text: async () => '' }; };	25	+	const reply = opts.reply \|\| { status: 200, ok: true, body: '' };
			26	+	globalThis.fetch = async (u) => { urls.push(u); return { status: reply.status, ok: reply.ok, text: async () => reply.body }; };
26		globalThis.document = {	27		globalThis.document = {
27		readyState: 'complete',	28		readyState: 'complete',
28		querySelector: (s) => (s === '#diffs' ? { dataset: {} } : null),	29		querySelector: (s) => (s === '#diffs' ? { dataset: {} } : null),
29		querySelectorAll: (s) => (s.includes(':not([data-manual])') ? phs.filter((p) => !('manual' in p.dataset)) : phs),	30		querySelectorAll: (s) => (s.includes(':not([data-manual])') ? phs.filter((p) => !('manual' in p.dataset)) : phs),
30	-	getElementById: () => null,	31	+	getElementById: opts.getElementById \|\| (() => null),
31	-	createElement: () => ({ content: { querySelectorAll: () => [] } }),	32	+	createElement: () => ({ content: { querySelectorAll: () => opts.files \|\| [] } }),
32		addEventListener() {},	33		addEventListener() {},
33		};	34		};
34		delete globalThis.ermiteLazy;	35		delete globalThis.ermiteLazy;

@@ -37,6 +38,7 @@ async function setup(phs, io = true) {
37		}	38		}
38		const settle = () => new Promise((r) => setTimeout(r, 0));	39		const settle = () => new Promise((r) => setTimeout(r, 0));
39		const restore = () => {	40		const restore = () => {
			41	+	delete globalThis.ermiteLazy;
40		for (const [k, v] of [['document', saved.document], ['fetch', saved.fetch], ['IntersectionObserver', saved.IO]]) {	42		for (const [k, v] of [['document', saved.document], ['fetch', saved.fetch], ['IntersectionObserver', saved.IO]]) {
41		if (v === undefined) delete globalThis[k]; else globalThis[k] = v;	43		if (v === undefined) delete globalThis[k]; else globalThis[k] = v;
42		}	44		}

@@ -96,3 +98,92 @@ test('a manual placeholder without an address is a plain link', async () => {
96		assert.equal(window.ermiteLazy.onClick({ target: a.link, button: 0, preventDefault() { assert.fail(); } }), false);	98		assert.equal(window.ermiteLazy.onClick({ target: a.link, button: 0, preventDefault() { assert.fail(); } }), false);
97		} finally { await settle(); restore(); }	99		} finally { await settle(); restore(); }
98		});	100		});
			101	+
			102	+	// A file of the page (old) or of a fragment (new), with the queries swap makes.
			103	+	function fileEl(id, { lazy = false, manual = false, open = false } = {}) {
			104	+	const f = { id, open, queried: [] };
			105	+	f.querySelector = (sel) => {
			106	+	if (sel === '.dlazy[data-manual]') return manual ? {} : null;
			107	+	if (sel === '.dlazy') return lazy \|\| manual ? {} : null;
			108	+	if (sel === '.ln a') return f.line \|\| null;
			109	+	return null;
			110	+	};
			111	+	f.querySelectorAll = () => [];
			112	+	return f;
			113	+	}
			114	+
			115	+	test('a loaded manual file opens and takes the focus without scrolling', async () => {
			116	+	const a = placeholder('/f?one=1&from=0', true);
			117	+	const old = fileEl('f1', { manual: true });
			118	+	const fresh = fileEl('f1');
			119	+	const focused = [];
			120	+	fresh.line = { focus: (o) => focused.push(o) };
			121	+	let current = old;
			122	+	old.replaceWith = (n) => { current = n; };
			123	+	try {
			124	+	await setup([a.ph], true, { reply: { status: 200, ok: true, body: '<x>' }, files: [fresh], getElementById: () => current });
			125	+	window.ermiteLazy.onClick({ target: a.link, button: 0, preventDefault() {} });
			126	+	await settle(); await settle();
			127	+	assert.equal(current, fresh);
			128	+	assert.equal(fresh.open, true);
			129	+	assert.deepEqual(focused, [{ preventScroll: true }]);
			130	+	} finally { await settle(); restore(); }
			131	+	});
			132	+
			133	+	test('a window that still holds a collapsed file keeps it closed', async () => {
			134	+	const w = placeholder('/w?from=25', false);
			135	+	const old = fileEl('f26', { lazy: true, open: true });
			136	+	const fresh = fileEl('f26', { manual: true, open: true });
			137	+	let replaced = null;
			138	+	old.replaceWith = (n) => { replaced = n; };
			139	+	try {
			140	+	await setup([w.ph], true, { reply: { status: 200, ok: true, body: '<x>' }, files: [fresh], getElementById: () => old });
			141	+	await window.ermiteLazy.load(w.ph);
			142	+	assert.equal(replaced, fresh);
			143	+	assert.equal(fresh.open, false);
			144	+	} finally { await settle(); restore(); }
			145	+	});
			146	+
			147	+	test('a 409 makes the page stale: no later load fetches', async () => {
			148	+	const a = placeholder('/f?one=1&from=0', true);
			149	+	const b = placeholder('/f?one=1&from=1', true);
			150	+	try {
			151	+	const { urls } = await setup([a.ph, b.ph], true, { reply: { status: 409, ok: false, body: '<p>moved</p>' } });
			152	+	await window.ermiteLazy.load(a.ph);
			153	+	assert.equal(a.ph.innerHTML, '<p>moved</p>');
			154	+	assert.equal(urls.length, 1);
			155	+	await window.ermiteLazy.load(b.ph);
			156	+	assert.equal(urls.length, 1, 'stale: nothing is fetched');
			157	+	} finally { await settle(); restore(); }
			158	+	});
			159	+
			160	+	test('a failed load clears the loading mark and can be retried', async () => {
			161	+	const a = placeholder('/f?one=1&from=0', true);
			162	+	const marks = new Set();
			163	+	a.ph.setAttribute = (n) => marks.add(n);
			164	+	a.ph.removeAttribute = (n) => marks.delete(n);
			165	+	try {
			166	+	const { urls } = await setup([a.ph], true, { reply: { status: 500, ok: false, body: 'boom' } });
			167	+	const p = window.ermiteLazy.load(a.ph);
			168	+	assert.ok(marks.has('data-loading'));
			169	+	await p;
			170	+	assert.equal(marks.has('data-loading'), false);
			171	+	await window.ermiteLazy.load(a.ph);
			172	+	assert.equal(urls.length, 2, 'a retry fetches again');
			173	+	} finally { await settle(); restore(); }
			174	+	});
			175	+
			176	+	test('Space is prevented on the link; a held key is ignored', async () => {
			177	+	const a = placeholder('/f?one=1&from=1', true);
			178	+	try {
			179	+	const { urls } = await setup([a.ph]);
			180	+	let prevented = 0;
			181	+	const ev = (extra) => ({ target: a.link, key: ' ', preventDefault() { prevented++; }, ...extra });
			182	+	assert.equal(window.ermiteLazy.onKey(ev({ repeat: true })), false);
			183	+	assert.equal(prevented, 0);
			184	+	assert.equal(urls.length, 0);
			185	+	assert.equal(window.ermiteLazy.onKey(ev({})), true);
			186	+	assert.equal(prevented, 1, 'Space does not scroll the page');
			187	+	assert.equal(urls.length, 1);
			188	+	} finally { await settle(); restore(); }
			189	+	});

M internal/adapters/web/mr.go +7 -4


@@ -4,6 +4,7 @@ package web
4	4
5	5		import (
6	6		"context"
	7	+	"errors"
7	8		"html"
8	9		"html/template"
9	10		"io"

@@ -569,15 +570,17 @@ func (s Server) mrPage(ctx context.Context, r http.Request, since bool) (templ
569	570		return nil, 0, errNotFound
570	571		}
571	572		// A page without scripts: ?file=i reads that collapsed file in full.
572		-	if i, ok := fileParam(r, len(res.Diff.Files)); ok && res.Diff.Files[i].Omitted.Collapsed() {
	573	+	// Without a pinned base, or when the pair moved since the first read, the
	574	+	// page is shown unspliced.
	575	+	if i, ok := fileParam(r, len(res.Diff.Files)); ok && res.Diff.Files[i].Omitted.Collapsed() && !res.DiffBaseID.IsZero() {
573	576		one, err := s.cfg.MergeRequests.Get(ctx, principal(ctx), d.repo.Name, n, ports.GetMergeRequestOptions{
574	577		SinceLastRead: since, DiffOnly: true, Base: res.DiffBaseID, Head: res.MergeRequest.HeadID,
575	578		Files: &ports.FileWindow{From: i, To: i + 1, Full: true},
576	579		})
577		-	if err != nil {
	580	+	if err != nil && !errors.Is(err, domain.ErrConflict) {
578	581		return nil, 0, err
579	582		}
580		-	if i < len(one.Diff.Files) {
	583	+	if err == nil && i < len(one.Diff.Files) {
581	584		res.Diff.Files[i] = one.Diff.Files[i]
582	585		}
583	586		}

@@ -611,7 +614,7 @@ func (s Server) mrPage(ctx context.Context, r http.Request, since bool) (templ
611	614		return nil, 0, err
612	615		}
613	616
614		-	p.Note = diffNote(res.Diff, len(p.Files), p.NFiles)
	617	+	p.Note = diffNote(res.Diff)
615	618		countNotes(p.Files, res.Comments)
616	619		if !res.DiffBaseID.IsZero() {
617	620		lazyFiles(p.Files, from, filesSrc(d.name(), n, since, res.DiffBaseID.String(), head))

 import (
 	"context"
+	"errors"
 	"html"
 	"html/template"
 	"io"
 		return nil, 0, errNotFound
 	}
 	// A page without scripts: ?file=i reads that collapsed file in full.
-	if i, ok := fileParam(r, len(res.Diff.Files)); ok && res.Diff.Files[i].Omitted.Collapsed() {
+	// Without a pinned base, or when the pair moved since the first read, the
+	// page is shown unspliced.
+	if i, ok := fileParam(r, len(res.Diff.Files)); ok && res.Diff.Files[i].Omitted.Collapsed() && !res.DiffBaseID.IsZero() {
 		one, err := s.cfg.MergeRequests.Get(ctx, principal(ctx), d.repo.Name, n, ports.GetMergeRequestOptions{
 			SinceLastRead: since, DiffOnly: true, Base: res.DiffBaseID, Head: res.MergeRequest.HeadID,
 			Files: &ports.FileWindow{From: i, To: i + 1, Full: true},
 		})
-		if err != nil {
+		if err != nil && !errors.Is(err, domain.ErrConflict) {
 			return nil, 0, err
 		}
-		if i < len(one.Diff.Files) {
+		if err == nil && i < len(one.Diff.Files) {
 			res.Diff.Files[i] = one.Diff.Files[i]
 		}
 	}
 		return nil, 0, err
 	}
-	p.Note = diffNote(res.Diff, len(p.Files), p.NFiles)
+	p.Note = diffNote(res.Diff)
 	countNotes(p.Files, res.Comments)
 	if !res.DiffBaseID.IsZero() {
 		lazyFiles(p.Files, from, filesSrc(d.name(), n, since, res.DiffBaseID.String(), head))

M internal/adapters/web/mr_files.go +8 -3


@@ -219,9 +219,14 @@ func (s Server) mrFiles(ctx context.Context, r http.Request) (templ.Component,
219	219		canWrite := signed && m.State == domain.MRStateOpen && !since
220	220		p := MRPage{Files: s.hl.files(ctx, res.Diff.Files), LooseInFiles: true, NoteAuthor: domain.AuthorName(principal(ctx))}
221	221		contextGaps(p.Files, res.Diff.Files, repoHref(d.name(), "context", head.String()))
222		-	collapseFiles(p.Files, res.Diff.Files, fileLinks{
223		-	src: filesSrc(d.name(), n, since, base.String(), head.String()), from: from, repo: d.name(), rev: head.String(),
224		-	})
	222	+	links := fileLinks{src: filesSrc(d.name(), n, since, base.String(), head.String()), from: from, repo: d.name(), rev: head.String()}
	223	+	if one {
	224	+	// A one-file read holds file from alone, at index from: the load
	225	+	// addresses of a window do not apply, only the raw link.
	226	+	p.Files[from].Raw = rawLink(res.Diff.Files[from], links)
	227	+	} else {
	228	+	collapseFiles(p.Files, res.Diff.Files, links)
	229	+	}
225	230		s.placeComments(&p, d.name(), n, res.Comments, m.HeadID.String(), !since, canWrite, nil)
226	231		window := p.Files[from:min(from+mrWindow, len(p.Files))]
227	232		if one {

 	canWrite := signed && m.State == domain.MRStateOpen && !since
 	p := MRPage{Files: s.hl.files(ctx, res.Diff.Files), LooseInFiles: true, NoteAuthor: domain.AuthorName(principal(ctx))}
 	contextGaps(p.Files, res.Diff.Files, repoHref(d.name(), "context", head.String()))
-	collapseFiles(p.Files, res.Diff.Files, fileLinks{
+	links := fileLinks{src: filesSrc(d.name(), n, since, base.String(), head.String()), from: from, repo: d.name(), rev: head.String()}
-		src: filesSrc(d.name(), n, since, base.String(), head.String()), from: from, repo: d.name(), rev: head.String(),
+	if one {
-	})
+		// A one-file read holds file from alone, at index from: the load
+		// addresses of a window do not apply, only the raw link.
+		p.Files[from].Raw = rawLink(res.Diff.Files[from], links)
+	} else {
+		collapseFiles(p.Files, res.Diff.Files, links)
+	}
 	s.placeComments(&p, d.name(), n, res.Comments, m.HeadID.String(), !since, canWrite, nil)
 	window := p.Files[from:min(from+mrWindow, len(p.Files))]
 	if one {

M internal/app/web_commit_test.go +28 -0


@@ -41,6 +41,34 @@ func TestWebCommitGolden(t *testing.T) {
41	41		}
42	42		}
43	43
	44	+	// A commit page reads the hunks of its first 300 files; the others are listed
	45	+	// collapsed and load on their own. A binary file or one without changed lines
	46	+	// past the window has nothing to load and gets no link.
	47	+	func TestWebCommitFileWindow(t *testing.T) {
	48	+	r := newMRRepo(t, domain.VisibilityPublic)
	49	+	b := browser{r.harness}
	50	+	for i := range 305 {
	51	+	require.NoError(t, os.MkdirAll(filepath.Join(r.wt, "many"), 0o755))
	52	+	require.NoError(t, os.WriteFile(filepath.Join(r.wt, fmt.Sprintf("many/f%03d.txt", i)), []byte(fmt.Sprintf("line %d\n", i)), 0o644))
	53	+	}
	54	+	require.NoError(t, os.WriteFile(filepath.Join(r.wt, "zbin.bin"), []byte("a\x00b"), 0o644))
	55	+	require.NoError(t, os.WriteFile(filepath.Join(r.wt, "zempty.txt"), nil, 0o644))
	56	+	r.mustGit(r.wt, "add", ".")
	57	+	r.mustGit(r.wt, "commit", "-q", "-m", "many files")
	58	+	r.mustGit(r.wt, "push", "-q", r.url, "feat/search")
	59	+
	60	+	page := b.get("/proj/commit/"+r.headOf("feat/search"), nil)
	61	+	require.Equal(t, 200, page.status)
	62	+	assert.Contains(t, page.body, "line 299</td>", "index 299 is read")
	63	+	assert.NotContains(t, page.body, "line 300</td>", "index 300 is collapsed")
	64	+	assert.Equal(t, 5, strings.Count(page.body, "collapsed to keep the page light"))
	65	+	assert.Equal(t, 5, strings.Count(page.body, "data-manual"), "binary and empty files past the window have nothing to load")
	66	+	assert.Contains(t, page.body, `<a href="?file=300#f301">load diff</a>`)
	67	+	assert.NotContains(t, page.body, `?file=299#`)
	68	+	assert.NotContains(t, page.body, `?file=305#`)
	69	+	assert.NotContains(t, page.body, `?file=306#`)
	70	+	}
	71	+
44	72		// The behaviours of the commit pages, asserted on their own.
45	73		func TestWebCommit(t *testing.T) {
46	74		h := newHarness(t, harnessOpts{})


@@ -41,6 +41,34 @@ func TestWebCommitGolden(t *testing.T) {
41	}	41		}
42	}	42		}
43		43
		44	+	// A commit page reads the hunks of its first 300 files; the others are listed
		45	+	// collapsed and load on their own. A binary file or one without changed lines
		46	+	// past the window has nothing to load and gets no link.
		47	+	func TestWebCommitFileWindow(t *testing.T) {
		48	+	r := newMRRepo(t, domain.VisibilityPublic)
		49	+	b := browser{r.harness}
		50	+	for i := range 305 {
		51	+	require.NoError(t, os.MkdirAll(filepath.Join(r.wt, "many"), 0o755))
		52	+	require.NoError(t, os.WriteFile(filepath.Join(r.wt, fmt.Sprintf("many/f%03d.txt", i)), []byte(fmt.Sprintf("line %d\n", i)), 0o644))
		53	+	}
		54	+	require.NoError(t, os.WriteFile(filepath.Join(r.wt, "zbin.bin"), []byte("a\x00b"), 0o644))
		55	+	require.NoError(t, os.WriteFile(filepath.Join(r.wt, "zempty.txt"), nil, 0o644))
		56	+	r.mustGit(r.wt, "add", ".")
		57	+	r.mustGit(r.wt, "commit", "-q", "-m", "many files")
		58	+	r.mustGit(r.wt, "push", "-q", r.url, "feat/search")
		59	+
		60	+	page := b.get("/proj/commit/"+r.headOf("feat/search"), nil)
		61	+	require.Equal(t, 200, page.status)
		62	+	assert.Contains(t, page.body, "line 299</td>", "index 299 is read")
		63	+	assert.NotContains(t, page.body, "line 300</td>", "index 300 is collapsed")
		64	+	assert.Equal(t, 5, strings.Count(page.body, "collapsed to keep the page light"))
		65	+	assert.Equal(t, 5, strings.Count(page.body, "data-manual"), "binary and empty files past the window have nothing to load")
		66	+	assert.Contains(t, page.body, `<a href="?file=300#f301">load diff</a>`)
		67	+	assert.NotContains(t, page.body, `?file=299#`)
		68	+	assert.NotContains(t, page.body, `?file=305#`)
		69	+	assert.NotContains(t, page.body, `?file=306#`)
		70	+	}
		71	+
44	// The behaviours of the commit pages, asserted on their own.	72		// The behaviours of the commit pages, asserted on their own.
45	func TestWebCommit(t *testing.T) {	73		func TestWebCommit(t *testing.T) {
46	h := newHarness(t, harnessOpts{})	74		h := newHarness(t, harnessOpts{})

M internal/app/web_mr_test.go +29 -1


@@ -943,7 +943,8 @@ func TestWebMergeRequestCollapsedFiles(t *testing.T) {
943	943		assert.Equal(t, 2, strings.Count(page.body, "data-manual"), "the large and the generated file")
944	944		assert.Contains(t, page.body, "generated file: collapsed")
945	945		assert.Contains(t, page.body, "large diff (2600 lines changed): collapsed")
946		-	assert.NotContains(t, page.body, "row 5<", "no rows for a collapsed file")
	946	+	assert.NotRegexp(t, `row \d+`, page.body, "no rows for a collapsed file")
	947	+	assert.NotContains(t, page.body, "generated line</td>")
947	948		assert.Contains(t, page.body, "small line")
948	949		assert.NotContains(t, page.body, "this diff is incomplete")
949	950		srcs := regexp.MustCompile(`data-src="([^"]*one=1&from=(\d+))" data-manual`).FindAllStringSubmatch(page.body, -1)

@@ -969,3 +970,30 @@ func TestWebMergeRequestCollapsedFiles(t *testing.T) {
969	970		assert.Equal(t, 1, strings.Count(own.body, "data-manual"), "that one file is spliced in")
970	971		}
971	972		}
	973	+
	974	+	// The page of a window without scripts: ?file=i splices that collapsed file in
	975	+	// and the load link of a window keeps the window.
	976	+	func TestWebMergeRequestWindowFileSplice(t *testing.T) {
	977	+	r := newMRRepo(t, domain.VisibilityPublic)
	978	+	b := browser{r.harness}
	979	+	for i := range 30 {
	980	+	r.writeFile(r.wt, fmt.Sprintf("a/f%02d.txt", i), []byte(fmt.Sprintf("line %d\n", i)))
	981	+	}
	982	+	r.writeFile(r.wt, "vendor/gen.txt", []byte("generated line\n"))
	983	+	r.mustGit(r.wt, "push", "-q", r.url, "feat/search")
	984	+	r.cli(t, adminToken, "mr", "create", "proj", "--source", "feat/search", "--title", "Window").ok(t)
	985	+
	986	+	// Files sort as a/f00..a/f29, search.go, vendor/gen.txt: the generated
	987	+	// file is index 31, in the window that starts at 25.
	988	+	win := b.get("/proj/merge-requests/1?from=25", nil)
	989	+	require.Equal(t, 200, win.status)
	990	+	m := regexp.MustCompile(`href="(\?from=25&file=(\d+)#f\d+)"`).FindStringSubmatch(win.body)
	991	+	require.Len(t, m, 3, "the load link keeps the window")
	992	+	assert.Equal(t, "31", m[2])
	993	+	assert.NotContains(t, win.body, "generated line</td>")
	994	+
	995	+	own := b.get("/proj/merge-requests/1?from=25&file=31", nil)
	996	+	require.Equal(t, 200, own.status)
	997	+	assert.Contains(t, own.body, "generated line</td>", "the file is spliced in")
	998	+	assert.NotContains(t, own.body, "data-manual")
	999	+	}


@@ -943,7 +943,8 @@ func TestWebMergeRequestCollapsedFiles(t *testing.T) {
943		assert.Equal(t, 2, strings.Count(page.body, "data-manual"), "the large and the generated file")	943		assert.Equal(t, 2, strings.Count(page.body, "data-manual"), "the large and the generated file")
944		assert.Contains(t, page.body, "generated file: collapsed")	944		assert.Contains(t, page.body, "generated file: collapsed")
945		assert.Contains(t, page.body, "large diff (2600 lines changed): collapsed")	945		assert.Contains(t, page.body, "large diff (2600 lines changed): collapsed")
946	-	assert.NotContains(t, page.body, "row 5<", "no rows for a collapsed file")	946	+	assert.NotRegexp(t, `row \d+`, page.body, "no rows for a collapsed file")
			947	+	assert.NotContains(t, page.body, "generated line</td>")
947		assert.Contains(t, page.body, "small line")	948		assert.Contains(t, page.body, "small line")
948		assert.NotContains(t, page.body, "this diff is incomplete")	949		assert.NotContains(t, page.body, "this diff is incomplete")
949		srcs := regexp.MustCompile(`data-src="([^"]*one=1&from=(\d+))" data-manual`).FindAllStringSubmatch(page.body, -1)	950		srcs := regexp.MustCompile(`data-src="([^"]*one=1&from=(\d+))" data-manual`).FindAllStringSubmatch(page.body, -1)

@@ -969,3 +970,30 @@ func TestWebMergeRequestCollapsedFiles(t *testing.T) {
969		assert.Equal(t, 1, strings.Count(own.body, "data-manual"), "that one file is spliced in")	970		assert.Equal(t, 1, strings.Count(own.body, "data-manual"), "that one file is spliced in")
970		}	971		}
971		}	972		}
			973	+
			974	+	// The page of a window without scripts: ?file=i splices that collapsed file in
			975	+	// and the load link of a window keeps the window.
			976	+	func TestWebMergeRequestWindowFileSplice(t *testing.T) {
			977	+	r := newMRRepo(t, domain.VisibilityPublic)
			978	+	b := browser{r.harness}
			979	+	for i := range 30 {
			980	+	r.writeFile(r.wt, fmt.Sprintf("a/f%02d.txt", i), []byte(fmt.Sprintf("line %d\n", i)))
			981	+	}
			982	+	r.writeFile(r.wt, "vendor/gen.txt", []byte("generated line\n"))
			983	+	r.mustGit(r.wt, "push", "-q", r.url, "feat/search")
			984	+	r.cli(t, adminToken, "mr", "create", "proj", "--source", "feat/search", "--title", "Window").ok(t)
			985	+
			986	+	// Files sort as a/f00..a/f29, search.go, vendor/gen.txt: the generated
			987	+	// file is index 31, in the window that starts at 25.
			988	+	win := b.get("/proj/merge-requests/1?from=25", nil)
			989	+	require.Equal(t, 200, win.status)
			990	+	m := regexp.MustCompile(`href="(\?from=25&file=(\d+)#f\d+)"`).FindStringSubmatch(win.body)
			991	+	require.Len(t, m, 3, "the load link keeps the window")
			992	+	assert.Equal(t, "31", m[2])
			993	+	assert.NotContains(t, win.body, "generated line</td>")
			994	+
			995	+	own := b.get("/proj/merge-requests/1?from=25&file=31", nil)
			996	+	require.Equal(t, 200, own.status)
			997	+	assert.Contains(t, own.body, "generated line</td>", "the file is spliced in")
			998	+	assert.NotContains(t, own.body, "data-manual")
			999	+	}

M internal/domain/budget.go +2 -0


@@ -84,6 +84,8 @@ func (b DiffBudget) Admit(binary bool, path string, changed int) (read bool, why
84	84		return false, OmitNone
85	85		case b.limits.SkipGenerated && IsGeneratedPath(path):
86	86		return false, OmitGenerated
	87	+	case b.limits.MaxAloneLines > 0 && changed > b.limits.MaxAloneLines:
	88	+	return false, OmitUnavailable
87	89		case b.limits.MaxFileLines > 0 && changed > b.limits.MaxFileLines:
88	90		return false, OmitLarge
89	91		case b.limits.MaxTotalLines > 0 && changed > b.linesLeft, b.bytesLeft <= 0:


@@ -84,6 +84,8 @@ func (b DiffBudget) Admit(binary bool, path string, changed int) (read bool, why
84	return false, OmitNone	84		return false, OmitNone
85	case b.limits.SkipGenerated && IsGeneratedPath(path):	85		case b.limits.SkipGenerated && IsGeneratedPath(path):
86	return false, OmitGenerated	86		return false, OmitGenerated
		87	+	case b.limits.MaxAloneLines > 0 && changed > b.limits.MaxAloneLines:
		88	+	return false, OmitUnavailable
87	case b.limits.MaxFileLines > 0 && changed > b.limits.MaxFileLines:	89		case b.limits.MaxFileLines > 0 && changed > b.limits.MaxFileLines:
88	return false, OmitLarge	90		return false, OmitLarge
89	case b.limits.MaxTotalLines > 0 && changed > b.linesLeft, b.bytesLeft <= 0:	91		case b.limits.MaxTotalLines > 0 && changed > b.linesLeft, b.bytesLeft <= 0:

M internal/domain/budget_test.go +11 -1


@@ -187,7 +187,7 @@ func TestFileOmissionCollapsed(t *testing.T) {
187	187
188	188		func TestOneFileDiffLimits(t *testing.T) {
189	189		l := OneFileDiffLimits(3)
190		-	if l.From != 3 \|\| l.To != 4 \|\| l.MaxFileLines != 0 \|\| l.MaxTotalLines != 0 \|\| l.MaxFiles != 0 \|\| l.MaxTotalBytes != DefaultDiffLimits().MaxTotalBytes {
	190	+	if l.From != 3 \|\| l.To != 4 \|\| l.MaxFileLines != 0 \|\| l.MaxAloneLines != MaxOneFileLines \|\| l.MaxTotalLines != 0 \|\| l.MaxFiles != 0 \|\| l.MaxTotalBytes != DefaultDiffLimits().MaxTotalBytes {
191	191		t.Fatalf("got %+v", l)
192	192		}
193	193		// A negative index (an unchecked IndexFunc) selects no file, not the whole diff.

@@ -230,3 +230,13 @@ func TestDiffBudgetContext(t *testing.T) {
230	230		t.Fatal("context must not exceed the byte budget")
231	231		}
232	232		}
	233	+
	234	+	func TestDiffBudgetAdmitAloneLines(t *testing.T) {
	235	+	b := NewDiffBudget(OneFileDiffLimits(0))
	236	+	if read, why := b.Admit(false, "a.go", MaxOneFileLines); !read \|\| why != OmitNone {
	237	+	t.Fatalf("at the cap: read=%v why=%v", read, why)
	238	+	}
	239	+	if read, why := b.Admit(false, "a.go", MaxOneFileLines+1); read \|\| why != OmitUnavailable {
	240	+	t.Fatalf("over the cap: read=%v why=%v, want OmitUnavailable", read, why)
	241	+	}
	242	+	}

 func TestOneFileDiffLimits(t *testing.T) {
 	l := OneFileDiffLimits(3)
-	if l.From != 3 || l.To != 4 || l.MaxFileLines != 0 || l.MaxTotalLines != 0 || l.MaxFiles != 0 || l.MaxTotalBytes != DefaultDiffLimits().MaxTotalBytes {
+	if l.From != 3 || l.To != 4 || l.MaxFileLines != 0 || l.MaxAloneLines != MaxOneFileLines || l.MaxTotalLines != 0 || l.MaxFiles != 0 || l.MaxTotalBytes != DefaultDiffLimits().MaxTotalBytes {
 		t.Fatalf("got %+v", l)
 	}
 	// A negative index (an unchecked IndexFunc) selects no file, not the whole diff.
 		t.Fatal("context must not exceed the byte budget")
 	}
 }
+func TestDiffBudgetAdmitAloneLines(t *testing.T) {
+	b := NewDiffBudget(OneFileDiffLimits(0))
+	if read, why := b.Admit(false, "a.go", MaxOneFileLines); !read || why != OmitNone {
+		t.Fatalf("at the cap: read=%v why=%v", read, why)
+	}
+	if read, why := b.Admit(false, "a.go", MaxOneFileLines+1); read || why != OmitUnavailable {
+		t.Fatalf("over the cap: read=%v why=%v, want OmitUnavailable", read, why)
+	}
+}

M internal/domain/git.go +13 -4


@@ -587,19 +587,28 @@ type DiffLimits struct {
587	587		// MaxTotalLines and MaxTotalBytes. To == 0 means no end, so the zero values
588	588		// select every file. MaxFileLines applies to each file of the window.
589	589		From, To int
	590	+	// MaxAloneLines is the most changed lines a file may have to be read at all;
	591	+	// a file over it is OmitUnavailable (it cannot be shown even alone), unlike
	592	+	// OmitLarge, which a read of that file alone can load. 0 means no limit.
	593	+	MaxAloneLines int
590	594		// SkipGenerated omits the files of the window that IsGeneratedPath
591	595		// recognizes (OmitGenerated), without reading their hunks.
592	596		SkipGenerated bool
593	597		}
594	598
595		-	// OneFileDiffLimits reads the hunks of file i only, with no line cap and
596		-	// MaxTotalBytes as the sole guard. i must be at least 0: From -1 with To 0
	599	+	// MaxOneFileLines is the most changed lines of a file read alone on the web
	600	+	// (OneFileDiffLimits). A file over it is OmitUnavailable, so a short-lines
	601	+	// file within the byte guard cannot produce millions of rows.
	602	+	const MaxOneFileLines = 50000
	603	+
	604	+	// OneFileDiffLimits reads the hunks of file i only, with no MaxFileLines cap;
	605	+	// MaxAloneLines (MaxOneFileLines) and MaxTotalBytes are the guards. i must be at least 0: From -1 with To 0
597	606		// would mean the whole diff, so a negative i reads no file at all.
598	607		func OneFileDiffLimits(i int) DiffLimits {
599	608		if i < 0 {
600		-	return DiffLimits{From: 1, To: 1, MaxTotalBytes: DefaultDiffLimits().MaxTotalBytes}
	609	+	return DiffLimits{From: 1, To: 1, MaxAloneLines: MaxOneFileLines, MaxTotalBytes: DefaultDiffLimits().MaxTotalBytes}
601	610		}
602		-	return DiffLimits{From: i, To: i + 1, MaxTotalBytes: DefaultDiffLimits().MaxTotalBytes}
	611	+	return DiffLimits{From: i, To: i + 1, MaxAloneLines: MaxOneFileLines, MaxTotalBytes: DefaultDiffLimits().MaxTotalBytes}
603	612		}
604	613
605	614		// DefaultDiffLimits are the limits of the API contract: 300 files, 2,000

 	// MaxTotalLines and MaxTotalBytes. To == 0 means no end, so the zero values
 	// select every file. MaxFileLines applies to each file of the window.
 	From, To int
+	// MaxAloneLines is the most changed lines a file may have to be read at all;
+	// a file over it is OmitUnavailable (it cannot be shown even alone), unlike
+	// OmitLarge, which a read of that file alone can load. 0 means no limit.
+	MaxAloneLines int
 	// SkipGenerated omits the files of the window that IsGeneratedPath
 	// recognizes (OmitGenerated), without reading their hunks.
 	SkipGenerated bool
 }
-// OneFileDiffLimits reads the hunks of file i only, with no line cap and
+// MaxOneFileLines is the most changed lines of a file read alone on the web
-// MaxTotalBytes as the sole guard. i must be at least 0: From -1 with To 0
+// (OneFileDiffLimits). A file over it is OmitUnavailable, so a short-lines
+// file within the byte guard cannot produce millions of rows.
+const MaxOneFileLines = 50000
+// OneFileDiffLimits reads the hunks of file i only, with no MaxFileLines cap;
+// MaxAloneLines (MaxOneFileLines) and MaxTotalBytes are the guards. i must be at least 0: From -1 with To 0
 // would mean the whole diff, so a negative i reads no file at all.
 func OneFileDiffLimits(i int) DiffLimits {
 	if i < 0 {
-		return DiffLimits{From: 1, To: 1, MaxTotalBytes: DefaultDiffLimits().MaxTotalBytes}
+		return DiffLimits{From: 1, To: 1, MaxAloneLines: MaxOneFileLines, MaxTotalBytes: DefaultDiffLimits().MaxTotalBytes}
 	}
-	return DiffLimits{From: i, To: i + 1, MaxTotalBytes: DefaultDiffLimits().MaxTotalBytes}
+	return DiffLimits{From: i, To: i + 1, MaxAloneLines: MaxOneFileLines, MaxTotalBytes: DefaultDiffLimits().MaxTotalBytes}
 }
 // DefaultDiffLimits are the limits of the API contract: 300 files, 2,000

M internal/readertest/diff.go +26 -0


@@ -156,4 +156,30 @@ func DiffOmissions(t *testing.T, mk Maker) {
156	156		for _, file := range d.Diff.Files {
157	157		assert.Equal(t, domain.OmitNone, file.Omitted)
158	158		}
	159	+
	160	+	// A one-file read is capped at MaxOneFileLines changed lines: the cap
	161	+	// itself loads, one more line is OmitUnavailable (it cannot be loaded).
	162	+	f.Lines("cap.txt", "c", domain.MaxOneFileLines).Lines("over.txt", "o", domain.MaxOneFileLines+1).Commit("caps")
	163	+	d, err = g.GetCommit(ctx, f.Name, domain.Revision{}, domain.DiffLimits{MaxTotalBytes: 1})
	164	+	require.NoError(t, err)
	165	+	capIdx, overIdx := -1, -1
	166	+	for i, file := range d.Diff.Files {
	167	+	switch file.NewPath {
	168	+	case "cap.txt":
	169	+	capIdx = i
	170	+	case "over.txt":
	171	+	overIdx = i
	172	+	}
	173	+	}
	174	+	require.GreaterOrEqual(t, capIdx, 0)
	175	+	require.GreaterOrEqual(t, overIdx, 0)
	176	+	d, err = g.GetCommit(ctx, f.Name, domain.Revision{}, domain.OneFileDiffLimits(capIdx))
	177	+	require.NoError(t, err)
	178	+	assert.Equal(t, domain.OmitNone, d.Diff.Files[capIdx].Omitted)
	179	+	require.Len(t, d.Diff.Files[capIdx].Hunks, 1)
	180	+	assert.Len(t, d.Diff.Files[capIdx].Hunks[0].Lines, domain.MaxOneFileLines)
	181	+	d, err = g.GetCommit(ctx, f.Name, domain.Revision{}, domain.OneFileDiffLimits(overIdx))
	182	+	require.NoError(t, err)
	183	+	assert.Equal(t, domain.OmitUnavailable, d.Diff.Files[overIdx].Omitted)
	184	+	assert.Empty(t, d.Diff.Files[overIdx].Hunks)
159	185		}


@@ -156,4 +156,30 @@ func DiffOmissions(t *testing.T, mk Maker) {
156	for _, file := range d.Diff.Files {	156		for _, file := range d.Diff.Files {
157	assert.Equal(t, domain.OmitNone, file.Omitted)	157		assert.Equal(t, domain.OmitNone, file.Omitted)
158	}	158		}
		159	+
		160	+	// A one-file read is capped at MaxOneFileLines changed lines: the cap
		161	+	// itself loads, one more line is OmitUnavailable (it cannot be loaded).
		162	+	f.Lines("cap.txt", "c", domain.MaxOneFileLines).Lines("over.txt", "o", domain.MaxOneFileLines+1).Commit("caps")
		163	+	d, err = g.GetCommit(ctx, f.Name, domain.Revision{}, domain.DiffLimits{MaxTotalBytes: 1})
		164	+	require.NoError(t, err)
		165	+	capIdx, overIdx := -1, -1
		166	+	for i, file := range d.Diff.Files {
		167	+	switch file.NewPath {
		168	+	case "cap.txt":
		169	+	capIdx = i
		170	+	case "over.txt":
		171	+	overIdx = i
		172	+	}
		173	+	}
		174	+	require.GreaterOrEqual(t, capIdx, 0)
		175	+	require.GreaterOrEqual(t, overIdx, 0)
		176	+	d, err = g.GetCommit(ctx, f.Name, domain.Revision{}, domain.OneFileDiffLimits(capIdx))
		177	+	require.NoError(t, err)
		178	+	assert.Equal(t, domain.OmitNone, d.Diff.Files[capIdx].Omitted)
		179	+	require.Len(t, d.Diff.Files[capIdx].Hunks, 1)
		180	+	assert.Len(t, d.Diff.Files[capIdx].Hunks[0].Lines, domain.MaxOneFileLines)
		181	+	d, err = g.GetCommit(ctx, f.Name, domain.Revision{}, domain.OneFileDiffLimits(overIdx))
		182	+	require.NoError(t, err)
		183	+	assert.Equal(t, domain.OmitUnavailable, d.Diff.Files[overIdx].Omitted)
		184	+	assert.Empty(t, d.Diff.Files[overIdx].Hunks)
159	}	185		}