enhance: improve commit page header (#39229)

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

models/asymkey/gpg_key_commit_verification.go

@@ -38,6 +38,8 @@ type SignCommit struct {
 }
 
 const (
+	VerificationReasonCommitNotSigned = "gpg.error.not_signed_commit"
+
 	// BadSignature is used as the reason when the signature has a KeyID that is in the db
 	// but no key that has that ID verifies the signature. This is a suspicious failure.
 	BadSignature = "gpg.error.probable_bad_signature"
@@ -183,3 +185,7 @@ func CalculateTrustStatus(verification *CommitVerification, repoTrustModel repo_
 
 	return err
 }
+
+func (cv *CommitVerification) IsCommitNotSigned() bool {
+	return !cv.Verified && cv.Reason == VerificationReasonCommitNotSigned
+}

modules/git/commit_message.go

@@ -177,6 +177,10 @@ func (c *Commit) AllAuthorIdentities() []*CommitIdentity {
 	return c.allAuthors
 }
 
+func (c *Commit) CommitterIsAuthor() bool {
+	return c.Committer.Name == c.Author.Name && c.Committer.Email == c.Author.Email
+}
+
 // Git identities are not RFC 5322 addresses: net/mail rejects names like "dependabot[bot]", so fall back to the angle-addr.
 func parseCommitIdentityValue(value string) (name, email string) {
 	if addr, err := mail.ParseAddress(value); err == nil {
@@ -188,14 +192,3 @@ func parseCommitIdentityValue(value string) (name, email string) {
 	}
 	return strings.TrimSpace(value[:begin]), strings.TrimSpace(value[begin+1 : end])
 }
-
-func (c *Commit) CoAuthorIdentities() (coAuthors []*CommitIdentity) {
-	all := c.AllAuthorIdentities()
-	if len(all) == 0 {
-		return nil
-	}
-	if all[0].role == commitIdentityRoleAuthor {
-		return all[1:]
-	}
-	return all
-}

modules/git/commit_message_test.go

@@ -87,36 +87,13 @@ func TestCommitMessageParticipants(t *testing.T) {
 				// if it is a problem, the caller should fix the problem (provide correct "author")
 				[]*CommitIdentity{idt("c", "c@m.com", roleCoAuthor)},
 			},
-		}
-		for _, c := range cases {
-			assert.Equal(t, c.identities, c.commit.AllAuthorIdentities(), "case: %s", c.name)
-		}
-	})
-	t.Run("CoAuthors", func(t *testing.T) {
-		cases := []testCase{
-			{
-				"GenuineCoAuthor",
-				&Commit{
-					Author: sig("a", "a@m.com"), Committer: sig("c", "c@m.com"),
-					CommitMessage: CommitMessage{MessageRaw: "Co-authored-by: x <x@m.com>"},
-				},
-				[]*CommitIdentity{idt("x", "x@m.com", roleCoAuthor)},
-			},
 			{
 				"CoAuthorIsCommitter",
 				&Commit{
 					Author: sig("a", "a@m.com"), Committer: sig("c", "c@m.com"),
 					CommitMessage: CommitMessage{MessageRaw: "Co-authored-by: c <c@m.com>"},
 				},
-				[]*CommitIdentity{idt("c", "c@m.com", roleCoAuthor)},
-			},
-			{
-				"CoAuthorIsAuthor",
-				&Commit{
-					Author: sig("a", "a@m.com"), Committer: sig("c", "c@m.com"),
-					CommitMessage: CommitMessage{MessageRaw: "Co-authored-by: a <a@m.com>"},
-				},
-				[]*CommitIdentity{},
+				[]*CommitIdentity{idt("a", "a@m.com", roleAuthor), idt("c", "c@m.com", roleCoAuthor)},
 			},
 			{
 				"CoAuthorNameOnlyAndDuplicate",
@@ -124,7 +101,7 @@ func TestCommitMessageParticipants(t *testing.T) {
 					Author: sig("a", "a@m.com"), Committer: sig("c", "c@m.com"),
 					CommitMessage: CommitMessage{MessageRaw: "Co-authored-by: b\nCo-authored-by: b\nCo-authored-by: c"},
 				},
-				[]*CommitIdentity{idt("b", "", roleCoAuthor), idt("c", "", roleCoAuthor)},
+				[]*CommitIdentity{idt("a", "a@m.com", roleAuthor), idt("b", "", roleCoAuthor), idt("c", "", roleCoAuthor)},
 			},
 			{
 				"CoAuthorNameNotAnEmailAddress", // names net/mail rejects, e.g. bots and names with a comma
@@ -133,13 +110,14 @@ func TestCommitMessageParticipants(t *testing.T) {
 					CommitMessage: CommitMessage{MessageRaw: "Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>\nCo-authored-by: Smith, John <j@m.com>"},
 				},
 				[]*CommitIdentity{
+					idt("a", "a@m.com", roleAuthor),
 					idt("dependabot[bot]", "49699333+dependabot[bot]@users.noreply.github.com", roleCoAuthor),
 					idt("Smith, John", "j@m.com", roleCoAuthor),
 				},
 			},
 		}
 		for _, c := range cases {
-			assert.Equal(t, c.identities, c.commit.CoAuthorIdentities(), "case: %s", c.name)
+			assert.Equal(t, c.identities, c.commit.AllAuthorIdentities(), "case: %s", c.name)
 		}
 	})
 }

options/locale/locale_en-US.json

@@ -2578,6 +2578,7 @@
   "repo.settings.rename_branch": "Rename branch",
   "repo.diff.browse_source": "Browse Source",
   "repo.diff.parent": "parent",
+  "repo.diff.parents": "parents",
   "repo.diff.commit": "commit",
   "repo.diff.git-notes": "Notes",
   "repo.diff.data_not_available": "Diff Content Not Available",
@@ -2627,8 +2628,8 @@
   "repo.diff.review.self_reject": "Pull request authors can't request changes on their own pull request",
   "repo.diff.review.reject": "Request changes",
   "repo.diff.review.self_approve": "Pull request authors can't approve their own pull request",
-  "repo.diff.committed_by": "committed by",
-  "repo.diff.coauthored_by": "co-authored by",
+  "repo.diff.committed_at": "%[1]s committed %[2]s",
+  "repo.diff.authored_committed_at": "%[1]s authored and %[2]s committed %[3]s",
   "repo.commits.avatar_stack_and": "and",
   "repo.commits.avatar_stack_people": "%d people",
   "repo.diff.protected": "Protected",

routers/web/repo/commit.go

@@ -393,9 +393,8 @@ func Diff(ctx *context.Context) {
 	ctx.Data["CommitStatuses"] = statuses
 
 	verification := asymkey_service.ParseCommitWithSignature(ctx, commit)
-	ctx.Data["Verification"] = verification
-	ctx.Data["Author"] = user_model.GetUserByGitAuthor(ctx, commit)
-	ctx.Data["CommitOtherParticipants"] = gituser.BuildAvatarStackData(ctx, commit.CoAuthorIdentities(), nil).Participants
+	ctx.Data["Verification"] = util.Iif(verification.IsCommitNotSigned(), nil, verification)
+	ctx.Data["CommitAvatarStackData"] = gituser.BuildAvatarStackData(ctx, commit.AllAuthorIdentities(), nil)
 	ctx.Data["Parents"] = parents
 	ctx.Data["DiffNotAvailable"] = diffShortStat.NumFiles == 0
 	ctx.Data["ShowDiffSummaryInToolbar"] = false

services/asymkey/commit.go

@@ -46,7 +46,7 @@ func ParseCommitWithSignatureCommitter(ctx context.Context, c *git.Commit, commi
 		return &asymkey_model.CommitVerification{
 			CommittingUser: committer,
 			Verified:       false,
-			Reason:         "gpg.error.not_signed_commit",
+			Reason:         asymkey_model.VerificationReasonCommitNotSigned,
 		}
 	}
 	// to support instance key, we need a fake committer user (not really needed, but legacy code accesses the committer without nil-check)

services/repository/files/commit.go

@@ -6,6 +6,7 @@ package files
 import (
 	"context"
 
+	"gitea.dev/models/asymkey"
 	"gitea.dev/modules/git"
 	"gitea.dev/modules/structs"
 	asymkey_service "gitea.dev/services/asymkey"
@@ -28,7 +29,7 @@ func GetPayloadCommitVerification(ctx context.Context, commit *git.Commit) *stru
 	verification.Verified = commitVerification.Verified
 	verification.Reason = commitVerification.Reason
 	if verification.Reason == "" && !verification.Verified {
-		verification.Reason = "gpg.error.not_signed_commit"
+		verification.Reason = asymkey.VerificationReasonCommitNotSigned
 	}
 	return verification
 }

templates/repo/commit_page.tmpl

@@ -3,16 +3,19 @@
 <div role="main" aria-label="{{.Title}}" class="page-content repository diff">
 	{{template "repo/header" .}}
 	<div class="ui container fluid padded">
-		<div class="ui top attached header clearing segment tw-relative commit-header">
-			<div class="tw-flex tw-mb-4 tw-gap-1">
-				<h3 class="tw-mb-0 tw-flex-1"><span class="commit-summary" title="{{.Commit.MessageTitle}}">{{ctx.RenderUtils.RenderCommitMessage .Commit.MessageUTF8 $.Repository}}</span>{{template "repo/commit_statuses" dict "Status" .CommitStatus "Statuses" .CommitStatuses "AdditionalClasses" "tw-inline"}}</h3>
+		<div class="ui top attached header segment">
+			<div class="flex-left-right tw-mb-4 tw-items-start">
+				<h3 class="tw-m-0">
+					<span class="commit-summary tw-align-middle" title="{{.Commit.MessageTitle}}">{{ctx.RenderUtils.RenderCommitMessage .Commit.MessageUTF8 $.Repository}}</span>
+					{{template "repo/commit_statuses" dict "Status" .CommitStatus "Statuses" .CommitStatuses}}
+				</h3>
 				{{if not $.PageIsWiki}}
 					<div class="commit-header-buttons">
-						<a class="ui primary tiny button" href="{{.SourcePath}}">
+						<a class="ui tiny button" href="{{.SourcePath}}">
 							{{ctx.Locale.Tr "repo.diff.browse_source"}}
 						</a>
 						{{if and ($.Permission.CanWrite ctx.Consts.RepoUnitTypeCode) (not $.Repository.IsArchived) (not .IsDeleted)}}{{- /* */ -}}
-							<div class="ui dropdown primary tiny button">
+							<div class="ui dropdown tiny button">
 								{{ctx.Locale.Tr "repo.commit.operations"}}
 								{{svg "octicon-triangle-down" 14 "dropdown icon"}}
 								<div class="menu">
@@ -126,48 +129,21 @@
 		</div>
 
 		<div class="ui attached segment flex-text-block tw-flex-wrap">
-			<div class="flex-text-inline">
-				{{if .Author}}
-					{{ctx.AvatarUtils.Avatar .Author 20}}
-					<strong>{{.Author.GetShortDisplayNameLinkHTML}}</strong>
+			{{$authors := ctx.RenderUtils.AvatarStackWithNames .CommitAvatarStackData}}
+			{{$committedAt := DateUtils.TimeSince .Commit.Committer.When}}
+			{{if .Commit.CommitterIsAuthor}}
+				<span class="flex-text-inline tw-text-text-light">{{ctx.Locale.Tr "repo.diff.committed_at" $authors $committedAt}}</span>
+			{{else}}
+				{{$committerAvatar := ""}}{{$committerDisplayName := ""}}
+				{{if .Verification.CommittingUser}}
+					{{$committerAvatar = ctx.AvatarUtils.Avatar .Verification.CommittingUser 20}}
+					{{$committerDisplayName = .Verification.CommittingUser.GetShortDisplayNameLinkHTML}}
 				{{else}}
-					{{ctx.AvatarUtils.AvatarByEmail .Commit.Author.Email .Commit.Author.Email 20}}
-					<strong>{{.Commit.Author.Name}}</strong>
+					{{$committerAvatar = ctx.AvatarUtils.AvatarByEmail .Commit.Committer.Email .Commit.Committer.Email 20}}
+					{{$committerDisplayName = .Commit.Committer.Name}}
 				{{end}}
-			</div>
-			<span class="tw-text-text-light">{{DateUtils.TimeSince .Commit.Committer.When}}</span>
-
-			{{$committerIsAuthor := and (eq .Commit.Committer.Name .Commit.Author.Name) (eq .Commit.Committer.Email .Commit.Author.Email)}}
-			{{if not $committerIsAuthor}}
-				<div class="flex-text-inline">
-					<span class="tw-text-text-light">{{ctx.Locale.Tr "repo.diff.committed_by"}}</span>
-					{{if and .Verification.CommittingUser}}
-						{{ctx.AvatarUtils.Avatar .Verification.CommittingUser 20}}
-						<strong>{{.Verification.CommittingUser.GetShortDisplayNameLinkHTML}}</strong>
-					{{else}}
-						{{ctx.AvatarUtils.AvatarByEmail .Commit.Committer.Email .Commit.Committer.Email 20}}
-						<strong>{{.Commit.Committer.Name}}</strong>
-					{{end}}
-				</div>
-			{{end}}
-
-			{{if .CommitOtherParticipants}}
-				<div class="flex-text-inline">
-					<span class="tw-text-text-light">{{ctx.Locale.Tr "repo.diff.coauthored_by"}}</span>
-					{{range $participant := .CommitOtherParticipants}}
-						{{$user := $participant.GiteaUser}}
-						{{$gitIdentity := $participant.GitIdentity}}
-						{{if $user}}
-							{{ctx.AvatarUtils.Avatar $user 20}}
-							<strong>{{$user.GetShortDisplayNameLinkHTML}}</strong>
-						{{else}}
-							{{$gitName := $gitIdentity.Name}}
-							{{$gitEmail := $gitIdentity.Email}}
-							{{ctx.AvatarUtils.AvatarByEmail $gitEmail $gitEmail 20}}{{/* use the same layout as the "author" above */}}
-							<strong>{{$gitName}}</strong>
-						{{end}}
-					{{end}}
-				</div>
+				{{$committer := HTMLFormat `%s <strong>%s</strong>` $committerAvatar $committerDisplayName}}
+				<span class="flex-text-inline tw-text-text-light">{{ctx.Locale.Tr "repo.diff.authored_committed_at" $authors $committer $committedAt}}</span>
 			{{end}}
 
 			{{if .Verification}}
@@ -176,18 +152,19 @@
 
 			<div class="tw-flex-1"></div>
 
-			<div class="flex-text-inline tw-gap-5">
+			<div class="flex-text-block tw-flex-wrap">
 				{{if .Parents}}
-					<div class="flex-text-inline">
-						<span>{{ctx.Locale.Tr "repo.diff.parent"}}</span>
-						{{range .Parents}}
-							<a class="ui label commit-id-short" href="{{$commitLinkBase}}/{{PathEscape .}}">{{ShortSha .}}</a>
+					<div class="flex-text-inline tw-flex-wrap">
+						<span class="tw-text-text-light">{{len .Parents}} {{ctx.Locale.TrN (len .Parents) "repo.diff.parent" "repo.diff.parents"}}</span>
+						{{range $parentCommitID := .Parents}}
+							<a class="ui label commit-id-short" href="{{$commitLinkBase}}/{{PathEscape $parentCommitID}}">{{ShortSha $parentCommitID}}</a>
 						{{end}}
 					</div>
 				{{end}}
 				<div class="flex-text-inline">
-					<span>{{ctx.Locale.Tr "repo.diff.commit"}}</span>
+					<span class="flex-text-inline tw-text-text-light">{{ctx.Locale.Tr "repo.diff.commit"}}</span>
 					<a class="ui label commit-id-short" href="{{$commitLinkBase}}/{{PathEscape .CommitID}}">{{ShortSha .CommitID}}</a>
+					<button class="btn interact-fg tw-p-1" data-clipboard-text="{{.CommitID}}" data-tooltip-content="{{ctx.Locale.Tr "copy_hash"}}">{{svg "octicon-copy" 14}}</button>
 				</div>
 			</div>
 		</div>

web_src/css/avatar.css

@@ -23,9 +23,8 @@ img.ui.avatar,
   max-width: 240px;
 }
 
-/* use semibold for latest commit author */
-.latest-commit .avatar-stack-names > a,
-.latest-commit .avatar-stack-names > .avatar-stack-popup-trigger {
+.avatar-stack-names > a,
+.avatar-stack-names > .avatar-stack-popup-trigger {
   font-weight: var(--font-weight-semibold);
 }
 

web_src/css/repo.css

@@ -131,7 +131,8 @@
   margin-bottom: 12px;
 }
 
-.commit-summary {
+.latest-commit .commit-summary,
+.repo-file-item .commit-summary {
   flex: 1;
   overflow-wrap: anywhere;
   overflow: hidden;
@@ -139,11 +140,6 @@
   text-overflow: ellipsis;
 }
 
-.commit-header .commit-summary,
-td .commit-summary {
-  white-space: normal;
-}
-
 .latest-commit {
   display: flex;
   flex: 1;
@@ -1686,10 +1682,6 @@ tbody.commit-list {
   .repository.view.issue .comment-list .comment-header-right .role-label {
     display: none;
   }
-  .commit-header h3 {
-    flex-basis: auto !important;
-    margin-bottom: 0.5rem !important;
-  }
   .commit-table {
     overflow-x: auto;
   }