Gitfed
bastien-mrq/gitfed/ Commits/ ca2be6a

Real pagination for the commit list

Replaces the old hard maxCommitsShown = 200 cutoff (nothing rendered past it) with ?page= pagination, 50 commits per page. Adds ListCommitsPage end to end (gitexec, admin.Ops, adminrpc) built on git log --skip=/--max-count=. A page number past the last one 404s rather than rendering an empty list indistinguishable from a repo with no commits, matching the "unknown value 404s" treatment already used for branch/ref validation elsewhere. Verified live against a 60-commit repo: page 1 shows 50 with a working Next link, page 2 shows the remaining 10 with Prev only, page 3 404s. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

bastien-mrq 2026-07-30 14:18 commit ca2be6aecb5eea1bf89634732e347a8a5cf8676c parent c7287f7a3644ab8698f00fec72d9c05af50275bd
10 files changed +178 −10
M cmd/gitfed-web/handlers_repo_commits.go +45 −9
M cmd/gitfed-web/render.go +4 −0
M internal/admin/admin.go +9 −0
M internal/adminrpc/client.go +6 −0
M internal/adminrpc/protocol.go +7 −0
M internal/adminrpc/server.go +8 −0
M internal/gitexec/gitexec.go +15 −1
M internal/gitexec/gitexec_test.go +78 −0
M internal/i18n/strings_en.go +3 −0
M internal/i18n/strings_fr.go +3 −0
cmd/gitfed-web/handlers_repo_commits.go
diff --git a/cmd/gitfed-web/handlers_repo_commits.go b/cmd/gitfed-web/handlers_repo_commits.go index d797240..920326c 100644 --- a/cmd/gitfed-web/handlers_repo_commits.go +++ b/cmd/gitfed-web/handlers_repo_commits.go @@ -4,6 +4,7 @@ import ( "bytes" "html/template" "net/http" + "strconv" "strings" "git.neuromancer.ovh/bastien-mrq/gitfed/internal/gitexec" @@ -11,15 +12,17 @@ import ( "git.neuromancer.ovh/bastien-mrq/gitfed/internal/store" ) -// maxCommitsShown caps how much history one page renders — plenty for -// browsing, and bounds how much `git log` output gitfed-web ever buffers. -const maxCommitsShown = 200 +// commitsPerPage bounds how much `git log` output one page request ever +// buffers — real pagination (?page=) reaches the rest instead of the old +// hard 200-commit cutoff with nothing beyond it. +const commitsPerPage = 50 var repoCommitsTpl = newTpl("repo-commits", ` {{.Flash}} <div class="gf-crumbs"><a href="/r/{{.Repo.Name}}">{{.Repo.Name}}</a><span class="sep">/</span>{{t .Lang "repo.commits_title"}}</div> <div class="gf-repo-head"> <h1>{{t .Lang "repo.commits_title"}}</h1> + {{if .Total}}<span class="count">{{t .Lang "repo.stat_commits" .Total}}</span>{{end}} </div> <div class="gf-card gf-commit-list"> @@ -35,6 +38,14 @@ var repoCommitsTpl = newTpl("repo-commits", ` <div class="gf-commit-row"><span class="muted">{{t .Lang "repo.commits_empty"}}</span></div> {{end}} </div> + +{{if or .HasPrev .HasNext}} +<div class="gf-pager"> + {{if .HasPrev}}<a href="/repo-commits/{{.Repo.Name}}?page={{.PrevPage}}" class="gf-btn">← {{t .Lang "repo.pager_prev"}}</a>{{else}}<span class="gf-btn disabled">← {{t .Lang "repo.pager_prev"}}</span>{{end}} + <span class="muted">{{t .Lang "repo.pager_page" .Page}}</span> + {{if .HasNext}}<a href="/repo-commits/{{.Repo.Name}}?page={{.NextPage}}" class="gf-btn">{{t .Lang "repo.pager_next"}} →</a>{{else}}<span class="gf-btn disabled">{{t .Lang "repo.pager_next"}} →</span>{{end}} +</div> +{{end}} `) func (s *server) handleRepoCommits(w http.ResponseWriter, r *http.Request) { @@ -47,7 +58,27 @@ func (s *server) handleRepoCommits(w http.ResponseWriter, r *http.Request) { return } - commits, _, err := s.ops.ListCommits(name, maxCommitsShown) + page, _ := strconv.Atoi(r.URL.Query().Get("page")) + if page < 1 { + page = 1 + } + offset := (page - 1) * commitsPerPage + + commits, _, err := s.ops.ListCommitsPage(name, commitsPerPage, offset) + if err != nil { + s.serverError(w, r, err) + return + } + // A real page beyond the last one (e.g. a stale bookmark after history + // was rewritten) — same "unknown value 404s" treatment used elsewhere, + // rather than silently rendering an empty list that looks like a repo + // with no commits at all. + if len(commits) == 0 && page > 1 { + http.NotFound(w, r) + return + } + + total, _, err := s.ops.CountCommits(name) if err != nil { s.serverError(w, r, err) return @@ -55,11 +86,16 @@ func (s *server) handleRepoCommits(w http.ResponseWriter, r *http.Request) { var buf bytes.Buffer _ = repoCommitsTpl.Execute(&buf, struct { - Repo store.Repo - Lang string - Commits []gitexec.Commit - Flash template.HTML - }{repo, string(lang), commits, flash(r)}) + Repo store.Repo + Lang string + Commits []gitexec.Commit + Total, Page, PrevPage, NextPage int + HasPrev, HasNext bool + Flash template.HTML + }{ + repo, string(lang), commits, total, page, page - 1, page + 1, + page > 1, offset+len(commits) < total, flash(r), + }) s.render(w, r, name+" — "+i18n.T(lang, "repo.commits_title"), "home", template.HTML(buf.String())) }
cmd/gitfed-web/render.go
diff --git a/cmd/gitfed-web/render.go b/cmd/gitfed-web/render.go index d7ad3db..e2ee5ed 100644 --- a/cmd/gitfed-web/render.go +++ b/cmd/gitfed-web/render.go @@ -423,8 +423,12 @@ const shellHeadSrc = `<!doctype html> .gf-crumbs .sep { color: var(--text-faint); margin: 0 0.35em; } .gf-repo-head { display: flex; align-items: center; gap: 0.55rem; flex-wrap: wrap; margin-top: 0.6rem; } .gf-repo-head h1 { font-size: 1.3rem; margin: 0; font-weight: 700; } + .gf-repo-head .count { font-family: var(--mono); font-size: 0.82rem; color: var(--text-faint); } .gf-actions-row { display: flex; align-items: center; gap: 0.6rem; flex-wrap: wrap; margin: 1rem 0 0.7rem; } .gf-actions-row .gf-btn { margin-top: 0; } + .gf-pager { display: flex; align-items: center; justify-content: center; gap: 1rem; margin: 1rem 0; } + .gf-pager .gf-btn { margin-top: 0; } + .gf-pager .gf-btn.disabled { opacity: 0.4; pointer-events: none; } .gf-branch-dropdown { position: relative; } .gf-branch-dropdown > summary { list-style: none; } .gf-branch-dropdown > summary::-webkit-details-marker { display: none; }
internal/admin/admin.go
diff --git a/internal/admin/admin.go b/internal/admin/admin.go index bc47df6..c8a53c1 100644 --- a/internal/admin/admin.go +++ b/internal/admin/admin.go @@ -63,6 +63,7 @@ type Ops interface { GetRepoFileAtRef(name, ref, path string) (content string, found bool, err error) GetRepoBranch(name string) (branch string, found bool, err error) ListCommits(name string, limit int) (commits []gitexec.Commit, found bool, err error) + ListCommitsPage(name string, limit, offset int) (commits []gitexec.Commit, found bool, err error) CountCommits(name string) (count int, found bool, err error) CountContributors(name string) (count int, found bool, err error) DominantLanguage(name string) (language string, found bool, err error) @@ -486,6 +487,14 @@ func (a *Admin) ListCommits(name string, limit int) ([]gitexec.Commit, bool, err return gitexec.ListCommits(repo.Path, limit) } +func (a *Admin) ListCommitsPage(name string, limit, offset int) ([]gitexec.Commit, bool, error) { + repo, err := a.Store.GetRepo(name) + if err != nil { + return nil, false, err + } + return gitexec.ListCommitsPage(repo.Path, limit, offset) +} + func (a *Admin) CountCommits(name string) (int, bool, error) { repo, err := a.Store.GetRepo(name) if err != nil {
internal/adminrpc/client.go
diff --git a/internal/adminrpc/client.go b/internal/adminrpc/client.go index 3031a10..d53a8ea 100644 --- a/internal/adminrpc/client.go +++ b/internal/adminrpc/client.go @@ -311,6 +311,12 @@ func (c *Client) ListCommits(name string, limit int) ([]gitexec.Commit, bool, er return out.Commits, out.Found, err } +func (c *Client) ListCommitsPage(name string, limit, offset int) ([]gitexec.Commit, bool, error) { + var out listCommitsResult + err := c.call(methodListCommitsPage, nameLimitOffsetArgs{Name: name, Limit: limit, Offset: offset}, &out) + return out.Commits, out.Found, err +} + func (c *Client) ShowCommit(name, hash string) (gitexec.CommitDetail, bool, error) { var out showCommitResult err := c.call(methodShowCommit, nameHashArgs{Name: name, Hash: hash}, &out)
internal/adminrpc/protocol.go
diff --git a/internal/adminrpc/protocol.go b/internal/adminrpc/protocol.go index 80909b3..d224c89 100644 --- a/internal/adminrpc/protocol.go +++ b/internal/adminrpc/protocol.go @@ -53,6 +53,7 @@ const ( methodDeleteSession = "DeleteSession" methodCheckAccess = "CheckAccess" methodListCommits = "ListCommits" + methodListCommitsPage = "ListCommitsPage" methodCountCommits = "CountCommits" methodCountContributors = "CountContributors" methodDominantLanguage = "DominantLanguage" @@ -308,6 +309,12 @@ type nameLimitArgs struct { Limit int `json:"limit"` } +type nameLimitOffsetArgs struct { + Name string `json:"name"` + Limit int `json:"limit"` + Offset int `json:"offset"` +} + type listCommitsResult struct { Commits []gitexec.Commit `json:"commits"` Found bool `json:"found"`
internal/adminrpc/server.go
diff --git a/internal/adminrpc/server.go b/internal/adminrpc/server.go index 4710858..0af8fe1 100644 --- a/internal/adminrpc/server.go +++ b/internal/adminrpc/server.go @@ -367,6 +367,14 @@ func (s *Server) dispatch(req wireRequest) (any, error) { commits, found, err := s.ops.ListCommits(a.Name, a.Limit) return listCommitsResult{Commits: commits, Found: found}, err + case methodListCommitsPage: + var a nameLimitOffsetArgs + if err := json.Unmarshal(req.Args, &a); err != nil { + return nil, err + } + commits, found, err := s.ops.ListCommitsPage(a.Name, a.Limit, a.Offset) + return listCommitsResult{Commits: commits, Found: found}, err + case methodShowCommit: var a nameHashArgs if err := json.Unmarshal(req.Args, &a); err != nil {
internal/gitexec/gitexec.go
diff --git a/internal/gitexec/gitexec.go b/internal/gitexec/gitexec.go index 32d1369..d811a77 100644 --- a/internal/gitexec/gitexec.go +++ b/internal/gitexec/gitexec.go @@ -507,13 +507,27 @@ func DominantLanguage(repoPath string) (language string, found bool, err error) // branch (see resolveDefaultRef), most recent first. found is false (nil // error) for a repo with no commits yet. func ListCommits(repoPath string, limit int) (commits []Commit, found bool, err error) { + return ListCommitsPage(repoPath, limit, 0) +} + +// ListCommitsPage is ListCommits with an additional offset (via `git log +// --skip=`), for the commits page's pagination — most recent first, so +// offset 0 is always the newest page regardless of how many commits have +// landed since a caller last asked. +func ListCommitsPage(repoPath string, limit, offset int) (commits []Commit, found bool, err error) { ref, ok := resolveDefaultRef(repoPath) if !ok { return nil, false, nil } format := strings.Join([]string{"%H", "%h", "%an", "%ae", "%aI", "%s"}, commitFieldSep) + commitRecordSep - cmd := exec.Command("git", "--git-dir="+repoPath, "log", "--max-count="+strconv.Itoa(limit), "--format="+format, ref) + args := []string{"--git-dir=" + repoPath, "log", "--max-count=" + strconv.Itoa(limit)} + if offset > 0 { + args = append(args, "--skip="+strconv.Itoa(offset)) + } + args = append(args, "--format="+format, ref) + + cmd := exec.Command("git", args...) var stdout, stderr bytes.Buffer cmd.Stdout = &stdout cmd.Stderr = &stderr
internal/gitexec/gitexec_test.go
diff --git a/internal/gitexec/gitexec_test.go b/internal/gitexec/gitexec_test.go index b2a1ee2..a2fb6d9 100644 --- a/internal/gitexec/gitexec_test.go +++ b/internal/gitexec/gitexec_test.go @@ -2,6 +2,7 @@ package gitexec import ( "context" + "fmt" "os" "os/exec" "path/filepath" @@ -544,6 +545,83 @@ func TestDominantLanguageNoRecognizedFiles(t *testing.T) { } } +func TestListCommitsPage(t *testing.T) { + tmp := t.TempDir() + barePath := filepath.Join(tmp, "repo.git") + if err := InitBareRepo(barePath); err != nil { + t.Fatalf("init bare repo: %v", err) + } + + work := filepath.Join(tmp, "work") + if err := os.Mkdir(work, 0755); err != nil { + t.Fatal(err) + } + run(t, work, "git", "init", "-q", "-b", "main") + // 5 commits, subjects "commit 0" (oldest) .. "commit 4" (newest). + for i := 0; i < 5; i++ { + if err := os.WriteFile(filepath.Join(work, "f"), []byte{byte(i)}, 0644); err != nil { + t.Fatal(err) + } + run(t, work, "git", "add", "f") + run(t, work, "git", "commit", "-q", "-m", fmt.Sprintf("commit %d", i)) + } + run(t, work, "git", "remote", "add", "origin", barePath) + run(t, work, "git", "push", "-q", "origin", "main") + + // Page 1 (limit 2, offset 0): newest two, most-recent-first. + page1, found, err := ListCommitsPage(barePath, 2, 0) + if err != nil || !found { + t.Fatalf("page1: found=%v err=%v", found, err) + } + if len(page1) != 2 || page1[0].Subject != "commit 4" || page1[1].Subject != "commit 3" { + t.Fatalf("page1 = %+v, want [commit 4, commit 3]", subjectsOf(page1)) + } + + // Page 2 (limit 2, offset 2): next two. + page2, found, err := ListCommitsPage(barePath, 2, 2) + if err != nil || !found { + t.Fatalf("page2: found=%v err=%v", found, err) + } + if len(page2) != 2 || page2[0].Subject != "commit 2" || page2[1].Subject != "commit 1" { + t.Fatalf("page2 = %+v, want [commit 2, commit 1]", subjectsOf(page2)) + } + + // Page 3 (limit 2, offset 4): only the oldest one left. + page3, found, err := ListCommitsPage(barePath, 2, 4) + if err != nil || !found { + t.Fatalf("page3: found=%v err=%v", found, err) + } + if len(page3) != 1 || page3[0].Subject != "commit 0" { + t.Fatalf("page3 = %+v, want [commit 0]", subjectsOf(page3)) + } + + // Past the end: found=true (repo has commits), just an empty page. + page4, found, err := ListCommitsPage(barePath, 2, 10) + if err != nil || !found { + t.Fatalf("page4: found=%v err=%v", found, err) + } + if len(page4) != 0 { + t.Fatalf("page4 = %+v, want none", subjectsOf(page4)) + } + + // ListCommits (no offset) must match ListCommitsPage(..., 0). + plain, _, err := ListCommits(barePath, 2) + if err != nil { + t.Fatalf("ListCommits: %v", err) + } + if len(plain) != 2 || plain[0].Subject != page1[0].Subject || plain[1].Subject != page1[1].Subject { + t.Fatalf("ListCommits = %+v, want same as ListCommitsPage(_, 2, 0) = %+v", subjectsOf(plain), subjectsOf(page1)) + } +} + +func subjectsOf(commits []Commit) []string { + subjects := make([]string, len(commits)) + for i, c := range commits { + subjects[i] = c.Subject + } + return subjects +} + func TestDefaultBranchNameViaHEAD(t *testing.T) { tmp := t.TempDir() barePath := filepath.Join(tmp, "repo.git")
internal/i18n/strings_en.go
diff --git a/internal/i18n/strings_en.go b/internal/i18n/strings_en.go index c03dd70..f61a24d 100644 --- a/internal/i18n/strings_en.go +++ b/internal/i18n/strings_en.go @@ -290,6 +290,9 @@ var en = map[string]string{ "repo.stat_contributors": "%d contributors", "repo.commits_title": "Commits", "repo.commits_empty": "No commits yet.", + "repo.pager_prev": "Previous", + "repo.pager_next": "Next", + "repo.pager_page": "Page %d", "repo.commit_label": "commit", "repo.parent_label": "parent", "repo.files_changed": "files changed",
internal/i18n/strings_fr.go
diff --git a/internal/i18n/strings_fr.go b/internal/i18n/strings_fr.go index bcbaf10..a75f757 100644 --- a/internal/i18n/strings_fr.go +++ b/internal/i18n/strings_fr.go @@ -290,6 +290,9 @@ var fr = map[string]string{ "repo.stat_contributors": "%d contributeurs", "repo.commits_title": "Commits", "repo.commits_empty": "Aucun commit pour le moment.", + "repo.pager_prev": "Précédent", + "repo.pager_next": "Suivant", + "repo.pager_page": "Page %d", "repo.commit_label": "commit", "repo.parent_label": "parent", "repo.files_changed": "fichiers modifiés",