detached 55fcc8f

#14 feat(diff): collapse generated and large files with on-demand load

state
merged · 55fcc8f · fast-forward
branch
feat/diff-large-files → main
opened
bnema ·
updated

Web diffs no longer cut files arbitrarily.

  • Every changed file is listed on MR, commit and compare pages. Generated files (lockfiles, vendor/, node_modules/, minified or generated code) and files over 2,000 changed lines render collapsed, with no rows.
  • "load diff" fetches that one file in place (one request, one file read, no scroll jump). Without JS, ?file=i reloads the page with the file spliced in.
  • One-file loads are capped at 8 MiB and 50,000 changed lines; beyond, the file shows a raw link and takes no line comment. Line comments accept any line the page can load.
  • Only changed lines count against the diff line limits (context lines no longer do).
  • API/CLI: files gain omitted (large, budget, unavailable) via ermite-api v0.11.0; truncated keeps its meaning.
  • static/js/lazy.js holds the shared lazy loader for MR, commit and compare pages.

Depends on !13: will be rebased after it merges (conflicts expected in mr.go, mr.js and goldens).

Checks: go build/vet/test, node --test (170), templ -check, Chromium check of load diff, scroll, focus, Enter, no-JS and idle mutations.

CommitSubjectAuthorDateSignatureStatus
55fcc8frefactor(diff): move the commit page window and the one-file splice into the usecasesbriceGPG Verified
d00be2edocs(diff): name every cause of an unavailable file in the APIbriceGPG Verified
88a1a56refactor(web): share the fragment layout rule and document the API omission reasonsbriceGPG Verified
de918e2docs(diff): say that files over the one-file cap take no line commentbriceGPG Verified
4af5505fix(diff): cap one-file loads at 50,000 changed lines and harden the web splicebriceGPG Verified
eb480cbfix(web): keep collapsed files closed in loaded windows and ignore held keysbriceGPG Verified
81bd47ffeat(web): collapse generated and large diff files with on-demand loadbriceGPG Verified
1b58f08test(diff): comment on a generated file and collapse it in commit windowsbriceGPG Verified
2a79463feat(diff): share one window policy for MR, commit and comment readsbriceGPG Verified
b1b81dffix(diff): omit a file over the byte budget of earlier files as budgetbriceGPG Verified
0e23da4feat(diff): tell why a diff file has no hunksbriceGPG Verified
7fe3b18build(deps): use ermite-api v0.11.0 for the file omission reasonbriceGPG Verified
e9b7a51test(diff): drop the duplicate changed-line limit testbriceGPG Verified
03ad5a8fix(diff): count only changed lines against the diff line limitsbriceGPG Verified
1e5691efeat(domain): classify generated and vendored files by pathbriceGPG Verified

Checks 8

61 files · +2371 -675
8/8
M docs/merge-requests.md +8 -0
M go.mod +1 -1
M go.sum +2 -2
M internal/adapters/gitcli/diff.go +38 -30
M internal/adapters/gitcli/diff_test.go +40 -38
M internal/adapters/rpc/convert.go +17 -1
M internal/adapters/rpc/convert_test.go +18 -0
M internal/adapters/rpc/git.go +1 -1
M internal/adapters/rpc/git_test.go +1 -1
M internal/adapters/web/assets.go +2 -1
M internal/adapters/web/browse.go +2 -2
M internal/adapters/web/commit.go +57 -16
M internal/adapters/web/compare.go +4 -2
M internal/adapters/web/diff.go +109 -5
M internal/adapters/web/diff.templ +10 -1
M internal/adapters/web/diff_highlight.go +2 -2
M internal/adapters/web/diff_highlight_test.go +2 -2
M internal/adapters/web/diff_templ.go +235 -173
M internal/adapters/web/diff_test.go +51 -2
A internal/adapters/web/jstest/lazy.test.mjs +248 -0
M internal/adapters/web/jstest/mr.test.mjs +1 -0
M internal/adapters/web/mr.go +8 -2
M internal/adapters/web/mr_files.go +36 -10
M internal/adapters/web/shell.templ +3 -0
M internal/adapters/web/shell_templ.go +199 -180
M internal/adapters/web/static/css/60-mr.css +3 -1
A internal/adapters/web/static/js/lazy.js +170 -0
M internal/adapters/web/static/js/mr.js +8 -91
M internal/adapters/web/view.go +3 -0
M internal/adapters/web/web.go +1 -0
M internal/app/cli_mr.go +21 -10
M internal/app/cli_mr_test.go +15 -0
M internal/app/testdata/web/commit-big.html +6 -3
M internal/app/testdata/web/commit-binary.html +2 -0
M internal/app/testdata/web/commit-merge.html +2 -0
M internal/app/testdata/web/commit-modify-split.html +2 -0
M internal/app/testdata/web/commit-modify.html +2 -0
M internal/app/testdata/web/commit-rename.html +2 -0
M internal/app/testdata/web/commit-root.html +2 -0
M internal/app/testdata/web/commit-trojan.html +2 -0
M internal/app/web_commit_test.go +63 -5
M internal/app/web_mr_test.go +77 -0
M internal/domain/budget.go +60 -11
M internal/domain/budget_test.go +136 -17
A internal/domain/generated.go +46 -0
A internal/domain/generated_test.go +51 -0
M internal/domain/git.go +84 -19
M internal/domain/mergerequest_test.go +1 -1
M internal/ports/dto.go +33 -3
M internal/ports/mocks/BrowseService.go +18 -12
M internal/ports/services.go +1 -1
@@ -95,7 +95,7 @@ type BrowseService interface {
9595 ListPaths(ctx context.Context, p domain.Principal, repo domain.RepoName, rev domain.Revision) (domain.PathList, error)
9696 OpenBlob(ctx context.Context, p domain.Principal, repo domain.RepoName, rev domain.Revision, path domain.TreePath) (domain.BlobInfo, io.ReadCloser, error)
9797 ListCommits(ctx context.Context, p domain.Principal, repo domain.RepoName, params ListCommitsParams) (HistoryPage, error)
98- GetCommit(ctx context.Context, p domain.Principal, repo domain.RepoName, rev domain.Revision) (domain.CommitDetail, error)
98+ GetCommit(ctx context.Context, p domain.Principal, repo domain.RepoName, rev domain.Revision, opts GetCommitOptions) (domain.CommitDetail, error)
9999 // RepoStats counts the commits of the branch whose head is given, and
100100 // names the latest tag.
101101 RepoStats(ctx context.Context, p domain.Principal, repo domain.RepoName, head domain.ObjectID, now time.Time) (RepoStats, error)
@@ -95,7 +95,7 @@ type BrowseService interface {
95 ListPaths(ctx context.Context, p domain.Principal, repo domain.RepoName, rev domain.Revision) (domain.PathList, error)95 ListPaths(ctx context.Context, p domain.Principal, repo domain.RepoName, rev domain.Revision) (domain.PathList, error)
96 OpenBlob(ctx context.Context, p domain.Principal, repo domain.RepoName, rev domain.Revision, path domain.TreePath) (domain.BlobInfo, io.ReadCloser, error)96 OpenBlob(ctx context.Context, p domain.Principal, repo domain.RepoName, rev domain.Revision, path domain.TreePath) (domain.BlobInfo, io.ReadCloser, error)
97 ListCommits(ctx context.Context, p domain.Principal, repo domain.RepoName, params ListCommitsParams) (HistoryPage, error)97 ListCommits(ctx context.Context, p domain.Principal, repo domain.RepoName, params ListCommitsParams) (HistoryPage, error)
98- GetCommit(ctx context.Context, p domain.Principal, repo domain.RepoName, rev domain.Revision) (domain.CommitDetail, error)98+ GetCommit(ctx context.Context, p domain.Principal, repo domain.RepoName, rev domain.Revision, opts GetCommitOptions) (domain.CommitDetail, error)
99 // RepoStats counts the commits of the branch whose head is given, and99 // RepoStats counts the commits of the branch whose head is given, and
100 // names the latest tag.100 // names the latest tag.
101 RepoStats(ctx context.Context, p domain.Principal, repo domain.RepoName, head domain.ObjectID, now time.Time) (RepoStats, error)101 RepoStats(ctx context.Context, p domain.Principal, repo domain.RepoName, head domain.ObjectID, now time.Time) (RepoStats, error)
M internal/readertest/conformance.go +2 -0
@@ -31,6 +31,8 @@ func Conformance(t *testing.T, mk Maker) {
3131 name string
3232 fn func(*testing.T, Maker)
3333 }{
34+ {"DiffChangedLineLimits", DiffChangedLineLimits},
35+ {"DiffOmissions", DiffOmissions},
3436 {"ListRefs", testListRefs},
3537 {"ListRefsEmptyAndMissing", testListRefsEmptyAndMissing},
3638 {"DetachedHead", testDetachedHead},
@@ -31,6 +31,8 @@ func Conformance(t *testing.T, mk Maker) {
31 name string31 name string
32 fn func(*testing.T, Maker)32 fn func(*testing.T, Maker)
33 }{33 }{
34+ {"DiffChangedLineLimits", DiffChangedLineLimits},
35+ {"DiffOmissions", DiffOmissions},
34 {"ListRefs", testListRefs},36 {"ListRefs", testListRefs},
35 {"ListRefsEmptyAndMissing", testListRefsEmptyAndMissing},37 {"ListRefsEmptyAndMissing", testListRefsEmptyAndMissing},
36 {"DetachedHead", testDetachedHead},38 {"DetachedHead", testDetachedHead},
A internal/readertest/diff.go +185 -0
@@ -0,0 +1,185 @@
1+// SPDX-License-Identifier: AGPL-3.0-or-later
2+
3+package readertest
4+
5+import (
6+ "context"
7+ "fmt"
8+ "strings"
9+ "testing"
10+
11+ "github.com/stretchr/testify/assert"
12+ "github.com/stretchr/testify/require"
13+
14+ "bnema.dev/ermite/internal/domain"
15+)
16+
17+// DiffChangedLineLimits checks that context is retained without consuming the
18+// changed-line limits, but still consumes the byte budget.
19+func DiffChangedLineLimits(t *testing.T, mk Maker) {
20+ g, data := mk(t)
21+ f := NewFixture(t, data, "diff-context-budget")
22+ var before, after strings.Builder
23+ const hunks = 96
24+ for i := range hunks * 10 {
25+ line := fmt.Sprintf("context %04d", i)
26+ fmt.Fprintln(&before, line)
27+ if i%10 == 4 {
28+ line = fmt.Sprintf("changed %04d", i)
29+ }
30+ fmt.Fprintln(&after, line)
31+ }
32+ f.Write("many", before.String()).Commit("before")
33+ f.Write("many", after.String()).Commit("after")
34+ limits := domain.DiffLimits{MaxFileLines: hunks * 2, MaxTotalLines: hunks * 2, MaxTotalBytes: 1 << 20}
35+ d, err := g.GetCommit(context.Background(), f.Name, domain.Revision{}, limits)
36+ require.NoError(t, err)
37+ require.Len(t, d.Diff.Files, 1)
38+ file := d.Diff.Files[0]
39+ require.Equal(t, domain.OmitNone, file.Omitted)
40+ assert.False(t, d.Diff.Truncated)
41+ require.Len(t, file.Hunks, hunks)
42+ var changed, patchLines, textBytes, changedBytes int
43+ for _, h := range file.Hunks {
44+ for _, l := range h.Lines {
45+ patchLines++
46+ textBytes += len(l.Text)
47+ if l.Kind != domain.LineContext {
48+ changed++
49+ changedBytes += len(l.Text)
50+ }
51+ }
52+ }
53+ assert.Equal(t, hunks*2, changed)
54+ // Every hunk is read with no line limit too.
55+ free, err := g.GetCommit(context.Background(), f.Name, domain.Revision{}, domain.DiffLimits{MaxTotalBytes: 1 << 20})
56+ require.NoError(t, err)
57+ require.Len(t, free.Diff.Files[0].Hunks, hunks)
58+ assert.Greater(t, patchLines, limits.MaxFileLines)
59+ assert.Equal(t, hunks, file.Additions)
60+ assert.Equal(t, hunks, file.Deletions)
61+ // The exact byte boundary fits; one fewer byte must discard the file even
62+ // though every changed line's text alone easily fits.
63+ limits.MaxTotalBytes = textBytes
64+ d, err = g.GetCommit(context.Background(), f.Name, domain.Revision{}, limits)
65+ require.NoError(t, err)
66+ assert.Equal(t, domain.OmitNone, d.Diff.Files[0].Omitted)
67+ limits.MaxTotalBytes--
68+ require.Greater(t, limits.MaxTotalBytes, changedBytes)
69+ d, err = g.GetCommit(context.Background(), f.Name, domain.Revision{}, limits)
70+ require.NoError(t, err)
71+ assert.False(t, d.Diff.Truncated, "only MaxFiles sets Diff.Truncated")
72+ assert.Equal(t, domain.OmitUnavailable, d.Diff.Files[0].Omitted)
73+ assert.Empty(t, d.Diff.Files[0].Hunks)
74+}
75+
76+// DiffOmissions checks the omission reasons: generated files skipped without
77+// reading their hunks, one file read without a line cap, and the MaxFiles
78+// overflow being the only thing that sets Diff.Truncated.
79+func DiffOmissions(t *testing.T, mk Maker) {
80+ g, data := mk(t)
81+ ctx := context.Background()
82+ f := NewFixture(t, data, "diff-omissions")
83+ f.Write("base.txt", "x\n").Commit("base")
84+ f.Write("a.txt", "small\n")
85+ // The generated file (515 bytes) would overflow the 300-byte budget below
86+ // if it were read: OmitGenerated, not OmitUnavailable, proves its hunks
87+ // are not read.
88+ f.Write("go.sum", strings.Repeat("h1:"+strings.Repeat("z", 100)+"\n", 5))
89+ f.Lines("z-big.txt", "line", 2500)
90+ f.Commit("change")
91+
92+ limits := domain.DiffLimits{MaxTotalBytes: 300, MaxFileLines: 2000, SkipGenerated: true}
93+ d, err := g.GetCommit(ctx, f.Name, domain.Revision{}, limits)
94+ require.NoError(t, err)
95+ require.Len(t, d.Diff.Files, 3)
96+ byPath := map[string]domain.FileDiff{}
97+ for _, file := range d.Diff.Files {
98+ byPath[file.NewPath] = file
99+ }
100+ assert.Equal(t, domain.OmitNone, byPath["a.txt"].Omitted)
101+ assert.Len(t, byPath["a.txt"].Hunks, 1)
102+ assert.Equal(t, domain.OmitGenerated, byPath["go.sum"].Omitted)
103+ assert.Empty(t, byPath["go.sum"].Hunks)
104+ assert.Equal(t, 5, byPath["go.sum"].Additions)
105+ assert.Equal(t, domain.OmitLarge, byPath["z-big.txt"].Omitted)
106+ assert.Empty(t, byPath["z-big.txt"].Hunks)
107+ assert.False(t, d.Diff.Truncated)
108+
109+ // Read, go.sum overflows the bytes that a.txt, read before it, already
110+ // spent: it may fit on its own, so it is OmitBudget, loadable alone.
111+ limits.SkipGenerated = false
112+ d, err = g.GetCommit(ctx, f.Name, domain.Revision{}, limits)
113+ require.NoError(t, err)
114+ sum := -1
115+ for i, file := range d.Diff.Files {
116+ if file.NewPath == "go.sum" {
117+ sum = i
118+ assert.Equal(t, domain.OmitBudget, file.Omitted, "earlier files spent bytes")
119+ assert.Empty(t, file.Hunks)
120+ }
121+ }
122+ require.GreaterOrEqual(t, sum, 0)
123+ // Alone, it is still over the bytes: OmitUnavailable, a dead end.
124+ alone := domain.DiffLimits{From: sum, To: sum + 1, MaxTotalBytes: 300}
125+ d, err = g.GetCommit(ctx, f.Name, domain.Revision{}, alone)
126+ require.NoError(t, err)
127+ assert.Equal(t, domain.OmitUnavailable, d.Diff.Files[sum].Omitted, "over the bytes on its own")
128+ assert.Empty(t, d.Diff.Files[sum].Hunks)
129+
130+ // OneFileDiffLimits reads a file over 2,000 changed lines, and only it.
131+ idx := -1
132+ for i, file := range d.Diff.Files {
133+ if file.NewPath == "z-big.txt" {
134+ idx = i
135+ }
136+ }
137+ require.GreaterOrEqual(t, idx, 0)
138+ d, err = g.GetCommit(ctx, f.Name, domain.Revision{}, domain.OneFileDiffLimits(idx))
139+ require.NoError(t, err)
140+ for i, file := range d.Diff.Files {
141+ assert.Equal(t, domain.OmitNone, file.Omitted)
142+ if i == idx {
143+ require.Len(t, file.Hunks, 1)
144+ assert.Len(t, file.Hunks[0].Lines, 2500)
145+ } else {
146+ assert.Empty(t, file.Hunks)
147+ }
148+ }
149+ assert.False(t, d.Diff.Truncated)
150+
151+ // MaxFiles alone sets Diff.Truncated.
152+ d, err = g.GetCommit(ctx, f.Name, domain.Revision{}, domain.DiffLimits{MaxFiles: 2, MaxTotalBytes: 1 << 20})
153+ require.NoError(t, err)
154+ assert.Len(t, d.Diff.Files, 2)
155+ assert.True(t, d.Diff.Truncated)
156+ for _, file := range d.Diff.Files {
157+ assert.Equal(t, domain.OmitNone, file.Omitted)
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)
185+}
@@ -0,0 +1,185 @@
1+// SPDX-License-Identifier: AGPL-3.0-or-later
2+
3+package readertest
4+
5+import (
6+ "context"
7+ "fmt"
8+ "strings"
9+ "testing"
10+
11+ "github.com/stretchr/testify/assert"
12+ "github.com/stretchr/testify/require"
13+
14+ "bnema.dev/ermite/internal/domain"
15+)
16+
17+// DiffChangedLineLimits checks that context is retained without consuming the
18+// changed-line limits, but still consumes the byte budget.
19+func DiffChangedLineLimits(t *testing.T, mk Maker) {
20+ g, data := mk(t)
21+ f := NewFixture(t, data, "diff-context-budget")
22+ var before, after strings.Builder
23+ const hunks = 96
24+ for i := range hunks * 10 {
25+ line := fmt.Sprintf("context %04d", i)
26+ fmt.Fprintln(&before, line)
27+ if i%10 == 4 {
28+ line = fmt.Sprintf("changed %04d", i)
29+ }
30+ fmt.Fprintln(&after, line)
31+ }
32+ f.Write("many", before.String()).Commit("before")
33+ f.Write("many", after.String()).Commit("after")
34+ limits := domain.DiffLimits{MaxFileLines: hunks * 2, MaxTotalLines: hunks * 2, MaxTotalBytes: 1 << 20}
35+ d, err := g.GetCommit(context.Background(), f.Name, domain.Revision{}, limits)
36+ require.NoError(t, err)
37+ require.Len(t, d.Diff.Files, 1)
38+ file := d.Diff.Files[0]
39+ require.Equal(t, domain.OmitNone, file.Omitted)
40+ assert.False(t, d.Diff.Truncated)
41+ require.Len(t, file.Hunks, hunks)
42+ var changed, patchLines, textBytes, changedBytes int
43+ for _, h := range file.Hunks {
44+ for _, l := range h.Lines {
45+ patchLines++
46+ textBytes += len(l.Text)
47+ if l.Kind != domain.LineContext {
48+ changed++
49+ changedBytes += len(l.Text)
50+ }
51+ }
52+ }
53+ assert.Equal(t, hunks*2, changed)
54+ // Every hunk is read with no line limit too.
55+ free, err := g.GetCommit(context.Background(), f.Name, domain.Revision{}, domain.DiffLimits{MaxTotalBytes: 1 << 20})
56+ require.NoError(t, err)
57+ require.Len(t, free.Diff.Files[0].Hunks, hunks)
58+ assert.Greater(t, patchLines, limits.MaxFileLines)
59+ assert.Equal(t, hunks, file.Additions)
60+ assert.Equal(t, hunks, file.Deletions)
61+ // The exact byte boundary fits; one fewer byte must discard the file even
62+ // though every changed line's text alone easily fits.
63+ limits.MaxTotalBytes = textBytes
64+ d, err = g.GetCommit(context.Background(), f.Name, domain.Revision{}, limits)
65+ require.NoError(t, err)
66+ assert.Equal(t, domain.OmitNone, d.Diff.Files[0].Omitted)
67+ limits.MaxTotalBytes--
68+ require.Greater(t, limits.MaxTotalBytes, changedBytes)
69+ d, err = g.GetCommit(context.Background(), f.Name, domain.Revision{}, limits)
70+ require.NoError(t, err)
71+ assert.False(t, d.Diff.Truncated, "only MaxFiles sets Diff.Truncated")
72+ assert.Equal(t, domain.OmitUnavailable, d.Diff.Files[0].Omitted)
73+ assert.Empty(t, d.Diff.Files[0].Hunks)
74+}
75+
76+// DiffOmissions checks the omission reasons: generated files skipped without
77+// reading their hunks, one file read without a line cap, and the MaxFiles
78+// overflow being the only thing that sets Diff.Truncated.
79+func DiffOmissions(t *testing.T, mk Maker) {
80+ g, data := mk(t)
81+ ctx := context.Background()
82+ f := NewFixture(t, data, "diff-omissions")
83+ f.Write("base.txt", "x\n").Commit("base")
84+ f.Write("a.txt", "small\n")
85+ // The generated file (515 bytes) would overflow the 300-byte budget below
86+ // if it were read: OmitGenerated, not OmitUnavailable, proves its hunks
87+ // are not read.
88+ f.Write("go.sum", strings.Repeat("h1:"+strings.Repeat("z", 100)+"\n", 5))
89+ f.Lines("z-big.txt", "line", 2500)
90+ f.Commit("change")
91+
92+ limits := domain.DiffLimits{MaxTotalBytes: 300, MaxFileLines: 2000, SkipGenerated: true}
93+ d, err := g.GetCommit(ctx, f.Name, domain.Revision{}, limits)
94+ require.NoError(t, err)
95+ require.Len(t, d.Diff.Files, 3)
96+ byPath := map[string]domain.FileDiff{}
97+ for _, file := range d.Diff.Files {
98+ byPath[file.NewPath] = file
99+ }
100+ assert.Equal(t, domain.OmitNone, byPath["a.txt"].Omitted)
101+ assert.Len(t, byPath["a.txt"].Hunks, 1)
102+ assert.Equal(t, domain.OmitGenerated, byPath["go.sum"].Omitted)
103+ assert.Empty(t, byPath["go.sum"].Hunks)
104+ assert.Equal(t, 5, byPath["go.sum"].Additions)
105+ assert.Equal(t, domain.OmitLarge, byPath["z-big.txt"].Omitted)
106+ assert.Empty(t, byPath["z-big.txt"].Hunks)
107+ assert.False(t, d.Diff.Truncated)
108+
109+ // Read, go.sum overflows the bytes that a.txt, read before it, already
110+ // spent: it may fit on its own, so it is OmitBudget, loadable alone.
111+ limits.SkipGenerated = false
112+ d, err = g.GetCommit(ctx, f.Name, domain.Revision{}, limits)
113+ require.NoError(t, err)
114+ sum := -1
115+ for i, file := range d.Diff.Files {
116+ if file.NewPath == "go.sum" {
117+ sum = i
118+ assert.Equal(t, domain.OmitBudget, file.Omitted, "earlier files spent bytes")
119+ assert.Empty(t, file.Hunks)
120+ }
121+ }
122+ require.GreaterOrEqual(t, sum, 0)
123+ // Alone, it is still over the bytes: OmitUnavailable, a dead end.
124+ alone := domain.DiffLimits{From: sum, To: sum + 1, MaxTotalBytes: 300}
125+ d, err = g.GetCommit(ctx, f.Name, domain.Revision{}, alone)
126+ require.NoError(t, err)
127+ assert.Equal(t, domain.OmitUnavailable, d.Diff.Files[sum].Omitted, "over the bytes on its own")
128+ assert.Empty(t, d.Diff.Files[sum].Hunks)
129+
130+ // OneFileDiffLimits reads a file over 2,000 changed lines, and only it.
131+ idx := -1
132+ for i, file := range d.Diff.Files {
133+ if file.NewPath == "z-big.txt" {
134+ idx = i
135+ }
136+ }
137+ require.GreaterOrEqual(t, idx, 0)
138+ d, err = g.GetCommit(ctx, f.Name, domain.Revision{}, domain.OneFileDiffLimits(idx))
139+ require.NoError(t, err)
140+ for i, file := range d.Diff.Files {
141+ assert.Equal(t, domain.OmitNone, file.Omitted)
142+ if i == idx {
143+ require.Len(t, file.Hunks, 1)
144+ assert.Len(t, file.Hunks[0].Lines, 2500)
145+ } else {
146+ assert.Empty(t, file.Hunks)
147+ }
148+ }
149+ assert.False(t, d.Diff.Truncated)
150+
151+ // MaxFiles alone sets Diff.Truncated.
152+ d, err = g.GetCommit(ctx, f.Name, domain.Revision{}, domain.DiffLimits{MaxFiles: 2, MaxTotalBytes: 1 << 20})
153+ require.NoError(t, err)
154+ assert.Len(t, d.Diff.Files, 2)
155+ assert.True(t, d.Diff.Truncated)
156+ for _, file := range d.Diff.Files {
157+ assert.Equal(t, domain.OmitNone, file.Omitted)
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)
185+}
M internal/usecase/browse.go +27 -3
@@ -336,15 +336,39 @@ func decodeCommitToken(tok string, path domain.TreePath) (domain.ObjectID, int,
336336 }
337337
338338 // GetCommit returns a commit with its statistics and its diff against the
339-// first parent, within the default diff limits.
340-func (b *Browse) GetCommit(ctx context.Context, p domain.Principal, repo domain.RepoName, rev domain.Revision) (domain.CommitDetail, error) {
339+// first parent, within the default diff limits or, with opts.Files, the limits
340+// of that file window. An invalid window gives domain.ErrInvalid.
341+func (b *Browse) GetCommit(ctx context.Context, p domain.Principal, repo domain.RepoName, rev domain.Revision, opts ports.GetCommitOptions) (domain.CommitDetail, error) {
342+ if (opts.Page && opts.Files != nil) || (opts.Load != nil && !opts.Page) {
343+ return domain.CommitDetail{}, fmt.Errorf("%w: commit page options", domain.ErrInvalid)
344+ }
345+ win := opts.Files
346+ if opts.Page {
347+ win = &ports.FileWindow{From: 0, To: domain.CommitPageFiles}
348+ }
349+ limits, err := windowLimits(win)
350+ if err != nil {
351+ return domain.CommitDetail{}, err
352+ }
341353 if _, err := b.authorize(ctx, p, repo); err != nil {
342354 return domain.CommitDetail{}, err
343355 }
344- d, err := b.reader.GetCommit(ctx, repo, rev, domain.DefaultDiffLimits())
356+ d, err := b.reader.GetCommit(ctx, repo, rev, limits)
345357 if err != nil {
346358 return domain.CommitDetail{}, err
347359 }
360+ if opts.Page {
361+ d.Diff.CollapseFrom(domain.CommitPageFiles)
362+ if i := opts.Load; i != nil && *i >= 0 && *i < len(d.Diff.Files) && d.Diff.Files[*i].Omitted.Collapsed() {
363+ one, err := b.reader.GetCommit(ctx, repo, rev, domain.OneFileDiffLimits(*i))
364+ if err != nil {
365+ return domain.CommitDetail{}, err
366+ }
367+ if *i < len(one.Diff.Files) {
368+ d.Diff.Files[*i] = one.Diff.Files[*i]
369+ }
370+ }
371+ }
348372 if b.verifier != nil {
349373 one := []domain.Commit{d.Commit}
350374 b.verify(ctx, repo, one)
@@ -336,15 +336,39 @@ func decodeCommitToken(tok string, path domain.TreePath) (domain.ObjectID, int,
336 }336 }
337 337
338 // GetCommit returns a commit with its statistics and its diff against the338 // GetCommit returns a commit with its statistics and its diff against the
339-// first parent, within the default diff limits.339+// first parent, within the default diff limits or, with opts.Files, the limits
340-func (b *Browse) GetCommit(ctx context.Context, p domain.Principal, repo domain.RepoName, rev domain.Revision) (domain.CommitDetail, error) {340+// of that file window. An invalid window gives domain.ErrInvalid.
341+func (b *Browse) GetCommit(ctx context.Context, p domain.Principal, repo domain.RepoName, rev domain.Revision, opts ports.GetCommitOptions) (domain.CommitDetail, error) {
342+ if (opts.Page && opts.Files != nil) || (opts.Load != nil && !opts.Page) {
343+ return domain.CommitDetail{}, fmt.Errorf("%w: commit page options", domain.ErrInvalid)
344+ }
345+ win := opts.Files
346+ if opts.Page {
347+ win = &ports.FileWindow{From: 0, To: domain.CommitPageFiles}
348+ }
349+ limits, err := windowLimits(win)
350+ if err != nil {
351+ return domain.CommitDetail{}, err
352+ }
341 if _, err := b.authorize(ctx, p, repo); err != nil {353 if _, err := b.authorize(ctx, p, repo); err != nil {
342 return domain.CommitDetail{}, err354 return domain.CommitDetail{}, err
343 }355 }
344- d, err := b.reader.GetCommit(ctx, repo, rev, domain.DefaultDiffLimits())356+ d, err := b.reader.GetCommit(ctx, repo, rev, limits)
345 if err != nil {357 if err != nil {
346 return domain.CommitDetail{}, err358 return domain.CommitDetail{}, err
347 }359 }
360+ if opts.Page {
361+ d.Diff.CollapseFrom(domain.CommitPageFiles)
362+ if i := opts.Load; i != nil && *i >= 0 && *i < len(d.Diff.Files) && d.Diff.Files[*i].Omitted.Collapsed() {
363+ one, err := b.reader.GetCommit(ctx, repo, rev, domain.OneFileDiffLimits(*i))
364+ if err != nil {
365+ return domain.CommitDetail{}, err
366+ }
367+ if *i < len(one.Diff.Files) {
368+ d.Diff.Files[*i] = one.Diff.Files[*i]
369+ }
370+ }
371+ }
348 if b.verifier != nil {372 if b.verifier != nil {
349 one := []domain.Commit{d.Commit}373 one := []domain.Commit{d.Commit}
350 b.verify(ctx, repo, one)374 b.verify(ctx, repo, one)
M internal/usecase/browse_test.go +37 -4
@@ -9,6 +9,7 @@ import (
99 "fmt"
1010 "io"
1111 "runtime"
12+ "slices"
1213 "strconv"
1314 "strings"
1415 "sync"
@@ -104,7 +105,7 @@ func browseOps() []browseOp {
104105 r.EXPECT().ListCommits(mock.Anything, mock.Anything, mock.Anything).Return(ports.CommitPage{}, nil)
105106 }},
106107 {"GetCommit", func(b *usecase.Browse, ctx context.Context, p domain.Principal, repo domain.RepoName) error {
107- _, err := b.GetCommit(ctx, p, repo, domain.Revision{})
108+ _, err := b.GetCommit(ctx, p, repo, domain.Revision{}, ports.GetCommitOptions{})
108109 return err
109110 }, func(r *mocks.RepoReader) {
110111 r.EXPECT().GetCommit(mock.Anything, mock.Anything, mock.Anything, domain.DefaultDiffLimits()).Return(domain.CommitDetail{}, nil)
@@ -629,12 +630,44 @@ func TestBrowseVerifiesCommits(t *testing.T) {
629630 Return(domain.CommitDetail{Commit: c, Stats: domain.DiffStats{FilesChanged: 2}}, nil)
630631 v.EXPECT().VerifyCommits(ctx, pub.Name, []domain.Commit{c}).
631632 Return(map[domain.ObjectID]domain.CommitVerification{oid("a"): verified})
632- d, err := f.browse.GetCommit(ctx, domain.Anonymous(), pub.Name, domain.Revision{})
633+ d, err := f.browse.GetCommit(ctx, domain.Anonymous(), pub.Name, domain.Revision{}, ports.GetCommitOptions{})
633634 require.NoError(t, err)
634635 assert.Equal(t, verified, d.Commit.Verification)
635636 assert.Equal(t, 2, d.Stats.FilesChanged)
636637 })
637638
639+ t.Run("GetCommit Page collapses past the window and loads one file", func(t *testing.T) {
640+ t.Parallel()
641+ f, v := newFixture(t)
642+ v.EXPECT().VerifyCommits(ctx, pub.Name, mock.Anything).Return(nil).Maybe()
643+ f.meta.EXPECT().Get(ctx, pub.Name).Return(pub, nil).Maybe()
644+ files := make([]domain.FileDiff, domain.CommitPageFiles+2)
645+ for i := range files {
646+ files[i].Additions = 1
647+ }
648+ win := domain.DefaultDiffLimits()
649+ win.MaxFiles, win.From, win.To, win.SkipGenerated = 0, 0, domain.CommitPageFiles, true
650+ f.reader.EXPECT().GetCommit(ctx, pub.Name, mock.Anything, win).
651+ Return(domain.CommitDetail{Diff: domain.Diff{Files: slices.Clone(files)}}, nil)
652+ i := domain.CommitPageFiles + 1
653+ loaded := slices.Clone(files)
654+ loaded[i].Hunks = []domain.Hunk{{}}
655+ f.reader.EXPECT().GetCommit(ctx, pub.Name, mock.Anything, domain.OneFileDiffLimits(i)).
656+ Return(domain.CommitDetail{Diff: domain.Diff{Files: loaded}}, nil)
657+
658+ d, err := f.browse.GetCommit(ctx, domain.Anonymous(), pub.Name, domain.Revision{}, ports.GetCommitOptions{Page: true, Load: &i})
659+ require.NoError(t, err)
660+ assert.Equal(t, domain.OmitNone, d.Diff.Files[0].Omitted)
661+ assert.Equal(t, domain.OmitBudget, d.Diff.Files[domain.CommitPageFiles].Omitted)
662+ assert.Equal(t, domain.OmitNone, d.Diff.Files[i].Omitted)
663+ assert.NotEmpty(t, d.Diff.Files[i].Hunks)
664+
665+ _, err = f.browse.GetCommit(ctx, domain.Anonymous(), pub.Name, domain.Revision{}, ports.GetCommitOptions{Load: &i})
666+ assert.ErrorIs(t, err, domain.ErrInvalid)
667+ _, err = f.browse.GetCommit(ctx, domain.Anonymous(), pub.Name, domain.Revision{}, ports.GetCommitOptions{Page: true, Files: &ports.FileWindow{To: 1}})
668+ assert.ErrorIs(t, err, domain.ErrInvalid)
669+ })
670+
638671 t.Run("nothing is verified for an empty page or a failed read", func(t *testing.T) {
639672 t.Parallel()
640673 f, _ := newFixture(t) // the verifier mock fails the test if called
@@ -642,7 +675,7 @@ func TestBrowseVerifiesCommits(t *testing.T) {
642675 _, err := f.browse.ListCommits(ctx, domain.Anonymous(), pub.Name, ports.ListCommitsParams{})
643676 require.NoError(t, err)
644677 f.reader.EXPECT().GetCommit(ctx, pub.Name, mock.Anything, mock.Anything).Return(domain.CommitDetail{}, domain.ErrNotFound)
645- _, err = f.browse.GetCommit(ctx, domain.Anonymous(), pub.Name, domain.Revision{})
678+ _, err = f.browse.GetCommit(ctx, domain.Anonymous(), pub.Name, domain.Revision{}, ports.GetCommitOptions{})
646679 assert.ErrorIs(t, err, domain.ErrNotFound)
647680 })
648681
@@ -657,7 +690,7 @@ func TestBrowseVerifiesCommits(t *testing.T) {
657690 page, err := f.browse.ListCommits(ctx, domain.Anonymous(), pub.Name, ports.ListCommitsParams{})
658691 require.NoError(t, err)
659692 assert.Equal(t, domain.CommitVerification{}, page.Commits[0].Verification)
660- d, err := f.browse.GetCommit(ctx, domain.Anonymous(), pub.Name, domain.Revision{})
693+ d, err := f.browse.GetCommit(ctx, domain.Anonymous(), pub.Name, domain.Revision{}, ports.GetCommitOptions{})
661694 require.NoError(t, err)
662695 assert.Equal(t, domain.CommitVerification{}, d.Commit.Verification)
663696 })
@@ -9,6 +9,7 @@ import (
9 "fmt"9 "fmt"
10 "io"10 "io"
11 "runtime"11 "runtime"
12+ "slices"
12 "strconv"13 "strconv"
13 "strings"14 "strings"
14 "sync"15 "sync"
@@ -104,7 +105,7 @@ func browseOps() []browseOp {
104 r.EXPECT().ListCommits(mock.Anything, mock.Anything, mock.Anything).Return(ports.CommitPage{}, nil)105 r.EXPECT().ListCommits(mock.Anything, mock.Anything, mock.Anything).Return(ports.CommitPage{}, nil)
105 }},106 }},
106 {"GetCommit", func(b *usecase.Browse, ctx context.Context, p domain.Principal, repo domain.RepoName) error {107 {"GetCommit", func(b *usecase.Browse, ctx context.Context, p domain.Principal, repo domain.RepoName) error {
107- _, err := b.GetCommit(ctx, p, repo, domain.Revision{})108+ _, err := b.GetCommit(ctx, p, repo, domain.Revision{}, ports.GetCommitOptions{})
108 return err109 return err
109 }, func(r *mocks.RepoReader) {110 }, func(r *mocks.RepoReader) {
110 r.EXPECT().GetCommit(mock.Anything, mock.Anything, mock.Anything, domain.DefaultDiffLimits()).Return(domain.CommitDetail{}, nil)111 r.EXPECT().GetCommit(mock.Anything, mock.Anything, mock.Anything, domain.DefaultDiffLimits()).Return(domain.CommitDetail{}, nil)
@@ -629,12 +630,44 @@ func TestBrowseVerifiesCommits(t *testing.T) {
629 Return(domain.CommitDetail{Commit: c, Stats: domain.DiffStats{FilesChanged: 2}}, nil)630 Return(domain.CommitDetail{Commit: c, Stats: domain.DiffStats{FilesChanged: 2}}, nil)
630 v.EXPECT().VerifyCommits(ctx, pub.Name, []domain.Commit{c}).631 v.EXPECT().VerifyCommits(ctx, pub.Name, []domain.Commit{c}).
631 Return(map[domain.ObjectID]domain.CommitVerification{oid("a"): verified})632 Return(map[domain.ObjectID]domain.CommitVerification{oid("a"): verified})
632- d, err := f.browse.GetCommit(ctx, domain.Anonymous(), pub.Name, domain.Revision{})633+ d, err := f.browse.GetCommit(ctx, domain.Anonymous(), pub.Name, domain.Revision{}, ports.GetCommitOptions{})
633 require.NoError(t, err)634 require.NoError(t, err)
634 assert.Equal(t, verified, d.Commit.Verification)635 assert.Equal(t, verified, d.Commit.Verification)
635 assert.Equal(t, 2, d.Stats.FilesChanged)636 assert.Equal(t, 2, d.Stats.FilesChanged)
636 })637 })
637 638
639+ t.Run("GetCommit Page collapses past the window and loads one file", func(t *testing.T) {
640+ t.Parallel()
641+ f, v := newFixture(t)
642+ v.EXPECT().VerifyCommits(ctx, pub.Name, mock.Anything).Return(nil).Maybe()
643+ f.meta.EXPECT().Get(ctx, pub.Name).Return(pub, nil).Maybe()
644+ files := make([]domain.FileDiff, domain.CommitPageFiles+2)
645+ for i := range files {
646+ files[i].Additions = 1
647+ }
648+ win := domain.DefaultDiffLimits()
649+ win.MaxFiles, win.From, win.To, win.SkipGenerated = 0, 0, domain.CommitPageFiles, true
650+ f.reader.EXPECT().GetCommit(ctx, pub.Name, mock.Anything, win).
651+ Return(domain.CommitDetail{Diff: domain.Diff{Files: slices.Clone(files)}}, nil)
652+ i := domain.CommitPageFiles + 1
653+ loaded := slices.Clone(files)
654+ loaded[i].Hunks = []domain.Hunk{{}}
655+ f.reader.EXPECT().GetCommit(ctx, pub.Name, mock.Anything, domain.OneFileDiffLimits(i)).
656+ Return(domain.CommitDetail{Diff: domain.Diff{Files: loaded}}, nil)
657+
658+ d, err := f.browse.GetCommit(ctx, domain.Anonymous(), pub.Name, domain.Revision{}, ports.GetCommitOptions{Page: true, Load: &i})
659+ require.NoError(t, err)
660+ assert.Equal(t, domain.OmitNone, d.Diff.Files[0].Omitted)
661+ assert.Equal(t, domain.OmitBudget, d.Diff.Files[domain.CommitPageFiles].Omitted)
662+ assert.Equal(t, domain.OmitNone, d.Diff.Files[i].Omitted)
663+ assert.NotEmpty(t, d.Diff.Files[i].Hunks)
664+
665+ _, err = f.browse.GetCommit(ctx, domain.Anonymous(), pub.Name, domain.Revision{}, ports.GetCommitOptions{Load: &i})
666+ assert.ErrorIs(t, err, domain.ErrInvalid)
667+ _, err = f.browse.GetCommit(ctx, domain.Anonymous(), pub.Name, domain.Revision{}, ports.GetCommitOptions{Page: true, Files: &ports.FileWindow{To: 1}})
668+ assert.ErrorIs(t, err, domain.ErrInvalid)
669+ })
670+
638 t.Run("nothing is verified for an empty page or a failed read", func(t *testing.T) {671 t.Run("nothing is verified for an empty page or a failed read", func(t *testing.T) {
639 t.Parallel()672 t.Parallel()
640 f, _ := newFixture(t) // the verifier mock fails the test if called673 f, _ := newFixture(t) // the verifier mock fails the test if called
@@ -642,7 +675,7 @@ func TestBrowseVerifiesCommits(t *testing.T) {
642 _, err := f.browse.ListCommits(ctx, domain.Anonymous(), pub.Name, ports.ListCommitsParams{})675 _, err := f.browse.ListCommits(ctx, domain.Anonymous(), pub.Name, ports.ListCommitsParams{})
643 require.NoError(t, err)676 require.NoError(t, err)
644 f.reader.EXPECT().GetCommit(ctx, pub.Name, mock.Anything, mock.Anything).Return(domain.CommitDetail{}, domain.ErrNotFound)677 f.reader.EXPECT().GetCommit(ctx, pub.Name, mock.Anything, mock.Anything).Return(domain.CommitDetail{}, domain.ErrNotFound)
645- _, err = f.browse.GetCommit(ctx, domain.Anonymous(), pub.Name, domain.Revision{})678+ _, err = f.browse.GetCommit(ctx, domain.Anonymous(), pub.Name, domain.Revision{}, ports.GetCommitOptions{})
646 assert.ErrorIs(t, err, domain.ErrNotFound)679 assert.ErrorIs(t, err, domain.ErrNotFound)
647 })680 })
648 681
@@ -657,7 +690,7 @@ func TestBrowseVerifiesCommits(t *testing.T) {
657 page, err := f.browse.ListCommits(ctx, domain.Anonymous(), pub.Name, ports.ListCommitsParams{})690 page, err := f.browse.ListCommits(ctx, domain.Anonymous(), pub.Name, ports.ListCommitsParams{})
658 require.NoError(t, err)691 require.NoError(t, err)
659 assert.Equal(t, domain.CommitVerification{}, page.Commits[0].Verification)692 assert.Equal(t, domain.CommitVerification{}, page.Commits[0].Verification)
660- d, err := f.browse.GetCommit(ctx, domain.Anonymous(), pub.Name, domain.Revision{})693+ d, err := f.browse.GetCommit(ctx, domain.Anonymous(), pub.Name, domain.Revision{}, ports.GetCommitOptions{})
661 require.NoError(t, err)694 require.NoError(t, err)
662 assert.Equal(t, domain.CommitVerification{}, d.Commit.Verification)695 assert.Equal(t, domain.CommitVerification{}, d.Commit.Verification)
663 })696 })
M internal/usecase/ci_pull_test.go +1 -1
@@ -107,7 +107,7 @@ func TestFilePatch(t *testing.T) {
107107 assert.Equal(t, "@@ -1,2 +1,2 @@ func x()\n a\n-b\n+c\n\\ No newline at end of file\n@@ -9 +9 @@\n+z", got)
108108 _, ok = usecase.FilePatch(domain.FileDiff{Binary: true, Hunks: f.Hunks})
109109 assert.False(t, ok, "binary")
110- _, ok = usecase.FilePatch(domain.FileDiff{Truncated: true})
110+ _, ok = usecase.FilePatch(domain.FileDiff{Omitted: domain.OmitLarge})
111111 assert.False(t, ok, "truncated")
112112 _, ok = usecase.FilePatch(domain.FileDiff{Status: domain.FileRenamed})
113113 assert.False(t, ok, "a pure rename has no hunk")
@@ -107,7 +107,7 @@ func TestFilePatch(t *testing.T) {
107 assert.Equal(t, "@@ -1,2 +1,2 @@ func x()\n a\n-b\n+c\n\\ No newline at end of file\n@@ -9 +9 @@\n+z", got)107 assert.Equal(t, "@@ -1,2 +1,2 @@ func x()\n a\n-b\n+c\n\\ No newline at end of file\n@@ -9 +9 @@\n+z", got)
108 _, ok = usecase.FilePatch(domain.FileDiff{Binary: true, Hunks: f.Hunks})108 _, ok = usecase.FilePatch(domain.FileDiff{Binary: true, Hunks: f.Hunks})
109 assert.False(t, ok, "binary")109 assert.False(t, ok, "binary")
110- _, ok = usecase.FilePatch(domain.FileDiff{Truncated: true})110+ _, ok = usecase.FilePatch(domain.FileDiff{Omitted: domain.OmitLarge})
111 assert.False(t, ok, "truncated")111 assert.False(t, ok, "truncated")
112 _, ok = usecase.FilePatch(domain.FileDiff{Status: domain.FileRenamed})112 _, ok = usecase.FilePatch(domain.FileDiff{Status: domain.FileRenamed})
113 assert.False(t, ok, "a pure rename has no hunk")113 assert.False(t, ok, "a pure rename has no hunk")
M internal/usecase/ci_pullfiles.go +1 -1
@@ -212,7 +212,7 @@ func escapeURLPath(p string) string {
212212 // (hunk headers and lines, no file headers). Absent for a binary file, a pure
213213 // rename and a file too large for the diff limits, as on GitHub.
214214 func filePatch(f domain.FileDiff) (string, bool) {
215- if f.Binary || f.Truncated || len(f.Hunks) == 0 {
215+ if f.Binary || f.Omitted != domain.OmitNone || len(f.Hunks) == 0 {
216216 return "", false
217217 }
218218 var b strings.Builder
@@ -212,7 +212,7 @@ func escapeURLPath(p string) string {
212 // (hunk headers and lines, no file headers). Absent for a binary file, a pure212 // (hunk headers and lines, no file headers). Absent for a binary file, a pure
213 // rename and a file too large for the diff limits, as on GitHub.213 // rename and a file too large for the diff limits, as on GitHub.
214 func filePatch(f domain.FileDiff) (string, bool) {214 func filePatch(f domain.FileDiff) (string, bool) {
215- if f.Binary || f.Truncated || len(f.Hunks) == 0 {215+ if f.Binary || f.Omitted != domain.OmitNone || len(f.Hunks) == 0 {
216 return "", false216 return "", false
217 }217 }
218 var b strings.Builder218 var b strings.Builder
M internal/usecase/export_test.go +3 -0
@@ -217,3 +217,6 @@ func (s *Mergeability) RetryDueForTest(repo domain.RepoName) (time.Time, bool) {
217217 func (s *Mergeability) PopDueForTest(repo domain.RepoName, due time.Time) []domain.RepoName {
218218 return s.popDue(retryItem{key: repo, due: due})
219219 }
220+
221+// WindowLimits exposes windowLimits to the tests.
222+var WindowLimits = windowLimits
@@ -217,3 +217,6 @@ func (s *Mergeability) RetryDueForTest(repo domain.RepoName) (time.Time, bool) {
217 func (s *Mergeability) PopDueForTest(repo domain.RepoName, due time.Time) []domain.RepoName {217 func (s *Mergeability) PopDueForTest(repo domain.RepoName, due time.Time) []domain.RepoName {
218 return s.popDue(retryItem{key: repo, due: due})218 return s.popDue(retryItem{key: repo, due: due})
219 }219 }
220+
221+// WindowLimits exposes windowLimits to the tests.
222+var WindowLimits = windowLimits
M internal/usecase/mergerequests.go +42 -19
@@ -493,6 +493,26 @@ func (m *MergeRequests) get(ctx context.Context, p domain.Principal, name domain
493493 return repo, mr, nil
494494 }
495495
496+// windowLimits is the diff limits of a read: nil gives
497+// domain.DefaultDiffLimits (the API contract, generated files read); a window
498+// lists every file (MaxFiles 0), reads From..To and skips generated files; a
499+// Full window reads the one file From with domain.OneFileDiffLimits. An invalid
500+// window (negative, To < From, Full without To == From+1) is domain.ErrInvalid.
501+func windowLimits(w *ports.FileWindow) (domain.DiffLimits, error) {
502+ if w == nil {
503+ return domain.DefaultDiffLimits(), nil
504+ }
505+ if w.From < 0 || w.To < 0 || (w.To != 0 && w.To < w.From) || (w.Full && w.To != w.From+1) {
506+ return domain.DiffLimits{}, fmt.Errorf("%w: file window [%d, %d) full=%t", domain.ErrInvalid, w.From, w.To, w.Full)
507+ }
508+ if w.Full {
509+ return domain.OneFileDiffLimits(w.From), nil
510+ }
511+ l := domain.DefaultDiffLimits()
512+ l.MaxFiles, l.From, l.To, l.SkipGenerated = 0, w.From, w.To, true
513+ return l, nil
514+}
515+
496516 // Get returns a merge request with its commits, its diff, its comments and the
497517 // read position. The diff compares the merge base of the target and the head
498518 // with the head ("target...head"), within domain.DefaultDiffLimits. With
@@ -507,12 +527,9 @@ func (m *MergeRequests) get(ctx context.Context, p domain.Principal, name domain
507527 // diff is no longer the one the caller shows (before any diff is read);
508528 // opts.DiffOnly skips the commits. An invalid window gives domain.ErrInvalid.
509529 func (m *MergeRequests) Get(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, opts ports.GetMergeRequestOptions) (ports.MergeRequestDetail, error) {
510- limits := domain.DefaultDiffLimits()
511- if w := opts.Files; w != nil {
512- if w.From < 0 || w.To < 0 || (w.To != 0 && w.To < w.From) {
513- return ports.MergeRequestDetail{}, fmt.Errorf("%w: file window [%d, %d)", domain.ErrInvalid, w.From, w.To)
514- }
515- limits.MaxFiles, limits.From, limits.To = 0, w.From, w.To
530+ limits, err := windowLimits(opts.Files)
531+ if err != nil {
532+ return ports.MergeRequestDetail{}, err
516533 }
517534 _, mr, err := m.get(ctx, p, name, number)
518535 if err != nil {
@@ -604,6 +621,16 @@ func (m *MergeRequests) Get(ctx context.Context, p domain.Principal, name domain
604621 if d.Stats, d.Diff, err = m.git.DiffCommits(ctx, name, base, head, limits); err != nil {
605622 return ports.MergeRequestDetail{}, err
606623 }
624+ // The page of a client without scripts reads one collapsed file in full.
625+ if i := opts.Load; i != nil && *i >= 0 && *i < len(d.Diff.Files) && d.Diff.Files[*i].Omitted.Collapsed() {
626+ _, one, err := m.git.DiffCommits(ctx, name, base, head, domain.OneFileDiffLimits(*i))
627+ if err != nil {
628+ return ports.MergeRequestDetail{}, err
629+ }
630+ if *i < len(one.Files) {
631+ d.Diff.Files[*i] = one.Files[*i]
632+ }
633+ }
607634 return d, nil
608635 }
609636
@@ -763,18 +790,22 @@ func (m *MergeRequests) checkLine(ctx context.Context, mr domain.MergeRequest, p
763790 // file's hunks within the default total limits would reject a comment on a
764791 // file the earlier ones left no budget for. So a first read lists the files
765792 // with no hunks (From past the end: cheap, git is stopped at once), and a
766- // second reads the hunks of the one file within the default per-file limit
767- // and total limits, as the web view does for a window holding it.
793+ // second loads the one file in full (a Full window: the byte guard and
794+ // domain.MaxOneFileLines only, generated files included), as the web view
795+ // does when it loads a collapsed file: any line the page can show can be
796+ // commented on, and a file over those limits takes no line comment.
768797 list := domain.DefaultDiffLimits()
769798 list.MaxFiles, list.From = 0, math.MaxInt
770799 _, diff, err := m.git.DiffCommits(ctx, mr.Repo, base, mr.HeadID, list)
771800 if err != nil {
772801 return err
773802 }
774- idx := slices.IndexFunc(diff.Files, func(f domain.FileDiff) bool { return diffFileName(f) == params.Path })
803+ idx := slices.IndexFunc(diff.Files, func(f domain.FileDiff) bool { return f.CommentPath() == params.Path })
775804 if idx >= 0 {
776- one := domain.DefaultDiffLimits()
777- one.MaxFiles, one.From, one.To = 0, idx, idx+1
805+ one, err := windowLimits(&ports.FileWindow{From: idx, To: idx + 1, Full: true})
806+ if err != nil {
807+ return err
808+ }
778809 if _, diff, err = m.git.DiffCommits(ctx, mr.Repo, base, mr.HeadID, one); err != nil {
779810 return err
780811 }
@@ -788,14 +819,6 @@ func (m *MergeRequests) checkLine(ctx context.Context, mr domain.MergeRequest, p
788819 return v.Err()
789820 }
790821
791-// diffFileName is the path a comment designates for f, as Diff.HasLine reads it.
792-func diffFileName(f domain.FileDiff) string {
793- if f.Status == domain.FileDeleted {
794- return f.OldPath
795- }
796- return f.NewPath
797-}
798-
799822 // ResolveComment marks a comment resolved or unresolved. The principal needs
800823 // mr:write. An unknown comment gives domain.ErrNotFound and a request that is
801824 // not open domain.ErrPreconditionFailed. Setting the state the comment already
@@ -493,6 +493,26 @@ func (m *MergeRequests) get(ctx context.Context, p domain.Principal, name domain
493 return repo, mr, nil493 return repo, mr, nil
494 }494 }
495 495
496+// windowLimits is the diff limits of a read: nil gives
497+// domain.DefaultDiffLimits (the API contract, generated files read); a window
498+// lists every file (MaxFiles 0), reads From..To and skips generated files; a
499+// Full window reads the one file From with domain.OneFileDiffLimits. An invalid
500+// window (negative, To < From, Full without To == From+1) is domain.ErrInvalid.
501+func windowLimits(w *ports.FileWindow) (domain.DiffLimits, error) {
502+ if w == nil {
503+ return domain.DefaultDiffLimits(), nil
504+ }
505+ if w.From < 0 || w.To < 0 || (w.To != 0 && w.To < w.From) || (w.Full && w.To != w.From+1) {
506+ return domain.DiffLimits{}, fmt.Errorf("%w: file window [%d, %d) full=%t", domain.ErrInvalid, w.From, w.To, w.Full)
507+ }
508+ if w.Full {
509+ return domain.OneFileDiffLimits(w.From), nil
510+ }
511+ l := domain.DefaultDiffLimits()
512+ l.MaxFiles, l.From, l.To, l.SkipGenerated = 0, w.From, w.To, true
513+ return l, nil
514+}
515+
496 // Get returns a merge request with its commits, its diff, its comments and the516 // Get returns a merge request with its commits, its diff, its comments and the
497 // read position. The diff compares the merge base of the target and the head517 // read position. The diff compares the merge base of the target and the head
498 // with the head ("target...head"), within domain.DefaultDiffLimits. With518 // with the head ("target...head"), within domain.DefaultDiffLimits. With
@@ -507,12 +527,9 @@ func (m *MergeRequests) get(ctx context.Context, p domain.Principal, name domain
507 // diff is no longer the one the caller shows (before any diff is read);527 // diff is no longer the one the caller shows (before any diff is read);
508 // opts.DiffOnly skips the commits. An invalid window gives domain.ErrInvalid.528 // opts.DiffOnly skips the commits. An invalid window gives domain.ErrInvalid.
509 func (m *MergeRequests) Get(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, opts ports.GetMergeRequestOptions) (ports.MergeRequestDetail, error) {529 func (m *MergeRequests) Get(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, opts ports.GetMergeRequestOptions) (ports.MergeRequestDetail, error) {
510- limits := domain.DefaultDiffLimits()530+ limits, err := windowLimits(opts.Files)
511- if w := opts.Files; w != nil {531+ if err != nil {
512- if w.From < 0 || w.To < 0 || (w.To != 0 && w.To < w.From) {532+ return ports.MergeRequestDetail{}, err
513- return ports.MergeRequestDetail{}, fmt.Errorf("%w: file window [%d, %d)", domain.ErrInvalid, w.From, w.To)
514- }
515- limits.MaxFiles, limits.From, limits.To = 0, w.From, w.To
516 }533 }
517 _, mr, err := m.get(ctx, p, name, number)534 _, mr, err := m.get(ctx, p, name, number)
518 if err != nil {535 if err != nil {
@@ -604,6 +621,16 @@ func (m *MergeRequests) Get(ctx context.Context, p domain.Principal, name domain
604 if d.Stats, d.Diff, err = m.git.DiffCommits(ctx, name, base, head, limits); err != nil {621 if d.Stats, d.Diff, err = m.git.DiffCommits(ctx, name, base, head, limits); err != nil {
605 return ports.MergeRequestDetail{}, err622 return ports.MergeRequestDetail{}, err
606 }623 }
624+ // The page of a client without scripts reads one collapsed file in full.
625+ if i := opts.Load; i != nil && *i >= 0 && *i < len(d.Diff.Files) && d.Diff.Files[*i].Omitted.Collapsed() {
626+ _, one, err := m.git.DiffCommits(ctx, name, base, head, domain.OneFileDiffLimits(*i))
627+ if err != nil {
628+ return ports.MergeRequestDetail{}, err
629+ }
630+ if *i < len(one.Files) {
631+ d.Diff.Files[*i] = one.Files[*i]
632+ }
633+ }
607 return d, nil634 return d, nil
608 }635 }
609 636
@@ -763,18 +790,22 @@ func (m *MergeRequests) checkLine(ctx context.Context, mr domain.MergeRequest, p
763 // file's hunks within the default total limits would reject a comment on a790 // file's hunks within the default total limits would reject a comment on a
764 // file the earlier ones left no budget for. So a first read lists the files791 // file the earlier ones left no budget for. So a first read lists the files
765 // with no hunks (From past the end: cheap, git is stopped at once), and a792 // with no hunks (From past the end: cheap, git is stopped at once), and a
766- // second reads the hunks of the one file within the default per-file limit793+ // second loads the one file in full (a Full window: the byte guard and
767- // and total limits, as the web view does for a window holding it.794+ // domain.MaxOneFileLines only, generated files included), as the web view
795+ // does when it loads a collapsed file: any line the page can show can be
796+ // commented on, and a file over those limits takes no line comment.
768 list := domain.DefaultDiffLimits()797 list := domain.DefaultDiffLimits()
769 list.MaxFiles, list.From = 0, math.MaxInt798 list.MaxFiles, list.From = 0, math.MaxInt
770 _, diff, err := m.git.DiffCommits(ctx, mr.Repo, base, mr.HeadID, list)799 _, diff, err := m.git.DiffCommits(ctx, mr.Repo, base, mr.HeadID, list)
771 if err != nil {800 if err != nil {
772 return err801 return err
773 }802 }
774- idx := slices.IndexFunc(diff.Files, func(f domain.FileDiff) bool { return diffFileName(f) == params.Path })803+ idx := slices.IndexFunc(diff.Files, func(f domain.FileDiff) bool { return f.CommentPath() == params.Path })
775 if idx >= 0 {804 if idx >= 0 {
776- one := domain.DefaultDiffLimits()805+ one, err := windowLimits(&ports.FileWindow{From: idx, To: idx + 1, Full: true})
777- one.MaxFiles, one.From, one.To = 0, idx, idx+1806+ if err != nil {
807+ return err
808+ }
778 if _, diff, err = m.git.DiffCommits(ctx, mr.Repo, base, mr.HeadID, one); err != nil {809 if _, diff, err = m.git.DiffCommits(ctx, mr.Repo, base, mr.HeadID, one); err != nil {
779 return err810 return err
780 }811 }
@@ -788,14 +819,6 @@ func (m *MergeRequests) checkLine(ctx context.Context, mr domain.MergeRequest, p
788 return v.Err()819 return v.Err()
789 }820 }
790 821
791-// diffFileName is the path a comment designates for f, as Diff.HasLine reads it.
792-func diffFileName(f domain.FileDiff) string {
793- if f.Status == domain.FileDeleted {
794- return f.OldPath
795- }
796- return f.NewPath
797-}
798-
799 // ResolveComment marks a comment resolved or unresolved. The principal needs822 // ResolveComment marks a comment resolved or unresolved. The principal needs
800 // mr:write. An unknown comment gives domain.ErrNotFound and a request that is823 // mr:write. An unknown comment gives domain.ErrNotFound and a request that is
801 // not open domain.ErrPreconditionFailed. Setting the state the comment already824 // not open domain.ErrPreconditionFailed. Setting the state the comment already
M internal/usecase/mergerequests_files_test.go +1 -1
@@ -63,7 +63,7 @@ func TestGetMergeRequestFilesWindow(t *testing.T) {
6363 for i, f := range d.Diff.Files {
6464 assert.Equal(t, fmt.Sprintf("m/f%03d", i), f.NewPath)
6565 assert.Equal(t, 1, f.Additions)
66- assert.False(t, f.Truncated)
66+ assert.Equal(t, domain.OmitNone, f.Omitted)
6767 if i >= 305 && i < 310 {
6868 require.Len(t, f.Hunks, 1, f.NewPath)
6969 assert.Equal(t, fmt.Sprintf("line %d", i), f.Hunks[0].Lines[0].Text)
@@ -63,7 +63,7 @@ func TestGetMergeRequestFilesWindow(t *testing.T) {
63 for i, f := range d.Diff.Files {63 for i, f := range d.Diff.Files {
64 assert.Equal(t, fmt.Sprintf("m/f%03d", i), f.NewPath)64 assert.Equal(t, fmt.Sprintf("m/f%03d", i), f.NewPath)
65 assert.Equal(t, 1, f.Additions)65 assert.Equal(t, 1, f.Additions)
66- assert.False(t, f.Truncated)66+ assert.Equal(t, domain.OmitNone, f.Omitted)
67 if i >= 305 && i < 310 {67 if i >= 305 && i < 310 {
68 require.Len(t, f.Hunks, 1, f.NewPath)68 require.Len(t, f.Hunks, 1, f.NewPath)
69 assert.Equal(t, fmt.Sprintf("line %d", i), f.Hunks[0].Lines[0].Text)69 assert.Equal(t, fmt.Sprintf("line %d", i), f.Hunks[0].Lines[0].Text)
A internal/usecase/window_test.go +166 -0
@@ -0,0 +1,166 @@
1+// SPDX-License-Identifier: AGPL-3.0-or-later
2+
3+package usecase_test
4+
5+import (
6+ "fmt"
7+ "strings"
8+ "testing"
9+
10+ "github.com/stretchr/testify/assert"
11+ "github.com/stretchr/testify/require"
12+
13+ "bnema.dev/ermite/internal/domain"
14+ "bnema.dev/ermite/internal/ports"
15+ "bnema.dev/ermite/internal/usecase"
16+)
17+
18+func TestWindowLimits(t *testing.T) {
19+ def := domain.DefaultDiffLimits()
20+ win := def
21+ win.MaxFiles, win.From, win.To, win.SkipGenerated = 0, 2, 5, true
22+ open := win
23+ open.From, open.To = 7, 0
24+
25+ tests := []struct {
26+ name string
27+ w *ports.FileWindow
28+ want domain.DiffLimits
29+ bad bool
30+ }{
31+ {name: "nil is the default", want: def},
32+ {name: "a window", w: &ports.FileWindow{From: 2, To: 5}, want: win},
33+ {name: "an open window", w: &ports.FileWindow{From: 7}, want: open},
34+ {name: "full", w: &ports.FileWindow{From: 3, To: 4, Full: true}, want: domain.OneFileDiffLimits(3)},
35+ {name: "negative from", w: &ports.FileWindow{From: -1}, bad: true},
36+ {name: "negative to", w: &ports.FileWindow{To: -1}, bad: true},
37+ {name: "to before from", w: &ports.FileWindow{From: 5, To: 3}, bad: true},
38+ {name: "full without to", w: &ports.FileWindow{From: 3, Full: true}, bad: true},
39+ {name: "full of two files", w: &ports.FileWindow{From: 3, To: 5, Full: true}, bad: true},
40+ {name: "full of no file", w: &ports.FileWindow{From: 3, To: 3, Full: true}, bad: true},
41+ }
42+ for _, tt := range tests {
43+ t.Run(tt.name, func(t *testing.T) {
44+ got, err := usecase.WindowLimits(tt.w)
45+ if tt.bad {
46+ require.ErrorIs(t, err, domain.ErrInvalid)
47+ return
48+ }
49+ require.NoError(t, err)
50+ assert.Equal(t, tt.want, got)
51+ })
52+ }
53+}
54+
55+func TestGetMergeRequestFullAndGenerated(t *testing.T) {
56+ r := newMRRig(t)
57+ r.seed()
58+ r.sh("checkout", "-q", "-B", "feat", "main")
59+ r.commit(author, "a.txt", "one\n", "a")
60+ r.commit(author, "go.sum", "x y z\n", "sum")
61+ r.push("feat")
62+ n := r.open("feat").Number
63+
64+ t.Run("full with a wider window is invalid", func(t *testing.T) {
65+ _, err := r.mrs.Get(r.ctx, reader, r.repo, n, ports.GetMergeRequestOptions{Files: &ports.FileWindow{From: 0, To: 2, Full: true}})
66+ require.ErrorIs(t, err, domain.ErrInvalid)
67+ })
68+
69+ t.Run("a windowed read collapses a generated file, a nil read does not", func(t *testing.T) {
70+ d, err := r.mrs.Get(r.ctx, reader, r.repo, n, ports.GetMergeRequestOptions{Files: &ports.FileWindow{From: 0, To: 10}})
71+ require.NoError(t, err)
72+ byPath := map[string]domain.FileDiff{}
73+ for _, f := range d.Diff.Files {
74+ byPath[f.NewPath] = f
75+ }
76+ assert.Equal(t, domain.OmitGenerated, byPath["go.sum"].Omitted)
77+ assert.Empty(t, byPath["go.sum"].Hunks)
78+ assert.Equal(t, domain.OmitNone, byPath["a.txt"].Omitted)
79+ assert.NotEmpty(t, byPath["a.txt"].Hunks)
80+
81+ d, err = r.mrs.Get(r.ctx, reader, r.repo, n, ports.GetMergeRequestOptions{})
82+ require.NoError(t, err)
83+ for _, f := range d.Diff.Files {
84+ assert.Equal(t, domain.OmitNone, f.Omitted, f.NewPath)
85+ assert.NotEmpty(t, f.Hunks, f.NewPath)
86+ }
87+ })
88+}
89+
90+func bigContent(n int) string {
91+ var b strings.Builder
92+ for i := range n {
93+ fmt.Fprintf(&b, "line %d\n", i)
94+ }
95+ return b.String()
96+}
97+
98+func TestCheckLineBeyondFileLimit(t *testing.T) {
99+ r := newMRRig(t)
100+ r.seed()
101+ r.sh("checkout", "-q", "-B", "feat", "main")
102+ r.commit(author, "big.txt", bigContent(3000), "big")
103+ r.commit(author, "go.sum", "h1:x\nh1:y\n", "sum")
104+ r.push("feat")
105+ n := r.open("feat").Number
106+
107+ add := func(path string, line int) error {
108+ _, err := r.mrs.AddComment(r.ctx, mrW, r.repo, n, ports.AddCommentParams{Path: path, Line: line, Side: domain.SideNew, Text: "c"})
109+ return err
110+ }
111+ require.NoError(t, add("big.txt", 1))
112+ require.NoError(t, add("big.txt", 2500))
113+ require.NoError(t, add("big.txt", 3000))
114+ require.ErrorIs(t, add("big.txt", 3001), domain.ErrInvalid)
115+ // A generated file is collapsed in the web windows, but loadable: its
116+ // lines can be commented on.
117+ require.NoError(t, add("go.sum", 2))
118+}
119+
120+func TestBrowseGetCommitFullWindow(t *testing.T) {
121+ r := newMRRig(t)
122+ r.seed()
123+ r.commit(author, "a-big.txt", bigContent(3000), "big")
124+ id := r.head()
125+ r.push("main")
126+ b := usecase.NewBrowse(r.store, r.git)
127+ rev := domain.RevisionOf(id)
128+
129+ d, err := b.GetCommit(r.ctx, root, r.repo, rev, ports.GetCommitOptions{})
130+ require.NoError(t, err)
131+ require.Len(t, d.Diff.Files, 1)
132+ assert.Equal(t, domain.OmitLarge, d.Diff.Files[0].Omitted)
133+ assert.Empty(t, d.Diff.Files[0].Hunks)
134+
135+ d, err = b.GetCommit(r.ctx, root, r.repo, rev, ports.GetCommitOptions{Files: &ports.FileWindow{From: 0, To: 1, Full: true}})
136+ require.NoError(t, err)
137+ require.Len(t, d.Diff.Files, 1)
138+ f := d.Diff.Files[0]
139+ assert.Equal(t, domain.OmitNone, f.Omitted)
140+ require.NotEmpty(t, f.Hunks)
141+ assert.Equal(t, 3000, f.Additions)
142+
143+ _, err = b.GetCommit(r.ctx, root, r.repo, rev, ports.GetCommitOptions{Files: &ports.FileWindow{From: 0, To: 2, Full: true}})
144+ require.ErrorIs(t, err, domain.ErrInvalid)
145+}
146+
147+func TestBrowseGetCommitWindowCollapsesGenerated(t *testing.T) {
148+ r := newMRRig(t)
149+ r.seed()
150+ r.commit(author, "go.sum", "h1:x\n", "sum")
151+ id := r.head()
152+ r.push("main")
153+ b := usecase.NewBrowse(r.store, r.git)
154+ rev := domain.RevisionOf(id)
155+
156+ d, err := b.GetCommit(r.ctx, root, r.repo, rev, ports.GetCommitOptions{})
157+ require.NoError(t, err)
158+ require.Len(t, d.Diff.Files, 1)
159+ assert.Equal(t, domain.OmitNone, d.Diff.Files[0].Omitted, "API reads keep generated files")
160+
161+ d, err = b.GetCommit(r.ctx, root, r.repo, rev, ports.GetCommitOptions{Files: &ports.FileWindow{From: 0, To: 25}})
162+ require.NoError(t, err)
163+ require.Len(t, d.Diff.Files, 1)
164+ assert.Equal(t, domain.OmitGenerated, d.Diff.Files[0].Omitted, "web windows collapse them")
165+ assert.Empty(t, d.Diff.Files[0].Hunks)
166+}
@@ -0,0 +1,166 @@
1+// SPDX-License-Identifier: AGPL-3.0-or-later
2+
3+package usecase_test
4+
5+import (
6+ "fmt"
7+ "strings"
8+ "testing"
9+
10+ "github.com/stretchr/testify/assert"
11+ "github.com/stretchr/testify/require"
12+
13+ "bnema.dev/ermite/internal/domain"
14+ "bnema.dev/ermite/internal/ports"
15+ "bnema.dev/ermite/internal/usecase"
16+)
17+
18+func TestWindowLimits(t *testing.T) {
19+ def := domain.DefaultDiffLimits()
20+ win := def
21+ win.MaxFiles, win.From, win.To, win.SkipGenerated = 0, 2, 5, true
22+ open := win
23+ open.From, open.To = 7, 0
24+
25+ tests := []struct {
26+ name string
27+ w *ports.FileWindow
28+ want domain.DiffLimits
29+ bad bool
30+ }{
31+ {name: "nil is the default", want: def},
32+ {name: "a window", w: &ports.FileWindow{From: 2, To: 5}, want: win},
33+ {name: "an open window", w: &ports.FileWindow{From: 7}, want: open},
34+ {name: "full", w: &ports.FileWindow{From: 3, To: 4, Full: true}, want: domain.OneFileDiffLimits(3)},
35+ {name: "negative from", w: &ports.FileWindow{From: -1}, bad: true},
36+ {name: "negative to", w: &ports.FileWindow{To: -1}, bad: true},
37+ {name: "to before from", w: &ports.FileWindow{From: 5, To: 3}, bad: true},
38+ {name: "full without to", w: &ports.FileWindow{From: 3, Full: true}, bad: true},
39+ {name: "full of two files", w: &ports.FileWindow{From: 3, To: 5, Full: true}, bad: true},
40+ {name: "full of no file", w: &ports.FileWindow{From: 3, To: 3, Full: true}, bad: true},
41+ }
42+ for _, tt := range tests {
43+ t.Run(tt.name, func(t *testing.T) {
44+ got, err := usecase.WindowLimits(tt.w)
45+ if tt.bad {
46+ require.ErrorIs(t, err, domain.ErrInvalid)
47+ return
48+ }
49+ require.NoError(t, err)
50+ assert.Equal(t, tt.want, got)
51+ })
52+ }
53+}
54+
55+func TestGetMergeRequestFullAndGenerated(t *testing.T) {
56+ r := newMRRig(t)
57+ r.seed()
58+ r.sh("checkout", "-q", "-B", "feat", "main")
59+ r.commit(author, "a.txt", "one\n", "a")
60+ r.commit(author, "go.sum", "x y z\n", "sum")
61+ r.push("feat")
62+ n := r.open("feat").Number
63+
64+ t.Run("full with a wider window is invalid", func(t *testing.T) {
65+ _, err := r.mrs.Get(r.ctx, reader, r.repo, n, ports.GetMergeRequestOptions{Files: &ports.FileWindow{From: 0, To: 2, Full: true}})
66+ require.ErrorIs(t, err, domain.ErrInvalid)
67+ })
68+
69+ t.Run("a windowed read collapses a generated file, a nil read does not", func(t *testing.T) {
70+ d, err := r.mrs.Get(r.ctx, reader, r.repo, n, ports.GetMergeRequestOptions{Files: &ports.FileWindow{From: 0, To: 10}})
71+ require.NoError(t, err)
72+ byPath := map[string]domain.FileDiff{}
73+ for _, f := range d.Diff.Files {
74+ byPath[f.NewPath] = f
75+ }
76+ assert.Equal(t, domain.OmitGenerated, byPath["go.sum"].Omitted)
77+ assert.Empty(t, byPath["go.sum"].Hunks)
78+ assert.Equal(t, domain.OmitNone, byPath["a.txt"].Omitted)
79+ assert.NotEmpty(t, byPath["a.txt"].Hunks)
80+
81+ d, err = r.mrs.Get(r.ctx, reader, r.repo, n, ports.GetMergeRequestOptions{})
82+ require.NoError(t, err)
83+ for _, f := range d.Diff.Files {
84+ assert.Equal(t, domain.OmitNone, f.Omitted, f.NewPath)
85+ assert.NotEmpty(t, f.Hunks, f.NewPath)
86+ }
87+ })
88+}
89+
90+func bigContent(n int) string {
91+ var b strings.Builder
92+ for i := range n {
93+ fmt.Fprintf(&b, "line %d\n", i)
94+ }
95+ return b.String()
96+}
97+
98+func TestCheckLineBeyondFileLimit(t *testing.T) {
99+ r := newMRRig(t)
100+ r.seed()
101+ r.sh("checkout", "-q", "-B", "feat", "main")
102+ r.commit(author, "big.txt", bigContent(3000), "big")
103+ r.commit(author, "go.sum", "h1:x\nh1:y\n", "sum")
104+ r.push("feat")
105+ n := r.open("feat").Number
106+
107+ add := func(path string, line int) error {
108+ _, err := r.mrs.AddComment(r.ctx, mrW, r.repo, n, ports.AddCommentParams{Path: path, Line: line, Side: domain.SideNew, Text: "c"})
109+ return err
110+ }
111+ require.NoError(t, add("big.txt", 1))
112+ require.NoError(t, add("big.txt", 2500))
113+ require.NoError(t, add("big.txt", 3000))
114+ require.ErrorIs(t, add("big.txt", 3001), domain.ErrInvalid)
115+ // A generated file is collapsed in the web windows, but loadable: its
116+ // lines can be commented on.
117+ require.NoError(t, add("go.sum", 2))
118+}
119+
120+func TestBrowseGetCommitFullWindow(t *testing.T) {
121+ r := newMRRig(t)
122+ r.seed()
123+ r.commit(author, "a-big.txt", bigContent(3000), "big")
124+ id := r.head()
125+ r.push("main")
126+ b := usecase.NewBrowse(r.store, r.git)
127+ rev := domain.RevisionOf(id)
128+
129+ d, err := b.GetCommit(r.ctx, root, r.repo, rev, ports.GetCommitOptions{})
130+ require.NoError(t, err)
131+ require.Len(t, d.Diff.Files, 1)
132+ assert.Equal(t, domain.OmitLarge, d.Diff.Files[0].Omitted)
133+ assert.Empty(t, d.Diff.Files[0].Hunks)
134+
135+ d, err = b.GetCommit(r.ctx, root, r.repo, rev, ports.GetCommitOptions{Files: &ports.FileWindow{From: 0, To: 1, Full: true}})
136+ require.NoError(t, err)
137+ require.Len(t, d.Diff.Files, 1)
138+ f := d.Diff.Files[0]
139+ assert.Equal(t, domain.OmitNone, f.Omitted)
140+ require.NotEmpty(t, f.Hunks)
141+ assert.Equal(t, 3000, f.Additions)
142+
143+ _, err = b.GetCommit(r.ctx, root, r.repo, rev, ports.GetCommitOptions{Files: &ports.FileWindow{From: 0, To: 2, Full: true}})
144+ require.ErrorIs(t, err, domain.ErrInvalid)
145+}
146+
147+func TestBrowseGetCommitWindowCollapsesGenerated(t *testing.T) {
148+ r := newMRRig(t)
149+ r.seed()
150+ r.commit(author, "go.sum", "h1:x\n", "sum")
151+ id := r.head()
152+ r.push("main")
153+ b := usecase.NewBrowse(r.store, r.git)
154+ rev := domain.RevisionOf(id)
155+
156+ d, err := b.GetCommit(r.ctx, root, r.repo, rev, ports.GetCommitOptions{})
157+ require.NoError(t, err)
158+ require.Len(t, d.Diff.Files, 1)
159+ assert.Equal(t, domain.OmitNone, d.Diff.Files[0].Omitted, "API reads keep generated files")
160+
161+ d, err = b.GetCommit(r.ctx, root, r.repo, rev, ports.GetCommitOptions{Files: &ports.FileWindow{From: 0, To: 25}})
162+ require.NoError(t, err)
163+ require.Len(t, d.Diff.Files, 1)
164+ assert.Equal(t, domain.OmitGenerated, d.Diff.Files[0].Omitted, "web windows collapse them")
165+ assert.Empty(t, d.Diff.Files[0].Hunks)
166+}

Comments0 open / 0

no comments yet.

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
l
merge request: the list of files (j k move, enter opens, esc back)
M
merge request: the merge menu; there 1–3 pick the mode, D deletes the branch, enter merges
R
merge request: mark the head read
esc
close a dialog, leave a field

Branches 2

Tags 44

↑↓ moveenter openesc close