Skip to content

Commit d5305eb

Browse files
skarimskarim
committed
improved handling of merged branches
1 parent b7c15c1 commit d5305eb

9 files changed

Lines changed: 347 additions & 75 deletions

File tree

‎cmd/navigate.go‎

Lines changed: 82 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,13 @@ func runNavigate(cfg *config.Config, delta int) error {
7070
// Might be on the trunk
7171
if currentBranch == s.Trunk.Branch {
7272
if delta > 0 && len(s.Branches) > 0 {
73-
target := s.Branches[0].Branch
73+
targetIdx := s.FirstActiveBranchIndex()
74+
if targetIdx < 0 {
75+
// All merged — fall back to top branch with warning
76+
targetIdx = len(s.Branches) - 1
77+
cfg.Warningf("Warning: all branches in this stack have been merged")
78+
}
79+
target := s.Branches[targetIdx].Branch
7480
if err := git.CheckoutBranch(target); err != nil {
7581
return err
7682
}
@@ -84,12 +90,65 @@ func runNavigate(cfg *config.Config, delta int) error {
8490
return nil
8591
}
8692

87-
newIdx := idx + delta
88-
if newIdx < 0 {
89-
newIdx = 0
93+
onMerged := s.Branches[idx].IsMerged()
94+
if onMerged {
95+
cfg.Warningf("Warning: you are on merged branch %q", currentBranch)
9096
}
91-
if newIdx >= len(s.Branches) {
92-
newIdx = len(s.Branches) - 1
97+
98+
var newIdx int
99+
var skipped int
100+
101+
if onMerged {
102+
// Navigate relative to current position among ALL branches
103+
newIdx = idx + delta
104+
if newIdx < 0 {
105+
newIdx = 0
106+
}
107+
if newIdx >= len(s.Branches) {
108+
newIdx = len(s.Branches) - 1
109+
}
110+
} else {
111+
// Build list of active (non-merged) branch indices
112+
var activeIndices []int
113+
for i, b := range s.Branches {
114+
if !b.IsMerged() {
115+
activeIndices = append(activeIndices, i)
116+
}
117+
}
118+
119+
// Find current position in active list
120+
activePos := -1
121+
for i, ai := range activeIndices {
122+
if ai == idx {
123+
activePos = i
124+
break
125+
}
126+
}
127+
128+
newActivePos := activePos + delta
129+
if newActivePos < 0 {
130+
newActivePos = 0
131+
}
132+
if newActivePos >= len(activeIndices) {
133+
newActivePos = len(activeIndices) - 1
134+
}
135+
136+
newIdx = activeIndices[newActivePos]
137+
138+
// Count how many merged branches were skipped
139+
if newIdx > idx {
140+
for i := idx + 1; i < newIdx; i++ {
141+
if s.Branches[i].IsMerged() {
142+
skipped++
143+
}
144+
}
145+
} else if newIdx < idx {
146+
for i := newIdx + 1; i < idx; i++ {
147+
if s.Branches[i].IsMerged() {
148+
skipped++
149+
}
150+
}
151+
}
93152
}
94153

95154
if newIdx == idx {
@@ -106,6 +165,10 @@ func runNavigate(cfg *config.Config, delta int) error {
106165
return err
107166
}
108167

168+
if skipped > 0 {
169+
cfg.Printf("Skipped %d merged %s", skipped, plural(skipped, "branch", "branch(es)"))
170+
}
171+
109172
moved := newIdx - idx
110173
direction := "up"
111174
if moved < 0 {
@@ -129,13 +192,19 @@ func runNavigateToEnd(cfg *config.Config, top bool) error {
129192
return nil
130193
}
131194

132-
var target string
195+
var targetIdx int
133196
if top {
134-
target = s.Branches[len(s.Branches)-1].Branch
197+
targetIdx = len(s.Branches) - 1
135198
} else {
136-
target = s.Branches[0].Branch
199+
targetIdx = s.FirstActiveBranchIndex()
200+
if targetIdx < 0 {
201+
// All merged — fall back to first branch with warning
202+
targetIdx = 0
203+
cfg.Warningf("Warning: all branches in this stack have been merged")
204+
}
137205
}
138206

207+
target := s.Branches[targetIdx].Branch
139208
if target == currentBranch {
140209
if top {
141210
cfg.Printf("Already at the top of the stack")
@@ -149,6 +218,10 @@ func runNavigateToEnd(cfg *config.Config, top bool) error {
149218
return err
150219
}
151220

221+
if s.Branches[targetIdx].IsMerged() {
222+
cfg.Warningf("Warning: you are on merged branch %q", target)
223+
}
224+
152225
cfg.Successf("Switched to %s", target)
153226
return nil
154227
}

‎cmd/push.go‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,11 @@ func runPush(cfg *config.Config, opts *pushOptions) error {
7474
}
7575

7676
// Push all branches
77-
for _, b := range s.Branches {
77+
merged := s.MergedBranches()
78+
if len(merged) > 0 {
79+
cfg.Printf("Skipping %d merged %s", len(merged), plural(len(merged), "branch", "branches"))
80+
}
81+
for _, b := range s.ActiveBranches() {
7882
if opts.dryRun {
7983
cfg.Printf("Would push %s", b.Branch)
8084
continue
@@ -93,7 +97,10 @@ func runPush(cfg *config.Config, opts *pushOptions) error {
9397

9498
// Create or update PRs
9599
for i, b := range s.Branches {
96-
baseBranch := s.BaseBranch(b.Branch)
100+
if s.Branches[i].IsMerged() {
101+
continue
102+
}
103+
baseBranch := s.ActiveBaseBranch(b.Branch)
97104

98105
pr, err := client.FindPRForBranch(b.Branch)
99106
if err != nil {
@@ -150,10 +157,10 @@ func runPush(cfg *config.Config, opts *pushOptions) error {
150157

151158
// Update base commit hashes and sync PR state
152159
for i := range s.Branches {
153-
parent := s.Trunk.Branch
154-
if i > 0 {
155-
parent = s.Branches[i-1].Branch
160+
if s.Branches[i].IsMerged() {
161+
continue
156162
}
163+
parent := s.ActiveBaseBranch(s.Branches[i].Branch)
157164
if base, err := git.HeadSHA(parent); err == nil {
158165
s.Branches[i].Base = base
159166
}
@@ -168,6 +175,6 @@ func runPush(cfg *config.Config, opts *pushOptions) error {
168175
return nil
169176
}
170177

171-
cfg.Successf("Pushed and synced %d branches", len(s.Branches))
178+
cfg.Successf("Pushed and synced %d branches", len(s.ActiveBranches()))
172179
return nil
173180
}

‎cmd/rebase.go‎

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"fmt"
66
"os"
77
"path/filepath"
8+
"strings"
89

910
"github.com/github/gh-stack/internal/config"
1011
"github.com/github/gh-stack/internal/git"
@@ -132,6 +133,10 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error {
132133
currentIdx = 0
133134
}
134135

136+
if opts.upstack && currentIdx >= 0 && s.Branches[currentIdx].IsMerged() {
137+
cfg.Warningf("Current branch %q has already been merged", currentBranch)
138+
}
139+
135140
startIdx := 0
136141
endIdx := len(s.Branches)
137142

@@ -180,7 +185,7 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error {
180185

181186
// Skip branches whose PRs have already been merged (e.g. via squash).
182187
// Record state so subsequent branches can use --onto rebase.
183-
if br.PullRequest != nil && br.PullRequest.Merged {
188+
if br.IsMerged() {
184189
ontoOldBase = originalRefs[br.Branch]
185190
needsOnto = true
186191
cfg.Successf("Skipping %s (PR #%d merged)", br.Branch, br.PullRequest.Number)
@@ -192,7 +197,7 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error {
192197
newBase := s.Trunk.Branch
193198
for j := absIdx - 1; j >= 0; j-- {
194199
b := s.Branches[j]
195-
if b.PullRequest == nil || !b.PullRequest.Merged {
200+
if !b.IsMerged() {
196201
newBase = b.Branch
197202
break
198203
}
@@ -289,13 +294,13 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error {
289294

290295
for i := range s.Branches {
291296
// Skip merged branches when updating base SHAs.
292-
if s.Branches[i].PullRequest != nil && s.Branches[i].PullRequest.Merged {
297+
if s.Branches[i].IsMerged() {
293298
continue
294299
}
295300
// Find the first non-merged ancestor, or trunk.
296301
parent := s.Trunk.Branch
297302
for j := i - 1; j >= 0; j-- {
298-
if s.Branches[j].PullRequest == nil || !s.Branches[j].PullRequest.Merged {
303+
if !s.Branches[j].IsMerged() {
299304
parent = s.Branches[j].Branch
300305
break
301306
}
@@ -311,6 +316,15 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error {
311316

312317
_ = stack.Save(gitDir, sf)
313318

319+
merged := s.MergedBranches()
320+
if len(merged) > 0 {
321+
names := make([]string, len(merged))
322+
for i, m := range merged {
323+
names[i] = m.Branch
324+
}
325+
cfg.Printf("Skipped %d merged %s: %s", len(merged), plural(len(merged), "branch", "branches"), strings.Join(names, ", "))
326+
}
327+
314328
rangeDesc := "All branches in stack"
315329
if opts.downstack {
316330
rangeDesc = fmt.Sprintf("All downstack branches up to %s", currentBranch)
@@ -380,7 +394,7 @@ func continueRebase(cfg *config.Config, gitDir string) error {
380394

381395
// Skip branches whose PRs have already been merged.
382396
br := s.Branches[idx]
383-
if br.PullRequest != nil && br.PullRequest.Merged {
397+
if br.IsMerged() {
384398
state.OntoOldBase = state.OriginalRefs[branchName]
385399
state.UseOnto = true
386400
cfg.Successf("Skipping %s (PR #%d merged)", branchName, br.PullRequest.Number)
@@ -399,7 +413,7 @@ func continueRebase(cfg *config.Config, gitDir string) error {
399413
newBase := s.Trunk.Branch
400414
for j := idx - 1; j >= 0; j-- {
401415
b := s.Branches[j]
402-
if b.PullRequest == nil || !b.PullRequest.Merged {
416+
if !b.IsMerged() {
403417
newBase = b.Branch
404418
break
405419
}
@@ -484,13 +498,13 @@ func continueRebase(cfg *config.Config, gitDir string) error {
484498

485499
for i := range s.Branches {
486500
// Skip merged branches when updating base SHAs.
487-
if s.Branches[i].PullRequest != nil && s.Branches[i].PullRequest.Merged {
501+
if s.Branches[i].IsMerged() {
488502
continue
489503
}
490504
// Find the first non-merged ancestor, or trunk.
491505
parent := s.Trunk.Branch
492506
for j := i - 1; j >= 0; j-- {
493-
if s.Branches[j].PullRequest == nil || !s.Branches[j].PullRequest.Merged {
507+
if !s.Branches[j].IsMerged() {
494508
parent = s.Branches[j].Branch
495509
break
496510
}

0 commit comments

Comments
 (0)