From 30b1928c068b9ed58668cea85310ae1aefe7972d Mon Sep 17 00:00:00 2001 From: Nicolas Date: Thu, 7 May 2026 22:40:48 +0200 Subject: [PATCH] more cleanup --- models/user/user.go | 15 ++-- modules/git/commit.go | 79 ++++++++++++++-- modules/git/commit_test.go | 89 +++++++++++++++++++ modules/repository/commits.go | 44 ++++++++- modules/repository/commits_test.go | 5 ++ options/locale/locale_en-US.json | 1 + services/repository/gitgraph/graph_models.go | 20 +++-- .../repo/commit_coauthor_avatar_stack.tmpl | 52 ++++------- templates/repo/commit_coauthor_avatars.tmpl | 3 +- templates/user/dashboard/feeds.tmpl | 2 +- web_src/css/repo.css | 7 +- 11 files changed, 256 insertions(+), 61 deletions(-) diff --git a/models/user/user.go b/models/user/user.go index 7a45d70878..956e69edcf 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -1209,12 +1209,15 @@ func ValidateCommitsWithEmails(ctx context.Context, oldCommits []*git.Commit) ([ for _, c := range oldCommits { user := emailUserMap.GetByEmail(c.Author.Email) // FIXME: why ValidateCommitsWithEmails uses "Author", but ParseCommitsWithSignature uses "Committer"? coAuthorSigs := c.CoAuthorSignatures() - coAuthors := make([]*CoAuthorUser, 0, len(coAuthorSigs)) - for _, sig := range coAuthorSigs { - coAuthors = append(coAuthors, &CoAuthorUser{ - GiteaUser: emailUserMap.GetByEmail(sig.Email), - TrailerSignature: sig, - }) + var coAuthors []*CoAuthorUser + if len(coAuthorSigs) > 0 { + coAuthors = make([]*CoAuthorUser, 0, len(coAuthorSigs)) + for _, sig := range coAuthorSigs { + coAuthors = append(coAuthors, &CoAuthorUser{ + GiteaUser: emailUserMap.GetByEmail(sig.Email), + TrailerSignature: sig, + }) + } } newCommits = append(newCommits, &UserCommit{ User: user, diff --git a/modules/git/commit.go b/modules/git/commit.go index 34e6d64b86..4a23533d2f 100644 --- a/modules/git/commit.go +++ b/modules/git/commit.go @@ -69,26 +69,64 @@ func (c *CommitMessage) MessageBody() string { return *c.messageBody } +// cutTrailerPrefix matches ":" case-insensitively at the start of line and +// returns the trailing value plus whether a match was found. `git interpret-trailers` +// matches trailer tokens case-insensitively, so e.g. "Co-Authored-By:" is valid. +func cutTrailerPrefix(line, token string) (string, bool) { + if len(line) < len(token)+1 || line[len(token)] != ':' { + return "", false + } + if !strings.EqualFold(line[:len(token)], token) { + return "", false + } + return line[len(token)+1:], true +} + +func isTrailerLine(line string) bool { + token, rest, ok := strings.Cut(line, ":") + if !ok || strings.TrimSpace(rest) == "" { + return false + } + for _, r := range token { + if (r >= 'A' && r <= 'Z') || (r >= 'a' && r <= 'z') || (r >= '0' && r <= '9') || r == '-' { + continue + } + return false + } + return token != "" +} + // CoAuthorSignatures parses "Co-authored-by:" and "Co-committed-by:" trailers // from the trailing block of the commit message and returns deduplicated // Signature values. Only the last paragraph of the body is scanned so that // quoted or in-body occurrences (e.g. inside a revert/cherry-pick description) -// are not misinterpreted as trailers, matching `git interpret-trailers`. +// are not misinterpreted as trailers. The trailing paragraph must contain only +// trailer-shaped lines. +// Token matching is case-insensitive to match git's behaviour. func (c *CommitMessage) CoAuthorSignatures() []*Signature { if c.messageCoAuthors != nil { return *c.messageCoAuthors } var sigs []*Signature seen := make(map[string]struct{}) - body := strings.TrimRight(c.MessageBody(), "\n") - if idx := strings.LastIndex(body, "\n\n"); idx >= 0 { + body := strings.TrimRight(c.MessageBody(), "\r\n") + if idx := strings.LastIndex(body, "\r\n\r\n"); idx >= 0 { + body = body[idx+4:] + } else if idx := strings.LastIndex(body, "\n\n"); idx >= 0 { body = body[idx+2:] } - for line := range strings.SplitSeq(body, "\n") { - var rest string - var ok bool - if rest, ok = strings.CutPrefix(line, "Co-authored-by:"); !ok { - rest, ok = strings.CutPrefix(line, "Co-committed-by:") + lines := strings.Split(body, "\n") + for i, line := range lines { + lines[i] = strings.TrimRight(line, "\r") + if !isTrailerLine(lines[i]) { + c.messageCoAuthors = &sigs + return sigs + } + } + for _, line := range lines { + rest, ok := cutTrailerPrefix(line, "Co-authored-by") + if !ok { + rest, ok = cutTrailerPrefix(line, "Co-committed-by") if !ok { continue } @@ -113,6 +151,31 @@ func (c *CommitMessage) CoAuthorSignatures() []*Signature { return sigs } +// CoAuthorSignatures returns the commit's co-author trailers with the commit's +// own author and committer emails filtered out, so a contributor who copies +// themselves into a Co-authored-by line is not duplicated in the avatar stack. +func (c *Commit) CoAuthorSignatures() []*Signature { + raw := c.CommitMessage.CoAuthorSignatures() + if len(raw) == 0 { + return raw + } + exclude := make(map[string]struct{}, 2) + if c.Author != nil { + exclude[strings.ToLower(strings.TrimSpace(c.Author.Email))] = struct{}{} + } + if c.Committer != nil { + exclude[strings.ToLower(strings.TrimSpace(c.Committer.Email))] = struct{}{} + } + out := make([]*Signature, 0, len(raw)) + for _, sig := range raw { + if _, skip := exclude[sig.Email]; skip { + continue + } + out = append(out, sig) + } + return out +} + // ParentID returns oid of n-th parent (0-based index). // It returns nil if no such parent exists. func (c *Commit) ParentID(n int) (ObjectID, error) { diff --git a/modules/git/commit_test.go b/modules/git/commit_test.go index a7668e4deb..08116b0199 100644 --- a/modules/git/commit_test.go +++ b/modules/git/commit_test.go @@ -208,3 +208,92 @@ func Test_GetCommitBranchStart(t *testing.T) { assert.NotEmpty(t, startCommitID) assert.Equal(t, "95bb4d39648ee7e325106df01a621c530863a653", startCommitID) } + +func TestCoAuthorSignatures(t *testing.T) { + cases := []struct { + name string + body string + want []Signature + }{ + { + name: "empty", + body: "title", + want: nil, + }, + { + name: "single co-author", + body: "title\n\nbody text\n\nCo-authored-by: Jane ", + want: []Signature{{Name: "Jane", Email: "jane@example.com"}}, + }, + { + name: "case insensitive token", + body: "title\n\nCo-Authored-By: Jane \nco-committed-by: Bob ", + want: []Signature{ + {Name: "Jane", Email: "jane@example.com"}, + {Name: "Bob", Email: "bob@example.com"}, + }, + }, + { + name: "dedup by lowercased email", + body: "title\n\nCo-authored-by: Jane \nCo-authored-by: Janey ", + want: []Signature{{Name: "Jane", Email: "jane@example.com"}}, + }, + { + name: "in-body trailer ignored, only last paragraph counts", + body: "title\n\nCo-authored-by: Mallory \n\nactual body explaining revert\n\nCo-authored-by: Jane ", + want: []Signature{{Name: "Jane", Email: "jane@example.com"}}, + }, + { + name: "body text in trailing paragraph rejects co-author line", + body: "title\n\nbody text\nCo-authored-by: Jane ", + want: nil, + }, + { + name: "missing brackets is ignored", + body: "title\n\nCo-authored-by: Jane jane@example.com", + want: nil, + }, + { + name: "CRLF line endings", + body: "title\r\n\r\nCo-authored-by: Jane \r\nCo-committed-by: Bob ", + want: []Signature{ + {Name: "Jane", Email: "jane@example.com"}, + {Name: "Bob", Email: "bob@example.com"}, + }, + }, + { + name: "non-trailer line", + body: "title\n\nSigned-off-by: Jane ", + want: nil, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + cm := CommitMessage{MessageRaw: tc.body} + got := cm.CoAuthorSignatures() + assert.Len(t, got, len(tc.want)) + for i, w := range tc.want { + if i >= len(got) { + break + } + assert.Equal(t, w.Name, got[i].Name, "name[%d]", i) + assert.Equal(t, w.Email, got[i].Email, "email[%d]", i) + } + }) + } +} + +func TestCommitCoAuthorSignaturesFiltersAuthorAndCommitter(t *testing.T) { + c := &Commit{ + Author: &Signature{Name: "Jane", Email: "jane@example.com"}, + Committer: &Signature{Name: "Bob", Email: "bob@example.com"}, + CommitMessage: CommitMessage{MessageRaw: "title\n\n" + + "Co-authored-by: Jane Self \n" + + "Co-authored-by: Bob Self \n" + + "Co-authored-by: Carol "}, + } + got := c.CoAuthorSignatures() + if assert.Len(t, got, 1) { + assert.Equal(t, "carol@example.com", got[0].Email) + } +} diff --git a/modules/repository/commits.go b/modules/repository/commits.go index 546e881090..83ae10d8f5 100644 --- a/modules/repository/commits.go +++ b/modules/repository/commits.go @@ -29,10 +29,16 @@ type PushCommit struct { AuthorName string CommitterEmail string CommitterName string - CoAuthors []*git.Signature + CoAuthors []*PushCommitCoAuthor `json:",omitempty"` Timestamp time.Time } +// PushCommitCoAuthor represents a co-author in a push commit payload. +type PushCommitCoAuthor struct { + Name string + Email string +} + // PushCommits represents list of commits in a push operation. type PushCommits struct { Commits []*PushCommit @@ -149,6 +155,17 @@ func (pc *PushCommits) AvatarLink(ctx context.Context, email string) string { return v } +func pushCommitCoAuthorsFromSignatures(sigs []*git.Signature) []*PushCommitCoAuthor { + if len(sigs) == 0 { + return nil + } + coAuthors := make([]*PushCommitCoAuthor, len(sigs)) + for i, sig := range sigs { + coAuthors[i] = &PushCommitCoAuthor{Name: sig.Name, Email: sig.Email} + } + return coAuthors +} + // CommitToPushCommit transforms a git.Commit to PushCommit type. func CommitToPushCommit(commit *git.Commit) *PushCommit { return &PushCommit{ @@ -158,11 +175,34 @@ func CommitToPushCommit(commit *git.Commit) *PushCommit { AuthorName: commit.Author.Name, CommitterEmail: commit.Committer.Email, CommitterName: commit.Committer.Name, - CoAuthors: commit.CoAuthorSignatures(), + CoAuthors: pushCommitCoAuthorsFromSignatures(commit.CoAuthorSignatures()), Timestamp: commit.Author.When, } } +// AuthorSignature returns the push commit author as a git signature. +func (pc *PushCommit) AuthorSignature() *git.Signature { + return &git.Signature{ + Email: pc.AuthorEmail, + Name: pc.AuthorName, + } +} + +// CoAuthorUsers returns co-authors in the template view shape. +func (pc *PushCommit) CoAuthorUsers() []*user_model.CoAuthorUser { + if len(pc.CoAuthors) == 0 { + return nil + } + coAuthors := make([]*user_model.CoAuthorUser, len(pc.CoAuthors)) + for i, coAuthor := range pc.CoAuthors { + coAuthors[i] = &user_model.CoAuthorUser{TrailerSignature: &git.Signature{ + Name: coAuthor.Name, + Email: coAuthor.Email, + }} + } + return coAuthors +} + // GitToPushCommits transforms a list of git.Commits to PushCommits type. func GitToPushCommits(gitCommits []*git.Commit) *PushCommits { commits := make([]*PushCommit, 0, len(gitCommits)) diff --git a/modules/repository/commits_test.go b/modules/repository/commits_test.go index a4c0eea8b5..6228ae61a7 100644 --- a/modules/repository/commits_test.go +++ b/modules/repository/commits_test.go @@ -157,6 +157,11 @@ func TestCommitToPushCommit(t *testing.T) { assert.Equal(t, "jane@example.com", pushCommit.CoAuthors[0].Email) assert.Equal(t, "Jane Doe", pushCommit.CoAuthors[0].Name) } + assert.Equal(t, &git.Signature{Email: "example@example.com", Name: "John Doe"}, pushCommit.AuthorSignature()) + if assert.Len(t, pushCommit.CoAuthorUsers(), 1) { + assert.Equal(t, &git.Signature{Email: "jane@example.com", Name: "Jane Doe"}, pushCommit.CoAuthorUsers()[0].TrailerSignature) + assert.Nil(t, pushCommit.CoAuthorUsers()[0].GiteaUser) + } assert.Equal(t, now, pushCommit.Timestamp) } diff --git a/options/locale/locale_en-US.json b/options/locale/locale_en-US.json index 18acdb059c..b5ae421266 100644 --- a/options/locale/locale_en-US.json +++ b/options/locale/locale_en-US.json @@ -2591,6 +2591,7 @@ "repo.diff.committed_by": "committed by", "repo.diff.coauthored_by": "co-authored by", "repo.commits.coauthor_and": "and", + "repo.commits.coauthor_people": "%d people", "repo.commits.coauthor_others_1": "%d other", "repo.commits.coauthor_others_n": "%d others", "repo.diff.protected": "Protected", diff --git a/services/repository/gitgraph/graph_models.go b/services/repository/gitgraph/graph_models.go index 96ab2639c5..fb33d3c0a0 100644 --- a/services/repository/gitgraph/graph_models.go +++ b/services/repository/gitgraph/graph_models.go @@ -132,16 +132,18 @@ func (graph *Graph) LoadAndProcessCommits(ctx context.Context, repository *repo_ c.User = emailUserMap.GetByEmail(c.Commit.Author.Email) } coAuthorSigs := c.Commit.CoAuthorSignatures() - c.CoAuthors = make([]*user_model.CoAuthorUser, 0, len(coAuthorSigs)) - for _, sig := range coAuthorSigs { - var giteaUser *user_model.User - if emailUserMap != nil { - giteaUser = emailUserMap.GetByEmail(sig.Email) + if len(coAuthorSigs) > 0 { + c.CoAuthors = make([]*user_model.CoAuthorUser, 0, len(coAuthorSigs)) + for _, sig := range coAuthorSigs { + var giteaUser *user_model.User + if emailUserMap != nil { + giteaUser = emailUserMap.GetByEmail(sig.Email) + } + c.CoAuthors = append(c.CoAuthors, &user_model.CoAuthorUser{ + GiteaUser: giteaUser, + TrailerSignature: sig, + }) } - c.CoAuthors = append(c.CoAuthors, &user_model.CoAuthorUser{ - GiteaUser: giteaUser, - TrailerSignature: sig, - }) } c.Verification = asymkey_service.ParseCommitWithSignature(ctx, c.Commit) diff --git a/templates/repo/commit_coauthor_avatar_stack.tmpl b/templates/repo/commit_coauthor_avatar_stack.tmpl index 772e054aaa..f9bd9e031c 100644 --- a/templates/repo/commit_coauthor_avatar_stack.tmpl +++ b/templates/repo/commit_coauthor_avatar_stack.tmpl @@ -1,19 +1,19 @@ -{{if or .CoAuthors .CoAuthorSignatures}} +{{if .CoAuthors}} {{- $additionalClasses := .AdditionalClasses -}} - - {{- if .AuthorUser -}} - {{- ctx.AvatarUtils.Avatar .AuthorUser 20 -}} - {{- else -}} - {{- ctx.AvatarUtils.AvatarByEmail .AuthorSignature.Email .AuthorSignature.Name 20 -}} - {{- end -}} - {{- if .CoAuthors -}} - {{- $coCount := len .CoAuthors -}} - {{- $maxCo := 9 -}} - {{- $visibleCo := .CoAuthors -}} - {{- $overflow := 0 -}} - {{- if gt $coCount $maxCo -}} - {{- $visibleCo = slice .CoAuthors 0 $maxCo -}} - {{- $overflow = Eval $coCount "-" $maxCo -}} + {{- $coCount := len .CoAuthors -}} + {{- $maxCo := 9 -}} + {{- $visibleCo := .CoAuthors -}} + {{- $overflow := 0 -}} + {{- if gt $coCount $maxCo -}} + {{- $visibleCo = slice .CoAuthors 0 $maxCo -}} + {{- $overflow = Eval $coCount "-" $maxCo -}} + {{- end -}} + + + {{- if .AuthorUser -}} + {{- ctx.AvatarUtils.Avatar .AuthorUser 20 -}} + {{- else -}} + {{- ctx.AvatarUtils.AvatarByEmail .AuthorSignature.Email .AuthorSignature.Name 20 -}} {{- end -}} {{- range $visibleCo -}} {{- if .GiteaUser -}} @@ -22,24 +22,10 @@ {{- ctx.AvatarUtils.AvatarByEmail .TrailerSignature.Email .TrailerSignature.Name 20 -}} {{- end -}} {{- end -}} - {{- if gt $overflow 0 -}} - +{{$overflow}} - {{- end -}} - {{- else -}} - {{- $coCount := len .CoAuthorSignatures -}} - {{- $maxCo := 9 -}} - {{- $visibleCo := .CoAuthorSignatures -}} - {{- $overflow := 0 -}} - {{- if gt $coCount $maxCo -}} - {{- $visibleCo = slice .CoAuthorSignatures 0 $maxCo -}} - {{- $overflow = Eval $coCount "-" $maxCo -}} - {{- end -}} - {{- range $visibleCo -}} - {{- ctx.AvatarUtils.AvatarByEmail .Email .Name 20 -}} - {{- end -}} - {{- if gt $overflow 0 -}} - +{{$overflow}} - {{- end -}} + + {{- if gt $overflow 0 -}} + {{- $overflowLabel := ctx.Locale.TrN $overflow "repo.commits.coauthor_others_1" "repo.commits.coauthor_others_n" $overflow -}} + +{{$overflow}} {{- end -}} {{else if .AuthorUser}} diff --git a/templates/repo/commit_coauthor_avatars.tmpl b/templates/repo/commit_coauthor_avatars.tmpl index 04c04d5186..a6b73de794 100644 --- a/templates/repo/commit_coauthor_avatars.tmpl +++ b/templates/repo/commit_coauthor_avatars.tmpl @@ -29,7 +29,8 @@ {{- end -}} {{- end -}} {{- else -}} - {{- ctx.Locale.TrN $coCount "repo.commits.coauthor_others_1" "repo.commits.coauthor_others_n" $coCount -}} + {{- $peopleCount := Eval $coCount "+" 1 -}} + {{- ctx.Locale.Tr "repo.commits.coauthor_people" $peopleCount -}} {{- end -}} {{- else -}} {{- if .AuthorUser -}} diff --git a/templates/user/dashboard/feeds.tmpl b/templates/user/dashboard/feeds.tmpl index b3283a8b14..0981fea8f1 100644 --- a/templates/user/dashboard/feeds.tmpl +++ b/templates/user/dashboard/feeds.tmpl @@ -91,7 +91,7 @@ {{range $pushCommit := $push.Commits}} {{$commitLink := printf "%s/commit/%s" $repoLink .Sha1}}
- {{template "repo/commit_coauthor_avatar_stack" dict "AuthorSignature" (dict "Email" .AuthorEmail "Name" .AuthorName) "CoAuthorSignatures" .CoAuthors}} + {{template "repo/commit_coauthor_avatar_stack" dict "AuthorSignature" .AuthorSignature "CoAuthors" .CoAuthorUsers}} {{ShortSha .Sha1}} {{ctx.RenderUtils.RenderCommitMessage $pushCommit.Message $repo}} diff --git a/web_src/css/repo.css b/web_src/css/repo.css index 135eeec281..0a30cf6ce5 100644 --- a/web_src/css/repo.css +++ b/web_src/css/repo.css @@ -1408,13 +1408,18 @@ tbody.commit-list { white-space: nowrap; } -.coauthor-avatar-stack { +.coauthor-avatar-stack-wrapper { display: inline-flex; align-items: center; vertical-align: middle; margin-right: 0.25rem; } +.coauthor-avatar-stack { + display: inline-flex; + align-items: center; +} + /* each direct child of the stack is either (linked user) or (no account) */ .coauthor-avatar-stack > * { margin-left: -12px;