Skip to content

Commit eb0aee0

Browse files
134130claude
134130
and
claude
committed
fix: resolve multiple bugs and remove dead code
- Fix cherry-pick not executing when --merge flag is explicitly set (mergeStrategy was never assigned in the non-auto branch) - Fix draft PR incorrectly displayed as "open" in StateString/PRNumberString - Fix format string injection risk in main.go error logging - Remove duplicate gh API call in PRMergedWith by reusing already-fetched SHA - Fix typo "unabled" -> "unable" in NotInstalledError message - Replace panic with error return for unsupported commands in Run() - Remove unused WithSpinner dead code and its dependencies Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 0876444 commit eb0aee0

8 files changed

Lines changed: 12 additions & 59 deletions

File tree

‎cmd/gh-cherry-pick/main.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ func main() {
4747
ctx = log.CtxWithLogger(ctx)
4848

4949
if err := cherryPick.RunWithContext(ctx); err != nil {
50-
log.LoggerFromCtx(ctx).Failf(err.Error())
50+
log.LoggerFromCtx(ctx).Fail(err.Error())
5151
os.Exit(1)
5252
}
5353
}

‎git/cherry_pick.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,12 +113,13 @@ func (cherryPick *CherryPick) RunWithContext(ctx context.Context) error {
113113
if cherryPick.MergeStrategy == MergeStrategyAuto {
114114
logger.Infof("no merge strategy given, determining merge strategy")
115115

116-
if mergeStrategy, err = PRMergedWith(ctx, cherryPick.PRNumber); err != nil {
116+
if mergeStrategy, err = PRMergedWith(ctx, cherryPick.PRNumber, pr.MergeCommit.Sha); err != nil {
117117
return fmt.Errorf("error determining merge strategy: %w", err)
118118
}
119119

120120
logger.Successf("determined merge strategy as %s", color.Cyan(mergeStrategy))
121121
} else {
122+
mergeStrategy = cherryPick.MergeStrategy
122123
logger.Infof("use merge strategy %s with given flag", color.Cyan(cherryPick.MergeStrategy))
123124
}
124125

‎git/command.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ func (c *Command) Run(ctx context.Context, mods ...CommandModifier) error {
2929
if err != nil {
3030
if errors.Is(err, exec.ErrNotFound) {
3131
return &NotInstalledError{
32-
message: fmt.Sprintf("unabled to find %s executable in PATH; please install %s before retrying", c.cmd, c.cmd),
32+
message: fmt.Sprintf("unable to find %s executable in PATH; please install %s before retrying", c.cmd, c.cmd),
3333
err: err,
3434
}
3535
}
@@ -67,7 +67,7 @@ func (c *Command) Run(ctx context.Context, mods ...CommandModifier) error {
6767
}
6868
return &ge
6969
default:
70-
panic(fmt.Sprintf("unsupported command: %s", c.cmd))
70+
return fmt.Errorf("unsupported command: %s", c.cmd)
7171
}
7272
}
7373

‎git/pr_merged_with.go‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package git
22

33
import (
4-
"bytes"
54
"context"
65
"fmt"
76
"strconv"
@@ -25,14 +24,7 @@ func (m MergeStrategy) Validate() error {
2524
}
2625
}
2726

28-
func PRMergedWith(ctx context.Context, prNumber int) (MergeStrategy, error) {
29-
stdout := &bytes.Buffer{}
30-
args := []string{"pr", "view", strconv.Itoa(prNumber), "--json", "mergeCommit", "--jq", ".mergeCommit.oid"}
31-
if err := NewCommand("gh", args...).Run(ctx, WithStdout(stdout)); err != nil {
32-
return "", fmt.Errorf("failed to get merge commit SHA for PR #%d: %w", prNumber, err)
33-
}
34-
35-
mergeCommitSHA := strings.TrimSpace(stdout.String())
27+
func PRMergedWith(ctx context.Context, prNumber int, mergeCommitSHA string) (MergeStrategy, error) {
3628
if len(mergeCommitSHA) == 0 {
3729
return "", fmt.Errorf("failed to get merge commit SHA for PR #%d: PR not merged", prNumber)
3830
}

‎gitobj/pull_request.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,15 +33,15 @@ type PullRequest struct {
3333
func (pr PullRequest) StateString() string {
3434
switch pr.State {
3535
case PullRequestStateOpen:
36+
if pr.IsDraft {
37+
return color.Grey("draft")
38+
}
3639
return color.Green("open")
3740
case PullRequestStateClosed:
3841
return color.Red("closed")
3942
case PullRequestStateMerged:
4043
return color.Purple("merged")
4144
default:
42-
if pr.IsDraft {
43-
return color.Grey("draft")
44-
}
4545
return "UNKNOWN"
4646
}
4747
}
@@ -50,15 +50,15 @@ func (pr PullRequest) PRNumberString() string {
5050
str := fmt.Sprintf("#%d", pr.Number)
5151
switch pr.State {
5252
case PullRequestStateOpen:
53+
if pr.IsDraft {
54+
return color.Grey(str)
55+
}
5356
return color.Green(str)
5457
case PullRequestStateClosed:
5558
return color.Red(str)
5659
case PullRequestStateMerged:
5760
return color.Purple(str)
5861
default:
59-
if pr.IsDraft {
60-
return color.Grey(str)
61-
}
6262
return "UNKNOWN"
6363
}
6464
}

‎go.mod‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,21 +3,17 @@ module github.com/134130/gh-cherry-pick
33
go 1.25
44

55
require (
6-
github.com/briandowns/spinner v1.23.1
76
github.com/charmbracelet/lipgloss v1.0.0
87
github.com/cli/safeexec v1.0.0
98
)
109

1110
require (
1211
github.com/aymanbagabas/go-osc52/v2 v2.0.1 // indirect
1312
github.com/charmbracelet/x/ansi v0.4.2 // indirect
14-
github.com/fatih/color v1.18.0 // indirect
1513
github.com/lucasb-eyer/go-colorful v1.2.0 // indirect
16-
github.com/mattn/go-colorable v0.1.13 // indirect
1714
github.com/mattn/go-isatty v0.0.20 // indirect
1815
github.com/mattn/go-runewidth v0.0.15 // indirect
1916
github.com/muesli/termenv v0.15.2 // indirect
2017
github.com/rivo/uniseg v0.4.7 // indirect
2118
golang.org/x/sys v0.28.0 // indirect
22-
golang.org/x/term v0.27.0 // indirect
2319
)

‎go.sum‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,13 @@
11
github.com/aymanbagabas/go-osc52/v2 v2.0.1 h1:HwpRHbFMcZLEVr42D4p7XBqjyuxQH5SMiErDT4WkJ2k=
22
github.com/aymanbagabas/go-osc52/v2 v2.0.1/go.mod h1:uYgXzlJ7ZpABp8OJ+exZzJJhRNQ2ASbcXHWsFqH8hp8=
3-
github.com/briandowns/spinner v1.23.1 h1:t5fDPmScwUjozhDj4FA46p5acZWIPXYE30qW2Ptu650=
4-
github.com/briandowns/spinner v1.23.1/go.mod h1:LaZeM4wm2Ywy6vO571mvhQNRcWfRUnXOs0RcKV0wYKM=
53
github.com/charmbracelet/lipgloss v1.0.0 h1:O7VkGDvqEdGi93X+DeqsQ7PKHDgtQfF8j8/O2qFMQNg=
64
github.com/charmbracelet/lipgloss v1.0.0/go.mod h1:U5fy9Z+C38obMs+T+tJqst9VGzlOYGj4ri9reL3qUlo=
75
github.com/charmbracelet/x/ansi v0.4.2 h1:0JM6Aj/g/KC154/gOP4vfxun0ff6itogDYk41kof+qk=
86
github.com/charmbracelet/x/ansi v0.4.2/go.mod h1:dk73KoMTT5AX5BsX0KrqhsTqAnhZZoCBjs7dGWp4Ktw=
97
github.com/cli/safeexec v1.0.0 h1:0VngyaIyqACHdcMNWfo6+KdUYnqEr2Sg+bSP1pdF+dI=
108
github.com/cli/safeexec v1.0.0/go.mod h1:Z/D4tTN8Vs5gXYHDCbaM1S/anmEDnJb1iW0+EJ5zx3Q=
11-
github.com/fatih/color v1.18.0 h1:S8gINlzdQ840/4pfAwic/ZE0djQEH3wM94VfqLTZcOM=
12-
github.com/fatih/color v1.18.0/go.mod h1:4FelSpRwEGDpQ12mAdzqdOukCy4u8WUtOY6lkT/6HfU=
139
github.com/lucasb-eyer/go-colorful v1.2.0 h1:1nnpGOrhyZZuNyfu1QjKiUICQ74+3FNCN69Aj6K7nkY=
1410
github.com/lucasb-eyer/go-colorful v1.2.0/go.mod h1:R4dSotOR9KMtayYi1e77YzuveK+i7ruzyGqttikkLy0=
15-
github.com/mattn/go-colorable v0.1.13 h1:fFA4WZxdEF4tXPZVKMLwD8oUnCTTo08duU7wxecdEvA=
16-
github.com/mattn/go-colorable v0.1.13/go.mod h1:7S9/ev0klgBDR4GtXTXX8a3vIGJpMovkB8vQcUbaXHg=
17-
github.com/mattn/go-isatty v0.0.16/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/yFXSvRLM=
1811
github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY=
1912
github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y=
2013
github.com/mattn/go-runewidth v0.0.15 h1:UNAjwbU9l54TA3KzvqLGxwWjHmMgBUVhBiTjelZgg3U=
@@ -24,9 +17,6 @@ github.com/muesli/termenv v0.15.2/go.mod h1:Epx+iuz8sNs7mNKhxzH4fWXGNpZwUaJKRS1n
2417
github.com/rivo/uniseg v0.2.0/go.mod h1:J6wj4VEh+S6ZtnVlnTBMWIodfgj8LQOQFoIToxlJtxc=
2518
github.com/rivo/uniseg v0.4.7 h1:WUdvkW8uEhrYfLC4ZzdpI2ztxP1I582+49Oc5Mq64VQ=
2619
github.com/rivo/uniseg v0.4.7/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUcx88=
27-
golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
2820
golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
2921
golang.org/x/sys v0.28.0 h1:Fksou7UEQUWlKvIdsqzJmUmCX3cZuD2+P3XyyzwMhlA=
3022
golang.org/x/sys v0.28.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
31-
golang.org/x/term v0.27.0 h1:WP60Sv1nlK1T6SupCHbXzSaN0b9wUmsPoRS9b61A23Q=
32-
golang.org/x/term v0.27.0/go.mod h1:iMsnZpn0cago0GOrHO2+Y7u7JPn5AylBrcoWkElMTSM=

‎internal/tui/spinner.go‎

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,6 @@ import (
44
"context"
55
"fmt"
66
"os"
7-
"time"
8-
9-
"github.com/briandowns/spinner"
107

118
internalColor "github.com/134130/gh-cherry-pick/internal/color"
129
"github.com/134130/gh-cherry-pick/internal/log"
@@ -26,26 +23,3 @@ func WithStep(ctx context.Context, title string, f func(ctx context.Context, log
2623
return
2724
}
2825

29-
func WithSpinner(ctx context.Context, title string, f func(ctx context.Context, logger log.Logger) error) (err error) {
30-
logger := log.LoggerFromCtx(ctx)
31-
logger.IncreaseIndent()
32-
33-
sp := spinner.New(spinner.CharSets[14], 40*time.Millisecond, spinner.WithColor("cyan"))
34-
sp.Suffix = " " + title
35-
sp.FinalMSG = fmt.Sprintf("%s %s\n", internalColor.Green("✔"), title)
36-
sp.Start()
37-
defer func() {
38-
sp.Stop()
39-
logger.DecreaseIndent()
40-
41-
if err != nil {
42-
logger.Failf(err.Error())
43-
} else {
44-
logger.Successf(title)
45-
}
46-
}()
47-
48-
err = f(ctx, logger)
49-
50-
return
51-
}

0 commit comments

Comments
 (0)