Skip to content

Commit 9d2a7cf

Browse files
committed
use multiline commit msgs for pr body
1 parent a27a4f5 commit 9d2a7cf

5 files changed

Lines changed: 125 additions & 136 deletions

File tree

cmd/push.go

Lines changed: 27 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -112,15 +112,24 @@ func runPush(cfg *config.Config, opts *pushOptions) error {
112112
// Create new PR — auto-generate title from commits/branch name,
113113
// then prompt interactively unless --auto or non-interactive.
114114
baseBranchForDiff := s.ActiveBaseBranch(b.Branch)
115-
title := defaultPRTitle(baseBranchForDiff, b.Branch)
115+
title, commitBody := defaultPRTitleBody(baseBranchForDiff, b.Branch)
116+
originalTitle := title
116117
if !opts.auto && cfg.IsInteractive() {
117118
p := prompter.New(cfg.In, cfg.Out, cfg.Err)
118119
input, err := p.Input(fmt.Sprintf("Title for PR (branch %s):", b.Branch), title)
119120
if err == nil && input != "" {
120121
title = input
121122
}
122123
}
123-
body := generatePRBody(s, b.Branch)
124+
125+
// If the user changed the title and the commit had a multi-line
126+
// message, put the full commit message in the PR body so no
127+
// content is lost.
128+
prBody := commitBody
129+
if title != originalTitle && commitBody != "" {
130+
prBody = originalTitle + "\n\n" + commitBody
131+
}
132+
body := generatePRBody(prBody)
124133

125134
newPR, createErr := client.CreatePR(baseBranch, b.Branch, title, body, opts.draft)
126135
if createErr != nil {
@@ -188,56 +197,33 @@ func runPush(cfg *config.Config, opts *pushOptions) error {
188197
return nil
189198
}
190199

191-
// defaultPRTitle generates a PR title from the branch's commits.
192-
// If there is exactly one commit, use its subject. Otherwise, humanize the
193-
// branch name (replace hyphens/underscores with spaces).
194-
func defaultPRTitle(base, head string) string {
200+
// defaultPRTitleBody generates a PR title and body from the branch's commits.
201+
// If there is exactly one commit, use its subject as the title and its body
202+
// (if any) as the PR body. Otherwise, humanize the branch name for the title.
203+
func defaultPRTitleBody(base, head string) (string, string) {
195204
commits, err := git.LogRange(base, head)
196205
if err == nil && len(commits) == 1 {
197-
return commits[0].Subject
206+
return commits[0].Subject, strings.TrimSpace(commits[0].Body)
198207
}
199-
return humanize(head)
208+
return humanize(head), ""
200209
}
201210

202-
// generatePRBody builds a rich PR description showing the downstack branches,
203-
// the current branch, and a footer with links to the CLI and feedback form.
204-
func generatePRBody(s *stack.Stack, currentBranch string) string {
205-
var lines []string
206-
207-
// Current branch entry (always first)
208-
lines = append(lines, fmt.Sprintf("- `%s` ← *this PR*", currentBranch))
211+
// generatePRBody builds a PR description from the commit body (if any)
212+
// and a footer linking to the CLI and feedback form.
213+
func generatePRBody(commitBody string) string {
214+
var parts []string
209215

210-
// Walk downstack from just below current to the bottom, skipping merged branches
211-
found := false
212-
for i := len(s.Branches) - 1; i >= 0; i-- {
213-
b := s.Branches[i]
214-
if b.Branch == currentBranch {
215-
found = true
216-
continue
217-
}
218-
if !found {
219-
continue
220-
}
221-
if b.IsMerged() {
222-
continue
223-
}
224-
if b.PullRequest != nil && b.PullRequest.URL != "" {
225-
lines = append(lines, fmt.Sprintf("- `%s` %s", b.Branch, b.PullRequest.URL))
226-
} else {
227-
lines = append(lines, fmt.Sprintf("- `%s`", b.Branch))
228-
}
216+
if commitBody != "" {
217+
parts = append(parts, commitBody)
229218
}
230219

231-
// Trunk entry
232-
lines = append(lines, fmt.Sprintf("- `%s` (base)", s.Trunk.Branch))
233-
234-
body := "---\n\n**Stacked Pull Requests**\n" + strings.Join(lines, "\n")
235-
body += fmt.Sprintf(
236-
"\n\n<sub>Stack created with <a href=\"https://github.com/github/gh-stack\">GitHub Stacks CLI</a> • <a href=\"%s\">Give Feedback 💬</a></sub>",
220+
footer := fmt.Sprintf(
221+
"<sub>Stack created with <a href=\"https://github.com/github/gh-stack\">GitHub Stacks CLI</a> • <a href=\"%s\">Give Feedback 💬</a></sub>",
237222
feedbackBaseURL,
238223
)
224+
parts = append(parts, footer)
239225

240-
return body
226+
return strings.Join(parts, "\n\n---\n\n")
241227
}
242228

243229
// humanize replaces hyphens and underscores with spaces.

cmd/push_test.go

Lines changed: 12 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -3,119 +3,41 @@ package cmd
33
import (
44
"testing"
55

6-
"github.com/github/gh-stack/internal/stack"
76
"github.com/stretchr/testify/assert"
87
)
98

109
func TestGeneratePRBody(t *testing.T) {
1110
tests := []struct {
12-
name string
13-
stack *stack.Stack
14-
currentBranch string
15-
wantContains []string
16-
wantAbsent []string
11+
name string
12+
commitBody string
13+
wantContains []string
1714
}{
1815
{
19-
name: "single branch stack",
20-
stack: &stack.Stack{
21-
Trunk: stack.BranchRef{Branch: "main"},
22-
Branches: []stack.BranchRef{{Branch: "feature"}},
23-
},
24-
currentBranch: "feature",
16+
name: "empty commit body",
17+
commitBody: "",
2518
wantContains: []string{
26-
"---",
27-
"**Stacked Pull Requests**",
28-
"- `feature` ← *this PR*",
29-
"- `main` (base)",
3019
"GitHub Stacks CLI",
3120
feedbackBaseURL,
21+
"<sub>",
3222
},
3323
},
3424
{
35-
name: "multi-branch current is topmost",
36-
stack: &stack.Stack{
37-
Trunk: stack.BranchRef{Branch: "main"},
38-
Branches: []stack.BranchRef{
39-
{Branch: "part-1", PullRequest: &stack.PullRequestRef{Number: 1, URL: "https://github.com/org/repo/pull/1"}},
40-
{Branch: "part-2", PullRequest: &stack.PullRequestRef{Number: 2, URL: "https://github.com/org/repo/pull/2"}},
41-
{Branch: "part-3"},
42-
},
43-
},
44-
currentBranch: "part-3",
25+
name: "with commit body",
26+
commitBody: "This is a detailed description\nof the change.",
4527
wantContains: []string{
46-
"- `part-3` ← *this PR*",
47-
"- `part-2` https://github.com/org/repo/pull/2",
48-
"- `part-1` https://github.com/org/repo/pull/1",
49-
"- `main` (base)",
50-
},
51-
},
52-
{
53-
name: "current is in the middle excludes upstack",
54-
stack: &stack.Stack{
55-
Trunk: stack.BranchRef{Branch: "main"},
56-
Branches: []stack.BranchRef{
57-
{Branch: "part-1", PullRequest: &stack.PullRequestRef{Number: 1, URL: "https://github.com/org/repo/pull/1"}},
58-
{Branch: "part-2"},
59-
{Branch: "part-3"},
60-
},
61-
},
62-
currentBranch: "part-2",
63-
wantContains: []string{
64-
"- `part-2` ← *this PR*",
65-
"- `part-1` https://github.com/org/repo/pull/1",
66-
"- `main` (base)",
67-
},
68-
wantAbsent: []string{
69-
"part-3",
70-
},
71-
},
72-
{
73-
name: "merged branches are skipped",
74-
stack: &stack.Stack{
75-
Trunk: stack.BranchRef{Branch: "main"},
76-
Branches: []stack.BranchRef{
77-
{Branch: "part-1", PullRequest: &stack.PullRequestRef{Number: 1, URL: "https://github.com/org/repo/pull/1", Merged: true}},
78-
{Branch: "part-2", PullRequest: &stack.PullRequestRef{Number: 2, URL: "https://github.com/org/repo/pull/2"}},
79-
{Branch: "part-3"},
80-
},
81-
},
82-
currentBranch: "part-3",
83-
wantContains: []string{
84-
"- `part-3` ← *this PR*",
85-
"- `part-2` https://github.com/org/repo/pull/2",
86-
"- `main` (base)",
87-
},
88-
wantAbsent: []string{
89-
"part-1",
90-
},
91-
},
92-
{
93-
name: "downstack branch without PR shows branch name only",
94-
stack: &stack.Stack{
95-
Trunk: stack.BranchRef{Branch: "main"},
96-
Branches: []stack.BranchRef{
97-
{Branch: "part-1"},
98-
{Branch: "part-2"},
99-
},
100-
},
101-
currentBranch: "part-2",
102-
wantContains: []string{
103-
"- `part-2` ← *this PR*",
104-
"- `part-1`",
105-
"- `main` (base)",
28+
"This is a detailed description\nof the change.",
29+
"GitHub Stacks CLI",
30+
"<sub>",
10631
},
10732
},
10833
}
10934

11035
for _, tt := range tests {
11136
t.Run(tt.name, func(t *testing.T) {
112-
got := generatePRBody(tt.stack, tt.currentBranch)
37+
got := generatePRBody(tt.commitBody)
11338
for _, want := range tt.wantContains {
11439
assert.Contains(t, got, want)
11540
}
116-
for _, absent := range tt.wantAbsent {
117-
assert.NotContains(t, got, absent)
118-
}
11941
})
12042
}
12143
}

internal/git/git.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ var client = &cligit.Client{}
1717
type CommitInfo struct {
1818
SHA string
1919
Subject string
20+
Body string
2021
Time time.Time
2122
}
2223

internal/git/gitops.go

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ func (d *defaultOps) Log(ref string, maxCount int) ([]CommitInfo, error) {
264264
}
265265

266266
func (d *defaultOps) LogRange(base, head string) ([]CommitInfo, error) {
267-
format := "%H\t%s\t%at"
267+
format := "%H%x01%B%x01%at%x00"
268268
rangeSpec := base + ".." + head
269269
output, err := run("log", rangeSpec, "--format="+format)
270270
if err != nil {
@@ -275,21 +275,40 @@ func (d *defaultOps) LogRange(base, head string) ([]CommitInfo, error) {
275275
}
276276

277277
var commits []CommitInfo
278-
for _, line := range strings.Split(output, "\n") {
279-
parts := strings.SplitN(line, "\t", 3)
278+
for _, record := range strings.Split(output, "\x00") {
279+
record = strings.TrimSpace(record)
280+
if record == "" {
281+
continue
282+
}
283+
parts := strings.SplitN(record, "\x01", 3)
280284
if len(parts) < 3 {
281285
continue
282286
}
283-
ts, _ := strconv.ParseInt(parts[2], 10, 64)
287+
ts, _ := strconv.ParseInt(strings.TrimSpace(parts[2]), 10, 64)
288+
subject, body := splitCommitMessage(parts[1])
284289
commits = append(commits, CommitInfo{
285290
SHA: parts[0],
286-
Subject: parts[1],
291+
Subject: subject,
292+
Body: body,
287293
Time: time.Unix(ts, 0),
288294
})
289295
}
290296
return commits, nil
291297
}
292298

299+
// splitCommitMessage splits a full commit message into subject (first line)
300+
// and body (remaining lines with leading/trailing blank lines trimmed).
301+
func splitCommitMessage(msg string) (subject, body string) {
302+
msg = strings.TrimSpace(msg)
303+
if i := strings.IndexByte(msg, '\n'); i >= 0 {
304+
subject = msg[:i]
305+
body = strings.TrimSpace(msg[i+1:])
306+
} else {
307+
subject = msg
308+
}
309+
return
310+
}
311+
293312
func (d *defaultOps) DiffStatRange(base, head string) (additions, deletions int, err error) {
294313
output, err := run("diff", "--numstat", base+".."+head)
295314
if err != nil {

internal/git/gitops_test.go

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
package git
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
)
8+
9+
func TestSplitCommitMessage(t *testing.T) {
10+
tests := []struct {
11+
name string
12+
msg string
13+
wantSubject string
14+
wantBody string
15+
}{
16+
{
17+
name: "single line",
18+
msg: "Fix the bug",
19+
wantSubject: "Fix the bug",
20+
wantBody: "",
21+
},
22+
{
23+
name: "subject and body with blank separator",
24+
msg: "Fix the bug\n\nMore details about the fix.",
25+
wantSubject: "Fix the bug",
26+
wantBody: "More details about the fix.",
27+
},
28+
{
29+
name: "multi-line without blank separator",
30+
msg: "Fix the bug\nMore details\nEven more",
31+
wantSubject: "Fix the bug",
32+
wantBody: "More details\nEven more",
33+
},
34+
{
35+
name: "body with leading and trailing blank lines trimmed",
36+
msg: "Fix the bug\n\n\nSome body text\n\n",
37+
wantSubject: "Fix the bug",
38+
wantBody: "Some body text",
39+
},
40+
{
41+
name: "whitespace-only body",
42+
msg: "Fix the bug\n\n \n\n",
43+
wantSubject: "Fix the bug",
44+
wantBody: "",
45+
},
46+
{
47+
name: "leading whitespace on message trimmed",
48+
msg: "\n Fix the bug\n\nBody here",
49+
wantSubject: "Fix the bug",
50+
wantBody: "Body here",
51+
},
52+
}
53+
54+
for _, tt := range tests {
55+
t.Run(tt.name, func(t *testing.T) {
56+
subject, body := splitCommitMessage(tt.msg)
57+
assert.Equal(t, tt.wantSubject, subject)
58+
assert.Equal(t, tt.wantBody, body)
59+
})
60+
}
61+
}

0 commit comments

Comments
 (0)