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 a deleted file instead of failing the page

bef0c093da662024c936eb2c230cf2a3521ecbb9 · Keenan Nemetz · 8d ago · parent f64c77e4

Open a deleted file instead of failing the page

A deletion has an old path and no new one, and pathspec returned both sides to
git. Git refuses an empty pathspec outright, so opening a deleted file on a
commit page answered 500 and the file could not be reviewed at all. The per-file
read this branch gives the Files tab would have extended that to every deleted
file in a change.

Return one path for a deletion, both only for a rename, and none when a row has
no paths at all. No paths means there is no patch to ask for, which is not the
same as every path: callers stop before asking git instead of printing the whole
commit into a single file's pane.

TestPathspecNamesTheSideThatExists pins the four shapes, and
TestCommitPageShowsADeletedFile is the page that used to 500. The request Files
test now carries a deletion too, and opens it.

server/commit_test.go +37

@@ -5,6 +5,7 @@ package server_test
5 5 // costs one patch rather than a hundred.
6 6
7 7 import (
8 "net/http"
8 9 "os"
9 10 "path/filepath"
10 11 "strings"
@@ -127,6 +128,42 @@ func TestCommitPageOpensOneFileAtATime(t *testing.T) {
127 128 }
128 129 }
129 130
131 // TestCommitPageShowsADeletedFile: a deletion has one path and no new one, and
132 // asking git for both sides used to fail the page outright — the file could not
133 // be reviewed at all.
134 func TestCommitPageShowsADeletedFile(t *testing.T) {
135 t.Parallel()
136 f := newFixture(t)
137 f.publish(f.repo)
138
139 work := workRepo(t, map[string]string{"gone.txt": "first line\nsecond line\n", "keep.txt": "keep\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 _, errs, err := runGit(t, work, "rm", "-q", "gone.txt"); err != nil {
144 t.Fatalf("rm: %s", errs)
145 }
146 if _, errs, err := runGit(t, work, "commit", "-qm", "drop gone.txt"); err != nil {
147 t.Fatalf("commit: %s", errs)
148 }
149 if _, errs, err := f.git(work, "push", f.sshURL(f.repo), "main:refs/heads/main"); err != nil {
150 t.Fatalf("push: %s", errs)
151 }
152 commitURL := "/r/" + f.repo + "/commit/" + mustSHA(t, work)
153
154 code, page := f.get(commitURL)
155 if code != http.StatusOK {
156 t.Fatalf("commit page = %d:\n%.2000s", code, page)
157 }
158 code, page = f.get(commitURL + "/gone.txt")
159 if code != http.StatusOK {
160 t.Fatalf("deleted file = %d:\n%.2000s", code, page)
161 }
162 if !strings.Contains(page, `data-highlight-side="old">first line<`) {
163 t.Fatalf("the deleted file has no patch:\n%.2000s", page)
164 }
165 }
166
130 167 // TestCommitPageShowsTheWholeOfOneFilesPatch: the page renders one file's patch,
131 168 // not the whole commit's, so the file that is open is shown whole. A byte cut
132 169 // would hide the end of a large file and, worse, every file named after it.