Sign in

Search issue titles and bodies, change requests and comments. Private repositories you cannot read are not included.

vole
Clone

Read only, no credentials:

https://rodent.software/git/vole.git

Write over SSH with a registered key:

git clone ssh://git@0.0.0.0:2222/vole.git

A free and open-source forge that lives in your Git repo.

Open one file at a time on the commit page

628d45239eb09af4e46c9dc779d30144e96c7e6f · Keenan Nemetz · 11d ago · parent 816cc543

Open one file at a time on the commit page

A commit page rendered every patch it contained, so a broad commit turned one
page load into every blob it touched. It now opens the file tree beside the diff
of a single file, the same shape a change request's Files tab uses: the tree is
built from `git show --name-status`, only the selected path's patch is read, and
a tree click swaps the pane over HTMX. `/commit/{sha}/{path}` deep-links a file
and a 512 KiB cap still bounds one file's patch.

The file tree became one widget for three surfaces rather than two, so its
context gained a Commit field and its href/target/status rules moved onto the
view (Branch, Diff, Href).

The pane's header is now one row: the file's name on the left and its line
counts on the right, with the separate "1 file changed" summary line dropped
when a single file fills the pane. The total-line remains for the multi-file
diff a compare renders.

server/commit_test.go +173

@@ -0,0 +1,173 @@
1 package server_test
2
3 // A commit is reviewed one file at a time. The page lists what the commit touched
4 // and renders the diff of the file that is open, so a commit over a hundred files
5 // costs one patch rather than a hundred.
6
7 import (
8 "os"
9 "path/filepath"
10 "strings"
11 "testing"
12 )
13
14 func TestCommitPageOpensOneFileAtATime(t *testing.T) {
15 t.Parallel()
16 f := newFixture(t)
17 f.publish(f.repo)
18
19 work := workRepo(t, map[string]string{
20 "readme.md": "v1 line\n",
21 "keep.txt": "keep\n",
22 "old.txt": "moved\n",
23 })
24 if _, errs, err := f.git(work, "push", f.sshURL(f.repo), "main:refs/heads/main"); err != nil {
25 t.Fatalf("initial push: %s", errs)
26 }
27 // One commit that adds, modifies, deletes and renames, so the tree has more
28 // than one shape of row and more than one directory.
29 if err := os.WriteFile(filepath.Join(work, "readme.md"), []byte("v1 line\nv2 line\n"), 0o644); err != nil {
30 t.Fatal(err)
31 }
32 if err := os.MkdirAll(filepath.Join(work, "docs"), 0o755); err != nil {
33 t.Fatal(err)
34 }
35 if err := os.WriteFile(filepath.Join(work, "docs", "guide.md"), []byte("a guide\n"), 0o644); err != nil {
36 t.Fatal(err)
37 }
38 if err := os.Remove(filepath.Join(work, "keep.txt")); err != nil {
39 t.Fatal(err)
40 }
41 if _, errs, err := runGit(t, work, "mv", "old.txt", "new.txt"); err != nil {
42 t.Fatalf("mv: %s", errs)
43 }
44 if _, errs, err := runGit(t, work, "add", "-A"); err != nil {
45 t.Fatalf("add: %s", errs)
46 }
47 if _, errs, err := runGit(t, work, "commit", "-qm", "broad change"); err != nil {
48 t.Fatalf("commit: %s", errs)
49 }
50 if _, errs, err := f.git(work, "push", f.sshURL(f.repo), "main:refs/heads/main"); err != nil {
51 t.Fatalf("second push: %s", errs)
52 }
53 sha := mustSHA(t, work)
54 commitURL := "/r/" + f.repo + "/commit/" + sha
55
56 // The commit page is the tree beside one file's diff, not every diff at once.
57 code, page := f.get(commitURL)
58 if code != 200 {
59 t.Fatalf("commit page = %d", code)
60 }
61 for _, want := range []string{
62 `id="review-tree"`, `id="review-main"`,
63 `data-status="M"`, `data-status="A"`, `data-status="D"`, `data-status="R"`,
64 commitURL + "/readme.md", commitURL + "/docs/guide.md", commitURL + "/new.txt",
65 } {
66 if !strings.Contains(page, want) {
67 t.Fatalf("commit page has no %q:\n%s", want, page)
68 }
69 }
70 // Only one patch reaches the page. The first file opens, and the rest are rows
71 // in the tree rather than rendered diffs.
72 if n := strings.Count(page, `<section class="diff"`); n != 1 {
73 t.Fatalf("the page rendered %d diffs, want 1:\n%s", n, page)
74 }
75 if !strings.Contains(page, "a guide") {
76 t.Fatalf("the first changed file did not open:\n%s", page)
77 }
78 // The name and the counts share one row, name on the left and counts on the
79 // right, with no second summary line above them.
80 stat := strings.Index(page, `class="diffstat"`)
81 if stat < 0 {
82 t.Fatalf("the diff header has no counts row:\n%s", page)
83 }
84 if strings.Contains(page, `class="diffsum"`) {
85 t.Fatalf("a single-file pane still renders a separate summary line:\n%s", page)
86 }
87 if name := strings.Index(page, `class="path"`); name < 0 || name > stat {
88 t.Fatalf("the file name is not to the left of its counts:\n%s", page)
89 }
90 row := page[stat:]
91 if end := strings.Index(row, "</h3>"); end < 0 || !strings.Contains(row[:end], `<span class="adds">+1</span>`) {
92 t.Fatalf("the header row does not hold the file's counts:\n%s", page)
93 }
94 for _, unwanted := range []string{"v2 line", "moved"} {
95 if strings.Contains(page, unwanted) {
96 t.Fatalf("the page rendered another file's diff (%q):\n%s", unwanted, page)
97 }
98 }
99
100 // A deep link opens that file instead of the first one.
101 code, page = f.get(commitURL + "/readme.md")
102 if code != 200 {
103 t.Fatalf("deep link = %d", code)
104 }
105 if !strings.Contains(page, "v2 line") || !strings.Contains(page, `data-path="readme.md"`) ||
106 strings.Contains(page, "a guide") {
107 t.Fatalf("the deep link did not open readme.md alone:\n%s", page)
108 }
109
110 // A renamed file keeps both of its paths: the diff is a rename, not a new file.
111 _, page = f.get(commitURL + "/new.txt")
112 if !strings.Contains(page, "old.txt → new.txt") {
113 t.Fatalf("the renamed file's diff does not name both paths:\n%s", page)
114 }
115
116 // A tree click returns only the pane it replaces.
117 resp, piece := f.htmxGet(t, commitURL+"/docs/guide.md", "")
118 if resp.StatusCode != 200 || strings.Contains(piece, "<!doctype html>") ||
119 !strings.Contains(piece, `id="review-main"`) || !strings.Contains(piece, "a guide") ||
120 strings.Contains(piece, `id="review-tree"`) {
121 t.Fatalf("commit fragment = %d:\n%s", resp.StatusCode, piece)
122 }
123
124 // A path the commit did not touch is a 404, not an empty pane.
125 if code, _ := f.get(commitURL + "/not-there.txt"); code != 404 {
126 t.Fatalf("untouched path = %d, want 404", code)
127 }
128 }
129
130 // TestCommitPageCutsOneFilesPatch: the cap is what stops a generated diff from
131 // being rendered in full, and it applies to the file that is open rather than to
132 // the whole commit.
133 func TestCommitPageCutsOneFilesPatch(t *testing.T) {
134 t.Parallel()
135 f := newFixture(t)
136 f.publish(f.repo)
137
138 // One line longer than the 512 KiB cap, so the file that is open is cut.
139 work := workRepo(t, map[string]string{"small.txt": "one\n"})
140 if _, errs, err := f.git(work, "push", f.sshURL(f.repo), "main:refs/heads/main"); err != nil {
141 t.Fatalf("initial push: %s", errs)
142 }
143 if err := os.WriteFile(filepath.Join(work, "big.txt"), []byte(strings.Repeat("x", 513<<10)+"\n"), 0o644); err != nil {
144 t.Fatal(err)
145 }
146 if err := os.WriteFile(filepath.Join(work, "small.txt"), []byte("one\ntwo\n"), 0o644); err != nil {
147 t.Fatal(err)
148 }
149 if _, errs, err := runGit(t, work, "add", "-A"); err != nil {
150 t.Fatalf("add: %s", errs)
151 }
152 if _, errs, err := runGit(t, work, "commit", "-qm", "a big file"); err != nil {
153 t.Fatalf("commit: %s", errs)
154 }
155 if _, errs, err := f.git(work, "push", f.sshURL(f.repo), "main:refs/heads/main"); err != nil {
156 t.Fatalf("push: %s", errs)
157 }
158 sha := mustSHA(t, work)
159 commitURL := "/r/" + f.repo + "/commit/" + sha
160
161 _, page := f.get(commitURL + "/big.txt")
162 if !strings.Contains(page, "showing the first 512 KiB") {
163 t.Fatalf("the cut file is not labelled as cut:\n%.2000s", page)
164 }
165 // The small file is whole, and cutting the big one did not hide its row.
166 _, page = f.get(commitURL + "/small.txt")
167 if strings.Contains(page, "showing the first 512 KiB") {
168 t.Fatalf("the small file was reported as cut:\n%s", page)
169 }
170 if !strings.Contains(page, commitURL+"/big.txt") {
171 t.Fatalf("the cut file is missing from the tree:\n%s", page)
172 }
173 }