Skip to content

Commit daee05f

Browse files
committed
Refine the merge TUI and simplify the async-merge client
Follow-up polish for `gh stack merge` (the command itself landed in the previous commit). These changes refine the interactive wizard, enrich the PR picker, and replace the merge client's bespoke HTTP handling with the standard go-gh REST client. Wizard and stepper: - Redesign the top stepper as a segmented bar: completed steps are green, the active step is the brightest, and upcoming steps are dimmed. Steps are separated by a Powerline arrow that blends into the shading, with a graceful fallback to abutting segments on terminals that lack the glyph (e.g. Apple Terminal). Set GH_STACK_POWERLINE=1/0 to override detection. - Show the stack number in the header ("Merge stack #123"). - Hide the header and stepper once the merge is submitted so the live progress view stands on its own. PR picker: - Render each pull request on two lines: the title (white/black, a touch bolder when selected) above its "#number • branch" (gray, fainter when deselected). Titles are fetched in one batched GraphQL query (PRTitles) and fall back to the branch name. - Scroll long stacks in a fixed 10-item window with persistent "N more" indicators, so the list no longer jumps as those hints appear and disappear. Add shift+up / shift+down to jump to the top or bottom. Progress and outcome: - Always render a status line ("Submitting merge request...") so it does not pop in later and shift the view, and normalize messages to end in an ellipsis. - Print the final result from the command layer rather than the TUI: a success line that includes the merge commit SHA ("Merged #1, #2 into main (abc1234)"), an atomic-rollback note on failure, a distinct message when the user stops watching an in-flight merge, and "Cancelled operation, nothing merged" on cancel. - Clamp every rendered line to the terminal width so resizing no longer leaves duplicated header lines behind, and make truncation ANSI-aware. Async-merge client: - Use the go-gh REST client (c.rest.Put / c.rest.Get) for both the submit and poll endpoints, removing the bespoke http.Client, base-URL helper, and manual response decoding. The REST client discards non-2xx bodies, but that only costs the rare 400 message and 409 UUID: real merge failures still surface through the 200 poll body, and the in-range PRs are validated open, non-draft, and non-merged before submitting. - Add classifyAsyncMergeError to map status codes to clear errors (404 unavailable, 409 already exists, 400 no longer mergeable) and drop the now-unused AsyncMergeResult.StatusCode field. Rework the client tests to drive the REST client through a stub http.RoundTripper.
1 parent 056116f commit daee05f

10 files changed

Lines changed: 585 additions & 273 deletions

File tree

cmd/merge.go

Lines changed: 42 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ func runMerge(cfg *config.Config, opts *mergeOptions, args []string) error {
141141
base := remoteStack.Base.Ref
142142

143143
if cfg.IsInteractive() && !opts.yes {
144-
return runMergeInteractive(cfg, client, base, candidates, allowed, mergeCfg.DefaultMethod, method, preselectIndex, opts)
144+
return runMergeInteractive(cfg, client, remoteStack.Number, base, candidates, allowed, mergeCfg.DefaultMethod, method, preselectIndex, opts)
145145
}
146146

147147
// Non-interactive (or --yes): merge the whole stack (or up to the given PR)
@@ -260,16 +260,30 @@ func resolveActiveRemoteStack(cfg *config.Config, client github.ClientOps) (*git
260260
return rs, nil
261261
}
262262

263-
func runMergeInteractive(cfg *config.Config, client github.ClientOps, base string, candidates []mergeview.PRItem, allowed []string, viewerDefault, methodFlag string, preselectIndex int, opts *mergeOptions) error {
263+
func runMergeInteractive(cfg *config.Config, client github.ClientOps, stackNumber int, base string, candidates []mergeview.PRItem, allowed []string, viewerDefault, methodFlag string, preselectIndex int, opts *mergeOptions) error {
264264
defaultMethod := viewerDefault
265265
if methodFlag != "" {
266266
defaultMethod = methodFlag
267267
}
268268

269+
// Enrich the picker with PR titles (best-effort; the branch is shown either way).
270+
nums := make([]int, len(candidates))
271+
for i, c := range candidates {
272+
nums[i] = c.Number
273+
}
274+
if titles, err := client.PRTitles(nums); err == nil {
275+
for i := range candidates {
276+
if t := titles[candidates[i].Number]; t != "" {
277+
candidates[i].Title = t
278+
}
279+
}
280+
}
281+
269282
submit, poll := mergeFuncs(client)
270283

271284
model := mergeview.New(mergeview.Options{
272285
PRs: candidates,
286+
StackNumber: stackNumber,
273287
BaseRef: base,
274288
AllowedMethods: allowed,
275289
DefaultMethod: defaultMethod,
@@ -292,13 +306,21 @@ func runMergeInteractive(cfg *config.Config, client github.ClientOps, base strin
292306
warnAsyncMergeUnavailable(cfg)
293307
return ErrStacksUnavailable
294308
}
309+
cfg.Errorf("merge failed: %s", out.Err)
295310
return ErrAPIFailure
296311
case out.Merged:
312+
mergedSuccess(cfg, prNumberList(out.MergedPRs), base, out.SHA)
297313
return nil
298314
case out.Failed:
315+
cfg.Errorf("merge failed: %s", out.Message)
316+
cfg.Printf("The stack is atomic, so nothing was merged.")
299317
return mergeFailureExit(out.Message)
318+
case out.WatchStopped:
319+
cfg.Infof("Stopped watching. Merge is still in progress. Check the pull requests on GitHub.")
320+
return ErrSilent
300321
default:
301-
// Cancelled, or watching was stopped while the merge continued.
322+
// Cancelled via esc/ctrl+c before submitting.
323+
cfg.Infof("Cancelled operation, nothing merged")
302324
return ErrSilent
303325
}
304326
}
@@ -320,18 +342,15 @@ func runMergeHeadless(cfg *config.Config, client github.ClientOps, base string,
320342
}
321343

322344
if res.Merged {
323-
cfg.Successf("Merged %s into %s", list, base)
345+
mergedSuccess(cfg, list, base, res.Details.SHA)
324346
return nil
325347
}
326-
if !res.Queued {
327-
cfg.Errorf("cannot merge: %s", res.Details.Message)
328-
return ErrAPIFailure
329-
}
330-
if res.StatusCode == http.StatusConflict {
331-
cfg.Infof("A merge request already exists for this stack; tracking it.")
332-
}
333348

334349
uuid := res.Details.UUID
350+
if uuid == "" {
351+
cfg.Errorf("merge did not start as expected")
352+
return ErrAPIFailure
353+
}
335354
interval := opts.pollInterval
336355
if interval <= 0 {
337356
interval = time.Second
@@ -350,10 +369,7 @@ func runMergeHeadless(cfg *config.Config, client github.ClientOps, base string,
350369
return ErrAPIFailure
351370
}
352371
if status.Merged {
353-
cfg.Successf("Merged %s into %s", list, base)
354-
if sha := status.Details.SHA; sha != "" {
355-
cfg.Printf(" Merge commit %s", shortMergeSHA(sha))
356-
}
372+
mergedSuccess(cfg, list, base, status.Details.SHA)
357373
return nil
358374
}
359375
if !status.Queued {
@@ -414,7 +430,7 @@ func mergeCandidates(rs *github.RemoteStack) (items []mergeview.PRItem, blocker
414430
b := pr
415431
return items, &b
416432
}
417-
items = append(items, mergeview.PRItem{Number: pr.Number, Title: pr.Head.Ref})
433+
items = append(items, mergeview.PRItem{Number: pr.Number, Branch: pr.Head.Ref})
418434
}
419435
return items, nil
420436
}
@@ -572,6 +588,16 @@ func shortMergeSHA(sha string) string {
572588
return sha
573589
}
574590

591+
// mergedSuccess prints the merge success line, appending the merge commit SHA in
592+
// parentheses when known: "Merged #1, #2 into main (abc1234)".
593+
func mergedSuccess(cfg *config.Config, list, base, sha string) {
594+
if sha != "" {
595+
cfg.Successf("Merged %s into %s (%s)", list, base, shortMergeSHA(sha))
596+
return
597+
}
598+
cfg.Successf("Merged %s into %s", list, base)
599+
}
600+
575601
func isNotFound(err error) bool {
576602
var httpErr *api.HTTPError
577603
return errors.As(err, &httpErr) && httpErr.StatusCode == http.StatusNotFound

cmd/merge_test.go

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package cmd
22

33
import (
4+
"errors"
45
"net/http"
56
"testing"
67
"time"
@@ -92,10 +93,10 @@ func TestRunMerge_NoArg_MergesWholeStack(t *testing.T) {
9293
},
9394
MergeStackAsyncFn: func(pr int, method string) (*github.AsyncMergeResult, error) {
9495
gotPR, gotMethod = pr, method
95-
return &github.AsyncMergeResult{Queued: true, Details: github.AsyncMergeDetails{UUID: "u"}, StatusCode: 202}, nil
96+
return &github.AsyncMergeResult{Queued: true, Details: github.AsyncMergeDetails{UUID: "u"}}, nil
9697
},
9798
GetAsyncMergeResultFn: func(pr int, uuid string) (*github.AsyncMergeResult, error) {
98-
return &github.AsyncMergeResult{Merged: true, Details: github.AsyncMergeDetails{SHA: "abc1234"}, StatusCode: 200}, nil
99+
return &github.AsyncMergeResult{Merged: true, Details: github.AsyncMergeDetails{SHA: "abc1234"}}, nil
99100
},
100101
}
101102

@@ -294,15 +295,16 @@ func TestRunMerge_SubmitNotMergeable(t *testing.T) {
294295
return remoteStack(7, "main", openStackPR(1, "b1"), openStackPR(2, "b2")), nil
295296
},
296297
MergeStackAsyncFn: func(pr int, method string) (*github.AsyncMergeResult, error) {
297-
return &github.AsyncMergeResult{Queued: false, Merged: false, Details: github.AsyncMergeDetails{Message: "Pull request is closed."}, StatusCode: 400}, nil
298+
return nil, errors.New("the stack can no longer be merged as requested; refresh and try again")
298299
},
299300
}
300301

301302
err := runMerge(cfg, fastOptions(), []string{"7"})
302303
output := collectOutput(cfg, outR, errR)
303304

304305
assert.ErrorIs(t, err, ErrAPIFailure)
305-
assert.Contains(t, output, "cannot merge: Pull request is closed.")
306+
assert.Contains(t, output, "failed to start merge")
307+
assert.Contains(t, output, "can no longer be merged")
306308
}
307309

308310
func TestRunMerge_PollFailedConflict(t *testing.T) {
@@ -312,7 +314,7 @@ func TestRunMerge_PollFailedConflict(t *testing.T) {
312314
return remoteStack(7, "main", openStackPR(1, "b1"), openStackPR(2, "b2")), nil
313315
},
314316
MergeStackAsyncFn: func(pr int, method string) (*github.AsyncMergeResult, error) {
315-
return &github.AsyncMergeResult{Queued: true, Details: github.AsyncMergeDetails{UUID: "u"}, StatusCode: 202}, nil
317+
return &github.AsyncMergeResult{Queued: true, Details: github.AsyncMergeDetails{UUID: "u"}}, nil
316318
},
317319
GetAsyncMergeResultFn: func(pr int, uuid string) (*github.AsyncMergeResult, error) {
318320
return &github.AsyncMergeResult{Queued: false, Merged: false, Details: github.AsyncMergeDetails{Message: "Merge conflict: could not merge."}}, nil
@@ -334,7 +336,7 @@ func TestRunMerge_AlreadyMergedOnSubmit(t *testing.T) {
334336
return remoteStack(7, "main", openStackPR(1, "b1"), openStackPR(2, "b2")), nil
335337
},
336338
MergeStackAsyncFn: func(pr int, method string) (*github.AsyncMergeResult, error) {
337-
return &github.AsyncMergeResult{Merged: true, Details: github.AsyncMergeDetails{SHA: "abc"}, StatusCode: 200}, nil
339+
return &github.AsyncMergeResult{Merged: true, Details: github.AsyncMergeDetails{SHA: "abc"}}, nil
338340
},
339341
}
340342

@@ -453,13 +455,13 @@ func TestMergeCandidates(t *testing.T) {
453455
t.Run("leading merged skipped", func(t *testing.T) {
454456
items, blocker := mergeCandidates(remoteStack(1, "main", mergedStackPR(1, "a"), openStackPR(2, "b"), openStackPR(3, "c")))
455457
assert.Nil(t, blocker)
456-
assert.Equal(t, []mergeview.PRItem{{Number: 2, Title: "b"}, {Number: 3, Title: "c"}}, items)
458+
assert.Equal(t, []mergeview.PRItem{{Number: 2, Branch: "b"}, {Number: 3, Branch: "c"}}, items)
457459
})
458460
t.Run("draft blocks above", func(t *testing.T) {
459461
items, blocker := mergeCandidates(remoteStack(1, "main", openStackPR(1, "a"), draftStackPR(2, "b"), openStackPR(3, "c")))
460462
require.NotNil(t, blocker)
461463
assert.Equal(t, 2, blocker.Number)
462-
assert.Equal(t, []mergeview.PRItem{{Number: 1, Title: "a"}}, items)
464+
assert.Equal(t, []mergeview.PRItem{{Number: 1, Branch: "a"}}, items)
463465
})
464466
t.Run("closed blocks", func(t *testing.T) {
465467
items, blocker := mergeCandidates(remoteStack(1, "main", closedStackPR(1, "a"), openStackPR(2, "b")))

internal/github/client_interface.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ type ClientOps interface {
2020
RepoMergeConfig() (*RepoMergeConfig, error)
2121
MergeStackAsync(prNumber int, method string) (*AsyncMergeResult, error)
2222
GetAsyncMergeResult(prNumber int, uuid string) (*AsyncMergeResult, error)
23+
PRTitles(numbers []int) (map[int]string, error)
2324
}
2425

2526
// Compile-time check that Client satisfies ClientOps.

0 commit comments

Comments
 (0)