tenseleyflow/shithub / ea86242

Browse files

Align pull request merge flow

Authored by mfwolffe <wolffemf@dukes.jmu.edu>
SHA
ea8624223c026e64abe77417e970fd9dd9ebeab7
Parents
5eb7056
Tree
806a06c

10 changed files

StatusFile+-
M docs/internal/pull-requests.md 12 2
M internal/repos/git/branchops.go 33 0
A internal/repos/git/branchops_test.go 48 0
M internal/web/handlers/repo/issues.go 18 0
A internal/web/handlers/repo/pull_view_helpers_test.go 55 0
M internal/web/handlers/repo/pulls.go 127 22
M internal/web/static/css/shithub.css 53 6
A internal/web/static/js/pull-view.js 37 0
M internal/web/templates/_layout.html 1 0
M internal/web/templates/repo/pull_view.html 43 9
docs/internal/pull-requests.mdmodified
@@ -51,6 +51,7 @@ so a self-merge can't be opened. Cross-fork PRs land in S27.
5151
 | `POST /{owner}/{repo}/pulls/{number}/state`           | RequireUser   |
5252
 | `POST /{owner}/{repo}/pulls/{number}/ready`           | RequireUser   |
5353
 | `POST /{owner}/{repo}/pulls/{number}/merge`           | RequireUser   |
54
+| `POST /{owner}/{repo}/pulls/{number}/delete-branch`   | RequireUser   |
5455
5556
 The pull-request list's "New pull request" button starts at
5657
 `/{owner}/{repo}/compare`, where the user picks base/head refs. Once
@@ -170,8 +171,17 @@ noreply emails are post-MVP.
170171
 - Files tab uses the existing S19 diff renderer fed from
171172
   `compareSourceMergeBase` (three-dot diff, base...head).
172173
 - The Checks tab is a placeholder — real check runs land in S24.
173
-- The merge form hides disallowed methods and preselects the repo's
174
-  `default_merge_method`.
174
+- The merge action is a GitHub-style two-step flow: the ready merge
175
+  row opens a confirmation panel with editable commit subject/body,
176
+  hidden disallowed methods, and the repo's `default_merge_method`
177
+  preselected. The handler still validates the selected method
178
+  server-side.
179
+- After a successful same-repo merge, the conversation shows the
180
+  merged timeline event plus the "successfully merged and closed"
181
+  branch-cleanup card. The timeline event links to the merge commit's
182
+  commit detail page. Deleting the head branch uses
183
+  `git update-ref -d refs/heads/<head> <expected_head_oid>` so a
184
+  branch that moved after merge cannot be removed accidentally.
175185
176186
 ## Errors
177187
internal/repos/git/branchops.gomodified
@@ -99,6 +99,39 @@ func UpdateRefCAS(ctx context.Context, gitDir, ref, newOID, oldOID string) error
9999
 // ref moved between our read and our update.
100100
 var ErrRefRaced = errors.New("repogit: ref moved concurrently")
101101
102
+// DeleteBranch removes refs/heads/<branch>. When oldOID is non-empty,
103
+// git's update-ref compare-and-delete guard is used so a branch that
104
+// moved after the caller rendered the page is not deleted accidentally.
105
+func DeleteBranch(ctx context.Context, gitDir, branch, oldOID string) error {
106
+	branch = strings.TrimSpace(branch)
107
+	if branch == "" || strings.HasPrefix(branch, "-") {
108
+		return ErrRefNotFound
109
+	}
110
+	check := exec.CommandContext(ctx, "git", "-C", gitDir, "check-ref-format", "--branch", branch)
111
+	if out, err := check.CombinedOutput(); err != nil {
112
+		return fmt.Errorf("check-ref-format %s: %w (%s)", branch, err, strings.TrimSpace(string(out)))
113
+	}
114
+	ref := "refs/heads/" + branch
115
+	args := []string{"-C", gitDir, "update-ref", "-d", ref}
116
+	if strings.TrimSpace(oldOID) != "" {
117
+		args = append(args, oldOID)
118
+	}
119
+	cmd := exec.CommandContext(ctx, "git", args...)
120
+	out, err := cmd.CombinedOutput()
121
+	if err == nil {
122
+		return nil
123
+	}
124
+	msg := strings.TrimSpace(string(out))
125
+	switch {
126
+	case strings.Contains(msg, "unable to resolve reference"), strings.Contains(msg, "reference does not exist"):
127
+		return ErrRefNotFound
128
+	case strings.Contains(msg, "cannot lock ref"), strings.Contains(msg, "expected"):
129
+		return ErrRefRaced
130
+	default:
131
+		return fmt.Errorf("delete-ref %s: %w (%s)", ref, err, msg)
132
+	}
133
+}
134
+
102135
 // FetchIntoNamespace fetches a single ref from `srcRepoDir` into
103136
 // `dstRepoDir` under the supplied refspec. The dst-side ref name is
104137
 // the second half of the refspec. Used by S27 cross-fork PR support
internal/repos/git/branchops_test.goadded
@@ -0,0 +1,48 @@
1
+// SPDX-License-Identifier: AGPL-3.0-or-later
2
+
3
+package git_test
4
+
5
+import (
6
+	"context"
7
+	"errors"
8
+	"strings"
9
+	"testing"
10
+	"time"
11
+
12
+	repogit "github.com/tenseleyFlow/shithub/internal/repos/git"
13
+)
14
+
15
+func TestDeleteBranchUsesExpectedOID(t *testing.T) {
16
+	t.Parallel()
17
+	ctx := context.Background()
18
+	gitDir := initBare(t)
19
+	when := time.Date(2026, 5, 12, 12, 0, 0, 0, time.UTC)
20
+	base, err := repogit.InitialCommit{
21
+		GitDir:      gitDir,
22
+		AuthorName:  "Alice",
23
+		AuthorEmail: "alice@example.com",
24
+		Branch:      "trunk",
25
+		When:        when,
26
+		Files:       []repogit.FileEntry{{Path: "README.md", Body: []byte("base\n")}},
27
+	}.Build(ctx)
28
+	if err != nil {
29
+		t.Fatalf("Build base: %v", err)
30
+	}
31
+	if out, err := gitCmd("-C", gitDir, "update-ref", "refs/heads/topic", base).CombinedOutput(); err != nil {
32
+		t.Fatalf("create branch: %v\n%s", err, out)
33
+	}
34
+
35
+	if err := repogit.DeleteBranch(ctx, gitDir, "topic", strings.Repeat("f", 40)); !errors.Is(err, repogit.ErrRefRaced) {
36
+		t.Fatalf("DeleteBranch stale oid = %v, want ErrRefRaced", err)
37
+	}
38
+	if _, err := repogit.ResolveRefOID(ctx, gitDir, "refs/heads/topic"); err != nil {
39
+		t.Fatalf("topic should still exist after stale delete: %v", err)
40
+	}
41
+
42
+	if err := repogit.DeleteBranch(ctx, gitDir, "topic", base); err != nil {
43
+		t.Fatalf("DeleteBranch current oid: %v", err)
44
+	}
45
+	if _, err := repogit.ResolveRefOID(ctx, gitDir, "refs/heads/topic"); !errors.Is(err, repogit.ErrRefNotFound) {
46
+		t.Fatalf("topic lookup after delete = %v, want ErrRefNotFound", err)
47
+	}
48
+}
internal/web/handlers/repo/issues.gomodified
@@ -370,6 +370,8 @@ type issueTimelineRow struct {
370370
 	LabelName   string
371371
 	LabelColor  string
372372
 	CommentID   int64
373
+	CommitSHA   string
374
+	ShortCommit string
373375
 	LinkedState bool
374376
 }
375377
@@ -411,6 +413,10 @@ func (h *Handlers) issueTimelineRows(
411413
 			row.CommentID = id
412414
 			row.LinkedState = e.Kind == "closed" || e.Kind == "reopened"
413415
 		}
416
+		if commit := metaString(meta, "commit"); commit != "" {
417
+			row.CommitSHA = commit
418
+			row.ShortCommit = shortSHA(commit)
419
+		}
414420
 		if id := metaInt64(meta, "label_id"); id != 0 {
415421
 			if l, ok := labelByID[id]; ok {
416422
 				row.LabelName = l.Name
@@ -471,6 +477,16 @@ func metaInt64(meta map[string]any, key string) int64 {
471477
 	}
472478
 }
473479
480
+func metaString(meta map[string]any, key string) string {
481
+	if meta == nil {
482
+		return ""
483
+	}
484
+	if s, ok := meta[key].(string); ok {
485
+		return s
486
+	}
487
+	return ""
488
+}
489
+
474490
 func issueEventMessage(kind string) string {
475491
 	switch kind {
476492
 	case "closed":
@@ -495,6 +511,8 @@ func issueEventMessage(kind string) string {
495511
 		return "unassigned a user"
496512
 	case "referenced":
497513
 		return "referenced this issue"
514
+	case "merged":
515
+		return "merged commit"
498516
 	default:
499517
 		return kind
500518
 	}
internal/web/handlers/repo/pull_view_helpers_test.goadded
@@ -0,0 +1,55 @@
1
+// SPDX-License-Identifier: AGPL-3.0-or-later
2
+
3
+package repo
4
+
5
+import (
6
+	"encoding/json"
7
+	"testing"
8
+	"time"
9
+
10
+	"github.com/jackc/pgx/v5/pgtype"
11
+
12
+	issuesdb "github.com/tenseleyFlow/shithub/internal/issues/sqlc"
13
+	pullsdb "github.com/tenseleyFlow/shithub/internal/pulls/sqlc"
14
+)
15
+
16
+func TestDefaultMergeSubjectUsesPullNumberAndHeadOwner(t *testing.T) {
17
+	t.Parallel()
18
+	got := defaultMergeSubject(pullsdb.GetPullRequestByRepoAndNumberRow{
19
+		INumber: 138,
20
+		HeadRef: "s41h/dogfood-ga-audit",
21
+	}, "tenseleyFlow")
22
+	want := "Merge pull request #138 from tenseleyFlow/s41h/dogfood-ga-audit"
23
+	if got != want {
24
+		t.Fatalf("defaultMergeSubject = %q, want %q", got, want)
25
+	}
26
+}
27
+
28
+func TestIssueTimelineRowsDecoratesMergedEvent(t *testing.T) {
29
+	t.Parallel()
30
+	meta, err := json.Marshal(map[string]string{
31
+		"method": "merge",
32
+		"commit": "5eb70568f00dbabe111111111111111111111111",
33
+	})
34
+	if err != nil {
35
+		t.Fatal(err)
36
+	}
37
+	rows := (*Handlers)(nil).issueTimelineRows(nil, []issuesdb.IssueEvent{{
38
+		Kind:        "merged",
39
+		Meta:        meta,
40
+		ActorUserID: pgtype.Int8{Int64: 7, Valid: true},
41
+		CreatedAt:   pgtype.Timestamptz{Time: time.Unix(10, 0), Valid: true},
42
+	}}, nil, nil, func(id int64) string {
43
+		if id == 7 {
44
+			return "mfwolffe"
45
+		}
46
+		return ""
47
+	})
48
+	if len(rows) != 1 {
49
+		t.Fatalf("rows len = %d, want 1", len(rows))
50
+	}
51
+	row := rows[0]
52
+	if row.Message != "merged commit" || row.CommitSHA == "" || row.ShortCommit != "5eb7056" || row.ActorName != "mfwolffe" {
53
+		t.Fatalf("merged row = %#v", row)
54
+	}
55
+}
internal/web/handlers/repo/pulls.gomodified
@@ -99,6 +99,7 @@ func (h *Handlers) MountPulls(r chi.Router) {
9999
 		r.Post("/{owner}/{repo}/pulls/{number}/state", h.pullSetState)
100100
 		r.Post("/{owner}/{repo}/pulls/{number}/ready", h.pullSetReady)
101101
 		r.Post("/{owner}/{repo}/pulls/{number}/merge", h.pullMerge)
102
+		r.Post("/{owner}/{repo}/pulls/{number}/delete-branch", h.pullDeleteHeadBranch)
102103
 	})
103104
 	// S23 review surface — its own group so the auth-required wrapper
104105
 	// is shared cleanly without rewriting this file's existing one.
@@ -396,6 +397,38 @@ func (h *Handlers) renderPullPage(w http.ResponseWriter, r *http.Request, tab st
396397
 		pr.BaseOid != "" && pr.HeadOid != "" {
397398
 		h.kickMergeability(r, pr.IID)
398399
 	}
400
+	viewer := middleware.CurrentUserFromContext(r.Context())
401
+	actor := viewer.PolicyActor()
402
+	pdeps := policy.Deps{Pool: h.d.Pool}
403
+	repoRef := policy.NewRepoRefFromRepo(row)
404
+	stateRef := repoRef
405
+	if pr.IAuthorUserID.Valid {
406
+		stateRef.AuthorUserID = pr.IAuthorUserID.Int64
407
+	}
408
+	canReviewPull := policy.Can(r.Context(), pdeps, actor, policy.ActionPullReview, repoRef).Allow
409
+	canMergePull := policy.Can(r.Context(), pdeps, actor, policy.ActionPullMerge, repoRef).Allow
410
+	canSetPullState := policy.Can(r.Context(), pdeps, actor, policy.ActionPullClose, stateRef).Allow
411
+	canReadyPull := policy.Can(r.Context(), pdeps, actor, policy.ActionPullCreate, repoRef).Allow
412
+	headOwner := owner.Username
413
+	if pr.HeadRepoID != 0 {
414
+		if headRepo, err := h.rq.GetRepoOwnerUsernameByID(r.Context(), h.d.Pool, pr.HeadRepoID); err == nil {
415
+			if ownerName := repoOwnerName(headRepo.OwnerUsername); ownerName != "" {
416
+				headOwner = ownerName
417
+			}
418
+		}
419
+	}
420
+	headBranchExists := false
421
+	if pr.HeadRepoID == row.ID && pr.HeadRef != "" && pr.HeadRef != row.DefaultBranch {
422
+		if gitDir, err := h.d.RepoFS.RepoPath(owner.Username, row.Name); err == nil {
423
+			if _, err := repogit.ResolveRefOID(r.Context(), gitDir, "refs/heads/"+pr.HeadRef); err == nil {
424
+				headBranchExists = true
425
+			}
426
+		}
427
+	}
428
+	defaultMethod := string(row.DefaultMergeMethod)
429
+	if defaultMethod == "" {
430
+		defaultMethod = "merge"
431
+	}
399432
 	checkGroups := h.pullCheckGroups(r.Context(), row.ID, pr.HeadOid)
400433
 	stats := h.pullStats(r.Context(), pr, checkGroups)
401434
 	data := map[string]any{
@@ -411,21 +444,20 @@ func (h *Handlers) renderPullPage(w http.ResponseWriter, r *http.Request, tab st
411444
 		"CSRFToken":             middleware.CSRFTokenForRequest(r),
412445
 		"RepoActions":           h.repoActions(r, row.ID),
413446
 		"RepoCounts":            h.subnavCounts(r.Context(), row.ID, row.ForkCount),
414
-		"CanSettings":  h.canViewSettings(middleware.CurrentUserFromContext(r.Context())),
447
+		"CanSettings":           h.canViewSettings(viewer),
415448
 		"ActiveSubnav":          "pulls",
416
-	}
417
-	viewer := middleware.CurrentUserFromContext(r.Context())
418
-	actor := viewer.PolicyActor()
419
-	pdeps := policy.Deps{Pool: h.d.Pool}
420
-	repoRef := policy.NewRepoRefFromRepo(row)
421
-	stateRef := repoRef
422
-	if pr.IAuthorUserID.Valid {
423
-		stateRef.AuthorUserID = pr.IAuthorUserID.Int64
424
-	}
425
-	data["CanReviewPull"] = policy.Can(r.Context(), pdeps, actor, policy.ActionPullReview, repoRef).Allow
426
-	data["CanMergePull"] = policy.Can(r.Context(), pdeps, actor, policy.ActionPullMerge, repoRef).Allow
427
-	data["CanSetPullState"] = policy.Can(r.Context(), pdeps, actor, policy.ActionPullClose, stateRef).Allow
428
-	data["CanReadyPull"] = policy.Can(r.Context(), pdeps, actor, policy.ActionPullCreate, repoRef).Allow
449
+		"UsePullViewJS":         true,
450
+		"MergeDefaultMethod":    defaultMethod,
451
+		"MergeFormSubject":      defaultMergeSubject(pr, headOwner),
452
+		"MergeFormBody":         strings.TrimSpace(pr.ITitle),
453
+		"MergeAuthorLine":       h.mergeAuthorLine(r.Context(), viewer),
454
+		"CanDeleteHeadBranch":   canMergePull && pr.MergedAt.Valid && pr.HeadRepoID == row.ID && pr.HeadRef != row.DefaultBranch && headBranchExists,
455
+		"HeadBranchAlreadyGone": pr.MergedAt.Valid && pr.HeadRepoID == row.ID && pr.HeadRef != row.DefaultBranch && !headBranchExists,
456
+	}
457
+	data["CanReviewPull"] = canReviewPull
458
+	data["CanMergePull"] = canMergePull
459
+	data["CanSetPullState"] = canSetPullState
460
+	data["CanReadyPull"] = canReadyPull
429461
 	for k, v := range extras {
430462
 		data[k] = v
431463
 	}
@@ -433,6 +465,41 @@ func (h *Handlers) renderPullPage(w http.ResponseWriter, r *http.Request, tab st
433465
 	_ = h.d.Render.RenderPage(w, r, "repo/pull_view", data)
434466
 }
435467
468
+func repoOwnerName(raw interface{}) string {
469
+	switch v := raw.(type) {
470
+	case string:
471
+		return v
472
+	case []byte:
473
+		return string(v)
474
+	default:
475
+		return ""
476
+	}
477
+}
478
+
479
+func defaultMergeSubject(pr pullsdb.GetPullRequestByRepoAndNumberRow, headOwner string) string {
480
+	head := strings.TrimSpace(headOwner)
481
+	if head == "" {
482
+		head = "head"
483
+	}
484
+	if pr.HeadRef != "" {
485
+		head += "/" + pr.HeadRef
486
+	}
487
+	return "Merge pull request #" + strconv.FormatInt(pr.INumber, 10) + " from " + head
488
+}
489
+
490
+func (h *Handlers) mergeAuthorLine(ctx context.Context, viewer middleware.CurrentUser) string {
491
+	if viewer.IsAnonymous() {
492
+		return ""
493
+	}
494
+	email := viewer.Username + "@noreply.shithub.local"
495
+	if u, err := h.uq.GetUserByID(ctx, h.d.Pool, viewer.ID); err == nil && u.PrimaryEmailID.Valid {
496
+		if em, err := h.uq.GetUserEmailByID(ctx, h.d.Pool, u.PrimaryEmailID.Int64); err == nil && em.Verified {
497
+			email = string(em.Email)
498
+		}
499
+	}
500
+	return "This commit will be authored by " + email + "."
501
+}
502
+
436503
 func (h *Handlers) pullStats(ctx context.Context, pr pullsdb.GetPullRequestByRepoAndNumberRow, checkGroups []pullCheckSuiteView) pullPageStats {
437504
 	stats := pullPageStats{CheckState: "none"}
438505
 	if comments, err := h.iq.ListIssueComments(ctx, h.d.Pool, pr.IID); err == nil {
@@ -972,6 +1039,44 @@ func (h *Handlers) pullMerge(w http.ResponseWriter, r *http.Request) {
9721039
 	h.redirectPull(w, r, owner.Username, row.Name, pr.INumber)
9731040
 }
9741041
1042
+func (h *Handlers) pullDeleteHeadBranch(w http.ResponseWriter, r *http.Request) {
1043
+	row, owner, ok := h.loadRepoAndAuthorize(w, r, policy.ActionPullMerge)
1044
+	if !ok {
1045
+		return
1046
+	}
1047
+	pr, ok := h.loadPullByNumber(w, r, row.ID)
1048
+	if !ok {
1049
+		return
1050
+	}
1051
+	if err := r.ParseForm(); err != nil {
1052
+		h.d.Render.HTTPError(w, r, http.StatusBadRequest, "form parse")
1053
+		return
1054
+	}
1055
+	if !pr.MergedAt.Valid || pr.HeadRepoID != row.ID || strings.TrimSpace(pr.HeadRef) == "" || pr.HeadRef == row.DefaultBranch {
1056
+		h.d.Render.HTTPError(w, r, http.StatusConflict, "head branch cannot be deleted from this pull request")
1057
+		return
1058
+	}
1059
+	gitDir, err := h.d.RepoFS.RepoPath(owner.Username, row.Name)
1060
+	if err != nil {
1061
+		h.d.Render.HTTPError(w, r, http.StatusNotFound, "")
1062
+		return
1063
+	}
1064
+	if err := repogit.DeleteBranch(r.Context(), gitDir, pr.HeadRef, pr.HeadOid); err != nil {
1065
+		if errors.Is(err, repogit.ErrRefNotFound) {
1066
+			h.redirectPull(w, r, owner.Username, row.Name, pr.INumber)
1067
+			return
1068
+		}
1069
+		if errors.Is(err, repogit.ErrRefRaced) {
1070
+			h.d.Render.HTTPError(w, r, http.StatusConflict, "branch moved after this pull request was merged")
1071
+			return
1072
+		}
1073
+		h.d.Logger.WarnContext(r.Context(), "pulls: delete head branch", "error", err, "repo_id", row.ID, "pr", pr.INumber, "branch", pr.HeadRef)
1074
+		h.d.Render.HTTPError(w, r, http.StatusInternalServerError, "")
1075
+		return
1076
+	}
1077
+	h.redirectPull(w, r, owner.Username, row.Name, pr.INumber)
1078
+}
1079
+
9751080
 func (h *Handlers) handlePullWriteError(w http.ResponseWriter, r *http.Request, err error) {
9761081
 	switch {
9771082
 	case errors.Is(err, pulls.ErrAlreadyMerged):
internal/web/static/css/shithub.cssmodified
@@ -8442,7 +8442,8 @@ button.shithub-repo-action {
84428442
 .shithub-event {
84438443
   color: var(--fg-muted);
84448444
   font-size: 0.85rem;
8445
-  display: flex;
8445
+  display: grid;
8446
+  grid-template-columns: auto minmax(0, 1fr) auto;
84468447
   align-items: center;
84478448
   gap: 0.5rem;
84488449
   padding: 0.55rem 0;
@@ -8475,6 +8476,11 @@ button.shithub-repo-action {
84758476
 .shithub-event-icon svg { width: 0.8rem; height: 0.8rem; }
84768477
 .shithub-event-linked .shithub-event-icon { color: var(--fg-muted); }
84778478
 .shithub-event a { font-weight: 600; }
8479
+.shithub-event-text { min-width: 0; }
8480
+.shithub-event-actions {
8481
+  display: flex;
8482
+  gap: 0.5rem;
8483
+}
84788484
 .shithub-comment-form { display: flex; flex-direction: column; gap: 0.5rem; margin-top: 1rem; }
84798485
 .shithub-comment-form textarea, .shithub-issue-form textarea, .shithub-issue-form input[type=text] {
84808486
   font: inherit; padding: 0.5rem; border: 1px solid var(--border-default); border-radius: 6px; width: 100%;
@@ -8681,16 +8687,17 @@ button.shithub-repo-action {
86818687
   padding: 0.85rem 1rem;
86828688
 }
86838689
 .shithub-pull-deploy-box p { margin: 0.15rem 0 0; }
8684
-.shithub-pull-merge-box-ready { border-color: rgba(26, 127, 55, 0.55); }
8690
+.shithub-pull-merge-box-ready { border-color: rgba(26, 127, 55, 0.45); }
86858691
 .shithub-pull-merge-box-blocked { border-color: rgba(207, 34, 46, 0.55); }
86868692
 .shithub-pull-merge-box-pending { border-color: rgba(154, 103, 0, 0.55); }
86878693
 .shithub-pull-merge-box-merged { border-color: rgba(130, 80, 223, 0.55); }
86888694
 .shithub-pull-merge-row {
86898695
   display: grid;
8690
-  grid-template-columns: 2rem minmax(0, 1fr);
8696
+  grid-template-columns: 2rem minmax(0, 1fr) auto;
86918697
   gap: 0.75rem;
86928698
   padding: 0.85rem 1rem;
86938699
   border-bottom: 1px solid var(--border-default);
8700
+  align-items: start;
86948701
 }
86958702
 .shithub-pull-merge-row:last-child { border-bottom: none; }
86968703
 .shithub-pull-merge-row p { margin: 0.15rem 0 0; }
@@ -8709,6 +8716,11 @@ button.shithub-repo-action {
87098716
 .shithub-pull-check-row-failure .shithub-pull-status-icon { color: #cf222e; }
87108717
 .shithub-pull-check-row-pending .shithub-pull-status-icon { color: #bf8700; }
87118718
 .shithub-pull-status-icon-merged { color: #8250df; }
8719
+.shithub-mono-link {
8720
+  font-family: ui-monospace, SFMono-Regular, Consolas, "Liberation Mono", monospace;
8721
+  font-size: 0.92em;
8722
+  color: var(--fg-muted);
8723
+}
87128724
 .shithub-pull-merge-checks {
87138725
   list-style: none;
87148726
   margin: 0;
@@ -8732,13 +8744,13 @@ button.shithub-repo-action {
87328744
   background: var(--canvas-subtle);
87338745
   border-radius: 0 0 6px 6px;
87348746
 }
8735
-.shithub-pull-merge-form {
8747
+.shithub-pull-merge-choice {
87368748
   display: flex;
87378749
   align-items: center;
87388750
   gap: 0;
87398751
   margin: 0;
87408752
 }
8741
-.shithub-pull-merge-form select {
8753
+.shithub-pull-merge-choice select {
87428754
   min-height: 32px;
87438755
   padding: 0 0.55rem;
87448756
   border: 1px solid var(--border-default);
@@ -8748,7 +8760,42 @@ button.shithub-repo-action {
87488760
   color: var(--fg-default);
87498761
   font: inherit;
87508762
 }
8751
-.shithub-pull-merge-form .shithub-button { border-radius: 6px 0 0 6px; }
8763
+.shithub-pull-merge-choice .shithub-button { border-radius: 6px 0 0 6px; }
8764
+.shithub-pull-merge-confirm {
8765
+  display: grid;
8766
+  gap: 0.85rem;
8767
+  flex: 1 1 100%;
8768
+  margin: 0;
8769
+}
8770
+.shithub-pull-merge-confirm[hidden] { display: none; }
8771
+.shithub-pull-merge-confirm label {
8772
+  display: grid;
8773
+  gap: 0.35rem;
8774
+  font-weight: 600;
8775
+}
8776
+.shithub-pull-merge-confirm input,
8777
+.shithub-pull-merge-confirm textarea {
8778
+  width: 100%;
8779
+  border: 1px solid var(--border-default);
8780
+  border-radius: 6px;
8781
+  background: var(--canvas-default);
8782
+  color: var(--fg-default);
8783
+  font: inherit;
8784
+  padding: 0.55rem 0.65rem;
8785
+}
8786
+.shithub-pull-merge-confirm textarea {
8787
+  min-height: 10rem;
8788
+  resize: vertical;
8789
+  line-height: 1.5;
8790
+}
8791
+.shithub-pull-delete-branch-form {
8792
+  align-self: center;
8793
+  margin: 0;
8794
+}
8795
+.shithub-pull-branch-deleted {
8796
+  align-self: center;
8797
+  white-space: nowrap;
8798
+}
87528799
 .shithub-pull-state-form { margin-top: 0.5rem; }
87538800
 .shithub-pull-refs { display: flex; gap: 0.4rem; align-items: flex-end; }
87548801
 .shithub-pull-refs label { flex: 1; }
internal/web/static/js/pull-view.jsadded
@@ -0,0 +1,37 @@
1
+// SPDX-License-Identifier: AGPL-3.0-or-later
2
+
3
+(function () {
4
+  function initMergeBox(root) {
5
+    const openButton = root.querySelector("[data-pull-merge-open]");
6
+    const cancelButton = root.querySelector("[data-pull-merge-cancel]");
7
+    const choice = root.querySelector("[data-pull-merge-choice]");
8
+    const confirm = root.querySelector("[data-pull-merge-confirm]");
9
+    const method = root.querySelector("[data-pull-merge-method]");
10
+    const confirmMethod = root.querySelector("[data-pull-merge-confirm-method]");
11
+    const subject = root.querySelector("[data-pull-merge-subject]");
12
+    if (!openButton || !choice || !confirm) return;
13
+
14
+    function syncMethod() {
15
+      if (method && confirmMethod) confirmMethod.value = method.value;
16
+    }
17
+
18
+    openButton.addEventListener("click", function () {
19
+      syncMethod();
20
+      choice.hidden = true;
21
+      confirm.hidden = false;
22
+      if (subject) subject.focus();
23
+    });
24
+
25
+    if (cancelButton) {
26
+      cancelButton.addEventListener("click", function () {
27
+        confirm.hidden = true;
28
+        choice.hidden = false;
29
+        openButton.focus();
30
+      });
31
+    }
32
+
33
+    if (method) method.addEventListener("change", syncMethod);
34
+  }
35
+
36
+  document.querySelectorAll("[data-pull-merge-box]").forEach(initMergeBox);
37
+})();
internal/web/templates/_layout.htmlmodified
@@ -37,6 +37,7 @@
3737
   <link rel="stylesheet" href="/static/css/chroma.css">
3838
   {{ if flag . "UseHTMX" }}<script src="/static/vendor/htmx/htmx.min.js" defer></script>{{ end }}
3939
   {{ if flag . "UseCompareJS" }}<script src="/static/js/compare.js" defer></script>{{ end }}
40
+  {{ if flag . "UsePullViewJS" }}<script src="/static/js/pull-view.js" defer></script>{{ end }}
4041
   {{ if flag . "UseCommentEditor" }}<script src="/static/js/comment-editor.js" defer></script>{{ end }}
4142
 </head>
4243
 <body class="shithub-body">
internal/web/templates/repo/pull_view.htmlmodified
@@ -87,7 +87,7 @@
8787
               <time datetime="{{ .PR.ICreatedAt.Time.Format "2006-01-02T15:04:05Z" }}">{{ relativeTime .PR.ICreatedAt.Time }}</time>
8888
             </div>
8989
             <div class="shithub-comment-body markdown-body">
90
-              {{ if .PR.IBodyHtmlCached.Valid }}{{ safeHTML .PR.IBodyHtmlCached.String }}{{ else }}<p>{{ .PR.IBody }}</p>{{ end }}
90
+              {{ if .PR.IBodyHtmlCached.Valid }}{{ safeHTML .PR.IBodyHtmlCached.String }}{{ else if .PR.IBody }}<p>{{ .PR.IBody }}</p>{{ else }}<p><em>No description provided.</em></p>{{ end }}
9191
             </div>
9292
           </div>
9393
@@ -108,6 +108,7 @@
108108
               <span class="shithub-event-icon" aria-hidden="true">
109109
                 {{ if eq .E.Kind "closed" }}{{ octicon "issue-closed" }}
110110
                 {{ else if eq .E.Kind "reopened" }}{{ octicon "issue-opened" }}
111
+                {{ else if eq .E.Kind "merged" }}{{ octicon "git-merge" }}
111112
                 {{ else if eq .E.Kind "locked" }}{{ octicon "lock" }}
112113
                 {{ else if eq .E.Kind "unlocked" }}{{ octicon "unlock" }}
113114
                 {{ else if or (eq .E.Kind "labeled") (eq .E.Kind "unlabeled") }}{{ octicon "tag" }}
@@ -115,9 +116,13 @@
115116
                 {{ else if or (eq .E.Kind "milestoned") (eq .E.Kind "demilestoned") }}{{ octicon "milestone" }}
116117
                 {{ else }}{{ octicon "comment" }}{{ end }}
117118
               </span>
118
-              <span>
119
+              <span class="shithub-event-text">
119120
                 {{ if .ActorName }}<a href="/{{ .ActorName }}">{{ .ActorName }}</a>{{ else }}Someone{{ end }}
120
-                {{ if .LabelName }}
121
+                {{ if eq .E.Kind "merged" }}
122
+                  merged commit
123
+                  {{ if .CommitSHA }}<a class="shithub-mono-link" href="/{{ $.Owner }}/{{ $.Repo.Name }}/commit/{{ .CommitSHA }}">{{ .ShortCommit }}</a>{{ end }}
124
+                  into <a class="shithub-branch-name" href="/{{ $.Owner }}/{{ $.Repo.Name }}/tree/{{ $.PR.BaseRef }}">{{ $.PR.BaseRef }}</a>
125
+                {{ else if .LabelName }}
121126
                   {{ if eq .E.Kind "labeled" }}added the{{ else }}removed the{{ end }}
122127
                   <span class="shithub-label" style="background-color: #{{ .LabelColor }}">{{ .LabelName }}</span>
123128
                   label
@@ -126,6 +131,11 @@
126131
                 {{ end }}
127132
                 <time datetime="{{ .CreatedAt.Format "2006-01-02T15:04:05Z" }}">{{ relativeTime .CreatedAt }}</time>
128133
               </span>
134
+              {{ if and (eq .E.Kind "merged") .CommitSHA }}
135
+              <span class="shithub-event-actions">
136
+                <a class="shithub-button" href="/{{ $.Owner }}/{{ $.Repo.Name }}/commit/{{ .CommitSHA }}">View details</a>
137
+              </span>
138
+              {{ end }}
129139
             </div>
130140
             {{ end }}
131141
           {{ end }}
@@ -140,14 +150,22 @@
140150
           </section>
141151
           {{ end }}
142152
143
-          <section class="shithub-pull-merge-box shithub-pull-merge-box-{{ if .PR.MergedAt.Valid }}merged{{ else if eq (printf "%s" .PR.IState) "closed" }}closed{{ else if eq $mergeable "clean" }}ready{{ else if eq $mergeable "dirty" }}blocked{{ else }}pending{{ end }}">
153
+          <section class="shithub-pull-merge-box shithub-pull-merge-box-{{ if .PR.MergedAt.Valid }}merged{{ else if eq (printf "%s" .PR.IState) "closed" }}closed{{ else if eq $mergeable "clean" }}ready{{ else if eq $mergeable "dirty" }}blocked{{ else }}pending{{ end }}" data-pull-merge-box>
144154
             {{ if .PR.MergedAt.Valid }}
145155
               <div class="shithub-pull-merge-row">
146156
                 <span class="shithub-pull-status-icon shithub-pull-status-icon-merged">{{ octicon "git-merge" }}</span>
147157
                 <div>
148158
                   <strong>Pull request successfully merged and closed</strong>
149
-                  <p class="shithub-muted">Merged via <code>{{ printf "%s" .PR.MergeMethod.PrMergeMethod }}</code>.</p>
159
+                  <p class="shithub-muted">You're all set &mdash; the <a class="shithub-branch-name" href="/{{ .Owner }}/{{ .Repo.Name }}/tree/{{ .PR.HeadRef }}">{{ .PR.HeadRef }}</a> branch can be safely deleted.</p>
150160
                 </div>
161
+                {{ if .CanDeleteHeadBranch }}
162
+                <form method="post" action="/{{ .Owner }}/{{ .Repo.Name }}/pulls/{{ .PR.INumber }}/delete-branch" class="shithub-pull-delete-branch-form">
163
+                  <input type="hidden" name="csrf_token" value="{{ .CSRFToken }}">
164
+                  <button type="submit" class="shithub-button">Delete branch</button>
165
+                </form>
166
+                {{ else if .HeadBranchAlreadyGone }}
167
+                  <span class="shithub-muted shithub-pull-branch-deleted">Branch deleted</span>
168
+                {{ end }}
151169
               </div>
152170
             {{ else if eq (printf "%s" .PR.IState) "closed" }}
153171
               <div class="shithub-pull-merge-row">
@@ -227,14 +245,30 @@
227245
               <div class="shithub-pull-merge-actions">
228246
                 {{ if .CanMergePull }}
229247
                   {{ if eq $mergeable "clean" }}
230
-                  <form method="post" action="/{{ .Owner }}/{{ .Repo.Name }}/pulls/{{ .PR.INumber }}/merge" class="shithub-pull-merge-form">
231
-                    <input type="hidden" name="csrf_token" value="{{ .CSRFToken }}">
232
-                    <button type="submit" class="shithub-button shithub-button-primary">Merge pull request</button>
233
-                    <select name="method" aria-label="Merge method">
248
+                  <div class="shithub-pull-merge-choice" data-pull-merge-choice>
249
+                    <button type="button" class="shithub-button shithub-button-primary" data-pull-merge-open>Merge pull request</button>
250
+                    <select aria-label="Merge method" data-pull-merge-method>
234251
                       {{ if .Repo.AllowMergeCommit }}<option value="merge" {{ if eq (printf "%s" .Repo.DefaultMergeMethod) "merge" }}selected{{ end }}>Merge commit</option>{{ end }}
235252
                       {{ if .Repo.AllowSquashMerge }}<option value="squash" {{ if eq (printf "%s" .Repo.DefaultMergeMethod) "squash" }}selected{{ end }}>Squash and merge</option>{{ end }}
236253
                       {{ if .Repo.AllowRebaseMerge }}<option value="rebase" {{ if eq (printf "%s" .Repo.DefaultMergeMethod) "rebase" }}selected{{ end }}>Rebase and merge</option>{{ end }}
237254
                     </select>
255
+                  </div>
256
+                  <form method="post" action="/{{ .Owner }}/{{ .Repo.Name }}/pulls/{{ .PR.INumber }}/merge" class="shithub-pull-merge-confirm" data-pull-merge-confirm hidden>
257
+                    <input type="hidden" name="csrf_token" value="{{ .CSRFToken }}">
258
+                    <input type="hidden" name="method" value="{{ .MergeDefaultMethod }}" data-pull-merge-confirm-method>
259
+                    <label>
260
+                      <span>Commit message</span>
261
+                      <input type="text" name="subject" value="{{ .MergeFormSubject }}" data-pull-merge-subject>
262
+                    </label>
263
+                    <label>
264
+                      <span>Extended description</span>
265
+                      <textarea name="body" rows="8">{{ .MergeFormBody }}</textarea>
266
+                    </label>
267
+                    {{ if .MergeAuthorLine }}<p class="shithub-muted">{{ .MergeAuthorLine }}</p>{{ end }}
268
+                    <div class="shithub-form-actions shithub-form-actions-start">
269
+                      <button type="submit" class="shithub-button shithub-button-primary">Confirm merge</button>
270
+                      <button type="button" class="shithub-button" data-pull-merge-cancel>Cancel</button>
271
+                    </div>
238272
                   </form>
239273
                   {{ end }}
240274
                 {{ end }}