refactor(web): share the fragment layout rule and document the API omission reasons
- commit
- 88a1a5619e1adf78a8d35e55659968ad98681c17
- author
- brice · · 2026-10-20 08:26 UTC
- committer
- brice · · 2026-10-20 09:16 UTC
- parent
- de918e2
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +12 -9
M docs/merge-requests.md +2 -0
| @@ -69,6 +69,8 @@ The merge request, commit and compare pages list every changed file. Generated f | |||
| 69 | 69 | ||
| 70 | 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, and its lines take no comment. | |
| 71 | 71 | ||
| 72 | + | The API and `mr show --diff --json` keep their limits (300 files, 2,000 changed lines per file, 20,000 in total, 8 MiB). A file read without its hunks has `truncated` set and `omitted` giving the reason: `large`, `budget` (the earlier files used the limits) or `unavailable` (too large to read even alone). | |
| 73 | + | ||
| 72 | 74 | ## Notes | |
| 73 | 75 | ||
| 74 | 76 | A note is the only review object. It is published as soon as it is written: there is no draft and no submit step. | |
| @@ -69,6 +69,8 @@ The merge request, commit and compare pages list every changed file. Generated f | |||||
| 69 | 69 | ||||
| 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, and its lines take no comment. | 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, and its lines take no comment. | ||
| 71 | 71 | ||||
| 72 | + | The API and `mr show --diff --json` keep their limits (300 files, 2,000 changed lines per file, 20,000 in total, 8 MiB). A file read without its hunks has `truncated` set and `omitted` giving the reason: `large`, `budget` (the earlier files used the limits) or `unavailable` (too large to read even alone). | |||
| 73 | + | ||||
| 72 | ## Notes | 74 | ## Notes | ||
| 73 | 75 | ||||
| 74 | A note is the only review object. It is published as soon as it is written: there is no draft and no submit step. | 76 | A note is the only review object. It is published as soon as it is written: there is no draft and no submit step. | ||
M internal/adapters/web/diff.go +9 -2
| @@ -380,7 +380,7 @@ func diffFiles(files []domain.FileDiff) []DiffFile { | |||
| 380 | 380 | ID: "f" + itoa(i+1), Status: statusLetter(f.Status), | |
| 381 | 381 | Add: f.Additions, Del: f.Deletions, Bin: f.Binary, | |
| 382 | 382 | // A file that is shown in full opens; a file with nothing to read | |
| 383 | - | // (binary, truncated) stays closed under its note. | |
| 383 | + | // (binary, omitted) stays closed under its note. | |
| 384 | 384 | Open: len(f.Hunks) > 0 && f.Omitted == domain.OmitNone, | |
| 385 | 385 | Manual: f.Omitted.Collapsed(), | |
| 386 | 386 | } | |
| @@ -535,6 +535,14 @@ func fileParam(r *http.Request, n int) (int, bool) { | |||
| 535 | 535 | // ?view= is the layout shown and ?layouts= says whether the page holds both. | |
| 536 | 536 | func fragmentView(r *http.Request, lines int) DiffView { | |
| 537 | 537 | v := newDiffView(r, lines, "", "") | |
| 538 | + | v.pageLayouts(r) | |
| 539 | + | return v | |
| 540 | + | } | |
| 541 | + | ||
| 542 | + | // pageLayouts makes a fragment send the layouts of the page that asks for | |
| 543 | + | // it: ?layouts=both when the page holds both and switches them itself, | |
| 544 | + | // ?layouts=one when it holds only the shown one. | |
| 545 | + | func (v *DiffView) pageLayouts(r *http.Request) { | |
| 538 | 546 | switch r.URL.Query().Get("layouts") { | |
| 539 | 547 | case "both": | |
| 540 | 548 | v.Both = true | |
| @@ -542,5 +550,4 @@ func fragmentView(r *http.Request, lines int) DiffView { | |||
| 542 | 550 | v.Both = false | |
| 543 | 551 | } | |
| 544 | 552 | v.ShowUnified, v.ShowSplit = v.Both || v.View == "unified", v.Both || v.View == "split" | |
| 545 | - | return v | |
| 546 | 553 | } | |
| @@ -380,7 +380,7 @@ func diffFiles(files []domain.FileDiff) []DiffFile { | |||||
| 380 | ID: "f" + itoa(i+1), Status: statusLetter(f.Status), | 380 | ID: "f" + itoa(i+1), Status: statusLetter(f.Status), | ||
| 381 | Add: f.Additions, Del: f.Deletions, Bin: f.Binary, | 381 | Add: f.Additions, Del: f.Deletions, Bin: f.Binary, | ||
| 382 | // A file that is shown in full opens; a file with nothing to read | 382 | // A file that is shown in full opens; a file with nothing to read | ||
| 383 | - | // (binary, truncated) stays closed under its note. | 383 | + | // (binary, omitted) stays closed under its note. |
| 384 | Open: len(f.Hunks) > 0 && f.Omitted == domain.OmitNone, | 384 | Open: len(f.Hunks) > 0 && f.Omitted == domain.OmitNone, | ||
| 385 | Manual: f.Omitted.Collapsed(), | 385 | Manual: f.Omitted.Collapsed(), | ||
| 386 | } | 386 | } | ||
| @@ -535,6 +535,14 @@ func fileParam(r *http.Request, n int) (int, bool) { | |||||
| 535 | // ?view= is the layout shown and ?layouts= says whether the page holds both. | 535 | // ?view= is the layout shown and ?layouts= says whether the page holds both. | ||
| 536 | func fragmentView(r *http.Request, lines int) DiffView { | 536 | func fragmentView(r *http.Request, lines int) DiffView { | ||
| 537 | v := newDiffView(r, lines, "", "") | 537 | v := newDiffView(r, lines, "", "") | ||
| 538 | + | v.pageLayouts(r) | |||
| 539 | + | return v | |||
| 540 | + | } | |||
| 541 | + | ||||
| 542 | + | // pageLayouts makes a fragment send the layouts of the page that asks for | |||
| 543 | + | // it: ?layouts=both when the page holds both and switches them itself, | |||
| 544 | + | // ?layouts=one when it holds only the shown one. | |||
| 545 | + | func (v *DiffView) pageLayouts(r *http.Request) { | |||
| 538 | switch r.URL.Query().Get("layouts") { | 546 | switch r.URL.Query().Get("layouts") { | ||
| 539 | case "both": | 547 | case "both": | ||
| 540 | v.Both = true | 548 | v.Both = true | ||
| @@ -542,5 +550,4 @@ func fragmentView(r *http.Request, lines int) DiffView { | |||||
| 542 | v.Both = false | 550 | v.Both = false | ||
| 543 | } | 551 | } | ||
| 544 | v.ShowUnified, v.ShowSplit = v.Both || v.View == "unified", v.Both || v.View == "split" | 552 | v.ShowUnified, v.ShowSplit = v.Both || v.View == "unified", v.Both || v.View == "split" | ||
| 545 | - | return v | |||
| 546 | } | 553 | } | ||
M internal/adapters/web/mr_files.go +1 -7
| @@ -241,12 +241,6 @@ func (s *Server) mrFiles(ctx context.Context, r *http.Request) (templ.Component, | |||
| 241 | 241 | } | |
| 242 | 242 | lines, _ := diffLines(window) | |
| 243 | 243 | v := newDiffView(r, lines, withFrom(self+"?view=unified", from), withFrom(self+"?view=split", from)) | |
| 244 | - | switch r.URL.Query().Get("layouts") { | |
| 245 | - | case "both": | |
| 246 | - | v.Both = true | |
| 247 | - | case "one": | |
| 248 | - | v.Both = false | |
| 249 | - | } | |
| 250 | - | v.ShowUnified, v.ShowSplit = v.Both || v.View == "unified", v.Both || v.View == "split" | |
| 244 | + | v.pageLayouts(r) | |
| 251 | 245 | return MRFiles(MRFilesPage{Files: window, DiffView: v}), http.StatusOK, nil | |
| 252 | 246 | } | |
| @@ -241,12 +241,6 @@ func (s *Server) mrFiles(ctx context.Context, r *http.Request) (templ.Component, | |||||
| 241 | } | 241 | } | ||
| 242 | lines, _ := diffLines(window) | 242 | lines, _ := diffLines(window) | ||
| 243 | v := newDiffView(r, lines, withFrom(self+"?view=unified", from), withFrom(self+"?view=split", from)) | 243 | v := newDiffView(r, lines, withFrom(self+"?view=unified", from), withFrom(self+"?view=split", from)) | ||
| 244 | - | switch r.URL.Query().Get("layouts") { | 244 | + | v.pageLayouts(r) |
| 245 | - | case "both": | |||
| 246 | - | v.Both = true | |||
| 247 | - | case "one": | |||
| 248 | - | v.Both = false | |||
| 249 | - | } | |||
| 250 | - | v.ShowUnified, v.ShowSplit = v.Both || v.View == "unified", v.Both || v.View == "split" | |||
| 251 | return MRFiles(MRFilesPage{Files: window, DiffView: v}), http.StatusOK, nil | 245 | return MRFiles(MRFilesPage{Files: window, DiffView: v}), http.StatusOK, nil | ||
| 252 | } | 246 | } | ||