diff --git a/README.md b/README.md index 9c4ea0da..2d73c401 100644 --- a/README.md +++ b/README.md @@ -557,9 +557,17 @@ gh stack view --short gh stack view --json ``` -`gh stack view --short` uses OSC 8 hyperlinks for PR numbers when the terminal -supports them. Otherwise, the full URL is shown for copy/paste. Set -`GH_STACK_HYPERLINKS=1` or `GH_STACK_HYPERLINKS=0` to override terminal detection. +Stack references follow the existing PR-link behavior in text output: OSC 8 +hyperlinks when supported, or the full URL for copy/paste otherwise. Stack links open +the repository's `/stack/` page; confirmed missing or removed stacks +are left unlinked. Set `GH_STACK_HYPERLINKS=1` or `GH_STACK_HYPERLINKS=0` to +override terminal detection. + +TUIs keep labels compact: click `#N` in `view` or `submit` to +open its page, or use your terminal's hyperlink gesture in the `checkout` +picker and `merge` wizard. Those inline views keep plain numbers when terminal +hyperlinks are unavailable. The environment override controls OSC 8 links, not +the application-handled clicks in `view` and `submit`. JSON output is unchanged. ### `gh stack unstack` diff --git a/cmd/checkout.go b/cmd/checkout.go index 4fe24f2c..a8481154 100644 --- a/cmd/checkout.go +++ b/cmd/checkout.go @@ -292,7 +292,7 @@ func checkoutRemoteStackByBranch(cfg *config.Config, sf *stack.StackFile, gitDir if len(matches) > 1 { stackNumbers := make([]string, len(matches)) for i, match := range matches { - stackNumbers[i] = strconv.Itoa(match.Number) + stackNumbers[i] = cfg.Link(strconv.Itoa(match.Number), cfg.StackURL(match.Number)) } cfg.Errorf("branch %q belongs to multiple stacks on GitHub (%s)", branch, strings.Join(stackNumbers, ", ")) @@ -384,9 +384,10 @@ func checkoutStackByNumber(cfg *config.Config, sf *stack.StackFile, gitDir strin } remoteStack, err := client.GetStack(stackNumber) - if err != nil || remoteStack == nil || len(remoteStack.PullRequests) == 0 { - // No such stack, stacks unavailable, or a transient failure — let the - // caller try the number as a PR number or branch name. + if err != nil { + return nil, "", errors.Join(errStackNumberNotFound, err) + } + if remoteStack == nil || len(remoteStack.PullRequests) == 0 { return nil, "", errStackNumberNotFound } @@ -474,7 +475,7 @@ func reconcileAndImportRemoteStack(cfg *config.Config, client github.ClientOps, if err := stack.Save(gitDir, sf); err != nil { return nil, "", stackSaveError(cfg, err) } - cfg.Successf("Local stack matches remote%s", stackLabel(remoteStack.Number)) + cfg.Successf("Local stack matches remote%s", linkedStackLabel(cfg, remoteStack.Number)) return localStack, targetBranch, nil } @@ -712,7 +713,7 @@ func importRemoteStack( // Update base SHAs from actual local refs updateBaseSHAs(s) - cfg.Successf("Imported stack with %d branches from GitHub%s", len(prs), stackLabel(remoteStackNumber)) + cfg.Successf("Imported stack with %d branches from GitHub%s", len(prs), linkedStackLabel(cfg, remoteStackNumber)) return s, nil } @@ -748,7 +749,7 @@ func interactiveCheckout(cfg *config.Config, sf *stack.StackFile, gitDir string) return nil, "", fmt.Errorf("no target specified; provide a branch name or PR number, or run interactively to select a stack") } - rows, remoteStacks := gatherCheckoutRows(cfg, sf) + rows, remoteStacks, links := gatherCheckoutRows(cfg, sf) if currentBranch, branchErr := git.CurrentBranch(); branchErr == nil { stackNumber, confirmed, confirmErr := offerRemoteStackForBranch(cfg, sf, remoteStacks, currentBranch) if confirmErr != nil { @@ -772,7 +773,7 @@ func interactiveCheckout(cfg *config.Config, sf *stack.StackFile, gitDir string) return nil, "", nil } - selected, ok, err := launchCheckoutPicker(rows) + selected, ok, err := launchCheckoutPicker(rows, links) if err != nil { return nil, "", err } @@ -789,14 +790,29 @@ func interactiveCheckout(cfg *config.Config, sf *stack.StackFile, gitDir string) // with the local stacks into the picker's rows. Any GitHub failure (stacks not // enabled for the repo, no auth, network error) gracefully degrades to a // local-only list. -func gatherCheckoutRows(cfg *config.Config, sf *stack.StackFile) ([]checkoutview.StackRow, []github.RemoteStack) { +func gatherCheckoutRows(cfg *config.Config, sf *stack.StackFile) ([]checkoutview.StackRow, []github.RemoteStack, map[int]string) { var remote []github.RemoteStack + var remoteNumbers map[int]bool // nil means the remote listing was unavailable if client, err := cfg.GitHubClient(); err == nil { if stacks, err := client.ListStacks(); err == nil { remote = stacks + remoteNumbers = make(map[int]bool, len(stacks)) + for _, s := range stacks { + remoteNumbers[s.Number] = true + } } } - return checkoutview.BuildRows(sf.Stacks, remote), remote + rows := checkoutview.BuildRows(sf.Stacks, remote) + links := make(map[int]string) + if repo, err := cfg.Repo(); err == nil && repo.Owner != "" && repo.Name != "" { + for _, row := range rows { + if row.Number > 0 && (remoteNumbers == nil || remoteNumbers[row.Number]) { + url := github.StackURL(repo.Host, repo.Owner, repo.Name, row.Number) + links[row.Number] = cfg.TerminalLink(row.NumberDisplay(), url) + } + } + } + return rows, remote, links } // offerRemoteStackForBranch asks to check out the unique active remote stack @@ -814,7 +830,8 @@ func offerRemoteStackForBranch(cfg *config.Config, sf *stack.StackFile, remote [ } stackNumber := matches[0].Number - prompt := fmt.Sprintf("Found stack #%d that includes branch %q. Check out stack #%d?", stackNumber, branch, stackNumber) + cfg.Infof("Found stack %s that includes branch %q.", cfg.StackLink(stackNumber), branch) + prompt := fmt.Sprintf("Check out stack #%d?", stackNumber) confirmed, err := confirmRemoteStackCheckout(cfg, prompt) if err != nil { if errors.Is(err, errInterrupt) { @@ -886,7 +903,11 @@ func resolveCheckoutSelection(cfg *config.Config, sf *stack.StackFile, gitDir st s, targetBranch, err := checkoutStackByNumber(cfg, sf, gitDir, selected.Number) if err != nil { if errors.Is(err, errStackNumberNotFound) { - cfg.Errorf("stack #%d could not be loaded from GitHub", selected.Number) + label := fmt.Sprintf("#%d", selected.Number) + if !isNotFound(err) { + label = cfg.StackLink(selected.Number) + } + cfg.Errorf("stack %s could not be loaded from GitHub", label) return nil, "", ErrAPIFailure } return nil, "", err @@ -899,8 +920,8 @@ func resolveCheckoutSelection(cfg *config.Config, sf *stack.StackFile, gitDir st // picker occupies only a few lines and leaves the surrounding terminal output // intact. Mouse motion is intentionally not enabled so the search field never // receives stray mouse bytes. -func launchCheckoutPicker(rows []checkoutview.StackRow) (checkoutview.StackRow, bool, error) { - p := tea.NewProgram(checkoutview.New(rows)) +func launchCheckoutPicker(rows []checkoutview.StackRow, links map[int]string) (checkoutview.StackRow, bool, error) { + p := tea.NewProgram(checkoutview.New(rows).WithNumberLinks(links)) finalModel, err := p.Run() if err != nil { return checkoutview.StackRow{}, false, fmt.Errorf("running stack picker: %w", err) diff --git a/cmd/checkout_picker_test.go b/cmd/checkout_picker_test.go index a47ed778..9f8f1a83 100644 --- a/cmd/checkout_picker_test.go +++ b/cmd/checkout_picker_test.go @@ -41,7 +41,7 @@ func TestGatherCheckoutRows_FallbackToLocalOnListError(t *testing.T) { }, }}} - rows, remote := gatherCheckoutRows(cfg, sf) + rows, remote, _ := gatherCheckoutRows(cfg, sf) require.Len(t, rows, 1, "falls back to a local-only list when ListStacks fails") assert.Empty(t, remote) assert.Equal(t, checkoutview.TypeLocal, rows[0].Type) @@ -70,7 +70,7 @@ func TestGatherCheckoutRows_IncludesRemoteOnlyStacks(t *testing.T) { Branches: []stack.BranchRef{{Branch: "local-a"}}, }}} - rows, remote := gatherCheckoutRows(cfg, sf) + rows, remote, _ := gatherCheckoutRows(cfg, sf) require.Len(t, rows, 2, "local and remote-only stacks are both listed") require.Len(t, remote, 1) @@ -173,23 +173,29 @@ func TestMatchingRemoteStacksForBranch(t *testing.T) { } func TestOfferRemoteStackForBranch_Confirmed(t *testing.T) { - cfg, _, _ := config.NewTestConfig() - cfg.ConfirmFn = func(prompt string, defaultValue bool) (bool, error) { - assert.Equal(t, `Found stack #7 that includes branch "feature". Check out stack #7?`, prompt) - assert.True(t, defaultValue) - return true, nil + for _, force := range []string{"0", "1"} { + t.Run(force, func(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", force) + cfg, outR, errR := config.NewTestConfig() + setTestRepo(cfg) + cfg.ConfirmFn = func(prompt string, defaultValue bool) (bool, error) { + assert.Equal(t, "Check out stack #7?", prompt) + assert.True(t, defaultValue) + return true, nil + } + number, confirmed, err := offerRemoteStackForBranch(cfg, &stack.StackFile{}, []github.RemoteStack{{ + Number: 7, + PRDetails: []github.RemoteStackPR{ + {Number: 10, State: "open", Head: github.RemoteStackPRHead{Ref: "feature"}}, + }, + }}, "feature") + output := collectOutput(cfg, outR, errR) + require.NoError(t, err) + assert.True(t, confirmed) + assert.Equal(t, 7, number) + assert.Contains(t, output, "Found stack "+cfg.StackLink(7)+` that includes branch "feature".`) + }) } - - number, confirmed, err := offerRemoteStackForBranch(cfg, &stack.StackFile{}, []github.RemoteStack{{ - Number: 7, - PRDetails: []github.RemoteStackPR{ - {Number: 10, State: "open", Head: github.RemoteStackPRHead{Ref: "feature"}}, - }, - }}, "feature") - - require.NoError(t, err) - assert.True(t, confirmed) - assert.Equal(t, 7, number) } func TestOfferRemoteStackForBranch_FallsBack(t *testing.T) { @@ -445,19 +451,33 @@ func TestResolveCheckoutSelection_RemoteRoutesToClone(t *testing.T) { } func TestResolveCheckoutSelection_RemoteLoadFailure(t *testing.T) { - cfg, _, _ := config.NewTestConfig() - cfg.GitHubClientOverride = &github.MockClient{ - GetStackFn: func(n int) (*github.RemoteStack, error) { - return nil, errors.New("boom") - }, + t.Setenv("GH_STACK_HYPERLINKS", "1") + for _, tt := range []struct { + name string + err error + link bool + }{ + {"transient failure", errors.New("boom"), true}, + {"deleted stack", &api.HTTPError{StatusCode: 404}, false}, + } { + t.Run(tt.name, func(t *testing.T) { + cfg, outR, errR := config.NewTestConfig() + setTestRepo(cfg) + cfg.GitHubClientOverride = &github.MockClient{ + GetStackFn: func(int) (*github.RemoteStack, error) { return nil, tt.err }, + } + sel := checkoutview.StackRow{Type: checkoutview.TypeRemote, Number: 7} + _, _, err := resolveCheckoutSelection(cfg, &stack.StackFile{}, t.TempDir(), sel) + output := collectOutput(cfg, outR, errR) + assert.ErrorIs(t, err, ErrAPIFailure) + if tt.link { + assert.Contains(t, output, "\x1b]8;;https://github.com/o/r/stack/7\x1b\\#7\x1b]8;;\x1b\\") + } else { + assert.Contains(t, output, "stack #7 could not be loaded") + assert.NotContains(t, output, "/stack/") + } + }) } - - sel := checkoutview.StackRow{Type: checkoutview.TypeRemote, Number: 7} - _, _, err := resolveCheckoutSelection(cfg, &stack.StackFile{}, t.TempDir(), sel) - - var exitErr *ExitError - require.ErrorAs(t, err, &exitErr) - assert.Equal(t, ErrAPIFailure, err) } func TestTopUnmergedBranch(t *testing.T) { diff --git a/cmd/checkout_test.go b/cmd/checkout_test.go index f6623838..bf7eca8d 100644 --- a/cmd/checkout_test.go +++ b/cmd/checkout_test.go @@ -8,15 +8,92 @@ import ( "strings" "testing" + tea "github.com/charmbracelet/bubbletea" + "github.com/charmbracelet/lipgloss" "github.com/cli/go-gh/v2/pkg/api" "github.com/github/gh-stack/internal/config" "github.com/github/gh-stack/internal/git" "github.com/github/gh-stack/internal/github" "github.com/github/gh-stack/internal/stack" + "github.com/github/gh-stack/internal/tui/checkoutview" + "github.com/muesli/termenv" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +func TestCheckoutPickerStackLinks(t *testing.T) { + profile := lipgloss.DefaultRenderer().ColorProfile() + t.Setenv("NO_COLOR", "") + t.Setenv("CLICOLOR_FORCE", "1") + lipgloss.SetColorProfile(termenv.ANSI256) + defer lipgloss.SetColorProfile(profile) + + for _, tt := range []struct { + name string + number int + empty, failed, missingRepo bool + force string + wantLink bool + }{ + {name: "matched stacks", number: 7, force: "1", wantLink: true}, + {name: "successful empty listing", number: 7, empty: true, force: "1"}, + {name: "failed listing", number: 7, failed: true, force: "1", wantLink: true}, + {name: "legacy ID match", force: "1", wantLink: true}, + {name: "links disabled", number: 7, force: "0"}, + {name: "missing repository", number: 7, missingRepo: true, force: "1"}, + } { + t.Run(tt.name, func(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", tt.force) + cfg := config.New() + setTestRepo(cfg) + cfg.RepoOverride.Host = "github.example.com" + if tt.missingRepo { + cfg.RepoOverride.Owner = "" + } + calls := 0 + cfg.GitHubClientOverride = &github.MockClient{ + ListStacksFn: func() ([]github.RemoteStack, error) { + calls++ + if tt.failed { + return nil, fmt.Errorf("offline") + } + if tt.empty { + return nil, nil + } + return []github.RemoteStack{ + {ID: 6154, Number: 7, PRDetails: []github.RemoteStackPR{linkPR(10, "tracked")}}, + {ID: 808, Number: 8, PRDetails: []github.RemoteStackPR{linkPR(11, "remote")}}, + }, nil + }, + GetStackFn: func(int) (*github.RemoteStack, error) { + t.Fatal("formatting picker links must not add a lookup") + return nil, nil + }, + } + sf := &stack.StackFile{Stacks: []stack.Stack{ + {Branches: []stack.BranchRef{{Branch: "unsubmitted"}}}, + {ID: "6154", Number: tt.number, Branches: []stack.BranchRef{{Branch: "tracked"}}}, + }} + rows, _, links := gatherCheckoutRows(cfg, sf) + m := checkoutview.New(rows).WithNumberLinks(links) + sized, _ := m.Update(tea.WindowSizeMsg{Width: 100, Height: 20}) + output := sized.View() + if tt.wantLink { + assert.Contains(t, output, "\x1b]8;;https://github.example.com/o/r/stack/7\x1b\\7\x1b]8;;\x1b\\") + } else { + assert.NotContains(t, output, "\x1b]8") + assert.NotContains(t, output, "https://") + } + if !tt.empty && !tt.failed && tt.wantLink { + assert.Contains(t, output, "/stack/8") + } + assert.NotContains(t, output, "/stack/0") + assert.Equal(t, 1, calls) + assert.Equal(t, tt.number, sf.Stacks[1].Number, "presentation must not mutate tracking") + }) + } +} + func TestCheckout_PrintPathRequiresTarget(t *testing.T) { cfg, outR, errR := config.NewTestConfig() cfg.ForceInteractive = true @@ -47,6 +124,7 @@ func TestCheckout_PrintPathAmbiguousBranch(t *testing.T) { } func TestCheckout_RemotePrintPathDoesNotImportDuringRecovery(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", "1") common, root, owner := t.TempDir(), t.TempDir(), t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(common, rebaseStateFile), []byte(`{"worktrees":{}}`), 0600)) restore := git.SetOps(&git.MockOps{ @@ -428,6 +506,7 @@ func TestCheckout_BranchNotFound(t *testing.T) { } func TestCheckout_RemoteBranchName_MultipleStacks(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", "0") gitDir := t.TempDir() checkoutCalled := false restore := git.SetOps(&git.MockOps{ @@ -443,6 +522,7 @@ func TestCheckout_RemoteBranchName_MultipleStacks(t *testing.T) { require.NoError(t, stack.Save(gitDir, &stack.StackFile{SchemaVersion: 1, Stacks: []stack.Stack{}})) cfg, outR, errR := config.NewTestConfig() + setTestRepo(cfg) cfg.GitHubClientOverride = &github.MockClient{ ListStacksFn: func() ([]github.RemoteStack, error) { return []github.RemoteStack{ @@ -471,7 +551,7 @@ func TestCheckout_RemoteBranchName_MultipleStacks(t *testing.T) { assert.ErrorIs(t, err, ErrDisambiguate) assert.False(t, checkoutCalled) - assert.Contains(t, output, `branch "feature" belongs to multiple stacks on GitHub (7, 8)`) + assert.Contains(t, output, `branch "feature" belongs to multiple stacks on GitHub (7 (https://github.com/o/r/stack/7), 8 (https://github.com/o/r/stack/8))`) assert.Contains(t, output, "stack or PR number") } @@ -663,6 +743,7 @@ func TestCheckout_NumericTarget_NewStack(t *testing.T) { } func TestCheckout_ByStackNumber(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", "1") gitDir := t.TempDir() var checkedOut string var createdBranches []string @@ -697,6 +778,7 @@ func TestCheckout_ByStackNumber(t *testing.T) { var gotStackNumber int cfg, outR, errR := config.NewTestConfig() + setTestRepo(cfg) cfg.GitHubClientOverride = &github.MockClient{ GetStackFn: func(n int) (*github.RemoteStack, error) { gotStackNumber = n @@ -716,6 +798,8 @@ func TestCheckout_ByStackNumber(t *testing.T) { } err := runCheckout(cfg, &checkoutOptions{target: "7"}) + require.NoError(t, err) + require.NoError(t, runCheckout(cfg, &checkoutOptions{target: "7"})) output := collectOutput(cfg, outR, errR) require.NoError(t, err) @@ -723,6 +807,10 @@ func TestCheckout_ByStackNumber(t *testing.T) { // The top-most (last) branch of the stack is checked out. assert.Equal(t, "feat-3", checkedOut) assert.Contains(t, output, "Imported stack with 3 branches") + link := "\x1b]8;;https://github.com/o/r/stack/7\x1b\\#7\x1b]8;;\x1b\\" + assert.Contains(t, output, "from GitHub (stack "+link+")") + assert.Contains(t, output, "Local stack matches remote (stack "+link+")") + assert.NotContains(t, output, "/stack/42") // Verify the stack was imported with both its internal id and stack number. sf, loadErr := stack.Load(gitDir) diff --git a/cmd/link.go b/cmd/link.go index 03f9e0d5..163099d6 100644 --- a/cmd/link.go +++ b/cmd/link.go @@ -271,7 +271,7 @@ func runLinkCreateOrUpdate(cfg *config.Config, client github.ClientOps, opts *li // chained on top of the stack's current top branch. func runLinkAdd(cfg *config.Config, client github.ClientOps, opts *linkOptions, target *github.RemoteStack, stacks []github.RemoteStack, prArgs []string, found []*resolvedArg, templateContent string) error { if opts.baseChanged { - cfg.Warningf("--base is ignored when adding to stack #%d (its base is fixed by the existing stack)", target.Number) + cfg.Warningf("--base is ignored when adding to stack %s (its base is fixed by the existing stack)", cfg.StackLink(target.Number)) } inTarget := make(map[int]bool, len(target.PRNumbers())) @@ -287,8 +287,8 @@ func runLinkAdd(cfg *config.Config, client github.ClientOps, opts *linkOptions, for i, arg := range prArgs { r := found[i] if r != nil && inTarget[r.prNumber] { - cfg.Infof("PR %s is already in stack #%d — skipping", - cfg.PRLink(r.prNumber, r.prURL), target.Number) + cfg.Infof("PR %s is already in stack %s — skipping", + cfg.PRLink(r.prNumber, r.prURL), cfg.StackLink(target.Number)) continue } appendArgs = append(appendArgs, arg) @@ -302,7 +302,7 @@ func runLinkAdd(cfg *config.Config, client github.ClientOps, opts *linkOptions, } if len(appendArgs) == 0 { - cfg.Successf("Stack #%d is already up to date", target.Number) + cfg.Successf("Stack %s is already up to date", cfg.StackLink(target.Number)) return nil } @@ -317,11 +317,23 @@ func runLinkAdd(cfg *config.Config, client github.ClientOps, opts *linkOptions, // full stack if the top branch can't be resolved from the listed stack. topBranch, err := stackTopBranch(target) if err != nil { - if full, gerr := client.GetStack(target.Number); gerr == nil && full != nil { + full, gerr := client.GetStack(target.Number) + if isNotFound(gerr) { + cfg.Errorf("Stack #%d no longer exists", target.Number) + return ErrNotInStack + } + if gerr == nil && full != nil { topBranch, err = stackTopBranch(full) } if err != nil { - cfg.Errorf("%s", err) + switch { + case errors.Is(err, errStackHasNoPRs): + cfg.Errorf("stack %s has no pull requests to append to", cfg.StackLink(target.Number)) + case errors.Is(err, errStackTopBranchMissing): + cfg.Errorf("could not determine the top branch of stack %s", cfg.StackLink(target.Number)) + default: + cfg.Errorf("%s", err) + } return ErrAPIFailure } } @@ -368,8 +380,8 @@ func ensureNotInOtherStack(cfg *config.Config, stacks []github.RemoteStack, targ continue } if sn, ok := owner[r.prNumber]; ok && sn != targetNumber { - cfg.Errorf("PR %s already belongs to stack #%d — unstack it first", - cfg.PRLink(r.prNumber, r.prURL), sn) + cfg.Errorf("PR %s already belongs to stack %s — unstack it first", + cfg.PRLink(r.prNumber, r.prURL), cfg.StackLink(sn)) invalid++ } } @@ -379,15 +391,20 @@ func ensureNotInOtherStack(cfg *config.Config, stacks []github.RemoteStack, targ return nil } +var ( + errStackHasNoPRs = errors.New("stack has no pull requests to append to") + errStackTopBranchMissing = errors.New("could not determine the top branch") +) + // stackTopBranch returns the head branch of the pull request at the top of the // stack — the base for the first PR appended on top of it. func stackTopBranch(s *github.RemoteStack) (string, error) { if len(s.PRDetails) == 0 { - return "", fmt.Errorf("stack #%d has no pull requests to append to", s.Number) + return "", errStackHasNoPRs } top := s.PRDetails[len(s.PRDetails)-1] if top.Head.Ref == "" { - return "", fmt.Errorf("could not determine the top branch of stack #%d", s.Number) + return "", errStackTopBranchMissing } return top.Head.Ref, nil } @@ -414,7 +431,7 @@ func addToStack(cfg *config.Config, client github.ClientOps, stackNumber int, de return ErrAPIFailure } - cfg.Successf("Added %d %s to stack #%d", len(delta), plural(len(delta), "PR", "PRs"), stackNumber) + cfg.Successf("Added %d %s to stack %s", len(delta), plural(len(delta), "PR", "PRs"), cfg.StackLink(stackNumber)) return nil } @@ -834,7 +851,7 @@ func createLink(cfg *config.Config, client github.ClientOps, prNumbers []int) er return ErrAPIFailure } - cfg.Successf("Created stack with %d PRs%s", len(prNumbers), stackLabel(rs.Number)) + cfg.Successf("Created stack with %d PRs%s", len(prNumbers), linkedStackLabel(cfg, rs.Number)) return nil } @@ -902,7 +919,7 @@ func updateLink(cfg *config.Config, client github.ClientOps, existing *github.Re return ErrAPIFailure } - cfg.Successf("Updated stack to %d PRs%s", len(prNumbers), stackLabel(rs.Number)) + cfg.Successf("Updated stack to %d PRs%s", len(prNumbers), linkedStackLabel(cfg, rs.Number)) return nil } diff --git a/cmd/link_test.go b/cmd/link_test.go index f377ae51..310f9ba9 100644 --- a/cmd/link_test.go +++ b/cmd/link_test.go @@ -99,11 +99,13 @@ func TestLink_NonRepositoryBranchNameStillFails(t *testing.T) { } func TestLink_PRNumbers_CreateNewStack(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", "1") restore := git.SetOps(newLinkGitMock()) defer restore() var createdPRs []int cfg, _, errR := config.NewTestConfig() + setTestRepo(cfg) cfg.GitHubClientOverride = &github.MockClient{ FindPRByNumberFn: func(n int) (*github.PullRequest, error) { return &github.PullRequest{ @@ -118,7 +120,7 @@ func TestLink_PRNumbers_CreateNewStack(t *testing.T) { }, CreateStackFn: func(prNumbers []int) (*github.RemoteStack, error) { createdPRs = prNumbers - return &github.RemoteStack{ID: 42, Number: 42}, nil + return &github.RemoteStack{ID: 6154, Number: 42, URL: "https://api.github.com/repos/o/r/stacks/42"}, nil }, } @@ -135,15 +137,20 @@ func TestLink_PRNumbers_CreateNewStack(t *testing.T) { assert.NoError(t, err) assert.Equal(t, []int{10, 20, 30}, createdPRs) assert.Contains(t, output, "Created stack with 3 PRs") + assert.Contains(t, output, "\x1b]8;;https://github.com/o/r/stack/42\x1b\\#42\x1b]8;;\x1b\\") + assert.NotContains(t, output, "/stack/6154") + assert.NotContains(t, output, "api.github.com") } func TestLink_PRNumbers_UpdateExistingStack(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", "0") restore := git.SetOps(newLinkGitMock()) defer restore() var updatedNumber int var updatedPRs []int cfg, _, errR := config.NewTestConfig() + setTestRepo(cfg) cfg.GitHubClientOverride = &github.MockClient{ FindPRByNumberFn: func(n int) (*github.PullRequest, error) { return &github.PullRequest{ @@ -179,6 +186,7 @@ func TestLink_PRNumbers_UpdateExistingStack(t *testing.T) { assert.Equal(t, 7, updatedNumber) assert.Equal(t, []int{30}, updatedPRs) assert.Contains(t, output, "Updated stack to 3 PRs") + assert.Contains(t, output, "#7 (https://github.com/o/r/stack/7)") } func TestLink_PRNumbers_ExactMatch_NoOp(t *testing.T) { @@ -2309,11 +2317,49 @@ func linkRemoteStack(number int, details ...github.RemoteStackPR) github.RemoteS return rs } +func TestLink_AddMode_StackLookupDiagnostics(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", "1") + for _, details := range [][]github.RemoteStackPR{nil, {linkPR(10, "")}} { + for _, notFound := range []bool{false, true} { + t.Run(fmt.Sprintf("details=%d/notFound=%t", len(details), notFound), func(t *testing.T) { + cfg, outR, errR := config.NewTestConfig() + setTestRepo(cfg) + target := github.RemoteStack{Number: 7, PullRequests: []int{10}, PRDetails: details} + client := &github.MockClient{ + GetStackFn: func(number int) (*github.RemoteStack, error) { + assert.Equal(t, 7, number) + if notFound { + return nil, &api.HTTPError{StatusCode: 404} + } + return &target, nil + }, + AddToStackFn: func(int, []int) (*github.RemoteStack, error) { + t.Fatal("a stack with unresolved metadata must not be updated") + return nil, nil + }, + } + err := runLinkAdd(cfg, client, &linkOptions{}, &target, []github.RemoteStack{target}, []string{"30"}, []*resolvedArg{{prNumber: 30}}, "") + output := collectOutput(cfg, outR, errR) + if notFound { + assert.ErrorIs(t, err, ErrNotInStack) + assert.Contains(t, output, "Stack #7 no longer exists") + assert.NotContains(t, output, "/stack/") + } else { + assert.ErrorIs(t, err, ErrAPIFailure) + assert.Contains(t, output, "\x1b]8;;https://github.com/o/r/stack/7\x1b\\#7\x1b]8;;\x1b\\") + } + }) + } + } +} + func TestLink_AddMode_AppendsPRNumberToStack(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", "1") defer mockRemoteOnlyGit()() var addNumber int var addPRs []int cfg, _, errR := config.NewTestConfig() + setTestRepo(cfg) cfg.GitHubClientOverride = &github.MockClient{ FindPRByNumberFn: func(n int) (*github.PullRequest, error) { return &github.PullRequest{ @@ -2352,7 +2398,7 @@ func TestLink_AddMode_AppendsPRNumberToStack(t *testing.T) { assert.NoError(t, err) assert.Equal(t, 7, addNumber) assert.Equal(t, []int{30}, addPRs) - assert.Contains(t, output, "Added 1 PR to stack #7") + assert.Contains(t, output, "Added 1 PR to stack "+cfg.StackLink(7)) } func TestLink_AddMode_CreatesPRForBranchOnTopOfStack(t *testing.T) { @@ -2401,8 +2447,10 @@ func TestLink_AddMode_CreatesPRForBranchOnTopOfStack(t *testing.T) { } func TestLink_AddMode_IdempotentWhenAllPresent(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", "0") defer mockRemoteOnlyGit()() cfg, _, errR := config.NewTestConfig() + setTestRepo(cfg) cfg.GitHubClientOverride = &github.MockClient{ FindPRByNumberFn: func(n int) (*github.PullRequest, error) { return &github.PullRequest{ @@ -2434,7 +2482,8 @@ func TestLink_AddMode_IdempotentWhenAllPresent(t *testing.T) { assert.NoError(t, err) assert.Contains(t, output, "already in stack #7") - assert.Contains(t, output, "Stack #7 is already up to date") + assert.Contains(t, output, "Stack #7 (https://github.com/o/r/stack/7) is already up to date") + assert.Contains(t, output, "already in stack #7 (https://github.com/o/r/stack/7)") } func TestLink_AddMode_SkipsPresentAppendsNew(t *testing.T) { @@ -2481,8 +2530,10 @@ func TestLink_AddMode_SkipsPresentAppendsNew(t *testing.T) { } func TestLink_AddMode_RejectsPRFromAnotherStack(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", "0") defer mockRemoteOnlyGit()() cfg, _, errR := config.NewTestConfig() + setTestRepo(cfg) cfg.GitHubClientOverride = &github.MockClient{ FindPRByNumberFn: func(n int) (*github.PullRequest, error) { return &github.PullRequest{ @@ -2514,7 +2565,7 @@ func TestLink_AddMode_RejectsPRFromAnotherStack(t *testing.T) { output := string(errOut) assert.ErrorIs(t, err, ErrInvalidArgs) - assert.Contains(t, output, "already belongs to stack #8") + assert.Contains(t, output, "already belongs to stack #8 (https://github.com/o/r/stack/8)") } func TestLink_AddMode_RejectsIneligibleNewPR(t *testing.T) { @@ -2649,9 +2700,11 @@ func TestLink_NumericFirstArgNotAStack_UsesCreateMode(t *testing.T) { } func TestLink_AddMode_WarnsWhenBaseFlagSet(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", "0") defer mockRemoteOnlyGit()() var addPRs []int cfg, _, errR := config.NewTestConfig() + setTestRepo(cfg) cfg.GitHubClientOverride = &github.MockClient{ FindPRByNumberFn: func(n int) (*github.PullRequest, error) { return &github.PullRequest{ @@ -2683,7 +2736,7 @@ func TestLink_AddMode_WarnsWhenBaseFlagSet(t *testing.T) { assert.NoError(t, err) assert.Equal(t, []int{30}, addPRs) - assert.Contains(t, output, "--base is ignored") + assert.Contains(t, output, "--base is ignored when adding to stack #7 (https://github.com/o/r/stack/7)") } func TestLink_AddMode_ChainsMultipleCreatedPRs(t *testing.T) { @@ -2772,8 +2825,10 @@ func TestLink_AddMode_AddToStack422(t *testing.T) { } func TestLink_AddMode_AddToStack404_StackGone(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", "1") defer mockRemoteOnlyGit()() cfg, _, errR := config.NewTestConfig() + setTestRepo(cfg) cfg.GitHubClientOverride = &github.MockClient{ FindPRByNumberFn: func(n int) (*github.PullRequest, error) { return &github.PullRequest{ @@ -2804,6 +2859,8 @@ func TestLink_AddMode_AddToStack404_StackGone(t *testing.T) { assert.ErrorIs(t, err, ErrNotInStack) assert.Contains(t, output, "no longer exists") + assert.Contains(t, output, "Stack #7 no longer exists") + assert.NotContains(t, output, "/stack/7") } func TestLink_AddMode_FetchesFullStackWhenListLacksHeadRefs(t *testing.T) { diff --git a/cmd/merge.go b/cmd/merge.go index 32ed9320..306c1f33 100644 --- a/cmd/merge.go +++ b/cmd/merge.go @@ -222,7 +222,7 @@ func resolveMergeStack(cfg *config.Config, client github.ClientOps, args []strin return rs, mergeTarget{}, nil } if err != nil && !isNotFound(err) { - cfg.Errorf("failed to fetch stack #%d: %s", n, err) + cfg.Errorf("failed to fetch stack %s: %s", cfg.StackLink(n), err) return nil, mergeTarget{}, ErrAPIFailure } @@ -298,7 +298,7 @@ func resolveActiveRemoteStack(cfg *config.Config, client github.ClientOps) (*git warnStacksUnavailable(cfg) return nil, ErrStacksUnavailable } - cfg.Errorf("failed to fetch stack #%d: %s", number, err) + cfg.Errorf("failed to fetch stack %s: %s", cfg.StackLink(number), err) return nil, ErrAPIFailure } return rs, nil @@ -328,6 +328,7 @@ func runMergeInteractive(cfg *config.Config, client github.ClientOps, stackNumbe model := mergeview.New(mergeview.Options{ PRs: candidates, StackNumber: stackNumber, + StackLink: cfg.TerminalLink(fmt.Sprintf("#%d", stackNumber), cfg.StackURL(stackNumber)), BaseRef: base, AllowedMethods: allowed, DefaultMethod: defaultMethod, diff --git a/cmd/merge_test.go b/cmd/merge_test.go index 11a7c7b4..61a21666 100644 --- a/cmd/merge_test.go +++ b/cmd/merge_test.go @@ -77,6 +77,47 @@ func setupLocalStack(t *testing.T, number int, currentBranch string, branches .. }) } +func TestResolveMergeStack_FetchFailureLinks(t *testing.T) { + for _, mode := range []string{"explicit", "active"} { + for _, force := range []string{"0", "1"} { + t.Run(mode+"/"+force, func(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", force) + var args []string + if mode == "active" { + setupLocalStack(t, 7, "feature", "feature") + } else { + args = []string{"7"} + } + cfg, outR, errR := config.NewTestConfig() + setTestRepo(cfg) + var calls int + client := &github.MockClient{ + GetStackFn: func(number int) (*github.RemoteStack, error) { + calls++ + assert.Equal(t, 7, number) + return nil, errors.New("offline") + }, + FindStackForPRFn: func(int) (*github.RemoteStack, error) { + t.Fatal("a transient stack failure must not fall back to PR lookup") + return nil, nil + }, + } + _, _, err := resolveMergeStack(cfg, client, args) + output := collectOutput(cfg, outR, errR) + assert.ErrorIs(t, err, ErrAPIFailure) + assert.Equal(t, 1, calls) + link := "#7 (https://github.com/o/r/stack/7)" + if force == "1" { + link = "\x1b]8;;https://github.com/o/r/stack/7\x1b\\#7\x1b]8;;\x1b\\" + } else { + assert.NotContains(t, output, "\x1b]8") + } + assert.Contains(t, output, "failed to fetch stack "+link+": offline") + }) + } + } +} + func TestRunMerge_NoArg_MergesWholeStack(t *testing.T) { setupLocalStack(t, 100, "b2", "b1", "b2", "b3") diff --git a/cmd/submit.go b/cmd/submit.go index ad75973d..36fe8c82 100644 --- a/cmd/submit.go +++ b/cmd/submit.go @@ -339,6 +339,7 @@ func collectPRDrafts(cfg *config.Config, client github.ClientOps, s *stack.Stack Version: Version, CanCreateStack: canCreateStack, StackNumber: s.Number, + StackURL: cfg.StackURL(s.Number), }) // Use cell-motion mouse mode (clicks, drag, and wheel) rather than all-motion. @@ -914,7 +915,7 @@ func reconcileUntrackedStack(cfg *config.Config, client github.ClientOps, s *sta s.Number = matched.Number if slicesEqual(matched.PRNumbers(), prNumbers) { - cfg.Successf("Linked to the existing stack on GitHub (%d PRs, already up to date)%s", len(prNumbers), stackLabel(matched.Number)) + cfg.Successf("Linked to the existing stack on GitHub (%d PRs, already up to date)%s", len(prNumbers), linkedStackLabel(cfg, matched.Number)) return true } @@ -977,7 +978,7 @@ func updateStack(cfg *config.Config, client github.ClientOps, s *stack.Stack) bo if slicesEqual(current, prNumbers) { s.ID = strconv.Itoa(remote.ID) s.Number = remote.Number - cfg.Successf("Stack on GitHub is up to date with %d PRs%s", len(prNumbers), stackLabel(remote.Number)) + cfg.Successf("Stack on GitHub is up to date with %d PRs%s", len(prNumbers), linkedStackLabel(cfg, remote.Number)) return true } @@ -1027,7 +1028,7 @@ func updateStack(cfg *config.Config, client github.ClientOps, s *stack.Stack) bo s.ID = strconv.Itoa(rs.ID) s.Number = rs.Number - cfg.Successf("Stack updated on GitHub with %d PRs%s", len(prNumbers), stackLabel(rs.Number)) + cfg.Successf("Stack updated on GitHub with %d PRs%s", len(prNumbers), linkedStackLabel(cfg, rs.Number)) return true } @@ -1043,7 +1044,7 @@ func createNewStack(cfg *config.Config, client github.ClientOps, s *stack.Stack) if err == nil { s.ID = strconv.Itoa(rs.ID) s.Number = rs.Number - cfg.Successf("Stack created on GitHub with %d PRs%s", len(prNumbers), stackLabel(rs.Number)) + cfg.Successf("Stack created on GitHub with %d PRs%s", len(prNumbers), linkedStackLabel(cfg, rs.Number)) return true } diff --git a/cmd/submit_test.go b/cmd/submit_test.go index f37ff2b3..2f4dfa96 100644 --- a/cmd/submit_test.go +++ b/cmd/submit_test.go @@ -843,6 +843,7 @@ func TestSyncStack_ExistingStack_UpdateFails(t *testing.T) { } func TestSyncStack_ExistingStack_Update404(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", "0") s := &stack.Stack{ ID: "99", Number: 99, @@ -872,6 +873,7 @@ func TestSyncStack_ExistingStack_Update404(t *testing.T) { } cfg, _, errR := config.NewTestConfig() + setTestRepo(cfg) syncStack(cfg, mock, s) cfg.Err.Close() @@ -881,6 +883,8 @@ func TestSyncStack_ExistingStack_Update404(t *testing.T) { assert.True(t, createCalled, "should fall through to CreateStack after 404") assert.Equal(t, "55", s.ID, "should set new stack ID from create response") assert.Contains(t, output, "Stack created on GitHub with 2 PRs") + assert.Contains(t, output, "#55 (https://github.com/o/r/stack/55)") + assert.NotContains(t, output, "/stack/99") } func TestSyncStack_AlreadyStacked_OurStack(t *testing.T) { @@ -1275,6 +1279,7 @@ func TestSyncStack_SkippedForSinglePR(t *testing.T) { } func TestSyncStack_MergedMembership(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", "1") for _, tc := range []struct { name string tracked bool @@ -1318,15 +1323,20 @@ func TestSyncStack_MergedMembership(t *testing.T) { }, } cfg, outR, errR := config.NewTestConfig() + setTestRepo(cfg) assert.True(t, syncStack(cfg, mock, s)) - commandOutput(t, cfg, outR, errR) + _, output := commandOutput(t, cfg, outR, errR) assert.Equal(t, tc.wantCreate, created) assert.Equal(t, tc.wantAdd, added) if tc.wantCreate != nil { assert.Equal(t, "88", s.ID) + assert.Contains(t, output, "\x1b]8;;https://github.com/o/r/stack/8\x1b\\#8\x1b]8;;\x1b\\") } else { assert.Equal(t, "77", s.ID) + assert.Contains(t, output, "\x1b]8;;https://github.com/o/r/stack/7\x1b\\#7\x1b]8;;\x1b\\") } + assert.NotContains(t, output, "/stack/77") + assert.NotContains(t, output, "/stack/88") assert.Equal(t, []string{"b1", "b2", "b3"}, s.BranchNames()) }) } diff --git a/cmd/sync_test.go b/cmd/sync_test.go index 1060fdd0..dc8e29b8 100644 --- a/cmd/sync_test.go +++ b/cmd/sync_test.go @@ -1773,6 +1773,7 @@ func runSyncWithGitHub(t *testing.T, gitMock *git.MockOps, ghMock *github.MockCl defer restore() cfg, _, errR := config.NewTestConfig() + setTestRepo(cfg) cfg.GitHubClientOverride = ghMock cmd := SyncCmd(cfg) cmd.SetOut(io.Discard) @@ -1788,6 +1789,7 @@ func runSyncWithGitHub(t *testing.T, gitMock *git.MockOps, ghMock *github.MockCl // branches already have open PRs but no stack exists on GitHub, sync creates the // stack object and reports "Stack synced". func TestSync_CreatesRemoteStackWhenPRsExist(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", "1") s := stack.Stack{ Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{{Branch: "b1"}, {Branch: "b2"}}, @@ -1818,6 +1820,7 @@ func TestSync_CreatesRemoteStackWhenPRsExist(t *testing.T) { assert.Equal(t, []int{101, 102}, createdWith, "should create the stack from both PR numbers") assert.Equal(t, 1, listCalls, "should issue exactly one ListStacks on the create path (no redundant round-trip)") assert.Contains(t, output, "Stack created on GitHub with 2 PRs") + assert.Contains(t, output, "\x1b]8;;https://github.com/o/r/stack/7\x1b\\#7\x1b]8;;\x1b\\") assert.Contains(t, output, "Stack synced") assert.NotContains(t, output, "Branches synced") @@ -1832,6 +1835,7 @@ func TestSync_CreatesRemoteStackWhenPRsExist(t *testing.T) { // already lists exactly the local PRs, sync records its ID without issuing a // redundant create/update and still reports "Stack synced". func TestSync_AdoptsExistingEqualRemoteStack(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", "0") s := stack.Stack{ Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{{Branch: "b1"}, {Branch: "b2"}}, @@ -1857,6 +1861,7 @@ func TestSync_AdoptsExistingEqualRemoteStack(t *testing.T) { output := runSyncWithGitHub(t, newSyncMockNoRebase(tmpDir, "b1"), ghMock) assert.Contains(t, output, "already up to date") + assert.Contains(t, output, "#9 (https://github.com/o/r/stack/9)") assert.Contains(t, output, "Stack synced") assert.NotContains(t, output, "Branches synced") @@ -1868,6 +1873,7 @@ func TestSync_AdoptsExistingEqualRemoteStack(t *testing.T) { // TestSync_UpdatesPartialRemoteStack verifies that when a remote stack contains // only some of the local PRs, sync updates it with the full list. func TestSync_UpdatesPartialRemoteStack(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", "1") s := stack.Stack{ Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{{Branch: "b1"}, {Branch: "b2"}, {Branch: "b3"}}, @@ -1902,6 +1908,7 @@ func TestSync_UpdatesPartialRemoteStack(t *testing.T) { assert.Equal(t, 9, updatedNumber) assert.Equal(t, []int{103}, addedWith) assert.Contains(t, output, "Stack updated on GitHub with 3 PRs") + assert.Contains(t, output, "\x1b]8;;https://github.com/o/r/stack/9\x1b\\#9\x1b]8;;\x1b\\") assert.Contains(t, output, "Stack synced") assert.NotContains(t, output, "Branches synced") @@ -2475,19 +2482,23 @@ func TestSync_Divergent_UseRemote_DirtyBlocked(t *testing.T) { // The keep-local resolution stops before pushing or recreating the stack. func TestSync_Divergent_DeleteRemote(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", "1") mergedAt := "2026-01-01T00:00:00Z" + mergedRemnant := &github.RemoteStack{ + ID: 9, Number: 9, PullRequests: []int{101}, + PRDetails: []github.RemoteStackPR{{Number: 101, MergedAt: &mergedAt}}, + } for _, tc := range []struct { - name string - remaining *github.RemoteStack - apiErr error - lookupFails bool - wantErr error + name string + remaining *github.RemoteStack + apiErr error + lookupFails bool + alreadyMerged bool + wantErr error }{ {name: "dissolved"}, - {name: "merged", remaining: &github.RemoteStack{ - ID: 9, Number: 9, PullRequests: []int{101}, - PRDetails: []github.RemoteStackPR{{Number: 101, MergedAt: &mergedAt}}, - }}, + {name: "merged", remaining: mergedRemnant}, + {name: "already merged", remaining: mergedRemnant, alreadyMerged: true}, {name: "queued", remaining: &github.RemoteStack{ID: 9, Number: 9, PullRequests: []int{102}}, wantErr: ErrConflict}, {name: "server rejection", apiErr: &api.HTTPError{StatusCode: 422, Message: "Pull requests cannot be removed"}, wantErr: ErrAPIFailure}, {name: "lookup failure", lookupFails: true, wantErr: ErrAPIFailure}, @@ -2504,6 +2515,9 @@ func TestSync_Divergent_DeleteRemote(t *testing.T) { if tc.lookupFails && lookups > 1 { return nil, assert.AnError } + if tc.alreadyMerged && lookups > 1 { + return []github.RemoteStack{*mergedRemnant}, nil + } return listStacks() } ghMock.UnstackFn = func(number int) (*github.RemoteStack, bool, error) { @@ -2522,12 +2536,13 @@ func TestSync_Divergent_DeleteRemote(t *testing.T) { mock := newSyncMockNoRebase(tmpDir, "b1") mock.PushFn = func(string, []string, bool, bool) error { pushed = true; return nil } output, err := runSyncCfg(t, mock, func(cfg *config.Config) { + setTestRepo(cfg) cfg.GitHubClientOverride = ghMock cfg.ForceInteractive = true cfg.SelectFn = func(_, _ string, _ []string) (int, error) { return 1, nil } }) - assert.Equal(t, !tc.lookupFails, unstacked) + assert.Equal(t, !tc.lookupFails && !tc.alreadyMerged, unstacked) assert.False(t, pushed) assert.NotContains(t, output, "Stack synced") assert.NotContains(t, output, "Branches synced") @@ -2545,11 +2560,16 @@ func TestSync_Divergent_DeleteRemote(t *testing.T) { assert.Contains(t, output, "gh stack submit") if tc.remaining != nil { assert.Contains(t, output, "remain stacked on GitHub") + link := "\x1b]8;;https://github.com/o/r/stack/9\x1b\\#9\x1b]8;;\x1b\\" + assert.Contains(t, output, "Merged pull requests remain stacked on GitHub (stack "+link+")") + if !tc.alreadyMerged { + assert.Contains(t, output, "Pull requests unstacked on GitHub (stack "+link+")") + } assert.NotContains(t, output, "Remote stack removed") assert.NotContains(t, output, "only the stack on GitHub was removed") assert.True(t, sf.Stacks[0].Branches[0].IsMerged()) } else { - assert.Contains(t, output, "Remote stack removed") + assert.Contains(t, output, "Remote stack removed (stack #9)") } } }) diff --git a/cmd/unstack.go b/cmd/unstack.go index f1380dee..40dd9bee 100644 --- a/cmd/unstack.go +++ b/cmd/unstack.go @@ -80,7 +80,7 @@ func runUnstack(cfg *config.Config, opts *unstackOptions) error { if !opts.local { return runRemoteUnstack(cfg, opts.stackNumber) } - cfg.Errorf("stack #%d is not tracked locally", opts.stackNumber) + cfg.Errorf("stack %s is not tracked locally", cfg.StackLink(opts.stackNumber)) return ErrNotInStack } // --local must never contact GitHub, so it uses a strictly local lookup @@ -93,7 +93,7 @@ func runUnstack(cfg *config.Config, opts *unstackOptions) error { if opts.local { // --local never contacts GitHub, and there is nothing to remove // locally, so there is nothing to do. - cfg.Errorf("stack #%d is not tracked locally", opts.stackNumber) + cfg.Errorf("stack %s is not tracked locally", cfg.StackLink(opts.stackNumber)) cfg.Printf("Omit %s to unstack it on GitHub", cfg.ColorCyan("--local")) return ErrNotInStack } @@ -208,7 +208,7 @@ func unstackNumberOnGitHub(cfg *config.Config, client github.ClientOps, number i if dissolved { cfg.Successf("Stack removed on GitHub%s", stackLabel(number)) } else { - cfg.Successf("Pull requests unstacked on GitHub%s", stackLabel(number)) + cfg.Successf("Pull requests unstacked on GitHub%s", linkedStackLabel(cfg, number)) cfg.Infof("Some pull requests remain stacked on GitHub") } return nil diff --git a/cmd/unstack_test.go b/cmd/unstack_test.go index df6fa739..f1ec922e 100644 --- a/cmd/unstack_test.go +++ b/cmd/unstack_test.go @@ -17,20 +17,36 @@ import ( ) func TestUnstack_NumberWithoutLocalRepository(t *testing.T) { - defer mockRemoteOnlyGit()() - cfg, outR, errR := config.NewTestConfig() - unstacked := 0 - cfg.GitHubClientOverride = &github.MockClient{ - UnstackFn: func(number int) (*github.RemoteStack, bool, error) { - unstacked = number - return nil, true, nil - }, + t.Setenv("GH_STACK_HYPERLINKS", "1") + for _, tc := range []struct { + name string + local bool + }{{"remote", false}, {"local", true}} { + t.Run(tc.name, func(t *testing.T) { + defer mockRemoteOnlyGit()() + cfg, outR, errR := config.NewTestConfig() + setTestRepo(cfg) + unstacked := 0 + cfg.GitHubClientOverride = &github.MockClient{ + UnstackFn: func(number int) (*github.RemoteStack, bool, error) { + unstacked = number + return nil, true, nil + }, + } + err := runUnstack(cfg, &unstackOptions{stackNumber: 7, local: tc.local}) + out, diagnostics := commandOutput(t, cfg, outR, errR) + if tc.local { + assert.ErrorIs(t, err, ErrNotInStack) + assert.Zero(t, unstacked) + assert.Contains(t, diagnostics, "stack "+cfg.StackLink(7)+" is not tracked locally") + } else { + require.NoError(t, err) + assert.Equal(t, 7, unstacked) + } + assert.Empty(t, out) + assert.NotContains(t, diagnostics, "not a git repository") + }) } - require.NoError(t, runUnstack(cfg, &unstackOptions{stackNumber: 7})) - out, diagnostics := commandOutput(t, cfg, outR, errR) - assert.Empty(t, out) - assert.Equal(t, 7, unstacked) - assert.NotContains(t, diagnostics, "not a git repository") } func writeTwoStacks(t *testing.T, dir string, s1, s2 stack.Stack) { @@ -487,6 +503,7 @@ func TestUnstack_ByStackNumber(t *testing.T) { } func TestUnstack_ByStackNumber_RemoteOnly_Dissolved(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", "1") // A stack number that isn't tracked locally is unstacked directly on GitHub // (remote-first), leaving unrelated local tracking untouched. gitDir := t.TempDir() @@ -505,6 +522,7 @@ func TestUnstack_ByStackNumber_RemoteOnly_Dissolved(t *testing.T) { var unstackedNumber int cfg, outR, errR := config.NewTestConfig() + setTestRepo(cfg) cfg.GitHubClientOverride = &github.MockClient{ UnstackFn: func(n int) (*github.RemoteStack, bool, error) { unstackedNumber = n @@ -517,6 +535,8 @@ func TestUnstack_ByStackNumber_RemoteOnly_Dissolved(t *testing.T) { require.NoError(t, err) assert.Equal(t, 999, unstackedNumber, "should unstack the requested number on GitHub") assert.Contains(t, output, "Stack removed on GitHub") + assert.Contains(t, output, "Stack removed on GitHub (stack #999)") + assert.NotContains(t, output, "/stack/999") // No local tracking was touched. assert.NotContains(t, output, "Stack removed from local tracking") @@ -527,6 +547,7 @@ func TestUnstack_ByStackNumber_RemoteOnly_Dissolved(t *testing.T) { } func TestUnstack_ByStackNumber_RemoteOnly_NotFound(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", "1") // With no local tracking to reconcile, a 404 means the targeted stack does // not exist on GitHub — a hard error, not an idempotent success. gitDir := t.TempDir() @@ -544,6 +565,7 @@ func TestUnstack_ByStackNumber_RemoteOnly_NotFound(t *testing.T) { }) cfg, outR, errR := config.NewTestConfig() + setTestRepo(cfg) cfg.GitHubClientOverride = &github.MockClient{ UnstackFn: func(int) (*github.RemoteStack, bool, error) { return nil, false, &api.HTTPError{StatusCode: 404, Message: "Not Found"} @@ -554,6 +576,7 @@ func TestUnstack_ByStackNumber_RemoteOnly_NotFound(t *testing.T) { assert.ErrorIs(t, err, ErrNotInStack) assert.Contains(t, output, "stack #999 not found on GitHub") + assert.NotContains(t, output, "/stack/999") assert.NotContains(t, output, "Stack removed") sf, err := stack.Load(gitDir) @@ -562,6 +585,7 @@ func TestUnstack_ByStackNumber_RemoteOnly_NotFound(t *testing.T) { } func TestUnstack_ByStackNumber_RemoteOnly_Partial(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", "1") // A partial remote-only unstack succeeds without changing unrelated local tracking. gitDir := t.TempDir() restore := git.SetOps(&git.MockOps{ @@ -578,6 +602,7 @@ func TestUnstack_ByStackNumber_RemoteOnly_Partial(t *testing.T) { }) cfg, outR, errR := config.NewTestConfig() + setTestRepo(cfg) cfg.GitHubClientOverride = &github.MockClient{ UnstackFn: func(int) (*github.RemoteStack, bool, error) { return &github.RemoteStack{ID: 555, Number: 999, PullRequests: []int{102}}, false, nil @@ -588,6 +613,7 @@ func TestUnstack_ByStackNumber_RemoteOnly_Partial(t *testing.T) { require.NoError(t, err) assert.Contains(t, output, "Pull requests unstacked on GitHub") + assert.Contains(t, output, "\x1b]8;;https://github.com/o/r/stack/999\x1b\\#999\x1b]8;;\x1b\\") assert.Contains(t, output, "Some pull requests remain stacked on GitHub") assert.NotContains(t, output, "auto-merge") assert.NotContains(t, output, "⚠") @@ -634,6 +660,7 @@ func TestUnstack_ByStackNumber_RemoteOnly_AllLocked(t *testing.T) { } func TestUnstack_ByStackNumber_NotTracked_LocalFlag(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", "0") // --local never contacts GitHub. Targeting a number that isn't tracked // locally with --local is an error: there is nothing to remove locally. gitDir := t.TempDir() @@ -652,6 +679,7 @@ func TestUnstack_ByStackNumber_NotTracked_LocalFlag(t *testing.T) { unstackCalled := false cfg, outR, errR := config.NewTestConfig() + setTestRepo(cfg) cfg.GitHubClientOverride = &github.MockClient{ UnstackFn: func(int) (*github.RemoteStack, bool, error) { unstackCalled = true @@ -663,7 +691,7 @@ func TestUnstack_ByStackNumber_NotTracked_LocalFlag(t *testing.T) { assert.ErrorIs(t, err, ErrNotInStack) assert.False(t, unstackCalled, "--local must never contact GitHub") - assert.Contains(t, output, "stack #999 is not tracked locally") + assert.Contains(t, output, "stack #999 (https://github.com/o/r/stack/999) is not tracked locally") sf, err := stack.Load(gitDir) require.NoError(t, err) @@ -671,6 +699,7 @@ func TestUnstack_ByStackNumber_NotTracked_LocalFlag(t *testing.T) { } func TestUnstack_ByStackNumber_LocalFlag_LegacyStack_NoRemoteCall(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", "0") // --local must never contact GitHub. A legacy stack (Number == 0) can only // be matched by number via a remote backfill (ListStacks); under --local // that lookup must be skipped and the number reported as not tracked. @@ -694,6 +723,7 @@ func TestUnstack_ByStackNumber_LocalFlag_LegacyStack_NoRemoteCall(t *testing.T) listCalled := false unstackCalled := false cfg, outR, errR := config.NewTestConfig() + setTestRepo(cfg) cfg.GitHubClientOverride = &github.MockClient{ ListStacksFn: func() ([]github.RemoteStack, error) { listCalled = true @@ -711,7 +741,7 @@ func TestUnstack_ByStackNumber_LocalFlag_LegacyStack_NoRemoteCall(t *testing.T) assert.ErrorIs(t, err, ErrNotInStack) assert.False(t, listCalled, "--local must not contact GitHub (no ListStacks backfill)") assert.False(t, unstackCalled, "--local must not contact GitHub") - assert.Contains(t, output, "stack #7 is not tracked locally") + assert.Contains(t, output, "stack #7 (https://github.com/o/r/stack/7) is not tracked locally") // Local tracking is untouched. sf, loadErr := stack.Load(gitDir) diff --git a/cmd/utils.go b/cmd/utils.go index c1a8f1a8..4750b368 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -90,6 +90,13 @@ func stackLabel(number int) string { return fmt.Sprintf(" (stack #%d)", number) } +func linkedStackLabel(cfg *config.Config, number int) string { + if number <= 0 { + return "" + } + return fmt.Sprintf(" (stack %s)", cfg.StackLink(number)) +} + // stackNumberByID resolves an internal stack ID (as stored in the local stack // file) to its human-facing stack number by consulting the remote stack list. // Returns ok=false when no remote stack matches the ID (e.g. it was deleted). @@ -2161,7 +2168,7 @@ func unstackAndKeepLocal(cfg *config.Config, client github.ClientOps, sf *stack. if remote == nil { cfg.Warningf("Remote stack already removed") } else if prepareStackRecreation(s, remote) { - cfg.Infof("Merged pull requests remain stacked on GitHub%s", stackLabel(remote.Number)) + cfg.Infof("Merged pull requests remain stacked on GitHub%s", linkedStackLabel(cfg, remote.Number)) } else if remaining, dissolved, unstackErr := client.Unstack(remote.Number); unstackErr != nil { var httpErr *api.HTTPError if errors.As(unstackErr, &httpErr) && httpErr.StatusCode == 404 { @@ -2176,8 +2183,8 @@ func unstackAndKeepLocal(cfg *config.Config, client github.ClientOps, sf *stack. } else if dissolved { cfg.Successf("Remote stack removed%s", stackLabel(remote.Number)) } else { - cfg.Successf("Pull requests unstacked on GitHub%s", stackLabel(remote.Number)) - cfg.Infof("Merged pull requests remain stacked on GitHub%s", stackLabel(remote.Number)) + cfg.Successf("Pull requests unstacked on GitHub%s", linkedStackLabel(cfg, remote.Number)) + cfg.Infof("Merged pull requests remain stacked on GitHub%s", linkedStackLabel(cfg, remote.Number)) } s.ID = "" diff --git a/cmd/utils_test.go b/cmd/utils_test.go index 304e27f7..fb8abcd2 100644 --- a/cmd/utils_test.go +++ b/cmd/utils_test.go @@ -1706,6 +1706,20 @@ func setTestRepo(cfg *config.Config) { cfg.RepoOverride = &repository.Repository{Host: "github.com", Owner: "o", Name: "r"} } +func TestLinkedStackLabel(t *testing.T) { + cfg := &config.Config{} + setTestRepo(cfg) + for _, force := range []string{"0", "1"} { + t.Run(force, func(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", force) + assert.Empty(t, linkedStackLabel(cfg, 0)) + assert.Empty(t, linkedStackLabel(cfg, -1)) + assert.Equal(t, " (stack "+cfg.StackLink(7)+")", linkedStackLabel(cfg, 7)) + assert.Equal(t, " (stack #7)", stackLabel(7)) + }) + } +} + func TestWarnStacksUnavailable_ShowsNotEnabled(t *testing.T) { cfg, _, errR := config.NewTestConfig() diff --git a/cmd/view.go b/cmd/view.go index 27b2c4b1..548f0488 100644 --- a/cmd/view.go +++ b/cmd/view.go @@ -145,7 +145,7 @@ func viewShort(cfg *config.Config, s *stack.Stack, currentBranch string) error { } if s.Number > 0 { - cfg.Outf("%s\n", cfg.ColorBold(fmt.Sprintf("Stack #%d", s.Number))) + cfg.Outf("%s\n", cfg.ColorBold("Stack "+cfg.StackLink(s.Number))) } for i := len(s.Branches) - 1; i >= 0; i-- { @@ -313,7 +313,7 @@ func viewFullTUI(cfg *config.Config, s *stack.Stack, currentBranch string, prDet reversed[len(nodes)-1-i] = n } - model := stackview.New(reversed, s.Trunk, Version, s.Number) + model := stackview.New(reversed, s.Trunk, Version, s.Number).WithStackURL(cfg.StackURL(s.Number)) p := tea.NewProgram( model, @@ -359,7 +359,7 @@ func viewFullStatic(cfg *config.Config, s *stack.Stack, currentBranch string) er var buf bytes.Buffer if s.Number > 0 { - fmt.Fprintf(&buf, "%s\n\n", cfg.ColorBold(fmt.Sprintf("Stack #%d", s.Number))) + fmt.Fprintf(&buf, "%s\n\n", cfg.ColorBold("Stack "+cfg.StackLink(s.Number))) } for i := len(s.Branches) - 1; i >= 0; i-- { diff --git a/cmd/view_test.go b/cmd/view_test.go index d0cbd45d..d8410f8f 100644 --- a/cmd/view_test.go +++ b/cmd/view_test.go @@ -16,6 +16,29 @@ import ( "github.com/stretchr/testify/require" ) +func TestViewStackLinks(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", "1") + for _, render := range []struct { + name string + fn func(*config.Config, *stack.Stack, string) error + }{ + {"short", viewShort}, + {"static", viewFullStatic}, + } { + t.Run(render.name, func(t *testing.T) { + cfg, outR, errR := config.NewTestConfig() + cfg.NonInteractive = true + setTestRepo(cfg) + cfg.GitHubClientOverride = &github.MockClient{} + s := &stack.Stack{ID: "6154", Number: 7, Trunk: stack.BranchRef{Branch: "main"}} + require.NoError(t, render.fn(cfg, s, "main")) + output, _ := commandOutput(t, cfg, outR, errR) + assert.Contains(t, output, "Stack \x1b]8;;https://github.com/o/r/stack/7\x1b\\#7\x1b]8;;\x1b\\") + assert.NotContains(t, output, "/stack/6154") + }) + } +} + func TestTimeAgo(t *testing.T) { tests := []struct { name string @@ -41,6 +64,7 @@ func TestTimeAgo(t *testing.T) { } func TestViewJSON(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", "1") original := git.CurrentOps() t.Cleanup(func() { assert.Same(t, original, git.CurrentOps()) }) restore := git.SetOps(&git.MockOps{ @@ -123,12 +147,18 @@ func TestViewJSON(t *testing.T) { cfg, outR, _ := config.NewTestConfig() defer outR.Close() + tt.stack.Number = 7 err := viewJSON(cfg, tt.stack, tt.currentBranch) require.NoError(t, err) cfg.Out.Close() raw, err := io.ReadAll(outR) require.NoError(t, err) + var fields map[string]json.RawMessage + require.NoError(t, json.Unmarshal(raw, &fields)) + assert.Len(t, fields, 3) + assert.NotContains(t, string(raw), "\\u001b") + assert.NotContains(t, string(raw), "/stack/") var got viewJSONOutput err = json.Unmarshal(raw, &got) diff --git a/docs/src/content/docs/reference/cli.md b/docs/src/content/docs/reference/cli.md index 01fb1f93..51ba4c0e 100644 --- a/docs/src/content/docs/reference/cli.md +++ b/docs/src/content/docs/reference/cli.md @@ -174,9 +174,17 @@ gh stack view --short gh stack view --json ``` -`gh stack view --short` uses OSC 8 hyperlinks for PR numbers when the terminal -supports them. Otherwise, the full URL is shown for copy/paste. Set -`GH_STACK_HYPERLINKS=1` or `GH_STACK_HYPERLINKS=0` to override terminal detection. +Stack references follow the existing PR-link behavior in text output: OSC 8 +hyperlinks when supported, or the full URL for copy/paste otherwise. Stack links open +the repository's `/stack/` page; confirmed missing or removed stacks +are left unlinked. Set `GH_STACK_HYPERLINKS=1` or `GH_STACK_HYPERLINKS=0` to +override terminal detection. + +TUIs keep labels compact: click `#N` in `view` or `submit` to +open its page, or use your terminal's hyperlink gesture in the `checkout` +picker and `merge` wizard. Those inline views keep plain numbers when terminal +hyperlinks are unavailable. The environment override controls OSC 8 links, not +the application-handled clicks in `view` and `submit`. JSON output is unchanged. ### `gh stack checkout` @@ -733,7 +741,7 @@ gh stack feedback "Support for reordering branches" | Variable | Values | Description | |----------|--------|-------------| | `GH_STACK_THEME` | `auto` (default), `light`, `dark` | Controls the color palette of the interactive screens (`submit`, `modify`, `view`) and all colored command output. Colors adapt to your terminal background automatically; set this to force the light or dark palette when a terminal doesn't report its background (some SSH or `tmux` setups). | -| `GH_STACK_HYPERLINKS` | `0`, `1` | Disables or enables OSC 8 hyperlinks when terminal detection is incorrect. Unsupported terminals show the full URL by default. | +| `GH_STACK_HYPERLINKS` | `0`, `1` | Disables or enables OSC 8 hyperlinks for PR and stack references when terminal detection is incorrect. Text output falls back to full URLs; compact TUIs keep plain labels. Does not control application-handled clicks in `view` and `submit`. | | `GH_STACK_NO_UPDATE_NOTIFIER` | Any non-empty value | Disables gh-stack's background release checks and upgrade notices. For example, set to `1` in automation that requires quiet stderr. | ```sh diff --git a/go.mod b/go.mod index d2addc15..be1500d8 100644 --- a/go.mod +++ b/go.mod @@ -9,6 +9,7 @@ require ( github.com/charmbracelet/bubbletea v1.3.10 github.com/charmbracelet/glamour v0.10.0 github.com/charmbracelet/lipgloss v1.1.1-0.20250404203927-76690c660834 + github.com/charmbracelet/x/ansi v0.11.7 github.com/cli/cli/v2 v2.97.0 github.com/cli/go-gh/v2 v2.16.2 github.com/cli/shurcooL-graphql v0.0.4 @@ -28,7 +29,6 @@ require ( github.com/aymanbagabas/go-osc52/v2 v2.0.1 // indirect github.com/aymerick/douceur v0.2.0 // indirect github.com/charmbracelet/colorprofile v0.4.3 // indirect - github.com/charmbracelet/x/ansi v0.11.7 // indirect github.com/charmbracelet/x/cellbuf v0.0.15 // indirect github.com/charmbracelet/x/exp/slice v0.0.0-20250630141444-821143405392 // indirect github.com/charmbracelet/x/term v0.2.2 // indirect diff --git a/internal/config/config.go b/internal/config/config.go index 686e7eb5..c37eaba3 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -177,8 +177,12 @@ func (c *Config) Outf(format string, args ...any) { // PRLink formats a PR number as a clickable terminal hyperlink when supported, // or includes the full URL as a copyable fallback. func (c *Config) PRLink(number int, url string) string { + return c.Link(fmt.Sprintf("#%d", number), url) +} + +// Link formats a reference with a terminal hyperlink or a copyable URL fallback. +func (c *Config) Link(label, url string) string { hyperlinksEnabled := supportsHyperlinks(c.Terminal.IsTerminalOutput()) - label := fmt.Sprintf("#%d", number) if url != "" { if !hyperlinksEnabled { return fmt.Sprintf("%s (%s)", label, url) @@ -191,6 +195,37 @@ func (c *Config) PRLink(number int, url string) string { return label } +// TerminalLink keeps a compact label when terminal hyperlinks are unavailable. +// Use Link's raw underline codes; per-rune TUI underlining can corrupt OSC 8. +func (c *Config) TerminalLink(label, url string) string { + if url == "" || !supportsHyperlinks(c.Terminal.IsTerminalOutput()) { + return label + } + return c.Link(label, url) +} + +// StackURL returns no destination when optional display identity is unavailable. +// Formatting a reference must not turn an otherwise successful command into an error. +func (c *Config) StackURL(number int) string { + if number <= 0 { + return "" + } + repo, err := c.Repo() + if err != nil || repo.Owner == "" || repo.Name == "" { + return "" + } + return ghapi.StackURL(repo.Host, repo.Owner, repo.Name, number) +} + +// StackLink formats a stack number using the same conventions as PRLink. +func (c *Config) StackLink(number int) string { + label := fmt.Sprintf("#%d", number) + if url := c.StackURL(number); url != "" { + return c.Link(label, url) + } + return label +} + func (c *Config) IsInteractive() bool { return !c.NonInteractive && (c.ForceInteractive || c.Terminal.IsTerminalOutput()) } diff --git a/internal/config/hyperlink_test.go b/internal/config/hyperlink_test.go index a505a7a8..c8739825 100644 --- a/internal/config/hyperlink_test.go +++ b/internal/config/hyperlink_test.go @@ -3,6 +3,7 @@ package config import ( "testing" + "github.com/cli/go-gh/v2/pkg/repository" "github.com/stretchr/testify/assert" ) @@ -133,3 +134,65 @@ func TestPRLinkFormatting(t *testing.T) { }) } } + +func TestStackLinkFormatting(t *testing.T) { + tests := []struct { + name, force, host, owner, repo string + number int + want string + }{ + {"hyperlink", "1", "github.com", "o", "r", 7, "\x1b]8;;https://github.com/o/r/stack/7\x1b\\#7\x1b]8;;\x1b\\"}, + {"fallback", "0", "github.com", "o", "r", 7, "#7 (https://github.com/o/r/stack/7)"}, + {"custom host", "0", "github.example.com", "o", "r", 7, "#7 (https://github.example.com/o/r/stack/7)"}, + {"default host", "0", "", "o", "r", 7, "#7 (https://github.com/o/r/stack/7)"}, + {"missing owner", "1", "github.com", "", "r", 7, "#7"}, + {"missing repo", "1", "github.com", "o", "", 7, "#7"}, + {"unknown number", "1", "github.com", "o", "r", 0, "#0"}, + {"invalid number", "1", "github.com", "o", "r", -1, "#-1"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", tt.force) + cfg := &Config{RepoOverride: &repository.Repository{Host: tt.host, Owner: tt.owner, Name: tt.repo}} + assert.Equal(t, tt.want, cfg.StackLink(tt.number)) + if tt.owner == "" || tt.repo == "" || tt.number <= 0 { + assert.Empty(t, cfg.StackURL(tt.number)) + } + }) + } +} + +func TestStackURL_RepositoryUnavailable(t *testing.T) { + t.Chdir(t.TempDir()) + t.Setenv("GH_REPO", "") + t.Setenv("GH_STACK_HYPERLINKS", "1") + cfg := &Config{} + assert.Empty(t, cfg.StackURL(7)) + assert.Equal(t, "#7", cfg.StackLink(7)) +} + +func TestTerminalLinkColor(t *testing.T) { + t.Setenv("NO_COLOR", "") + t.Setenv("CLICOLOR_FORCE", "1") + t.Setenv("GH_STACK_HYPERLINKS", "1") + assert.Equal(t, + "\x1b[4m\x1b]8;;https://github.com/o/r/stack/7\x1b\\#7\x1b]8;;\x1b\\\x1b[24m", + New().TerminalLink("#7", "https://github.com/o/r/stack/7")) +} + +func TestTerminalLink(t *testing.T) { + const url = "https://github.com/o/r/stack/7" + tests := []struct { + name, force, url, want string + }{ + {"enabled", "1", url, "\x1b]8;;https://github.com/o/r/stack/7\x1b\\7\x1b]8;;\x1b\\"}, + {"disabled", "0", url, "7"}, + {"missing URL", "1", "", "7"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", tt.force) + assert.Equal(t, tt.want, (&Config{}).TerminalLink("7", tt.url)) + }) + } +} diff --git a/internal/github/github.go b/internal/github/github.go index 9da4954a..bbae6480 100644 --- a/internal/github/github.go +++ b/internal/github/github.go @@ -95,6 +95,14 @@ func PRURL(host, owner, repo string, number int) string { return fmt.Sprintf("https://%s/%s/%s/pull/%d", host, owner, repo, number) } +// StackURL constructs the web URL for a repository-scoped stack number. +func StackURL(host, owner, repo string, number int) string { + if host == "" { + host = "github.com" + } + return fmt.Sprintf("https://%s/%s/%s/stack/%d", host, owner, repo, number) +} + // FindPRForBranch finds an open PR by head branch name. func (c *Client) FindPRForBranch(branch string) (*PullRequest, error) { var query struct { diff --git a/internal/github/github_test.go b/internal/github/github_test.go index baeff335..32d84a47 100644 --- a/internal/github/github_test.go +++ b/internal/github/github_test.go @@ -35,6 +35,23 @@ func TestPRURL(t *testing.T) { } } +func TestStackURL(t *testing.T) { + tests := []struct { + name, host, owner, repo string + number int + want string + }{ + {"github.com", "github.com", "o", "r", 360, "https://github.com/o/r/stack/360"}, + {"custom host", "github.example.com", "org", "repo", 7, "https://github.example.com/org/repo/stack/7"}, + {"default host", "", "o", "r", 7, "https://github.com/o/r/stack/7"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, StackURL(tt.host, tt.owner, tt.repo, tt.number)) + }) + } +} + func TestPullRequest_IsQueued(t *testing.T) { t.Run("not queued when MergeQueueEntry is nil", func(t *testing.T) { pr := &PullRequest{Number: 1} diff --git a/internal/tui/checkoutview/model.go b/internal/tui/checkoutview/model.go index 23853dcf..7ba22824 100644 --- a/internal/tui/checkoutview/model.go +++ b/internal/tui/checkoutview/model.go @@ -58,8 +58,9 @@ var keys = keyMap{ // Model is the Bubble Tea model for the interactive checkout stack picker. type Model struct { - rows []StackRow // all reconciled rows (stable order) - filtered []StackRow // rows matching the active tab + search query + rows []StackRow // all reconciled rows (stable order) + filtered []StackRow // rows matching the active tab + search query + numberLinks map[int]string tab tab query string @@ -82,6 +83,12 @@ func New(rows []StackRow) Model { return m } +// WithNumberLinks adds compact presentation labels without changing row data. +func (m Model) WithNumberLinks(links map[int]string) Model { + m.numberLinks = links + return m +} + // Result returns the selected row and whether one was chosen (Enter). It is // false when the user cancelled or nothing was selectable. func (m Model) Result() (StackRow, bool) { @@ -511,10 +518,13 @@ func (m Model) renderRow(r StackRow, selected bool, lay layout) string { // #, Branches, Base, Status, Type, Created. A missing stack number is // rendered faint so it stays unobtrusive. numStyle := numberStyle + number := r.NumberDisplay() if r.Number == 0 { numStyle = dimStyle + } else if link := m.numberLinks[r.Number]; r.Number > 0 && link != "" { + number = link } - b.WriteString(renderCell(r.NumberDisplay(), numStyle, lay.num, selected)) + b.WriteString(renderCell(number, numStyle, lay.num, selected)) b.WriteString(sepCell(selected)) b.WriteString(renderSummaryCell(r, lay.summary, selected)) b.WriteString(sepCell(selected)) diff --git a/internal/tui/checkoutview/model_test.go b/internal/tui/checkoutview/model_test.go index e0962f72..f5bb57d3 100644 --- a/internal/tui/checkoutview/model_test.go +++ b/internal/tui/checkoutview/model_test.go @@ -2,22 +2,21 @@ package checkoutview import ( "fmt" - "regexp" "strings" "testing" "time" tea "github.com/charmbracelet/bubbletea" "github.com/charmbracelet/lipgloss" + "github.com/charmbracelet/x/ansi" + "github.com/muesli/termenv" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/github/gh-stack/internal/stack" ) -var ansiRe = regexp.MustCompile("\x1b\\[[0-9;]*m") - -func stripANSI(s string) string { return ansiRe.ReplaceAllString(s, "") } +func stripANSI(s string) string { return ansi.Strip(s) } func runeKey(s string) tea.KeyMsg { return tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune(s)} } @@ -47,6 +46,70 @@ func sized(m Model) Model { return drive(m, tea.WindowSizeMsg{Width: 100, Height: 20}) } +func testNumberLinks(rows []StackRow) map[int]string { + links := make(map[int]string) + for _, row := range rows { + if row.Number > 0 { + links[row.Number] = fmt.Sprintf("\x1b[4m\x1b]8;;https://github.com/o/r/stack/%d\x1b\\%d\x1b]8;;\x1b\\\x1b[24m", row.Number, row.Number) + } + } + return links +} + +func TestNumberLinksPreserveRows(t *testing.T) { + profile := lipgloss.DefaultRenderer().ColorProfile() + lipgloss.SetColorProfile(termenv.ANSI256) + defer lipgloss.SetColorProfile(profile) + + rows := sampleRows() + m := sized(New(rows).WithNumberLinks(testNumberLinks(rows))) + plain := sized(New(rows)) + assert.Equal(t, stripANSI(plain.View()), stripANSI(m.View())) + assert.Contains(t, m.View(), testNumberLinks(rows)[55]) + assert.NotContains(t, m.View(), "/stack/0") + + // URL text is presentation only, not a search term. + searchURL := drive(m, runeKey("/"), runeKey("https")) + assert.Empty(t, searchURL.filtered) + m = drive(m, runeKey("/"), runeKey("42")) + require.Len(t, m.filtered, 1) + assert.Contains(t, m.View(), testNumberLinks(rows)[42]) + assert.NotContains(t, m.View(), "/stack/55") + m = drive(m, tea.KeyMsg{Type: tea.KeyEnter}) + row, selected := m.Result() + require.True(t, selected) + assert.Equal(t, rows[2], row) +} + +func TestNumberLinksScrollingAndClipping(t *testing.T) { + profile := lipgloss.DefaultRenderer().ColorProfile() + lipgloss.SetColorProfile(termenv.ANSI256) + defer lipgloss.SetColorProfile(profile) + + rows := manyRows(20) + rows[0].BottomBranch = "界面" + links := testNumberLinks(rows) + m := New(rows).WithNumberLinks(links) + for _, width := range []int{1, 2, 3, 6, 12, 24, 40, 60, 100} { + t.Run(fmt.Sprint(width), func(t *testing.T) { + mm := drive(m, tea.WindowSizeMsg{Width: width, Height: 11}) + for range 15 { + mm = drive(mm, tea.KeyMsg{Type: tea.KeyDown}) + } + output := mm.View() + for _, line := range strings.Split(output, "\n") { + assert.LessOrEqual(t, lipgloss.Width(line), width) + assert.Equal(t, strings.Count(line, "\x1b]8;;https://"), strings.Count(line, "\x1b]8;;\x1b\\"), "each line closes its hyperlinks") + } + assert.NotContains(t, output, "/stack/1000", "scrolled-out rows have no links") + if width == 100 { + assert.Contains(t, output, links[1015]) + } + assert.NotContains(t, stripANSI(output), "https://") + }) + } +} + func TestNew_InitialState(t *testing.T) { m := New(sampleRows()) assert.Equal(t, tabAll, m.tab) diff --git a/internal/tui/checkoutview/styles.go b/internal/tui/checkoutview/styles.go index 5cc811a5..5b3bec20 100644 --- a/internal/tui/checkoutview/styles.go +++ b/internal/tui/checkoutview/styles.go @@ -4,6 +4,7 @@ import ( "strings" "github.com/charmbracelet/lipgloss" + "github.com/charmbracelet/x/ansi" "github.com/github/gh-stack/internal/tui/shared" ) @@ -161,27 +162,5 @@ func truncate(s string, width int) string { if lipgloss.Width(s) <= width { return s } - var b strings.Builder - w := 0 - inEscape := false - for _, r := range s { - if r == '\x1b' { - inEscape = true - } - if inEscape { - b.WriteRune(r) - if r == 'm' { - inEscape = false - } - continue - } - if w >= width-1 { - b.WriteString("…") - b.WriteString("\x1b[0m") - break - } - b.WriteRune(r) - w++ - } - return b.String() + return ansi.Truncate(s, width, "…") + "\x1b[0m" } diff --git a/internal/tui/mergeview/model_test.go b/internal/tui/mergeview/model_test.go index 1de8f32c..f34cbdc8 100644 --- a/internal/tui/mergeview/model_test.go +++ b/internal/tui/mergeview/model_test.go @@ -8,6 +8,8 @@ import ( tea "github.com/charmbracelet/bubbletea" "github.com/charmbracelet/lipgloss" + "github.com/charmbracelet/x/ansi" + "github.com/muesli/termenv" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -410,9 +412,18 @@ func TestOutcome_SHA(t *testing.T) { assert.Equal(t, "deadbeef", m.Outcome().SHA) } +const stackLink42 = "\x1b[4m\x1b]8;;https://github.example.com/o/r/stack/42\x1b\\#42\x1b]8;;\x1b\\\x1b[24m" + func TestView_RendersBannerAndSteps(t *testing.T) { - m := New(baseOptions()) + profile := lipgloss.DefaultRenderer().ColorProfile() + lipgloss.SetColorProfile(termenv.ANSI256) + defer lipgloss.SetColorProfile(profile) + opts := baseOptions() + opts.StackNumber, opts.StackLink = 42, stackLink42 + m := New(opts) sel := m.View() + assert.Contains(t, sel, stackLink42) + sel = ansi.Strip(sel) assert.Contains(t, sel, "Merge stack") assert.Contains(t, sel, "Select PRs") assert.Contains(t, sel, "Select Merge Method") @@ -423,13 +434,48 @@ func TestView_RendersBannerAndSteps(t *testing.T) { m = step(m, keyType(tea.KeyTab)) method := m.View() + assert.Contains(t, method, stackLink42) + method = ansi.Strip(method) assert.Contains(t, method, "Squash and merge") // method labels, no subheading assert.Contains(t, method, "↓↑/jk") m = step(m, keyType(tea.KeyTab)) confirm := m.View() + assert.Contains(t, confirm, stackLink42) + confirm = ansi.Strip(confirm) assert.Contains(t, confirm, "Merge 3 PRs") assert.Contains(t, confirm, "#1, #2, #3") + m.opts.StackNumber = 0 + assert.NotContains(t, m.View(), "/stack/", "unknown numbers do not get a link") +} + +func TestBanner_StackLinksClipping(t *testing.T) { + profile := lipgloss.DefaultRenderer().ColorProfile() + lipgloss.SetColorProfile(termenv.ANSI256) + defer lipgloss.SetColorProfile(profile) + + opts := baseOptions() + opts.StackNumber = 12345 + opts.StackLink = "\x1b[4m\x1b]8;;https://github.com/o/r/stack/12345\x1b\\#12345\x1b]8;;\x1b\\\x1b[24m" + opts.PRs[0].Title = "你好世界" + linked := New(opts) + opts.StackLink = "" + plain := New(opts) + for _, width := range []int{1, 2, 6, 12, 13, 14, 15, 24, 40, 60, 100} { + for _, phase := range []Step{StepSelectPRs, StepMethod, StepConfirm} { + t.Run(fmt.Sprintf("%d/%d", width, phase), func(t *testing.T) { + linked.width, plain.width = width, width + linked.step, plain.step = phase, phase + output := linked.View() + assert.Equal(t, ansi.Strip(plain.View()), ansi.Strip(output)) + for _, line := range strings.Split(output, "\n") { + assert.LessOrEqual(t, lipgloss.Width(line), width) + assert.Equal(t, strings.Count(line, "\x1b]8;;https://"), strings.Count(line, "\x1b]8;;\x1b\\")) + } + assert.NotContains(t, ansi.Strip(output), "https://") + }) + } + } } func TestBanner_IncludesStackNumber(t *testing.T) { @@ -440,11 +486,14 @@ func TestBanner_IncludesStackNumber(t *testing.T) { } func TestProgress_HidesHeader(t *testing.T) { - m := New(baseOptions()) + opts := baseOptions() + opts.StackNumber, opts.StackLink = 42, stackLink42 + m := New(opts) m.step = StepProgress v := m.View() assert.NotContains(t, v, "Merge stack", "header/wizard hidden during progress") assert.NotContains(t, v, "Select PRs") + assert.NotContains(t, v, stackLink42) assert.Contains(t, v, "Merging") assert.NotContains(t, v, "…", "no trailing ellipsis on the Merging line") } diff --git a/internal/tui/mergeview/types.go b/internal/tui/mergeview/types.go index 8ee0277f..780821ed 100644 --- a/internal/tui/mergeview/types.go +++ b/internal/tui/mergeview/types.go @@ -80,6 +80,8 @@ type Options struct { PRs []PRItem // StackNumber is the repo-scoped stack number, shown in the header. StackNumber int + // StackLink is the compact terminal link for the stack number, when available. + StackLink string // BaseRef is the branch the stack merges into (for display). BaseRef string // RepoSlug is owner/repo, for display. diff --git a/internal/tui/mergeview/view.go b/internal/tui/mergeview/view.go index 7ad76d59..2faccf06 100644 --- a/internal/tui/mergeview/view.go +++ b/internal/tui/mergeview/view.go @@ -6,6 +6,7 @@ import ( "strings" "github.com/charmbracelet/lipgloss" + "github.com/charmbracelet/x/ansi" "github.com/github/gh-stack/internal/theme" ) @@ -73,7 +74,11 @@ func (m Model) View() string { func (m Model) banner() string { title := "Merge stack" if m.opts.StackNumber > 0 { - title = fmt.Sprintf("Merge stack #%d", m.opts.StackNumber) + label := fmt.Sprintf("#%d", m.opts.StackNumber) + if m.opts.StackLink != "" { + label = m.opts.StackLink + } + title = "Merge stack " + label } return titleStyle.Render(title) + "\n" + m.stepper() + "\n\n" } @@ -419,28 +424,5 @@ func truncate(s string, width int) string { if lipgloss.Width(s) <= width { return s } - var b strings.Builder - w := 0 - inEscape := false - for _, r := range s { - if r == '\x1b' { - inEscape = true - } - if inEscape { - b.WriteRune(r) - if r == 'm' { - inEscape = false - } - continue - } - rw := lipgloss.Width(string(r)) - if w+rw > width-1 { - b.WriteString("…") - b.WriteString("\x1b[0m") - break - } - b.WriteRune(r) - w += rw - } - return b.String() + return ansi.Truncate(s, width, "…") + "\x1b[0m" } diff --git a/internal/tui/shared/header.go b/internal/tui/shared/header.go index b40e27af..fa36a50d 100644 --- a/internal/tui/shared/header.go +++ b/internal/tui/shared/header.go @@ -36,9 +36,13 @@ type ShortcutEntry struct { type HeaderInfoLine struct { Icon string Label string + LinkLabel string // optional clickable text within Label + URL string // native-click target for LinkLabel IconStyle *lipgloss.Style // optional override; nil uses default HeaderInfoStyle (cyan) } +const headerInfoStartRow = 3 + // headerLeftMargin is the left padding, in columns, before the logo and the // info lines (which share this left edge). It is kept small so it visually // matches the header's top and bottom padding. @@ -113,7 +117,7 @@ func shortcutRowCount(cfg HeaderConfig) int { // keeps the box exactly as tall as its content, with no trailing empty row. func headerContentRows(cfg HeaderConfig) int { // title (row 0), subtitle (row 1), a gap (row 2), then the info lines. - info := 3 + len(cfg.InfoLines) + info := headerInfoStartRow + len(cfg.InfoLines) sc := shortcutRowCount(cfg) if sc > info { return sc @@ -127,20 +131,12 @@ func HeaderHeightFor(cfg HeaderConfig) int { return headerContentRows(cfg) + 2 } -// RenderHeader renders the full-width header box. -// Progressive disclosure as width narrows: first hides the art, then the -// info text, keeping keyboard shortcuts always visible. -func RenderHeader(b *strings.Builder, cfg HeaderConfig, width, height int) { - if width < 2 { - return - } - innerWidth := width - 2 +type shortcutLine struct { + text string + visWidth int +} - // Always build shortcut lines - type shortcutLine struct { - text string - visWidth int - } +func headerShortcuts(cfg HeaderConfig) ([]shortcutLine, int) { var shortcuts []shortcutLine maxShortcutWidth := 0 rightColWidth := 0 @@ -225,11 +221,65 @@ func RenderHeader(b *strings.Builder, cfg HeaderConfig, width, height int) { rightColWidth = maxShortcutWidth + 2 } + return shortcuts, rightColWidth +} + +func headerInfoFits(width, rightColWidth int) bool { + const infoMinWidth = 20 + return width-2 >= rightColWidth+infoMinWidth+4 +} + +// HeaderLinkAt returns the URL of a visible info label at the screen coordinates. +func HeaderLinkAt(cfg HeaderConfig, width, height, x, y int) string { + if !ShouldShowHeader(width, height) { + return "" + } + _, rightColWidth := headerShortcuts(cfg) + if !headerInfoFits(width, rightColWidth) { + return "" + } + i := y - 1 - headerInfoStartRow + if i < 0 || i >= len(cfg.InfoLines) { + return "" + } + info := cfg.InfoLines[i] + prefix, _, found := strings.Cut(info.Label, info.LinkLabel) + if info.URL == "" || info.LinkLabel == "" || !found { + return "" + } + start := 1 + headerLeftMargin + lipgloss.Width(info.Icon) + 1 + lipgloss.Width(prefix) + end := start + lipgloss.Width(info.LinkLabel) + if x >= start && x < end && x < width-1 { + return info.URL + } + return "" +} + +func renderHeaderInfoLabel(info HeaderInfoLine) string { + prefix, suffix, found := strings.Cut(info.Label, info.LinkLabel) + if info.URL == "" || info.LinkLabel == "" || !found { + return HeaderInfoLabelStyle.Render(" " + info.Label) + } + return HeaderInfoLabelStyle.Render(" "+prefix) + + HeaderInfoLabelStyle.Underline(true).Render(info.LinkLabel) + + HeaderInfoLabelStyle.Render(suffix) +} + +// RenderHeader renders the full-width header box. +// Progressive disclosure as width narrows: first hides the art, then the +// info text, keeping keyboard shortcuts always visible. +func RenderHeader(b *strings.Builder, cfg HeaderConfig, width, height int) { + if width < 2 { + return + } + innerWidth := width - 2 + shortcuts, rightColWidth := headerShortcuts(cfg) + // Determine what fits: shortcuts always shown, the logo and info are // progressive. The logo is image-or-nothing: it shows only when an // inline-image protocol is available and the viewport is wide enough. showArt := cfg.ShowArt && LogoAvailable() - showInfo := true + showInfo := headerInfoFits(width, rightColWidth) // Hide the logo when the viewport is too narrow or too short. The height // guard drops the logo a little before the rest of the header because a @@ -251,12 +301,6 @@ func RenderHeader(b *strings.Builder, cfg HeaderConfig, width, height int) { cr := headerContentRows(cfg) - // If info + shortcuts don't fit, hide info - infoMinWidth := 20 // rough minimum for title/info text - if innerWidth < rightColWidth+infoMinWidth+4 { - showInfo = false - } - // Map info lines to row indices infoByRow := make(map[int]string) if showInfo { @@ -265,7 +309,7 @@ func RenderHeader(b *strings.Builder, cfg HeaderConfig, width, height int) { infoByRow[1] = HeaderInfoLabelStyle.Render(cfg.Subtitle) } for i, info := range cfg.InfoLines { - row := 3 + i + row := headerInfoStartRow + i if row > cr-1 { break } @@ -273,7 +317,7 @@ func RenderHeader(b *strings.Builder, cfg HeaderConfig, width, height int) { if info.IconStyle != nil { iconStyle = *info.IconStyle } - infoByRow[row] = iconStyle.Render(info.Icon) + HeaderInfoLabelStyle.Render(" "+info.Label) + infoByRow[row] = iconStyle.Render(info.Icon) + renderHeaderInfoLabel(info) } } diff --git a/internal/tui/shared/header_test.go b/internal/tui/shared/header_test.go index 7fcc873d..f851eca3 100644 --- a/internal/tui/shared/header_test.go +++ b/internal/tui/shared/header_test.go @@ -1,8 +1,10 @@ package shared import ( + "strings" "testing" + "github.com/charmbracelet/lipgloss" "github.com/stretchr/testify/assert" ) @@ -39,3 +41,56 @@ func TestLogoHidesBeforeHeader(t *testing.T) { assert.False(t, artFitsViewport(MinWidthForArt, h), "logo should be hidden at height %d", h) } } + +func TestHeaderLinkAt(t *testing.T) { + const url = "https://github.com/o/r/stack/7" + cfg := HeaderConfig{ + Title: "View Stack", + InfoLines: []HeaderInfoLine{ + {Icon: "◆", Label: "Stack #7 | o/r", LinkLabel: "#7", URL: url}, + {Icon: "○", Label: "Base: main"}, + }, + Shortcuts: []ShortcutEntry{{Key: "q", Desc: "quit"}}, + } + tests := []struct { + name string + width, height, x, y int + want string + }{ + {"first cell", 100, 40, 10, 4, url}, + {"last cell", 100, 40, 11, 4, url}, + {"icon", 100, 40, 2, 4, ""}, + {"padding", 100, 40, 3, 4, ""}, + {"word", 100, 40, 4, 4, ""}, + {"space", 100, 40, 9, 4, ""}, + {"suffix", 100, 40, 12, 4, ""}, + {"other info", 100, 40, 4, 5, ""}, + {"title", 100, 40, 4, 1, ""}, + {"short", 100, MinHeightForHeader - 1, 10, 4, ""}, + {"narrow", MinWidthForHeader - 1, 40, 10, 4, ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, HeaderLinkAt(cfg, tt.width, tt.height, tt.x, tt.y)) + }) + } + + cfg.Shortcuts = []ShortcutEntry{{Key: "q", Desc: strings.Repeat("x", 35)}} + assert.True(t, ShouldShowHeader(60, 40)) + assert.Empty(t, HeaderLinkAt(cfg, 60, 40, 10, 4), "info can be hidden even when the header is visible") + var b strings.Builder + RenderHeader(&b, cfg, 60, 40) + assert.NotContains(t, b.String(), "Stack #7") + + cfg.Shortcuts = nil + cfg.InfoLines[0].URL = "" + assert.Empty(t, HeaderLinkAt(cfg, 100, 40, 10, 4)) +} + +func TestHeaderLinkPreservesLayout(t *testing.T) { + plain := HeaderInfoLine{Icon: "◆", Label: "Stack #7 | o/r", LinkLabel: "#7"} + linked := plain + linked.URL = "https://github.com/o/r/stack/7" + assert.Equal(t, lipgloss.Width(renderHeaderInfoLabel(plain)), lipgloss.Width(renderHeaderInfoLabel(linked))) + assert.NotContains(t, renderHeaderInfoLabel(linked), linked.URL, "native links stay compact") +} diff --git a/internal/tui/stackview/model.go b/internal/tui/stackview/model.go index 0c442bcd..deee92ff 100644 --- a/internal/tui/stackview/model.go +++ b/internal/tui/stackview/model.go @@ -67,6 +67,8 @@ type Model struct { trunk stack.BranchRef version string stackNumber int + stackURL string + openURL func(string) cursor int // index into nodes (displayed top-down, so 0 = top of stack) help help.Model width int @@ -114,6 +116,20 @@ func New(nodes []BranchNode, trunk stack.BranchRef, version string, stackNumber } } +// WithStackURL attaches the optional web destination to the stack header. +func (m Model) WithStackURL(url string) Model { + m.stackURL = url + return m +} + +func (m Model) openBrowser(url string) { + if m.openURL != nil { + m.openURL(url) + return + } + shared.OpenBrowserInBackground(url) +} + // CheckoutBranch returns the branch to checkout after the TUI exits, if any. func (m Model) CheckoutBranch() string { return m.checkoutBranch @@ -164,7 +180,7 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { if m.cursor >= 0 && m.cursor < len(m.nodes) { node := m.nodes[m.cursor] if node.PR != nil && node.PR.URL != "" { - shared.OpenBrowserInBackground(node.PR.URL) + m.openBrowser(node.PR.URL) } } return m, nil @@ -223,6 +239,11 @@ func toBranchNodeData(node BranchNode) shared.BranchNodeData { // handleMouseClick processes a mouse click at the given screen position. func (m Model) handleMouseClick(screenX, screenY int) (tea.Model, tea.Cmd) { + if url := shared.HeaderLinkAt(m.buildHeaderConfig(), m.width, m.height, screenX, screenY); url != "" { + m.openBrowser(url) + return m, nil + } + nodes := make([]shared.BranchNodeData, len(m.nodes)) for i, n := range m.nodes { nodes[i] = toBranchNodeData(n) @@ -241,7 +262,7 @@ func (m Model) handleMouseClick(screenX, screenY int) (tea.Model, tea.Cmd) { m.cursor = result.NodeIndex if result.OpenURL != "" { - shared.OpenBrowserInBackground(result.OpenURL) + m.openBrowser(result.OpenURL) } if result.ToggleFiles { m.nodes[result.NodeIndex].FilesExpanded = !m.nodes[result.NodeIndex].FilesExpanded @@ -448,7 +469,10 @@ func (m Model) buildHeaderConfig() shared.HeaderConfig { infoLines := make([]shared.HeaderInfoLine, 0, 3) if m.stackNumber > 0 { - infoLines = append(infoLines, shared.HeaderInfoLine{Icon: "◆", Label: fmt.Sprintf("Stack #%d", m.stackNumber)}) + infoLines = append(infoLines, shared.HeaderInfoLine{ + Icon: "◆", Label: fmt.Sprintf("Stack #%d", m.stackNumber), + LinkLabel: fmt.Sprintf("#%d", m.stackNumber), URL: m.stackURL, + }) } else { infoLines = append(infoLines, shared.HeaderInfoLine{Icon: "✓", Label: "Stack initialized"}) } diff --git a/internal/tui/stackview/model_test.go b/internal/tui/stackview/model_test.go index 8901172a..b95f60ab 100644 --- a/internal/tui/stackview/model_test.go +++ b/internal/tui/stackview/model_test.go @@ -42,6 +42,42 @@ func keyMsg(k string) tea.KeyMsg { var testTrunk = stack.BranchRef{Branch: "main"} +func TestStackHeaderLink(t *testing.T) { + const url = "https://github.example.com/o/r/stack/7" + for _, merged := range []bool{false, true} { + t.Run(fmt.Sprint(merged), func(t *testing.T) { + nodes := makeNodes("feature") + nodes[0].Ref.PullRequest = &stack.PullRequestRef{Number: 42, Merged: merged} + m := New(nodes, testTrunk, "0.0.1", 7).WithStackURL(url) + m.width, m.height = 100, 40 + beforeCursor := m.cursor + var opened []string + m.openURL = func(url string) { opened = append(opened, url) } + for _, x := range []int{4, 9, 12} { + _, _ = m.Update(tea.MouseMsg{Action: tea.MouseActionPress, Button: tea.MouseButtonLeft, X: x, Y: 4}) + } + assert.Empty(t, opened, "words, spaces, and padding are not links") + updated, cmd := m.Update(tea.MouseMsg{Action: tea.MouseActionPress, Button: tea.MouseButtonLeft, X: 10, Y: 4}) + result := updated.(Model) + assert.Equal(t, []string{url}, opened) + assert.Equal(t, beforeCursor, result.cursor) + assert.Empty(t, result.CheckoutBranch()) + assert.Nil(t, cmd) + }) + } +} + +func TestOpenPRKeyWithStackLink(t *testing.T) { + nodes := makeNodes("feature") + nodes[0].PR = &ghapi.PRDetails{Number: 42, URL: "https://github.com/o/r/pull/42"} + m := New(nodes, testTrunk, "0.0.1", 7).WithStackURL("https://github.com/o/r/stack/7") + var opened string + m.openURL = func(url string) { opened = url } + _, cmd := m.Update(keyMsg("o")) + assert.Equal(t, nodes[0].PR.URL, opened) + assert.Nil(t, cmd) +} + func TestNew_CursorAtCurrentBranch(t *testing.T) { nodes := makeNodes("b1", "b2", "b3") nodes[1].IsCurrent = true @@ -495,5 +531,6 @@ func TestBuildHeaderConfig_ShowsStackNumber(t *testing.T) { m0 := New(makeNodes("b1", "b2"), testTrunk, "0.0.1", 0) for _, line := range m0.buildHeaderConfig().InfoLines { assert.NotContains(t, line.Label, "Stack #") + assert.Empty(t, line.URL) } } diff --git a/internal/tui/submitview/model.go b/internal/tui/submitview/model.go index b1264fc7..97739eb2 100644 --- a/internal/tui/submitview/model.go +++ b/internal/tui/submitview/model.go @@ -53,6 +53,7 @@ type Options struct { // StackNumber is the human-facing stack number shown in the header. Zero // when the local stack has not yet been created on GitHub. StackNumber int + StackURL string } // Model is the Bubble Tea model backing the interactive `gh stack submit` TUI. @@ -69,6 +70,7 @@ type Model struct { // stackNumber is the human-facing stack number shown in the header (0 when // the stack has not been created on GitHub yet). stackNumber int + stackURL string cursor int // index into nodes (the focused branch) @@ -105,7 +107,7 @@ type Model struct { cancelled bool // openURL, when non-nil, is called instead of launching the system browser - // to open an existing PR. Tests inject a no-op so the suite never spawns a + // to open a PR or stack. Tests inject a no-op so the suite never spawns a // real browser. openURL func(string) @@ -157,6 +159,7 @@ func New(opts Options) Model { canCreateStack: opts.CanCreateStack, stackNumber: opts.StackNumber, + stackURL: opts.StackURL, titleArea: tia, descArea: ta, diff --git a/internal/tui/submitview/model_test.go b/internal/tui/submitview/model_test.go index 11c86309..8070f630 100644 --- a/internal/tui/submitview/model_test.go +++ b/internal/tui/submitview/model_test.go @@ -4,6 +4,7 @@ import ( "testing" tea "github.com/charmbracelet/bubbletea" + "github.com/charmbracelet/lipgloss" ghapi "github.com/github/gh-stack/internal/github" "github.com/github/gh-stack/internal/stack" "github.com/github/gh-stack/internal/tui/stackview" @@ -329,8 +330,12 @@ func TestHeaderConfig_StackNumberInRepoLine(t *testing.T) { RepoLabel: "myorg/myrepo", Version: "1.0.0", StackNumber: 7, + StackURL: "https://github.com/myorg/myrepo/stack/7", }) - assert.Equal(t, "Stack #7 • myorg/myrepo", m.buildHeaderConfig().InfoLines[0].Label) + info := m.buildHeaderConfig().InfoLines[0] + assert.Equal(t, "Stack #7 • myorg/myrepo", info.Label) + assert.Equal(t, "#7", info.LinkLabel) + assert.Equal(t, "https://github.com/myorg/myrepo/stack/7", info.URL) // Without a stack number, the first line is just the repo. assert.Equal(t, "Repo: myorg/myrepo", testModel(t, newNodes()).buildHeaderConfig().InfoLines[0].Label) @@ -375,6 +380,82 @@ func TestView_EmptyWhenNoSize(t *testing.T) { // --- mouse --- +func TestLeftPanel_StackLinkClipping(t *testing.T) { + m := testModel(t, newNodes()) + m.width = 70 + m.stackNumber = int(^uint(0) >> 1) + m.stackURL = "https://github.com/o/r/stack/7" + leftW, _ := m.panelWidths() + header := m.buildLeftRows(leftW - 2)[0].text + assert.LessOrEqual(t, lipgloss.Width(header), leftW-2) + assert.NotContains(t, header, m.stackURL) + last := 1 + lipgloss.Width(truncateVisible(m.stackHeading(), leftW-3)) + assert.True(t, m.leftStackLinkHit(last, m.panelTopRow())) + assert.False(t, m.leftStackLinkHit(last+1, m.panelTopRow())) + assert.False(t, m.leftStackLinkHit(leftW-1, m.panelTopRow())) + m.height, m.leftScroll = 26, 1 + assert.False(t, m.leftStackLinkHit(8, m.panelTopRow()), "scrolled-out headings are not clickable") + m.leftScroll, m.stackURL = 0, "" + assert.False(t, m.leftStackLinkHit(8, m.panelTopRow())) + m.stackNumber, m.stackURL = 0, "https://github.com/o/r/stack/7" + assert.False(t, m.leftStackLinkHit(8, m.panelTopRow())) +} + +func TestSidebarLinkAfterResizeWithClosedPR(t *testing.T) { + m := testModel(t, append(newNodes(), newNode("closed", StateClosed))) + m.stackNumber, m.stackURL = 7, "https://github.com/o/r/stack/7" + updated, _ := m.Update(tea.WindowSizeMsg{Width: 100, Height: 26}) + m = updated.(Model) + m.scrollLeft(1000) + require.Positive(t, m.leftScroll) + + leftW, _ := m.panelWidths() + rows := m.buildLeftRows(leftW - 2) + // The banner leaves just the heading scrolled out after this resize. + updated, _ = m.Update(tea.WindowSizeMsg{Width: 100, Height: len(rows) + m.headerHeight() + 3}) + m = updated.(Model) + assert.NotContains(t, stripANSI(m.View()), "STACK #7") + var opened []string + m.openURL = func(url string) { opened = append(opened, url) } + _, _ = m.Update(tea.MouseMsg{Action: tea.MouseActionPress, Button: tea.MouseButtonLeft, X: 8, Y: m.panelTopRow()}) + assert.Empty(t, opened, "the first visible sidebar row is blank, not a link") +} + +func TestMouse_StackLinks(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", "0") + const url = "https://github.example.com/o/r/stack/7" + m := testModel(t, newNodes()) + m.stackNumber, m.stackURL = 7, url + m.titleArea.SetValue("unsaved title") + m.descArea.SetValue("unsaved description") + beforeCursor, beforeField := m.cursor, m.focusedField + var opened []string + m.openURL = func(url string) { opened = append(opened, url) } + click := func(x, y int) { + updated, cmd := m.Update(tea.MouseMsg{Action: tea.MouseActionPress, Button: tea.MouseButtonLeft, X: x, Y: y}) + m = updated.(Model) + assert.Nil(t, cmd) + } + for _, point := range [][2]int{{4, 4}, {9, 4}, {12, 4}, {2, m.panelTopRow()}, {7, m.panelTopRow()}, {10, m.panelTopRow()}} { + click(point[0], point[1]) + } + assert.Empty(t, opened, "words, spaces, and adjacent text are not links") + click(10, 4) + click(8, m.panelTopRow()) + assert.Equal(t, []string{url, url}, opened) + assert.Equal(t, beforeCursor, m.cursor) + assert.Equal(t, beforeField, m.focusedField) + assert.Equal(t, "unsaved title", m.titleArea.Value()) + assert.Equal(t, "unsaved description", m.descArea.Value()) + assert.False(t, m.SubmitRequested()) + assert.False(t, m.Cancelled()) + m.showHelp = true + click(10, 4) + m.confirmingQuit = true + click(8, m.panelTopRow()) + assert.Len(t, opened, 2, "overlays block the underlying links") +} + func TestMouse_ClickBranchFocuses(t *testing.T) { m := testModel(t, newNodes()) y := m.panelTopRow() + 2 + 1 // branch index 1 diff --git a/internal/tui/submitview/mouse.go b/internal/tui/submitview/mouse.go index 924a762d..cadf76be 100644 --- a/internal/tui/submitview/mouse.go +++ b/internal/tui/submitview/mouse.go @@ -3,6 +3,7 @@ package submitview import ( tea "github.com/charmbracelet/bubbletea" "github.com/charmbracelet/lipgloss" + "github.com/github/gh-stack/internal/tui/shared" ) func (m Model) handleMouse(msg tea.MouseMsg) (tea.Model, tea.Cmd) { @@ -156,12 +157,34 @@ func (m Model) leftStackButtonHit(x, y int) bool { return x >= 1 && x < 1+lipgloss.Width(m.renderStackButton(leftW-2)) } +func (m Model) leftStackLinkHit(x, y int) bool { + if m.stackNumber <= 0 || m.stackURL == "" || y != m.panelTopRow() { + return false + } + leftW, _ := m.panelWidths() + rows := m.buildLeftRows(leftW - 2) + if clampScroll(m.leftScroll, len(rows), m.leftVisibleHeight()) != 0 { + return false + } + labelWidth := lipgloss.Width(truncateVisible(m.stackHeading(), leftW-3)) + return x >= 2+lipgloss.Width(stackHeadingPrefix) && x < 2+labelWidth && x < leftW-1 +} + // handleClick routes a left click to a branch row (left map) or an editor // element (right panel). func (m Model) handleClick(x, y int) (tea.Model, tea.Cmd) { m.statusMessage = "" m.statusIsError = false + if url := shared.HeaderLinkAt(m.buildHeaderConfig(), m.width, m.height, x, y); url != "" { + m.openBrowser(url) + return m, nil + } + if m.leftStackLinkHit(x, y) { + m.openBrowser(m.stackURL) + return m, nil + } + leftW, rightW := m.panelWidths() // Bottom-left STACK button: link the existing open PRs into a stack. diff --git a/internal/tui/submitview/render.go b/internal/tui/submitview/render.go index a3107e2d..9ac0defd 100644 --- a/internal/tui/submitview/render.go +++ b/internal/tui/submitview/render.go @@ -44,7 +44,10 @@ func (m Model) buildHeaderConfig() shared.HeaderConfig { infoLines := make([]shared.HeaderInfoLine, 0, 3) if m.stackNumber > 0 { - infoLines = append(infoLines, shared.HeaderInfoLine{Icon: "◆", Label: fmt.Sprintf("Stack #%d • %s", m.stackNumber, repo)}) + infoLines = append(infoLines, shared.HeaderInfoLine{ + Icon: "◆", Label: fmt.Sprintf("Stack #%d • %s", m.stackNumber, repo), + LinkLabel: fmt.Sprintf("#%d", m.stackNumber), URL: m.stackURL, + }) } else { infoLines = append(infoLines, shared.HeaderInfoLine{Icon: "◆", Label: "Repo: " + repo}) } diff --git a/internal/tui/submitview/screen.go b/internal/tui/submitview/screen.go index 6f2e88da..93eaec43 100644 --- a/internal/tui/submitview/screen.go +++ b/internal/tui/submitview/screen.go @@ -340,6 +340,10 @@ func (m *Model) openFocusedPR() { if url == "" { return } + m.openBrowser(url) +} + +func (m Model) openBrowser(url string) { if m.openURL != nil { m.openURL(url) return @@ -759,18 +763,28 @@ func leftPanelBox(content string, width, height int) string { Render(content) } +const stackHeadingPrefix = "STACK " + +func (m Model) stackHeading() string { + if m.stackNumber > 0 { + return stackHeadingPrefix + fmt.Sprintf("#%d", m.stackNumber) + } + return "STACK" +} + // buildLeftRows lays out the whole left panel: the STACK header, each branch's // node/name/meta lines, a spine gap between branches (shaded as padding around // the focused branch), and the trunk. It is deterministic given the // nodes/cursor/width, so the mouse layer can recompute it to resolve clicks. func (m Model) buildLeftRows(fullW int) []leftRow { cur := m.cursor - stackLabel := "STACK" - if m.stackNumber > 0 { - stackLabel = fmt.Sprintf("STACK #%d", m.stackNumber) + heading := truncateVisible(m.stackHeading(), fullW-1) + renderedHeading := sectionLabelStyle.Render(heading) + if number, ok := strings.CutPrefix(heading, stackHeadingPrefix); ok && m.stackURL != "" { + renderedHeading = sectionLabelStyle.Render(stackHeadingPrefix) + sectionLabelStyle.Underline(true).Render(number) } rows := []leftRow{ - {text: pad(1, false) + sectionLabelStyle.Render(stackLabel), branch: -1}, + {text: pad(1, false) + renderedHeading, branch: -1}, {text: m.gapRow(fullW, false, cur == 0), branch: -1}, // blank under STACK; top pad for branch 0 } for i := range m.nodes { @@ -928,6 +942,9 @@ func pad(n int, focused bool) string { // leftVisibleHeight is the number of timeline rows the left panel can show. func (m Model) leftVisibleHeight() int { h := m.contentHeight() - 2 - m.leftButtonRows() // panel border + reserved button rows + if m.renderClosedBanner() != "" { + h-- + } if h < 1 { h = 1 }