Skip to content

Commit e507bae

Browse files
committed
feat(cmd): refactor pr command and enhance log output
Refactor 'gh stack pr' to wrap 'gh pr create' interactively: - Pass through user flags after '--' (e.g., --fill, --web, --title) - Auto-draft PRs targeting non-trunk branches - Use FindPRByHead to discover newly created PRs - Improve error messages with actionable hints - Extract updateExistingPR helper for clarity Enhance 'gh stack log' to display PR URLs: - Show clickable URLs in tree view - Add URL column to porcelain output
1 parent f11af38 commit e507bae

4 files changed

Lines changed: 169 additions & 65 deletions

File tree

cmd/log.go

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77

88
"github.com/boneskull/gh-stack/internal/config"
99
"github.com/boneskull/gh-stack/internal/git"
10+
"github.com/boneskull/gh-stack/internal/github"
1011
"github.com/boneskull/gh-stack/internal/tree"
1112
"github.com/spf13/cobra"
1213
)
@@ -48,16 +49,19 @@ func runLog(cmd *cobra.Command, args []string) error {
4849
g := git.New(cwd)
4950
currentBranch, _ := g.CurrentBranch() //nolint:errcheck // empty string is fine for display
5051

52+
// Try to get GitHub client for PR URLs (optional - may fail if not in a GitHub repo)
53+
gh, _ := github.NewClient() //nolint:errcheck // nil is fine, URLs won't be shown
54+
5155
if logPorcelainFlag {
52-
printPorcelain(root, currentBranch)
56+
printPorcelain(root, currentBranch, gh)
5357
} else {
54-
printTree(root, "", true, currentBranch)
58+
printTree(root, "", true, currentBranch, gh)
5559
}
5660

5761
return nil
5862
}
5963

60-
func printTree(node *tree.Node, prefix string, isLast bool, current string) {
64+
func printTree(node *tree.Node, prefix string, isLast bool, current string, gh *github.Client) {
6165
// Determine the branch indicator
6266
connector := "├── "
6367
if isLast {
@@ -75,7 +79,11 @@ func printTree(node *tree.Node, prefix string, isLast bool, current string) {
7579

7680
prInfo := ""
7781
if node.PR > 0 {
78-
prInfo = fmt.Sprintf(" (#%d)", node.PR)
82+
if gh != nil {
83+
prInfo = fmt.Sprintf(" (#%d) %s", node.PR, gh.PRURL(node.PR))
84+
} else {
85+
prInfo = fmt.Sprintf(" (#%d)", node.PR)
86+
}
7987
}
8088

8189
fmt.Printf("%s%s%s%s%s\n", prefix, connector, marker, node.Name, prInfo)
@@ -92,11 +100,11 @@ func printTree(node *tree.Node, prefix string, isLast bool, current string) {
92100

93101
for i, child := range node.Children {
94102
isLastChild := i == len(node.Children)-1
95-
printTree(child, childPrefix, isLastChild, current)
103+
printTree(child, childPrefix, isLastChild, current, gh)
96104
}
97105
}
98106

99-
func printPorcelain(node *tree.Node, current string) {
107+
func printPorcelain(node *tree.Node, current string, gh *github.Client) {
100108
var printNode func(*tree.Node, int)
101109
printNode = func(n *tree.Node, depth int) {
102110
isCurrent := "0"
@@ -107,7 +115,11 @@ func printPorcelain(node *tree.Node, current string) {
107115
if n.Parent != nil {
108116
parent = n.Parent.Name
109117
}
110-
fmt.Printf("%s\t%s\t%d\t%s\n", n.Name, parent, n.PR, isCurrent)
118+
prURL := ""
119+
if n.PR > 0 && gh != nil {
120+
prURL = gh.PRURL(n.PR)
121+
}
122+
fmt.Printf("%s\t%s\t%d\t%s\t%s\n", n.Name, parent, n.PR, isCurrent, prURL)
111123
for _, child := range n.Children {
112124
printNode(child, depth+1)
113125
}

cmd/pr.go

Lines changed: 81 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -2,27 +2,40 @@
22
package cmd
33

44
import (
5+
"context"
56
"fmt"
67
"os"
78

9+
gh "github.com/cli/go-gh/v2"
10+
"github.com/spf13/cobra"
11+
812
"github.com/boneskull/gh-stack/internal/config"
913
"github.com/boneskull/gh-stack/internal/git"
1014
"github.com/boneskull/gh-stack/internal/github"
1115
"github.com/boneskull/gh-stack/internal/tree"
12-
"github.com/spf13/cobra"
1316
)
1417

1518
var prCmd = &cobra.Command{
16-
Use: "pr",
19+
Use: "pr [-- <gh-pr-create-flags>...]",
1720
Short: "Create or update a PR for the current branch",
18-
Long: `Create a new PR targeting the parent branch, or update an existing PR's base.`,
19-
RunE: runPR,
21+
Long: `Create a new PR targeting the parent branch, or update an existing PR's base.
22+
23+
This command wraps 'gh pr create', automatically setting the base branch to the
24+
stack parent. Any additional flags after '--' are passed through to 'gh pr create'.
25+
26+
Examples:
27+
gh stack pr # Interactive PR creation
28+
gh stack pr -- --title "My PR" # With title
29+
gh stack pr -- --fill --web # Fill from commits, open in browser
30+
gh stack pr --base main # Override base branch`,
31+
RunE: runPR,
32+
DisableFlagParsing: false,
2033
}
2134

2235
var prBaseFlag string
2336

2437
func init() {
25-
prCmd.Flags().StringVar(&prBaseFlag, "base", "", "override base branch")
38+
prCmd.Flags().StringVar(&prBaseFlag, "base", "", "override base branch (default: stack parent)")
2639
rootCmd.AddCommand(prCmd)
2740
}
2841

@@ -37,8 +50,7 @@ func runPR(cmd *cobra.Command, args []string) error {
3750
return err
3851
}
3952

40-
// Create GitHub client
41-
gh, err := github.NewClient()
53+
ghClient, err := github.NewClient()
4254
if err != nil {
4355
return err
4456
}
@@ -52,80 +64,91 @@ func runPR(cmd *cobra.Command, args []string) error {
5264
// Get parent (base branch)
5365
parent, err := cfg.GetParent(branch)
5466
if err != nil {
55-
return fmt.Errorf("branch %q is not tracked", branch)
67+
return fmt.Errorf("branch %q is not tracked; use 'gh stack create' or 'gh stack track' first", branch)
5668
}
5769

58-
base := prBaseFlag
59-
if base == "" {
60-
base = parent
61-
}
62-
63-
// Get trunk for draft decision and comment generation
6470
trunk, err := cfg.GetTrunk()
6571
if err != nil {
6672
return err
6773
}
6874

75+
base := prBaseFlag
76+
if base == "" {
77+
base = parent
78+
}
79+
6980
// Check if PR already exists
7081
existingPR, _ := cfg.GetPR(branch) //nolint:errcheck // 0 is fine if no PR
7182
if existingPR > 0 {
72-
// Update existing PR's base if needed
73-
fmt.Printf("PR #%d already exists, updating base to %q\n", existingPR, base)
74-
if updateErr := gh.UpdatePRBase(existingPR, base); updateErr != nil {
75-
return fmt.Errorf("failed to update PR base: %w", updateErr)
76-
}
77-
78-
// Update stack comment
79-
root, buildErr := tree.Build(cfg)
80-
if buildErr != nil {
81-
return fmt.Errorf("build tree: %w", buildErr)
82-
}
83-
comment := github.GenerateStackComment(root, branch, trunk)
84-
if comment != "" {
85-
if commentErr := gh.CreateOrUpdateStackComment(existingPR, comment); commentErr != nil {
86-
fmt.Printf("Warning: failed to update stack comment: %v\n", commentErr)
87-
}
88-
}
89-
return nil
83+
return updateExistingPR(ghClient, cfg, existingPR, branch, base, trunk)
9084
}
9185

92-
// Create new PR
93-
fmt.Printf("Creating PR for %q targeting %q...\n", branch, base)
86+
// Build args for gh pr create
87+
ghArgs := []string{"pr", "create", "--base", base}
9488

95-
var prNumber int
89+
// Auto-draft if not targeting trunk (middle of stack)
9690
if base != trunk {
97-
// Create as draft since it's part of a stack
98-
prNumber, err = gh.CreateDraftPR(branch, base, branch, "")
99-
if err != nil {
100-
return err
101-
}
102-
fmt.Printf("Created draft PR #%d for %s -> %s\n", prNumber, branch, base)
103-
} else {
104-
prNumber, err = gh.CreatePR(branch, base, branch, "")
105-
if err != nil {
106-
return err
107-
}
108-
fmt.Printf("Created PR #%d for %s -> %s\n", prNumber, branch, base)
91+
ghArgs = append(ghArgs, "--draft")
92+
fmt.Printf("Creating draft PR (base %q is not trunk %q)\n", base, trunk)
93+
}
94+
95+
// Pass through any additional args from user
96+
ghArgs = append(ghArgs, args...)
97+
98+
// Let user interact with gh pr create
99+
ctx := context.Background()
100+
if execErr := gh.ExecInteractive(ctx, ghArgs...); execErr != nil {
101+
return fmt.Errorf("gh pr create failed: %w", execErr)
102+
}
103+
104+
// Find the PR we just created
105+
pr, err := ghClient.FindPRByHead(branch)
106+
if err != nil {
107+
return fmt.Errorf("failed to find created PR: %w", err)
108+
}
109+
if pr == nil {
110+
// User might have cancelled
111+
fmt.Println("No PR was created.")
112+
return nil
109113
}
110114

111115
// Store PR number
112-
if setPRErr := cfg.SetPR(branch, prNumber); setPRErr != nil {
113-
return setPRErr
116+
if setErr := cfg.SetPR(branch, pr.Number); setErr != nil {
117+
return setErr
114118
}
115119

116120
// Post stack navigation comment
117-
root, buildErr := tree.Build(cfg)
118-
if buildErr != nil {
119-
return fmt.Errorf("build tree: %w", buildErr)
121+
root, err := tree.Build(cfg)
122+
if err != nil {
123+
return fmt.Errorf("build tree: %w", err)
124+
}
125+
126+
if err := ghClient.GenerateAndPostStackComment(root, branch, trunk, pr.Number); err != nil {
127+
fmt.Printf("Warning: failed to add stack comment: %v\n", err)
128+
}
129+
130+
fmt.Printf("Stored PR #%d for branch %q\n", pr.Number, branch)
131+
return nil
132+
}
133+
134+
// updateExistingPR updates the base branch and stack comment for an existing PR.
135+
func updateExistingPR(ghClient *github.Client, cfg *config.Config, prNumber int, branch, base, trunk string) error {
136+
fmt.Printf("PR #%d already exists, updating base to %q\n", prNumber, base)
137+
138+
if err := ghClient.UpdatePRBase(prNumber, base); err != nil {
139+
return fmt.Errorf("failed to update PR base: %w", err)
140+
}
141+
142+
// Update stack comment
143+
root, err := tree.Build(cfg)
144+
if err != nil {
145+
return fmt.Errorf("build tree: %w", err)
120146
}
121147

122-
comment := github.GenerateStackComment(root, branch, trunk)
123-
if comment != "" {
124-
if err := gh.CreateOrUpdateStackComment(prNumber, comment); err != nil {
125-
fmt.Printf("Warning: failed to add stack comment: %v\n", err)
126-
// Don't fail the command for comment issues
127-
}
148+
if err := ghClient.GenerateAndPostStackComment(root, branch, trunk, prNumber); err != nil {
149+
fmt.Printf("Warning: failed to update stack comment: %v\n", err)
128150
}
129151

152+
fmt.Println(ghClient.PRURL(prNumber))
130153
return nil
131154
}

internal/git/git_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,3 +256,4 @@ func TestNeedsRebase(t *testing.T) {
256256
t.Error("feature should need rebase after main moved forward")
257257
}
258258
}
259+

internal/github/github_test.go

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,3 +295,71 @@ func TestClient_UpdatePRBase(t *testing.T) {
295295
t.Fatalf("unexpected error: %v", err)
296296
}
297297
}
298+
299+
func TestClient_FindPRByHead(t *testing.T) {
300+
mock := &mockREST{
301+
getFn: func(path string, response any) error {
302+
expectedPath := "repos/owner/repo/pulls?head=owner:feature-branch&state=open"
303+
if path != expectedPath {
304+
t.Errorf("expected path %q, got %q", expectedPath, path)
305+
}
306+
307+
if prs, ok := response.(*[]PR); ok {
308+
*prs = []PR{
309+
{Number: 42, State: "open", Draft: false},
310+
}
311+
}
312+
return nil
313+
},
314+
}
315+
316+
client := NewClientWithREST(mock, "owner", "repo")
317+
pr, err := client.FindPRByHead("feature-branch")
318+
319+
if err != nil {
320+
t.Fatalf("unexpected error: %v", err)
321+
}
322+
if pr == nil {
323+
t.Fatal("expected PR, got nil")
324+
}
325+
if pr.Number != 42 {
326+
t.Errorf("expected PR number 42, got %d", pr.Number)
327+
}
328+
}
329+
330+
func TestClient_FindPRByHead_NotFound(t *testing.T) {
331+
mock := &mockREST{
332+
getFn: func(path string, response any) error {
333+
// Return empty slice (no PRs)
334+
if prs, ok := response.(*[]PR); ok {
335+
*prs = []PR{}
336+
}
337+
return nil
338+
},
339+
}
340+
341+
client := NewClientWithREST(mock, "owner", "repo")
342+
pr, err := client.FindPRByHead("nonexistent-branch")
343+
344+
if err != nil {
345+
t.Fatalf("unexpected error: %v", err)
346+
}
347+
if pr != nil {
348+
t.Errorf("expected nil PR, got %+v", pr)
349+
}
350+
}
351+
352+
func TestClient_FindPRByHead_Error(t *testing.T) {
353+
mock := &mockREST{
354+
getFn: func(path string, response any) error {
355+
return errors.New("API error")
356+
},
357+
}
358+
359+
client := NewClientWithREST(mock, "owner", "repo")
360+
_, err := client.FindPRByHead("feature")
361+
362+
if err == nil {
363+
t.Fatal("expected error, got nil")
364+
}
365+
}

0 commit comments

Comments
 (0)