From 93609e002af6fe72a1690d93a8a30f734db710f8 Mon Sep 17 00:00:00 2001 From: Sameen Karim Date: Thu, 22 Oct 2026 12:57:18 -0400 Subject: [PATCH 01/11] Add shared stack URL and hyperlink helpers --- cmd/utils.go | 7 +++ cmd/utils_test.go | 14 +++++ internal/config/config.go | 43 ++++++++++++++- internal/config/hyperlink_test.go | 54 +++++++++++++++++++ internal/github/github.go | 8 +++ internal/github/github_test.go | 17 ++++++ internal/tui/shared/header.go | 85 ++++++++++++++++++++++-------- internal/tui/shared/header_test.go | 54 +++++++++++++++++++ 8 files changed, 257 insertions(+), 25 deletions(-) diff --git a/cmd/utils.go b/cmd/utils.go index c1a8f1a8..61d00445 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). 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/internal/config/config.go b/internal/config/config.go index 686e7eb5..8aa43590 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -177,13 +177,17 @@ 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) } - label = fmt.Sprintf("\033]8;;%s\033\\%s\033]8;;\033\\", url, label) + label = hyperlink(label, url) } if c.Terminal.IsColorEnabled() { label = fmt.Sprintf("\033[4m%s\033[24m", label) @@ -191,6 +195,41 @@ func (c *Config) PRLink(number int, url string) string { return label } +// TerminalLink keeps a compact label when terminal hyperlinks are unavailable. +// TUI callers supply their own styling. +func (c *Config) TerminalLink(label, url string) string { + if url == "" || !supportsHyperlinks(c.Terminal.IsTerminalOutput()) { + return label + } + return hyperlink(label, url) +} + +func hyperlink(label, url string) string { + return fmt.Sprintf("\033]8;;%s\033\\%s\033]8;;\033\\", url, label) +} + +// 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..994b6563 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,56 @@ 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 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/shared/header.go b/internal/tui/shared/header.go index b40e27af..459ae935 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 + Suffix string // optional text after the label, outside its link + URL string // optional native-click target for the label 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,60 @@ 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] + start := 1 + headerLeftMargin + lipgloss.Width(info.Icon) + 1 + end := start + lipgloss.Width(info.Label) + if x >= start && x < end && x < width-1 { + return info.URL + } + return "" +} + +func renderHeaderInfoLabel(info HeaderInfoLine) string { + if info.URL == "" { + return HeaderInfoLabelStyle.Render(" " + info.Label + info.Suffix) + } + return HeaderInfoLabelStyle.Render(" ") + + HeaderInfoLabelStyle.Underline(true).Render(info.Label) + + HeaderInfoLabelStyle.Render(info.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 +296,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 +304,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 +312,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..9b631c5c 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" ) @@ -38,4 +40,56 @@ func TestLogoHidesBeforeHeader(t *testing.T) { assert.True(t, ShouldShowHeader(MinWidthForArt, h), "header should show at height %d", h) 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", Suffix: " | o/r", 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, 4, 4, url}, + {"last cell", 100, 40, 11, 4, url}, + {"icon", 100, 40, 2, 4, ""}, + {"padding", 100, 40, 3, 4, ""}, + {"suffix", 100, 40, 12, 4, ""}, + {"other info", 100, 40, 4, 5, ""}, + {"title", 100, 40, 4, 1, ""}, + {"short", 100, MinHeightForHeader - 1, 4, 4, ""}, + {"narrow", MinWidthForHeader - 1, 40, 4, 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, 4, 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, 4, 4)) +} + +func TestHeaderLinkPreservesLayout(t *testing.T) { + plain := HeaderInfoLine{Icon: "◆", Label: "Stack #7", Suffix: " | o/r"} + 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") } From 612101dbe851be8721026bc6ae021293ccab551d Mon Sep 17 00:00:00 2001 From: Sameen Karim Date: Thu, 22 Oct 2026 12:59:36 -0400 Subject: [PATCH 02/11] Link stack references in view output and TUI --- cmd/view.go | 6 +- cmd/view_test.go | 85 ++++++++++++++++++++++++++++ internal/tui/stackview/model.go | 27 ++++++++- internal/tui/stackview/model_test.go | 56 ++++++++++++++++++ 4 files changed, 168 insertions(+), 6 deletions(-) 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..198c0552 100644 --- a/cmd/view_test.go +++ b/cmd/view_test.go @@ -16,6 +16,91 @@ import ( "github.com/stretchr/testify/require" ) +func TestViewStackLinks(t *testing.T) { + restore := git.SetOps(&git.MockOps{ + IsAncestorFn: func(string, string) (bool, error) { return true, nil }, + }) + defer restore() + + for _, render := range []struct { + name string + fn func(*config.Config, *stack.Stack, string) error + }{ + {"short", viewShort}, + {"static", viewFullStatic}, + } { + for _, tt := range []struct { + name, force string + number int + want string + }{ + {"hyperlink", "1", 7, "Stack \x1b]8;;https://github.com/o/r/stack/7\x1b\\#7\x1b]8;;\x1b\\"}, + {"fallback", "0", 7, "Stack #7 (https://github.com/o/r/stack/7)"}, + {"unknown", "1", 0, ""}, + } { + t.Run(render.name+"/"+tt.name, func(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", tt.force) + cfg, outR, errR := config.NewTestConfig() + defer outR.Close() + defer errR.Close() + defer cfg.Err.Close() + defer cfg.Out.Close() + cfg.NonInteractive = true + setTestRepo(cfg) + cfg.GitHubClientOverride = &github.MockClient{} + s := &stack.Stack{ + ID: "6154", Number: tt.number, + Trunk: stack.BranchRef{Branch: "main"}, + Branches: []stack.BranchRef{{ + Branch: "feature", + PullRequest: &stack.PullRequestRef{ + Number: 42, URL: "https://github.com/o/r/pull/42", + }, + }}, + } + require.NoError(t, render.fn(cfg, s, "feature")) + require.NoError(t, cfg.Out.Close()) + raw, err := io.ReadAll(outR) + require.NoError(t, err) + if tt.want == "" { + assert.NotContains(t, string(raw), "Stack #") + assert.NotContains(t, string(raw), "/stack/") + } else { + assert.Contains(t, string(raw), tt.want) + } + assert.NotContains(t, string(raw), "/stack/6154") + assert.NotContains(t, string(raw), "/stack/42") + if tt.force == "0" { + assert.NotContains(t, string(raw), "\x1b]8") + } + }) + } + } +} + +func TestViewJSON_StackLinksDoNotChangeOutput(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", "1") + cfg, outR, errR := config.NewTestConfig() + defer cfg.Out.Close() + defer cfg.Err.Close() + defer outR.Close() + defer errR.Close() + setTestRepo(cfg) + s := &stack.Stack{Number: 7, Trunk: stack.BranchRef{Branch: "main"}} + require.NoError(t, viewJSON(cfg, s, "main")) + require.NoError(t, cfg.Out.Close()) + raw, err := io.ReadAll(outR) + require.NoError(t, err) + var out map[string]json.RawMessage + require.NoError(t, json.Unmarshal(raw, &out)) + assert.Len(t, out, 3) + assert.Contains(t, out, "trunk") + assert.Contains(t, out, "currentBranch") + assert.Contains(t, out, "branches") + assert.NotContains(t, string(raw), "\x1b") + assert.NotContains(t, string(raw), "/stack/") +} + func TestTimeAgo(t *testing.T) { tests := []struct { name string diff --git a/internal/tui/stackview/model.go b/internal/tui/stackview/model.go index 0c442bcd..cc20b2b9 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,7 @@ 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), 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..687660e5 100644 --- a/internal/tui/stackview/model_test.go +++ b/internal/tui/stackview/model_test.go @@ -42,6 +42,62 @@ 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 _, tt := range []struct { + name string + number int + width, height, x, y int + merged, noURL, wantHit bool + }{ + {name: "identifier", number: 7, width: 100, height: 40, x: 4, y: 4, wantHit: true}, + {name: "all merged", number: 7, width: 100, height: 40, x: 11, y: 4, merged: true, wantHit: true}, + {name: "unknown stack", width: 100, height: 40, x: 4, y: 4}, + {name: "unknown URL", number: 7, width: 100, height: 40, x: 4, y: 4, noURL: true}, + {name: "icon", number: 7, width: 100, height: 40, x: 2, y: 4}, + {name: "after label", number: 7, width: 100, height: 40, x: 12, y: 4}, + {name: "other line", number: 7, width: 100, height: 40, x: 4, y: 5}, + {name: "short header hidden", number: 7, width: 100, height: 20, x: 4, y: 4}, + {name: "narrow header hidden", number: 7, width: 40, height: 40, x: 4, y: 4}, + } { + t.Run(tt.name, func(t *testing.T) { + nodes := makeNodes("feature") + if tt.merged { + nodes[0].Ref.PullRequest = &stack.PullRequestRef{Number: 42, Merged: true} + } + m := New(nodes, testTrunk, "0.0.1", tt.number).WithStackURL(url) + if tt.noURL { + m = m.WithStackURL("") + } + m.width, m.height = tt.width, tt.height + beforeCursor := m.cursor + var opened []string + m.openURL = func(url string) { opened = append(opened, url) } + updated, cmd := m.Update(tea.MouseMsg{Action: tea.MouseActionPress, Button: tea.MouseButtonLeft, X: tt.x, Y: tt.y}) + result := updated.(Model) + if tt.wantHit { + assert.Equal(t, []string{url}, opened) + } else { + assert.Empty(t, 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 From 090e0b175390a74a3fef7c00cade14558c0da9a8 Mon Sep 17 00:00:00 2001 From: Sameen Karim Date: Thu, 22 Oct 2026 13:01:33 -0400 Subject: [PATCH 03/11] Link stack references in submit and sync output --- cmd/submit.go | 8 ++-- cmd/submit_test.go | 106 +++++++++++++++++++++++++++++++++++++++++++++ cmd/sync_test.go | 7 +++ 3 files changed, 117 insertions(+), 4 deletions(-) diff --git a/cmd/submit.go b/cmd/submit.go index ad75973d..ba971abd 100644 --- a/cmd/submit.go +++ b/cmd/submit.go @@ -914,7 +914,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 +977,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 +1027,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 +1043,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..42c58f38 100644 --- a/cmd/submit_test.go +++ b/cmd/submit_test.go @@ -736,6 +736,112 @@ func TestSubmit_Humanize(t *testing.T) { } } +func TestSyncStack_StackLinks(t *testing.T) { + for _, tt := range []struct { + name string + want string + }{ + {"create", "Stack created on GitHub with 2 PRs"}, + {"current", "Stack on GitHub is up to date with 2 PRs"}, + {"adopt", "Linked to the existing stack on GitHub (2 PRs, already up to date)"}, + {"update", "Stack updated on GitHub with 2 PRs"}, + {"recreate", "Stack created on GitHub with 2 PRs"}, + {"unknown number", "Stack created on GitHub with 2 PRs"}, + } { + for _, force := range []string{"0", "1"} { + t.Run(tt.name+"/"+force, func(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", force) + cfg, outR, errR := config.NewTestConfig() + defer cfg.Out.Close() + defer cfg.Err.Close() + defer outR.Close() + defer errR.Close() + setTestRepo(cfg) + s := &stack.Stack{ + Trunk: stack.BranchRef{Branch: "main"}, + Branches: []stack.BranchRef{ + {Branch: "b1", PullRequest: &stack.PullRequestRef{Number: 10}}, + {Branch: "b2", PullRequest: &stack.PullRequestRef{Number: 11}}, + }, + } + rs := &github.RemoteStack{ID: 6154, Number: 7, PullRequests: []int{10, 11}} + switch tt.name { + case "current", "update": + s.ID, s.Number = "6154", 7 + case "recreate": + s.ID, s.Number = "99", 99 + case "unknown number": + rs.Number = 0 + } + var lists, gets, creates, adds int + client := &github.MockClient{ + ListStacksFn: func() ([]github.RemoteStack, error) { + lists++ + if tt.name == "adopt" { + return []github.RemoteStack{*rs}, nil + } + return nil, nil + }, + GetStackFn: func(number int) (*github.RemoteStack, error) { + gets++ + if tt.name == "recreate" { + assert.Equal(t, 99, number) + return nil, &api.HTTPError{StatusCode: 404} + } + assert.Equal(t, 7, number) + if tt.name == "update" { + return &github.RemoteStack{ID: 6154, Number: 7, PullRequests: []int{10}}, nil + } + return rs, nil + }, + CreateStackFn: func(numbers []int) (*github.RemoteStack, error) { + creates++ + assert.Equal(t, []int{10, 11}, numbers) + return rs, nil + }, + AddToStackFn: func(number int, delta []int) (*github.RemoteStack, error) { + adds++ + assert.Equal(t, 7, number) + assert.Equal(t, []int{11}, delta) + return rs, nil + }, + } + require.True(t, syncStack(cfg, client, s)) + require.NoError(t, cfg.Err.Close()) + raw, err := io.ReadAll(errR) + require.NoError(t, err) + output := string(raw) + assert.Contains(t, output, tt.want) + if rs.Number == 0 { + assert.NotContains(t, output, "(stack ") + assert.NotContains(t, output, "/stack/") + } else if force == "1" { + assert.Contains(t, output, "\x1b]8;;https://github.com/o/r/stack/7\x1b\\#7\x1b]8;;\x1b\\") + } else { + assert.Contains(t, output, "#7 (https://github.com/o/r/stack/7)") + assert.NotContains(t, output, "\x1b]8") + } + assert.NotContains(t, output, "/stack/6154") + assert.NotContains(t, output, "/stack/99") + assert.Equal(t, "6154", s.ID) + assert.Equal(t, rs.Number, s.Number) + switch tt.name { + case "create", "unknown number": + assert.Equal(t, []int{1, 0, 1, 0}, []int{lists, gets, creates, adds}) + case "adopt": + assert.Equal(t, []int{1, 0, 0, 0}, []int{lists, gets, creates, adds}) + case "current": + assert.Equal(t, []int{0, 1, 0, 0}, []int{lists, gets, creates, adds}) + case "update": + assert.Equal(t, []int{0, 1, 0, 1}, []int{lists, gets, creates, adds}) + case "recreate": + assert.Equal(t, []int{0, 1, 1, 0}, []int{lists, gets, creates, adds}) + } + }) + } + } +} + func TestSyncStack_NewStack_CreateSuccess(t *testing.T) { s := &stack.Stack{ Trunk: stack.BranchRef{Branch: "main"}, diff --git a/cmd/sync_test.go b/cmd/sync_test.go index 1060fdd0..51e8881b 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") From 57659bd0fe4088cdebcb9888f250d24d0d100fb8 Mon Sep 17 00:00:00 2001 From: Sameen Karim Date: Thu, 22 Oct 2026 13:05:05 -0400 Subject: [PATCH 04/11] Make submit TUI stack labels clickable --- cmd/submit.go | 1 + internal/tui/submitview/model.go | 5 +- internal/tui/submitview/model_test.go | 103 +++++++++++++++++++++++++- internal/tui/submitview/mouse.go | 23 ++++++ internal/tui/submitview/render.go | 5 +- internal/tui/submitview/screen.go | 19 ++++- 6 files changed, 149 insertions(+), 7 deletions(-) diff --git a/cmd/submit.go b/cmd/submit.go index ba971abd..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. 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..394ce5d6 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", info.Label) + assert.Equal(t, " • myorg/myrepo", info.Suffix) + 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,102 @@ 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())) +} + +func TestMouse_StackLinks(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", "0") + const url = "https://github.example.com/o/r/stack/7" + tests := []struct { + name string + sidebar, unknown, noURL bool + help, confirming, scrolled bool + closed, short bool + x int + wantHit bool + }{ + {name: "header", x: 4, wantHit: true}, + {name: "header last cell", x: 11, wantHit: true}, + {name: "header icon", x: 2}, + {name: "header repo suffix", x: 12}, + {name: "header unknown number", x: 4, unknown: true}, + {name: "header unknown URL", x: 4, noURL: true}, + {name: "header hidden", x: 4, short: true}, + {name: "header help overlay", x: 4, help: true}, + {name: "header quit overlay", x: 4, confirming: true}, + {name: "sidebar", sidebar: true, x: 2, wantHit: true}, + {name: "sidebar last cell", sidebar: true, x: 9, wantHit: true}, + {name: "sidebar gutter", sidebar: true, x: 1}, + {name: "sidebar after label", sidebar: true, x: 10}, + {name: "sidebar unknown number", sidebar: true, x: 2, unknown: true}, + {name: "sidebar unknown URL", sidebar: true, x: 2, noURL: true}, + {name: "sidebar help overlay", sidebar: true, x: 2, help: true}, + {name: "sidebar quit overlay", sidebar: true, x: 2, confirming: true}, + {name: "sidebar scrolled away", sidebar: true, x: 2, scrolled: true}, + {name: "sidebar below closed banner", sidebar: true, x: 2, closed: true, wantHit: true}, + {name: "sidebar without header", sidebar: true, x: 2, short: true, wantHit: true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + nodes := newNodes() + if tt.closed { + nodes = append(nodes, newNode("closed", StateClosed)) + } + m := testModel(t, nodes) + m.stackNumber, m.stackURL = 7, url + if tt.unknown { + m.stackNumber = 0 + } + if tt.noURL { + m.stackURL = "" + } + if tt.scrolled { + m.height = 26 + m.leftScroll = 1 + } + if tt.short { + m.height = 20 + } + m.showHelp, m.confirmingQuit = tt.help, tt.confirming + 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) } + y := 4 + if tt.sidebar { + y = m.panelTopRow() + } + updated, cmd := m.Update(tea.MouseMsg{Action: tea.MouseActionPress, Button: tea.MouseButtonLeft, X: tt.x, Y: y}) + result := updated.(Model) + if tt.wantHit { + assert.Equal(t, []string{url}, opened) + assert.Equal(t, beforeCursor, result.cursor) + assert.Equal(t, beforeField, result.focusedField) + assert.Equal(t, "unsaved title", result.titleArea.Value()) + assert.Equal(t, "unsaved description", result.descArea.Value()) + assert.Nil(t, cmd) + } else { + assert.Empty(t, opened) + } + assert.False(t, result.SubmitRequested()) + assert.False(t, result.Cancelled()) + }) + } +} + 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..533f28eb 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 && 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..23a023d7 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", m.stackNumber), + Suffix: " • " + repo, 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..305c0d3c 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,25 @@ func leftPanelBox(content string, width, height int) string { Render(content) } +func (m Model) stackHeading() string { + if m.stackNumber > 0 { + return fmt.Sprintf("STACK #%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) + headingStyle := sectionLabelStyle + if m.stackNumber > 0 && m.stackURL != "" { + headingStyle = headingStyle.Underline(true) } rows := []leftRow{ - {text: pad(1, false) + sectionLabelStyle.Render(stackLabel), branch: -1}, + {text: pad(1, false) + headingStyle.Render(truncateVisible(m.stackHeading(), fullW-1)), branch: -1}, {text: m.gapRow(fullW, false, cur == 0), branch: -1}, // blank under STACK; top pad for branch 0 } for i := range m.nodes { From 54e32c75f19d8838f4460cf1849b363c849df2d9 Mon Sep 17 00:00:00 2001 From: Sameen Karim Date: Thu, 22 Oct 2026 13:10:24 -0400 Subject: [PATCH 05/11] Link stack references in checkout output and picker --- cmd/checkout.go | 33 +++++++++--- cmd/checkout_picker_test.go | 10 +++- cmd/checkout_test.go | 72 ++++++++++++++++++++++++- go.mod | 2 +- internal/tui/checkoutview/model.go | 19 +++++-- internal/tui/checkoutview/model_test.go | 62 +++++++++++++++++++-- internal/tui/checkoutview/styles.go | 25 +-------- 7 files changed, 181 insertions(+), 42 deletions(-) diff --git a/cmd/checkout.go b/cmd/checkout.go index 4fe24f2c..732a356a 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, ", ")) @@ -474,7 +474,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 +712,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 } @@ -772,7 +772,7 @@ func interactiveCheckout(cfg *config.Config, sf *stack.StackFile, gitDir string) return nil, "", nil } - selected, ok, err := launchCheckoutPicker(rows) + selected, ok, err := launchCheckoutPicker(cfg, rows) if err != nil { return nil, "", err } @@ -814,7 +814,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) + link := cfg.StackLink(stackNumber) + prompt := fmt.Sprintf("Found stack %s that includes branch %q. Check out stack %s?", link, branch, link) confirmed, err := confirmRemoteStackCheckout(cfg, prompt) if err != nil { if errors.Is(err, errInterrupt) { @@ -886,7 +887,7 @@ 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) + cfg.Errorf("stack %s could not be loaded from GitHub", cfg.StackLink(selected.Number)) return nil, "", ErrAPIFailure } return nil, "", err @@ -894,13 +895,29 @@ func resolveCheckoutSelection(cfg *config.Config, sf *stack.StackFile, gitDir st return s, targetBranch, nil } +func newCheckoutPicker(cfg *config.Config, rows []checkoutview.StackRow) checkoutview.Model { + model := checkoutview.New(rows) + repo, err := cfg.Repo() + if err != nil || repo.Owner == "" || repo.Name == "" { + return model + } + links := make(map[int]string, len(rows)) + for _, row := range rows { + if row.Number > 0 { + url := github.StackURL(repo.Host, repo.Owner, repo.Name, row.Number) + links[row.Number] = cfg.TerminalLink(row.NumberDisplay(), url) + } + } + return model.WithNumberLinks(links) +} + // launchCheckoutPicker runs the Bubble Tea stack picker and returns the selected // row (and whether one was chosen). It renders inline (no alt-screen) so the // 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(cfg *config.Config, rows []checkoutview.StackRow) (checkoutview.StackRow, bool, error) { + p := tea.NewProgram(newCheckoutPicker(cfg, rows)) 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..3ec253f3 100644 --- a/cmd/checkout_picker_test.go +++ b/cmd/checkout_picker_test.go @@ -173,9 +173,11 @@ func TestMatchingRemoteStacksForBranch(t *testing.T) { } func TestOfferRemoteStackForBranch_Confirmed(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", "0") cfg, _, _ := config.NewTestConfig() + setTestRepo(cfg) 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.Equal(t, `Found stack #7 (https://github.com/o/r/stack/7) that includes branch "feature". Check out stack #7 (https://github.com/o/r/stack/7)?`, prompt) assert.True(t, defaultValue) return true, nil } @@ -445,7 +447,9 @@ func TestResolveCheckoutSelection_RemoteRoutesToClone(t *testing.T) { } func TestResolveCheckoutSelection_RemoteLoadFailure(t *testing.T) { - cfg, _, _ := config.NewTestConfig() + t.Setenv("GH_STACK_HYPERLINKS", "1") + cfg, outR, errR := config.NewTestConfig() + setTestRepo(cfg) cfg.GitHubClientOverride = &github.MockClient{ GetStackFn: func(n int) (*github.RemoteStack, error) { return nil, errors.New("boom") @@ -458,6 +462,8 @@ func TestResolveCheckoutSelection_RemoteLoadFailure(t *testing.T) { var exitErr *ExitError require.ErrorAs(t, err, &exitErr) assert.Equal(t, ErrAPIFailure, err) + output := collectOutput(cfg, outR, errR) + assert.Contains(t, output, "\x1b]8;;https://github.com/o/r/stack/7\x1b\\#7\x1b]8;;\x1b\\") } func TestTopUnmergedBranch(t *testing.T) { diff --git a/cmd/checkout_test.go b/cmd/checkout_test.go index f6623838..e8da000d 100644 --- a/cmd/checkout_test.go +++ b/cmd/checkout_test.go @@ -8,15 +8,74 @@ import ( "strings" "testing" + tea "github.com/charmbracelet/bubbletea" "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/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +func TestCheckoutPickerStackLinks(t *testing.T) { + for _, force := range []string{"0", "1"} { + t.Run(force, func(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", force) + cfg := &config.Config{} + setTestRepo(cfg) + cfg.RepoOverride.Host = "github.example.com" + rows := []checkoutview.StackRow{ + {Number: 0, Type: checkoutview.TypeLocal, BottomBranch: "unsubmitted"}, + {Number: 7, Type: checkoutview.TypeLocal, BottomBranch: "tracked"}, + {Number: 8, Type: checkoutview.TypeRemote, BottomBranch: "remote"}, + } + m := newCheckoutPicker(cfg, rows) + sized, _ := m.Update(tea.WindowSizeMsg{Width: 100, Height: 20}) + output := sized.View() + if force == "1" { + for _, number := range []int{7, 8} { + assert.Contains(t, output, fmt.Sprintf("\x1b]8;;https://github.example.com/o/r/stack/%d\x1b\\%d\x1b]8;;\x1b\\", number, number)) + } + assert.NotContains(t, output, "/stack/0") + } else { + assert.NotContains(t, output, "\x1b]8") + assert.NotContains(t, output, "https://") + } + cfg.RepoOverride.Owner = "" + missing, _ := newCheckoutPicker(cfg, rows).Update(tea.WindowSizeMsg{Width: 100, Height: 20}) + assert.NotContains(t, missing.View(), "\x1b]8") + }) + } +} + +func TestCheckoutStackPromptLinks(t *testing.T) { + for _, force := range []string{"0", "1"} { + t.Run(force, func(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", force) + cfg := &config.Config{} + setTestRepo(cfg) + want := "#7 (https://github.com/o/r/stack/7)" + if force == "1" { + want = "\x1b]8;;https://github.com/o/r/stack/7\x1b\\#7\x1b]8;;\x1b\\" + } + cfg.ConfirmFn = func(prompt string, defaultValue bool) (bool, error) { + assert.Equal(t, fmt.Sprintf("Found stack %s that includes branch %q. Check out stack %s?", want, "feature", want), prompt) + assert.True(t, defaultValue) + return true, nil + } + number, confirmed, err := offerRemoteStackForBranch(cfg, &stack.StackFile{}, []github.RemoteStack{{ + ID: 6154, Number: 7, + PRDetails: []github.RemoteStackPR{{Number: 42, State: "open", Head: github.RemoteStackPRHead{Ref: "feature"}}}, + }}, "feature") + require.NoError(t, err) + assert.True(t, confirmed) + assert.Equal(t, 7, number) + }) + } +} + func TestCheckout_PrintPathRequiresTarget(t *testing.T) { cfg, outR, errR := config.NewTestConfig() cfg.ForceInteractive = true @@ -47,6 +106,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 +488,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 +504,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 +533,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 +725,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 +760,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 +780,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 +789,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/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/tui/checkoutview/model.go b/internal/tui/checkoutview/model.go index 23853dcf..c615f06b 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,16 @@ 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 != "" { + if link != number { + numStyle = numStyle.Underline(true) + } + 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..120d3ae1 100644 --- a/internal/tui/checkoutview/model_test.go +++ b/internal/tui/checkoutview/model_test.go @@ -2,22 +2,20 @@ package checkoutview import ( "fmt" - "regexp" "strings" "testing" "time" tea "github.com/charmbracelet/bubbletea" "github.com/charmbracelet/lipgloss" + "github.com/charmbracelet/x/ansi" "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 +45,62 @@ 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]8;;https://github.com/o/r/stack/%d\x1b\\%d\x1b]8;;\x1b\\", row.Number, row.Number) + } + } + return links +} + +func TestNumberLinksPreserveRows(t *testing.T) { + 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) { + 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" } From 90fe3c3aa5974d70a5fdac9636f626ef391a3af6 Mon Sep 17 00:00:00 2001 From: Sameen Karim Date: Thu, 22 Oct 2026 13:12:46 -0400 Subject: [PATCH 06/11] Link stack references in link command output --- cmd/link.go | 36 ++++++++++------ cmd/link_test.go | 108 ++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 131 insertions(+), 13 deletions(-) diff --git a/cmd/link.go b/cmd/link.go index 03f9e0d5..204f5bf2 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 } @@ -321,7 +321,14 @@ func runLinkAdd(cfg *config.Config, client github.ClientOps, opts *linkOptions, 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 +375,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 +386,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 +426,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 +846,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 +914,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..6008cdc7 100644 --- a/cmd/link_test.go +++ b/cmd/link_test.go @@ -2309,6 +2309,106 @@ func linkRemoteStack(number int, details ...github.RemoteStackPR) github.RemoteS return rs } +func TestLink_StackLinks(t *testing.T) { + tests := []struct { + name string + want string + number int + wantErr error + }{ + {"create", "Created stack with 2 PRs (stack %s)", 7, nil}, + {"update", "Updated stack to 2 PRs (stack %s)", 7, nil}, + {"append", "Added 1 PR to stack %s", 7, nil}, + {"noop", "Stack %s is already up to date", 7, nil}, + {"other stack", "already belongs to stack %s", 8, ErrInvalidArgs}, + {"no PR details", "stack %s has no pull requests to append to", 7, ErrAPIFailure}, + {"missing top branch", "could not determine the top branch of stack %s", 7, ErrAPIFailure}, + } + for _, tt := range tests { + for _, force := range []string{"0", "1"} { + t.Run(tt.name+"/"+force, func(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", force) + cfg, outR, errR := config.NewTestConfig() + defer cfg.Out.Close() + defer cfg.Err.Close() + defer outR.Close() + defer errR.Close() + setTestRepo(cfg) + target := linkRemoteStack(7, linkPR(10, "branch-10")) + remote := &github.RemoteStack{ + ID: 6154, Number: 7, PullRequests: []int{10, 11}, + URL: "https://api.github.com/repos/o/r/stacks/7", + } + client := &github.MockClient{ + CreateStackFn: func(numbers []int) (*github.RemoteStack, error) { + assert.Equal(t, []int{10, 11}, numbers) + return remote, nil + }, + AddToStackFn: func(number int, delta []int) (*github.RemoteStack, error) { + assert.Equal(t, 7, number) + assert.Equal(t, []int{11}, delta) + return remote, nil + }, + GetStackFn: func(number int) (*github.RemoteStack, error) { + assert.Equal(t, 7, number) + return &target, nil + }, + } + var err error + switch tt.name { + case "create": + err = createLink(cfg, client, []int{10, 11}) + case "update": + err = updateLink(cfg, client, &target, []int{10, 11}) + case "append": + err = addToStack(cfg, client, 7, []int{11}) + case "noop": + err = runLinkAdd(cfg, client, &linkOptions{baseChanged: true}, &target, []github.RemoteStack{target}, []string{"10"}, []*resolvedArg{{ + prNumber: 10, prURL: "https://github.com/o/r/pull/10", + }}, "") + case "other stack": + err = ensureNotInOtherStack(cfg, []github.RemoteStack{linkRemoteStack(8, linkPR(11, "branch-11"))}, 7, []*resolvedArg{{ + prNumber: 11, prURL: "https://github.com/o/r/pull/11", + }}) + default: + if tt.name == "no PR details" { + target.PRDetails = nil + } else { + target.PRDetails[0].Head.Ref = "" + } + _, lookupErr := stackTopBranch(&target) + require.Error(t, lookupErr) + assert.NotContains(t, lookupErr.Error(), "\x1b") + err = runLinkAdd(cfg, client, &linkOptions{}, &target, []github.RemoteStack{target}, []string{"11"}, []*resolvedArg{{prNumber: 11}}, "") + } + if tt.wantErr != nil { + assert.ErrorIs(t, err, tt.wantErr) + } else { + require.NoError(t, err) + } + require.NoError(t, cfg.Err.Close()) + raw, readErr := io.ReadAll(errR) + require.NoError(t, readErr) + output := string(raw) + label := fmt.Sprintf("#%d (https://github.com/o/r/stack/%d)", tt.number, tt.number) + if force == "1" { + label = fmt.Sprintf("\x1b]8;;https://github.com/o/r/stack/%d\x1b\\#%d\x1b]8;;\x1b\\", tt.number, tt.number) + } else { + assert.NotContains(t, output, "\x1b]8") + } + assert.Contains(t, output, fmt.Sprintf(tt.want, label)) + assert.NotContains(t, output, "/stack/6154") + assert.NotContains(t, output, "api.github.com") + if tt.name == "noop" { + assert.Contains(t, output, "--base is ignored when adding to stack "+label) + assert.Contains(t, output, "is already in stack "+label) + assert.Contains(t, output, "https://github.com/o/r/pull/10") + } + }) + } + } +} + func TestLink_AddMode_AppendsPRNumberToStack(t *testing.T) { defer mockRemoteOnlyGit()() var addNumber int @@ -2401,8 +2501,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 +2536,7 @@ 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") } func TestLink_AddMode_SkipsPresentAppendsNew(t *testing.T) { @@ -2772,8 +2874,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 +2908,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) { From 10559eaf8e169cfee9c018db845ea3ca9da7bb0c Mon Sep 17 00:00:00 2001 From: Sameen Karim Date: Thu, 22 Oct 2026 13:15:29 -0400 Subject: [PATCH 07/11] Link stack references in merge output and wizard --- cmd/merge.go | 5 ++- cmd/merge_test.go | 41 +++++++++++++++++ internal/tui/mergeview/model_test.go | 66 ++++++++++++++++++++++++++++ internal/tui/mergeview/types.go | 2 + internal/tui/mergeview/view.go | 32 +++----------- 5 files changed, 119 insertions(+), 27 deletions(-) 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/internal/tui/mergeview/model_test.go b/internal/tui/mergeview/model_test.go index 1de8f32c..50755407 100644 --- a/internal/tui/mergeview/model_test.go +++ b/internal/tui/mergeview/model_test.go @@ -8,6 +8,7 @@ import ( tea "github.com/charmbracelet/bubbletea" "github.com/charmbracelet/lipgloss" + "github.com/charmbracelet/x/ansi" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -432,6 +433,71 @@ func TestView_RendersBannerAndSteps(t *testing.T) { assert.Contains(t, confirm, "#1, #2, #3") } +func TestBanner_StackLinks(t *testing.T) { + const link = "\x1b]8;;https://github.example.com/o/r/stack/42\x1b\\#42\x1b]8;;\x1b\\" + for _, tt := range []struct { + name, label string + number int + linked bool + }{ + {"linked", link, 42, true}, + {"disabled", "#42", 42, false}, + {"missing URL", "", 42, false}, + {"unknown number", link, 0, false}, + } { + t.Run(tt.name, func(t *testing.T) { + opts := baseOptions() + opts.StackNumber, opts.StackLink = tt.number, tt.label + m := New(opts) + for _, phase := range []Step{StepSelectPRs, StepMethod, StepConfirm, StepProgress, StepDone} { + m.step = phase + output := m.View() + if phase == StepProgress || phase == StepDone { + assert.NotContains(t, output, "Merge stack") + assert.NotContains(t, output, "/stack/") + continue + } + want := "Merge stack" + if tt.number > 0 { + want += " #42" + } + assert.Contains(t, ansi.Strip(output), want+"\n") + if tt.linked { + assert.Contains(t, output, link) + } else { + assert.NotContains(t, output, "\x1b]8") + assert.NotContains(t, output, "https://") + } + } + }) + } +} + +func TestBanner_StackLinksClipping(t *testing.T) { + opts := baseOptions() + opts.StackNumber = 12345 + opts.StackLink = "\x1b]8;;https://github.com/o/r/stack/12345\x1b\\#12345\x1b]8;;\x1b\\" + 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) { opts := baseOptions() opts.StackNumber = 42 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..88f00cf9 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 != "" && m.opts.StackLink != label { + label = titleStyle.Underline(true).Render(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" } From 7a7506f55bc12f7129c8e1d02ea35f520ef85d8d Mon Sep 17 00:00:00 2001 From: Sameen Karim Date: Thu, 22 Oct 2026 13:17:40 -0400 Subject: [PATCH 08/11] Link surviving stacks in unstack and reconciliation output --- cmd/unstack.go | 6 +-- cmd/unstack_test.go | 94 ++++++++++++++++++++++++++++++++++++++++++++- cmd/utils.go | 6 +-- cmd/utils_test.go | 74 +++++++++++++++++++++++++++++++++++ 4 files changed, 172 insertions(+), 8 deletions(-) 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..c7451f71 100644 --- a/cmd/unstack_test.go +++ b/cmd/unstack_test.go @@ -16,6 +16,92 @@ import ( "github.com/stretchr/testify/require" ) +func TestUnstackNumberOnGitHub_StackLinks(t *testing.T) { + for _, outcome := range []string{"partial", "dissolved", "not found"} { + for _, force := range []string{"0", "1"} { + t.Run(outcome+"/"+force, func(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", force) + cfg, outR, errR := config.NewTestConfig() + setTestRepo(cfg) + calls := 0 + client := &github.MockClient{ + UnstackFn: func(number int) (*github.RemoteStack, bool, error) { + calls++ + assert.Equal(t, 7, number) + if outcome == "not found" { + return nil, false, &api.HTTPError{StatusCode: 404} + } + if outcome == "dissolved" { + return nil, true, nil + } + return &github.RemoteStack{ID: 6154, Number: 7, PullRequests: []int{42}}, false, nil + }, + } + err := unstackNumberOnGitHub(cfg, client, 7, false) + output := collectOutput(cfg, outR, errR) + assert.Equal(t, 1, calls) + switch outcome { + case "not found": + assert.ErrorIs(t, err, ErrNotInStack) + assert.Contains(t, output, "stack #7 not found on GitHub") + case "dissolved": + require.NoError(t, err) + assert.Contains(t, output, "Stack removed on GitHub (stack #7)") + case "partial": + require.NoError(t, err) + 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\\" + } + assert.Contains(t, output, "Pull requests unstacked on GitHub (stack "+link+")") + } + if outcome != "partial" { + assert.NotContains(t, output, "/stack/") + assert.NotContains(t, output, "\x1b]8") + } + }) + } + } +} + +func TestUnstack_LocalMissingStackLinks(t *testing.T) { + for _, localRepo := range []bool{false, true} { + name := "remote-only context" + if localRepo { + name = "local repository" + } + t.Run(name, func(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", "1") + if localRepo { + dir := t.TempDir() + writeStackFile(t, dir, stack.Stack{Number: 42, Trunk: stack.BranchRef{Branch: "main"}}) + restore := git.SetOps(&git.MockOps{ + GitDirFn: func() (string, error) { return dir, nil }, + }) + defer restore() + } else { + defer mockRemoteOnlyGit()() + } + cfg, outR, errR := config.NewTestConfig() + setTestRepo(cfg) + cfg.GitHubClientOverride = &github.MockClient{ + ListStacksFn: func() ([]github.RemoteStack, error) { + t.Fatal("--local must not look up remote stacks to format a link") + return nil, nil + }, + UnstackFn: func(int) (*github.RemoteStack, bool, error) { + t.Fatal("--local must not unstack on GitHub") + return nil, false, nil + }, + } + err := runUnstack(cfg, &unstackOptions{stackNumber: 7, local: true}) + output := collectOutput(cfg, outR, errR) + assert.ErrorIs(t, err, ErrNotInStack) + assert.Contains(t, output, "stack \x1b]8;;https://github.com/o/r/stack/7\x1b\\#7\x1b]8;;\x1b\\ is not tracked locally") + }) + } +} + func TestUnstack_NumberWithoutLocalRepository(t *testing.T) { defer mockRemoteOnlyGit()() cfg, outR, errR := config.NewTestConfig() @@ -634,6 +720,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 +739,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 +751,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 +759,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 +783,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 +801,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 61d00445..4750b368 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -2168,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 { @@ -2183,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 fb8abcd2..eafe7a75 100644 --- a/cmd/utils_test.go +++ b/cmd/utils_test.go @@ -1706,6 +1706,80 @@ func setTestRepo(cfg *config.Config) { cfg.RepoOverride = &repository.Repository{Host: "github.com", Owner: "o", Name: "r"} } +func TestUnstackAndKeepLocal_StackLinks(t *testing.T) { + for _, outcome := range []string{"already merged", "partial", "dissolved"} { + for _, force := range []string{"0", "1"} { + t.Run(outcome+"/"+force, func(t *testing.T) { + t.Setenv("GH_STACK_HYPERLINKS", force) + dir := t.TempDir() + writeStackFile(t, dir, stack.Stack{ + ID: "6154", Number: 7, Trunk: stack.BranchRef{Branch: "main"}, + Branches: []stack.BranchRef{ + {Branch: "b1", PullRequest: &stack.PullRequestRef{Number: 10}}, + {Branch: "b2", PullRequest: &stack.PullRequestRef{Number: 11}}, + }, + }) + sf, err := stack.Load(dir) + require.NoError(t, err) + s := &sf.Stacks[0] + remote := &github.RemoteStack{ID: 6154, Number: 7, PullRequests: []int{10, 11}} + if outcome == "already merged" { + s.Branches[0].PullRequest.Merged = true + remote.PullRequests = []int{10} + } + calls := 0 + client := &github.MockClient{ + UnstackFn: func(number int) (*github.RemoteStack, bool, error) { + calls++ + assert.Equal(t, 7, number) + if outcome == "dissolved" { + return nil, true, nil + } + mergedAt := "2026-01-01T00:00:00Z" + return &github.RemoteStack{ + ID: 6154, Number: 7, PullRequests: []int{10}, + PRDetails: []github.RemoteStackPR{{Number: 10, MergedAt: &mergedAt}}, + }, false, nil + }, + } + cfg, outR, errR := config.NewTestConfig() + setTestRepo(cfg) + err = unstackAndKeepLocal(cfg, client, sf, s, remote, dir) + output := collectOutput(cfg, outR, errR) + require.NoError(t, err) + if outcome == "already merged" { + assert.Zero(t, calls) + } else { + assert.Equal(t, 1, calls) + } + if outcome == "dissolved" { + assert.Contains(t, output, "Remote stack removed (stack #7)") + assert.NotContains(t, output, "/stack/") + assert.NotContains(t, output, "\x1b]8") + } else { + 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\\" + } + assert.Contains(t, output, "Merged pull requests remain stacked on GitHub (stack "+link+")") + if outcome == "partial" { + assert.Contains(t, output, "Pull requests unstacked on GitHub (stack "+link+")") + } + } + saved, err := stack.Load(dir) + require.NoError(t, err) + require.Len(t, saved.Stacks, 1) + assert.Empty(t, saved.Stacks[0].ID) + assert.Zero(t, saved.Stacks[0].Number) + assert.Equal(t, []string{"b1", "b2"}, saved.Stacks[0].BranchNames()) + if outcome != "dissolved" { + assert.True(t, saved.Stacks[0].Branches[0].PullRequest.Merged) + } + }) + } + } +} + func TestLinkedStackLabel(t *testing.T) { cfg := &config.Config{} setTestRepo(cfg) From 57ba15b8f44a1dd32c2aafa0951bf436d2b82f47 Mon Sep 17 00:00:00 2001 From: Sameen Karim Date: Thu, 22 Oct 2026 13:18:52 -0400 Subject: [PATCH 09/11] Document stack links and terminal fallbacks --- README.md | 14 +++++++++++--- docs/src/content/docs/reference/cli.md | 16 ++++++++++++---- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 9c4ea0da..13dadb6d 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. 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 the stack identifier 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/docs/src/content/docs/reference/cli.md b/docs/src/content/docs/reference/cli.md index 01fb1f93..0530fa19 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. 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 the stack identifier 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 From 34283c191a42ddafdf91ab198bc5bbc64fb482ec Mon Sep 17 00:00:00 2001 From: Sameen Karim Date: Thu, 22 Oct 2026 18:16:32 -0400 Subject: [PATCH 10/11] Fix stack link rendering and stale-stack handling --- README.md | 4 +- cmd/checkout.go | 60 ++++++----- cmd/checkout_picker_test.go | 84 ++++++++------- cmd/checkout_test.go | 102 ++++++++++-------- cmd/link.go | 7 +- cmd/link_test.go | 129 +++++++---------------- docs/src/content/docs/reference/cli.md | 4 +- internal/config/config.go | 10 +- internal/config/hyperlink_test.go | 9 ++ internal/tui/checkoutview/model.go | 3 - internal/tui/checkoutview/model_test.go | 11 +- internal/tui/mergeview/model_test.go | 69 +++++-------- internal/tui/mergeview/view.go | 4 +- internal/tui/shared/header.go | 23 +++-- internal/tui/shared/header_test.go | 17 +-- internal/tui/stackview/model.go | 5 +- internal/tui/stackview/model_test.go | 43 +++----- internal/tui/submitview/model_test.go | 132 ++++++++++-------------- internal/tui/submitview/mouse.go | 2 +- internal/tui/submitview/render.go | 4 +- internal/tui/submitview/screen.go | 16 ++- 21 files changed, 350 insertions(+), 388 deletions(-) diff --git a/README.md b/README.md index 13dadb6d..2d73c401 100644 --- a/README.md +++ b/README.md @@ -558,12 +558,12 @@ gh stack view --json ``` Stack references follow the existing PR-link behavior in text output: OSC 8 -hyperlinks when supported, or the full URL for copy/paste otherwise. Links open +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 the stack identifier in `view` or `submit` to +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 diff --git a/cmd/checkout.go b/cmd/checkout.go index 732a356a..a8481154 100644 --- a/cmd/checkout.go +++ b/cmd/checkout.go @@ -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 } @@ -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(cfg, 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 + } + } + } + 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 checkoutview.BuildRows(sf.Stacks, remote), remote + return rows, remote, links } // offerRemoteStackForBranch asks to check out the unique active remote stack @@ -814,8 +830,8 @@ func offerRemoteStackForBranch(cfg *config.Config, sf *stack.StackFile, remote [ } stackNumber := matches[0].Number - link := cfg.StackLink(stackNumber) - prompt := fmt.Sprintf("Found stack %s that includes branch %q. Check out stack %s?", link, branch, link) + 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) { @@ -887,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 %s could not be loaded from GitHub", cfg.StackLink(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 @@ -895,29 +915,13 @@ func resolveCheckoutSelection(cfg *config.Config, sf *stack.StackFile, gitDir st return s, targetBranch, nil } -func newCheckoutPicker(cfg *config.Config, rows []checkoutview.StackRow) checkoutview.Model { - model := checkoutview.New(rows) - repo, err := cfg.Repo() - if err != nil || repo.Owner == "" || repo.Name == "" { - return model - } - links := make(map[int]string, len(rows)) - for _, row := range rows { - if row.Number > 0 { - url := github.StackURL(repo.Host, repo.Owner, repo.Name, row.Number) - links[row.Number] = cfg.TerminalLink(row.NumberDisplay(), url) - } - } - return model.WithNumberLinks(links) -} - // launchCheckoutPicker runs the Bubble Tea stack picker and returns the selected // row (and whether one was chosen). It renders inline (no alt-screen) so the // 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(cfg *config.Config, rows []checkoutview.StackRow) (checkoutview.StackRow, bool, error) { - p := tea.NewProgram(newCheckoutPicker(cfg, 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 3ec253f3..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,25 +173,29 @@ func TestMatchingRemoteStacksForBranch(t *testing.T) { } func TestOfferRemoteStackForBranch_Confirmed(t *testing.T) { - t.Setenv("GH_STACK_HYPERLINKS", "0") - cfg, _, _ := config.NewTestConfig() - setTestRepo(cfg) - cfg.ConfirmFn = func(prompt string, defaultValue bool) (bool, error) { - assert.Equal(t, `Found stack #7 (https://github.com/o/r/stack/7) that includes branch "feature". Check out stack #7 (https://github.com/o/r/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) { @@ -448,22 +452,32 @@ func TestResolveCheckoutSelection_RemoteRoutesToClone(t *testing.T) { func TestResolveCheckoutSelection_RemoteLoadFailure(t *testing.T) { t.Setenv("GH_STACK_HYPERLINKS", "1") - cfg, outR, errR := config.NewTestConfig() - setTestRepo(cfg) - cfg.GitHubClientOverride = &github.MockClient{ - GetStackFn: func(n int) (*github.RemoteStack, error) { - return nil, errors.New("boom") - }, + 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) - output := collectOutput(cfg, outR, errR) - assert.Contains(t, output, "\x1b]8;;https://github.com/o/r/stack/7\x1b\\#7\x1b]8;;\x1b\\") } func TestTopUnmergedBranch(t *testing.T) { diff --git a/cmd/checkout_test.go b/cmd/checkout_test.go index e8da000d..bf7eca8d 100644 --- a/cmd/checkout_test.go +++ b/cmd/checkout_test.go @@ -9,69 +9,87 @@ import ( "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) { - for _, force := range []string{"0", "1"} { - t.Run(force, func(t *testing.T) { - t.Setenv("GH_STACK_HYPERLINKS", force) - cfg := &config.Config{} + 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" - rows := []checkoutview.StackRow{ - {Number: 0, Type: checkoutview.TypeLocal, BottomBranch: "unsubmitted"}, - {Number: 7, Type: checkoutview.TypeLocal, BottomBranch: "tracked"}, - {Number: 8, Type: checkoutview.TypeRemote, BottomBranch: "remote"}, + if tt.missingRepo { + cfg.RepoOverride.Owner = "" } - m := newCheckoutPicker(cfg, rows) + 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 force == "1" { - for _, number := range []int{7, 8} { - assert.Contains(t, output, fmt.Sprintf("\x1b]8;;https://github.example.com/o/r/stack/%d\x1b\\%d\x1b]8;;\x1b\\", number, number)) - } - assert.NotContains(t, output, "/stack/0") + 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://") } - cfg.RepoOverride.Owner = "" - missing, _ := newCheckoutPicker(cfg, rows).Update(tea.WindowSizeMsg{Width: 100, Height: 20}) - assert.NotContains(t, missing.View(), "\x1b]8") - }) - } -} - -func TestCheckoutStackPromptLinks(t *testing.T) { - for _, force := range []string{"0", "1"} { - t.Run(force, func(t *testing.T) { - t.Setenv("GH_STACK_HYPERLINKS", force) - cfg := &config.Config{} - setTestRepo(cfg) - want := "#7 (https://github.com/o/r/stack/7)" - if force == "1" { - want = "\x1b]8;;https://github.com/o/r/stack/7\x1b\\#7\x1b]8;;\x1b\\" + if !tt.empty && !tt.failed && tt.wantLink { + assert.Contains(t, output, "/stack/8") } - cfg.ConfirmFn = func(prompt string, defaultValue bool) (bool, error) { - assert.Equal(t, fmt.Sprintf("Found stack %s that includes branch %q. Check out stack %s?", want, "feature", want), prompt) - assert.True(t, defaultValue) - return true, nil - } - number, confirmed, err := offerRemoteStackForBranch(cfg, &stack.StackFile{}, []github.RemoteStack{{ - ID: 6154, Number: 7, - PRDetails: []github.RemoteStackPR{{Number: 42, State: "open", Head: github.RemoteStackPRHead{Ref: "feature"}}}, - }}, "feature") - require.NoError(t, err) - assert.True(t, confirmed) - assert.Equal(t, 7, number) + 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") }) } } diff --git a/cmd/link.go b/cmd/link.go index 204f5bf2..163099d6 100644 --- a/cmd/link.go +++ b/cmd/link.go @@ -317,7 +317,12 @@ 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 { diff --git a/cmd/link_test.go b/cmd/link_test.go index 6008cdc7..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,100 +2317,36 @@ func linkRemoteStack(number int, details ...github.RemoteStackPR) github.RemoteS return rs } -func TestLink_StackLinks(t *testing.T) { - tests := []struct { - name string - want string - number int - wantErr error - }{ - {"create", "Created stack with 2 PRs (stack %s)", 7, nil}, - {"update", "Updated stack to 2 PRs (stack %s)", 7, nil}, - {"append", "Added 1 PR to stack %s", 7, nil}, - {"noop", "Stack %s is already up to date", 7, nil}, - {"other stack", "already belongs to stack %s", 8, ErrInvalidArgs}, - {"no PR details", "stack %s has no pull requests to append to", 7, ErrAPIFailure}, - {"missing top branch", "could not determine the top branch of stack %s", 7, ErrAPIFailure}, - } - for _, tt := range tests { - for _, force := range []string{"0", "1"} { - t.Run(tt.name+"/"+force, func(t *testing.T) { - t.Setenv("GH_STACK_HYPERLINKS", force) +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() - defer cfg.Out.Close() - defer cfg.Err.Close() - defer outR.Close() - defer errR.Close() setTestRepo(cfg) - target := linkRemoteStack(7, linkPR(10, "branch-10")) - remote := &github.RemoteStack{ - ID: 6154, Number: 7, PullRequests: []int{10, 11}, - URL: "https://api.github.com/repos/o/r/stacks/7", - } + target := github.RemoteStack{Number: 7, PullRequests: []int{10}, PRDetails: details} client := &github.MockClient{ - CreateStackFn: func(numbers []int) (*github.RemoteStack, error) { - assert.Equal(t, []int{10, 11}, numbers) - return remote, nil - }, - AddToStackFn: func(number int, delta []int) (*github.RemoteStack, error) { - assert.Equal(t, 7, number) - assert.Equal(t, []int{11}, delta) - return remote, nil - }, 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 + }, } - var err error - switch tt.name { - case "create": - err = createLink(cfg, client, []int{10, 11}) - case "update": - err = updateLink(cfg, client, &target, []int{10, 11}) - case "append": - err = addToStack(cfg, client, 7, []int{11}) - case "noop": - err = runLinkAdd(cfg, client, &linkOptions{baseChanged: true}, &target, []github.RemoteStack{target}, []string{"10"}, []*resolvedArg{{ - prNumber: 10, prURL: "https://github.com/o/r/pull/10", - }}, "") - case "other stack": - err = ensureNotInOtherStack(cfg, []github.RemoteStack{linkRemoteStack(8, linkPR(11, "branch-11"))}, 7, []*resolvedArg{{ - prNumber: 11, prURL: "https://github.com/o/r/pull/11", - }}) - default: - if tt.name == "no PR details" { - target.PRDetails = nil - } else { - target.PRDetails[0].Head.Ref = "" - } - _, lookupErr := stackTopBranch(&target) - require.Error(t, lookupErr) - assert.NotContains(t, lookupErr.Error(), "\x1b") - err = runLinkAdd(cfg, client, &linkOptions{}, &target, []github.RemoteStack{target}, []string{"11"}, []*resolvedArg{{prNumber: 11}}, "") - } - if tt.wantErr != nil { - assert.ErrorIs(t, err, tt.wantErr) - } else { - require.NoError(t, err) - } - require.NoError(t, cfg.Err.Close()) - raw, readErr := io.ReadAll(errR) - require.NoError(t, readErr) - output := string(raw) - label := fmt.Sprintf("#%d (https://github.com/o/r/stack/%d)", tt.number, tt.number) - if force == "1" { - label = fmt.Sprintf("\x1b]8;;https://github.com/o/r/stack/%d\x1b\\#%d\x1b]8;;\x1b\\", tt.number, tt.number) + 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.NotContains(t, output, "\x1b]8") - } - assert.Contains(t, output, fmt.Sprintf(tt.want, label)) - assert.NotContains(t, output, "/stack/6154") - assert.NotContains(t, output, "api.github.com") - if tt.name == "noop" { - assert.Contains(t, output, "--base is ignored when adding to stack "+label) - assert.Contains(t, output, "is already in stack "+label) - assert.Contains(t, output, "https://github.com/o/r/pull/10") + assert.ErrorIs(t, err, ErrAPIFailure) + assert.Contains(t, output, "\x1b]8;;https://github.com/o/r/stack/7\x1b\\#7\x1b]8;;\x1b\\") } }) } @@ -2410,10 +2354,12 @@ func TestLink_StackLinks(t *testing.T) { } 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{ @@ -2452,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) { @@ -2537,6 +2483,7 @@ 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 (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) { @@ -2583,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{ @@ -2616,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) { @@ -2751,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{ @@ -2785,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) { diff --git a/docs/src/content/docs/reference/cli.md b/docs/src/content/docs/reference/cli.md index 0530fa19..51ba4c0e 100644 --- a/docs/src/content/docs/reference/cli.md +++ b/docs/src/content/docs/reference/cli.md @@ -175,12 +175,12 @@ gh stack view --json ``` Stack references follow the existing PR-link behavior in text output: OSC 8 -hyperlinks when supported, or the full URL for copy/paste otherwise. Links open +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 the stack identifier in `view` or `submit` to +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 diff --git a/internal/config/config.go b/internal/config/config.go index 8aa43590..c37eaba3 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -187,7 +187,7 @@ func (c *Config) Link(label, url string) string { if !hyperlinksEnabled { return fmt.Sprintf("%s (%s)", label, url) } - label = hyperlink(label, url) + label = fmt.Sprintf("\033]8;;%s\033\\%s\033]8;;\033\\", url, label) } if c.Terminal.IsColorEnabled() { label = fmt.Sprintf("\033[4m%s\033[24m", label) @@ -196,16 +196,12 @@ func (c *Config) Link(label, url string) string { } // TerminalLink keeps a compact label when terminal hyperlinks are unavailable. -// TUI callers supply their own styling. +// 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 hyperlink(label, url) -} - -func hyperlink(label, url string) string { - return fmt.Sprintf("\033]8;;%s\033\\%s\033]8;;\033\\", url, label) + return c.Link(label, url) } // StackURL returns no destination when optional display identity is unavailable. diff --git a/internal/config/hyperlink_test.go b/internal/config/hyperlink_test.go index 994b6563..c8739825 100644 --- a/internal/config/hyperlink_test.go +++ b/internal/config/hyperlink_test.go @@ -171,6 +171,15 @@ func TestStackURL_RepositoryUnavailable(t *testing.T) { 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 { diff --git a/internal/tui/checkoutview/model.go b/internal/tui/checkoutview/model.go index c615f06b..7ba22824 100644 --- a/internal/tui/checkoutview/model.go +++ b/internal/tui/checkoutview/model.go @@ -522,9 +522,6 @@ func (m Model) renderRow(r StackRow, selected bool, lay layout) string { if r.Number == 0 { numStyle = dimStyle } else if link := m.numberLinks[r.Number]; r.Number > 0 && link != "" { - if link != number { - numStyle = numStyle.Underline(true) - } number = link } b.WriteString(renderCell(number, numStyle, lay.num, selected)) diff --git a/internal/tui/checkoutview/model_test.go b/internal/tui/checkoutview/model_test.go index 120d3ae1..f5bb57d3 100644 --- a/internal/tui/checkoutview/model_test.go +++ b/internal/tui/checkoutview/model_test.go @@ -9,6 +9,7 @@ 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" @@ -49,13 +50,17 @@ 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]8;;https://github.com/o/r/stack/%d\x1b\\%d\x1b]8;;\x1b\\", row.Number, row.Number) + 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)) @@ -77,6 +82,10 @@ func TestNumberLinksPreserveRows(t *testing.T) { } 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) diff --git a/internal/tui/mergeview/model_test.go b/internal/tui/mergeview/model_test.go index 50755407..f34cbdc8 100644 --- a/internal/tui/mergeview/model_test.go +++ b/internal/tui/mergeview/model_test.go @@ -9,6 +9,7 @@ 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" ) @@ -411,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") @@ -424,59 +434,29 @@ 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") -} - -func TestBanner_StackLinks(t *testing.T) { - const link = "\x1b]8;;https://github.example.com/o/r/stack/42\x1b\\#42\x1b]8;;\x1b\\" - for _, tt := range []struct { - name, label string - number int - linked bool - }{ - {"linked", link, 42, true}, - {"disabled", "#42", 42, false}, - {"missing URL", "", 42, false}, - {"unknown number", link, 0, false}, - } { - t.Run(tt.name, func(t *testing.T) { - opts := baseOptions() - opts.StackNumber, opts.StackLink = tt.number, tt.label - m := New(opts) - for _, phase := range []Step{StepSelectPRs, StepMethod, StepConfirm, StepProgress, StepDone} { - m.step = phase - output := m.View() - if phase == StepProgress || phase == StepDone { - assert.NotContains(t, output, "Merge stack") - assert.NotContains(t, output, "/stack/") - continue - } - want := "Merge stack" - if tt.number > 0 { - want += " #42" - } - assert.Contains(t, ansi.Strip(output), want+"\n") - if tt.linked { - assert.Contains(t, output, link) - } else { - assert.NotContains(t, output, "\x1b]8") - assert.NotContains(t, output, "https://") - } - } - }) - } + 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]8;;https://github.com/o/r/stack/12345\x1b\\#12345\x1b]8;;\x1b\\" + 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 = "" @@ -506,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/view.go b/internal/tui/mergeview/view.go index 88f00cf9..2faccf06 100644 --- a/internal/tui/mergeview/view.go +++ b/internal/tui/mergeview/view.go @@ -75,8 +75,8 @@ func (m Model) banner() string { title := "Merge stack" if m.opts.StackNumber > 0 { label := fmt.Sprintf("#%d", m.opts.StackNumber) - if m.opts.StackLink != "" && m.opts.StackLink != label { - label = titleStyle.Underline(true).Render(m.opts.StackLink) + if m.opts.StackLink != "" { + label = m.opts.StackLink } title = "Merge stack " + label } diff --git a/internal/tui/shared/header.go b/internal/tui/shared/header.go index 459ae935..fa36a50d 100644 --- a/internal/tui/shared/header.go +++ b/internal/tui/shared/header.go @@ -36,8 +36,8 @@ type ShortcutEntry struct { type HeaderInfoLine struct { Icon string Label string - Suffix string // optional text after the label, outside its link - URL string // optional native-click target for the label + LinkLabel string // optional clickable text within Label + URL string // native-click target for LinkLabel IconStyle *lipgloss.Style // optional override; nil uses default HeaderInfoStyle (cyan) } @@ -243,8 +243,12 @@ func HeaderLinkAt(cfg HeaderConfig, width, height, x, y int) string { return "" } info := cfg.InfoLines[i] - start := 1 + headerLeftMargin + lipgloss.Width(info.Icon) + 1 - end := start + lipgloss.Width(info.Label) + 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 } @@ -252,12 +256,13 @@ func HeaderLinkAt(cfg HeaderConfig, width, height, x, y int) string { } func renderHeaderInfoLabel(info HeaderInfoLine) string { - if info.URL == "" { - return HeaderInfoLabelStyle.Render(" " + info.Label + info.Suffix) + prefix, suffix, found := strings.Cut(info.Label, info.LinkLabel) + if info.URL == "" || info.LinkLabel == "" || !found { + return HeaderInfoLabelStyle.Render(" " + info.Label) } - return HeaderInfoLabelStyle.Render(" ") + - HeaderInfoLabelStyle.Underline(true).Render(info.Label) + - HeaderInfoLabelStyle.Render(info.Suffix) + return HeaderInfoLabelStyle.Render(" "+prefix) + + HeaderInfoLabelStyle.Underline(true).Render(info.LinkLabel) + + HeaderInfoLabelStyle.Render(suffix) } // RenderHeader renders the full-width header box. diff --git a/internal/tui/shared/header_test.go b/internal/tui/shared/header_test.go index 9b631c5c..f851eca3 100644 --- a/internal/tui/shared/header_test.go +++ b/internal/tui/shared/header_test.go @@ -40,7 +40,6 @@ func TestLogoHidesBeforeHeader(t *testing.T) { assert.True(t, ShouldShowHeader(MinWidthForArt, h), "header should show at height %d", h) assert.False(t, artFitsViewport(MinWidthForArt, h), "logo should be hidden at height %d", h) } - } func TestHeaderLinkAt(t *testing.T) { @@ -48,7 +47,7 @@ func TestHeaderLinkAt(t *testing.T) { cfg := HeaderConfig{ Title: "View Stack", InfoLines: []HeaderInfoLine{ - {Icon: "◆", Label: "Stack #7", Suffix: " | o/r", URL: url}, + {Icon: "◆", Label: "Stack #7 | o/r", LinkLabel: "#7", URL: url}, {Icon: "○", Label: "Base: main"}, }, Shortcuts: []ShortcutEntry{{Key: "q", Desc: "quit"}}, @@ -58,15 +57,17 @@ func TestHeaderLinkAt(t *testing.T) { width, height, x, y int want string }{ - {"first cell", 100, 40, 4, 4, url}, + {"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, 4, 4, ""}, - {"narrow", MinWidthForHeader - 1, 40, 4, 4, ""}, + {"short", 100, MinHeightForHeader - 1, 10, 4, ""}, + {"narrow", MinWidthForHeader - 1, 40, 10, 4, ""}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -76,18 +77,18 @@ func TestHeaderLinkAt(t *testing.T) { cfg.Shortcuts = []ShortcutEntry{{Key: "q", Desc: strings.Repeat("x", 35)}} assert.True(t, ShouldShowHeader(60, 40)) - assert.Empty(t, HeaderLinkAt(cfg, 60, 40, 4, 4), "info can be hidden even when the header is visible") + 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, 4, 4)) + assert.Empty(t, HeaderLinkAt(cfg, 100, 40, 10, 4)) } func TestHeaderLinkPreservesLayout(t *testing.T) { - plain := HeaderInfoLine{Icon: "◆", Label: "Stack #7", Suffix: " | o/r"} + 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))) diff --git a/internal/tui/stackview/model.go b/internal/tui/stackview/model.go index cc20b2b9..deee92ff 100644 --- a/internal/tui/stackview/model.go +++ b/internal/tui/stackview/model.go @@ -469,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), URL: m.stackURL}) + 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 687660e5..b95f60ab 100644 --- a/internal/tui/stackview/model_test.go +++ b/internal/tui/stackview/model_test.go @@ -44,42 +44,22 @@ var testTrunk = stack.BranchRef{Branch: "main"} func TestStackHeaderLink(t *testing.T) { const url = "https://github.example.com/o/r/stack/7" - for _, tt := range []struct { - name string - number int - width, height, x, y int - merged, noURL, wantHit bool - }{ - {name: "identifier", number: 7, width: 100, height: 40, x: 4, y: 4, wantHit: true}, - {name: "all merged", number: 7, width: 100, height: 40, x: 11, y: 4, merged: true, wantHit: true}, - {name: "unknown stack", width: 100, height: 40, x: 4, y: 4}, - {name: "unknown URL", number: 7, width: 100, height: 40, x: 4, y: 4, noURL: true}, - {name: "icon", number: 7, width: 100, height: 40, x: 2, y: 4}, - {name: "after label", number: 7, width: 100, height: 40, x: 12, y: 4}, - {name: "other line", number: 7, width: 100, height: 40, x: 4, y: 5}, - {name: "short header hidden", number: 7, width: 100, height: 20, x: 4, y: 4}, - {name: "narrow header hidden", number: 7, width: 40, height: 40, x: 4, y: 4}, - } { - t.Run(tt.name, func(t *testing.T) { + for _, merged := range []bool{false, true} { + t.Run(fmt.Sprint(merged), func(t *testing.T) { nodes := makeNodes("feature") - if tt.merged { - nodes[0].Ref.PullRequest = &stack.PullRequestRef{Number: 42, Merged: true} - } - m := New(nodes, testTrunk, "0.0.1", tt.number).WithStackURL(url) - if tt.noURL { - m = m.WithStackURL("") - } - m.width, m.height = tt.width, tt.height + 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) } - updated, cmd := m.Update(tea.MouseMsg{Action: tea.MouseActionPress, Button: tea.MouseButtonLeft, X: tt.x, Y: tt.y}) - result := updated.(Model) - if tt.wantHit { - assert.Equal(t, []string{url}, opened) - } else { - assert.Empty(t, opened) + 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) @@ -551,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_test.go b/internal/tui/submitview/model_test.go index 394ce5d6..8070f630 100644 --- a/internal/tui/submitview/model_test.go +++ b/internal/tui/submitview/model_test.go @@ -333,8 +333,8 @@ func TestHeaderConfig_StackNumberInRepoLine(t *testing.T) { StackURL: "https://github.com/myorg/myrepo/stack/7", }) info := m.buildHeaderConfig().InfoLines[0] - assert.Equal(t, "Stack #7", info.Label) - assert.Equal(t, " • myorg/myrepo", info.Suffix) + 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. @@ -393,87 +393,67 @@ func TestLeftPanel_StackLinkClipping(t *testing.T) { 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" - tests := []struct { - name string - sidebar, unknown, noURL bool - help, confirming, scrolled bool - closed, short bool - x int - wantHit bool - }{ - {name: "header", x: 4, wantHit: true}, - {name: "header last cell", x: 11, wantHit: true}, - {name: "header icon", x: 2}, - {name: "header repo suffix", x: 12}, - {name: "header unknown number", x: 4, unknown: true}, - {name: "header unknown URL", x: 4, noURL: true}, - {name: "header hidden", x: 4, short: true}, - {name: "header help overlay", x: 4, help: true}, - {name: "header quit overlay", x: 4, confirming: true}, - {name: "sidebar", sidebar: true, x: 2, wantHit: true}, - {name: "sidebar last cell", sidebar: true, x: 9, wantHit: true}, - {name: "sidebar gutter", sidebar: true, x: 1}, - {name: "sidebar after label", sidebar: true, x: 10}, - {name: "sidebar unknown number", sidebar: true, x: 2, unknown: true}, - {name: "sidebar unknown URL", sidebar: true, x: 2, noURL: true}, - {name: "sidebar help overlay", sidebar: true, x: 2, help: true}, - {name: "sidebar quit overlay", sidebar: true, x: 2, confirming: true}, - {name: "sidebar scrolled away", sidebar: true, x: 2, scrolled: true}, - {name: "sidebar below closed banner", sidebar: true, x: 2, closed: true, wantHit: true}, - {name: "sidebar without header", sidebar: true, x: 2, short: true, wantHit: true}, + 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 _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - nodes := newNodes() - if tt.closed { - nodes = append(nodes, newNode("closed", StateClosed)) - } - m := testModel(t, nodes) - m.stackNumber, m.stackURL = 7, url - if tt.unknown { - m.stackNumber = 0 - } - if tt.noURL { - m.stackURL = "" - } - if tt.scrolled { - m.height = 26 - m.leftScroll = 1 - } - if tt.short { - m.height = 20 - } - m.showHelp, m.confirmingQuit = tt.help, tt.confirming - 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) } - y := 4 - if tt.sidebar { - y = m.panelTopRow() - } - updated, cmd := m.Update(tea.MouseMsg{Action: tea.MouseActionPress, Button: tea.MouseButtonLeft, X: tt.x, Y: y}) - result := updated.(Model) - if tt.wantHit { - assert.Equal(t, []string{url}, opened) - assert.Equal(t, beforeCursor, result.cursor) - assert.Equal(t, beforeField, result.focusedField) - assert.Equal(t, "unsaved title", result.titleArea.Value()) - assert.Equal(t, "unsaved description", result.descArea.Value()) - assert.Nil(t, cmd) - } else { - assert.Empty(t, opened) - } - assert.False(t, result.SubmitRequested()) - assert.False(t, result.Cancelled()) - }) + 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) { diff --git a/internal/tui/submitview/mouse.go b/internal/tui/submitview/mouse.go index 533f28eb..cadf76be 100644 --- a/internal/tui/submitview/mouse.go +++ b/internal/tui/submitview/mouse.go @@ -167,7 +167,7 @@ func (m Model) leftStackLinkHit(x, y int) bool { return false } labelWidth := lipgloss.Width(truncateVisible(m.stackHeading(), leftW-3)) - return x >= 2 && x < 2+labelWidth && x < leftW-1 + 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 diff --git a/internal/tui/submitview/render.go b/internal/tui/submitview/render.go index 23a023d7..9ac0defd 100644 --- a/internal/tui/submitview/render.go +++ b/internal/tui/submitview/render.go @@ -45,8 +45,8 @@ 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), - Suffix: " • " + repo, URL: m.stackURL, + 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 305c0d3c..93eaec43 100644 --- a/internal/tui/submitview/screen.go +++ b/internal/tui/submitview/screen.go @@ -763,9 +763,11 @@ func leftPanelBox(content string, width, height int) string { Render(content) } +const stackHeadingPrefix = "STACK " + func (m Model) stackHeading() string { if m.stackNumber > 0 { - return fmt.Sprintf("STACK #%d", m.stackNumber) + return stackHeadingPrefix + fmt.Sprintf("#%d", m.stackNumber) } return "STACK" } @@ -776,12 +778,13 @@ func (m Model) stackHeading() string { // nodes/cursor/width, so the mouse layer can recompute it to resolve clicks. func (m Model) buildLeftRows(fullW int) []leftRow { cur := m.cursor - headingStyle := sectionLabelStyle - if m.stackNumber > 0 && m.stackURL != "" { - headingStyle = headingStyle.Underline(true) + 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) + headingStyle.Render(truncateVisible(m.stackHeading(), fullW-1)), 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 { @@ -939,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 } From 873861e242442ff0ebb9fdb3393e5130204f84b6 Mon Sep 17 00:00:00 2001 From: Sameen Karim Date: Thu, 22 Oct 2026 18:16:32 -0400 Subject: [PATCH 11/11] Reuse existing command tests for stack link coverage --- cmd/submit_test.go | 118 ++++------------------------------------ cmd/sync_test.go | 35 ++++++++---- cmd/unstack_test.go | 128 ++++++++++++-------------------------------- cmd/utils_test.go | 74 ------------------------- cmd/view_test.go | 93 +++++++------------------------- 5 files changed, 88 insertions(+), 360 deletions(-) diff --git a/cmd/submit_test.go b/cmd/submit_test.go index 42c58f38..2f4dfa96 100644 --- a/cmd/submit_test.go +++ b/cmd/submit_test.go @@ -736,112 +736,6 @@ func TestSubmit_Humanize(t *testing.T) { } } -func TestSyncStack_StackLinks(t *testing.T) { - for _, tt := range []struct { - name string - want string - }{ - {"create", "Stack created on GitHub with 2 PRs"}, - {"current", "Stack on GitHub is up to date with 2 PRs"}, - {"adopt", "Linked to the existing stack on GitHub (2 PRs, already up to date)"}, - {"update", "Stack updated on GitHub with 2 PRs"}, - {"recreate", "Stack created on GitHub with 2 PRs"}, - {"unknown number", "Stack created on GitHub with 2 PRs"}, - } { - for _, force := range []string{"0", "1"} { - t.Run(tt.name+"/"+force, func(t *testing.T) { - t.Setenv("GH_STACK_HYPERLINKS", force) - cfg, outR, errR := config.NewTestConfig() - defer cfg.Out.Close() - defer cfg.Err.Close() - defer outR.Close() - defer errR.Close() - setTestRepo(cfg) - s := &stack.Stack{ - Trunk: stack.BranchRef{Branch: "main"}, - Branches: []stack.BranchRef{ - {Branch: "b1", PullRequest: &stack.PullRequestRef{Number: 10}}, - {Branch: "b2", PullRequest: &stack.PullRequestRef{Number: 11}}, - }, - } - rs := &github.RemoteStack{ID: 6154, Number: 7, PullRequests: []int{10, 11}} - switch tt.name { - case "current", "update": - s.ID, s.Number = "6154", 7 - case "recreate": - s.ID, s.Number = "99", 99 - case "unknown number": - rs.Number = 0 - } - var lists, gets, creates, adds int - client := &github.MockClient{ - ListStacksFn: func() ([]github.RemoteStack, error) { - lists++ - if tt.name == "adopt" { - return []github.RemoteStack{*rs}, nil - } - return nil, nil - }, - GetStackFn: func(number int) (*github.RemoteStack, error) { - gets++ - if tt.name == "recreate" { - assert.Equal(t, 99, number) - return nil, &api.HTTPError{StatusCode: 404} - } - assert.Equal(t, 7, number) - if tt.name == "update" { - return &github.RemoteStack{ID: 6154, Number: 7, PullRequests: []int{10}}, nil - } - return rs, nil - }, - CreateStackFn: func(numbers []int) (*github.RemoteStack, error) { - creates++ - assert.Equal(t, []int{10, 11}, numbers) - return rs, nil - }, - AddToStackFn: func(number int, delta []int) (*github.RemoteStack, error) { - adds++ - assert.Equal(t, 7, number) - assert.Equal(t, []int{11}, delta) - return rs, nil - }, - } - require.True(t, syncStack(cfg, client, s)) - require.NoError(t, cfg.Err.Close()) - raw, err := io.ReadAll(errR) - require.NoError(t, err) - output := string(raw) - assert.Contains(t, output, tt.want) - if rs.Number == 0 { - assert.NotContains(t, output, "(stack ") - assert.NotContains(t, output, "/stack/") - } else if force == "1" { - assert.Contains(t, output, "\x1b]8;;https://github.com/o/r/stack/7\x1b\\#7\x1b]8;;\x1b\\") - } else { - assert.Contains(t, output, "#7 (https://github.com/o/r/stack/7)") - assert.NotContains(t, output, "\x1b]8") - } - assert.NotContains(t, output, "/stack/6154") - assert.NotContains(t, output, "/stack/99") - assert.Equal(t, "6154", s.ID) - assert.Equal(t, rs.Number, s.Number) - switch tt.name { - case "create", "unknown number": - assert.Equal(t, []int{1, 0, 1, 0}, []int{lists, gets, creates, adds}) - case "adopt": - assert.Equal(t, []int{1, 0, 0, 0}, []int{lists, gets, creates, adds}) - case "current": - assert.Equal(t, []int{0, 1, 0, 0}, []int{lists, gets, creates, adds}) - case "update": - assert.Equal(t, []int{0, 1, 0, 1}, []int{lists, gets, creates, adds}) - case "recreate": - assert.Equal(t, []int{0, 1, 1, 0}, []int{lists, gets, creates, adds}) - } - }) - } - } -} - func TestSyncStack_NewStack_CreateSuccess(t *testing.T) { s := &stack.Stack{ Trunk: stack.BranchRef{Branch: "main"}, @@ -949,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, @@ -978,6 +873,7 @@ func TestSyncStack_ExistingStack_Update404(t *testing.T) { } cfg, _, errR := config.NewTestConfig() + setTestRepo(cfg) syncStack(cfg, mock, s) cfg.Err.Close() @@ -987,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) { @@ -1381,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 @@ -1424,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 51e8881b..dc8e29b8 100644 --- a/cmd/sync_test.go +++ b/cmd/sync_test.go @@ -2482,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}, @@ -2511,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) { @@ -2529,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") @@ -2552,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_test.go b/cmd/unstack_test.go index c7451f71..f1ec922e 100644 --- a/cmd/unstack_test.go +++ b/cmd/unstack_test.go @@ -16,109 +16,39 @@ import ( "github.com/stretchr/testify/require" ) -func TestUnstackNumberOnGitHub_StackLinks(t *testing.T) { - for _, outcome := range []string{"partial", "dissolved", "not found"} { - for _, force := range []string{"0", "1"} { - t.Run(outcome+"/"+force, func(t *testing.T) { - t.Setenv("GH_STACK_HYPERLINKS", force) - cfg, outR, errR := config.NewTestConfig() - setTestRepo(cfg) - calls := 0 - client := &github.MockClient{ - UnstackFn: func(number int) (*github.RemoteStack, bool, error) { - calls++ - assert.Equal(t, 7, number) - if outcome == "not found" { - return nil, false, &api.HTTPError{StatusCode: 404} - } - if outcome == "dissolved" { - return nil, true, nil - } - return &github.RemoteStack{ID: 6154, Number: 7, PullRequests: []int{42}}, false, nil - }, - } - err := unstackNumberOnGitHub(cfg, client, 7, false) - output := collectOutput(cfg, outR, errR) - assert.Equal(t, 1, calls) - switch outcome { - case "not found": - assert.ErrorIs(t, err, ErrNotInStack) - assert.Contains(t, output, "stack #7 not found on GitHub") - case "dissolved": - require.NoError(t, err) - assert.Contains(t, output, "Stack removed on GitHub (stack #7)") - case "partial": - require.NoError(t, err) - 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\\" - } - assert.Contains(t, output, "Pull requests unstacked on GitHub (stack "+link+")") - } - if outcome != "partial" { - assert.NotContains(t, output, "/stack/") - assert.NotContains(t, output, "\x1b]8") - } - }) - } - } -} - -func TestUnstack_LocalMissingStackLinks(t *testing.T) { - for _, localRepo := range []bool{false, true} { - name := "remote-only context" - if localRepo { - name = "local repository" - } - t.Run(name, func(t *testing.T) { - t.Setenv("GH_STACK_HYPERLINKS", "1") - if localRepo { - dir := t.TempDir() - writeStackFile(t, dir, stack.Stack{Number: 42, Trunk: stack.BranchRef{Branch: "main"}}) - restore := git.SetOps(&git.MockOps{ - GitDirFn: func() (string, error) { return dir, nil }, - }) - defer restore() - } else { - defer mockRemoteOnlyGit()() - } +func TestUnstack_NumberWithoutLocalRepository(t *testing.T) { + 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{ - ListStacksFn: func() ([]github.RemoteStack, error) { - t.Fatal("--local must not look up remote stacks to format a link") - return nil, nil - }, - UnstackFn: func(int) (*github.RemoteStack, bool, error) { - t.Fatal("--local must not unstack on GitHub") - return nil, false, nil + UnstackFn: func(number int) (*github.RemoteStack, bool, error) { + unstacked = number + return nil, true, nil }, } - err := runUnstack(cfg, &unstackOptions{stackNumber: 7, local: true}) - output := collectOutput(cfg, outR, errR) - assert.ErrorIs(t, err, ErrNotInStack) - assert.Contains(t, output, "stack \x1b]8;;https://github.com/o/r/stack/7\x1b\\#7\x1b]8;;\x1b\\ is not tracked locally") + 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") }) } } -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 - }, - } - 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) { t.Helper() sf := &stack.StackFile{ @@ -573,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() @@ -591,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 @@ -603,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") @@ -613,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() @@ -630,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"} @@ -640,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) @@ -648,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{ @@ -664,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 @@ -674,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, "⚠") diff --git a/cmd/utils_test.go b/cmd/utils_test.go index eafe7a75..fb8abcd2 100644 --- a/cmd/utils_test.go +++ b/cmd/utils_test.go @@ -1706,80 +1706,6 @@ func setTestRepo(cfg *config.Config) { cfg.RepoOverride = &repository.Repository{Host: "github.com", Owner: "o", Name: "r"} } -func TestUnstackAndKeepLocal_StackLinks(t *testing.T) { - for _, outcome := range []string{"already merged", "partial", "dissolved"} { - for _, force := range []string{"0", "1"} { - t.Run(outcome+"/"+force, func(t *testing.T) { - t.Setenv("GH_STACK_HYPERLINKS", force) - dir := t.TempDir() - writeStackFile(t, dir, stack.Stack{ - ID: "6154", Number: 7, Trunk: stack.BranchRef{Branch: "main"}, - Branches: []stack.BranchRef{ - {Branch: "b1", PullRequest: &stack.PullRequestRef{Number: 10}}, - {Branch: "b2", PullRequest: &stack.PullRequestRef{Number: 11}}, - }, - }) - sf, err := stack.Load(dir) - require.NoError(t, err) - s := &sf.Stacks[0] - remote := &github.RemoteStack{ID: 6154, Number: 7, PullRequests: []int{10, 11}} - if outcome == "already merged" { - s.Branches[0].PullRequest.Merged = true - remote.PullRequests = []int{10} - } - calls := 0 - client := &github.MockClient{ - UnstackFn: func(number int) (*github.RemoteStack, bool, error) { - calls++ - assert.Equal(t, 7, number) - if outcome == "dissolved" { - return nil, true, nil - } - mergedAt := "2026-01-01T00:00:00Z" - return &github.RemoteStack{ - ID: 6154, Number: 7, PullRequests: []int{10}, - PRDetails: []github.RemoteStackPR{{Number: 10, MergedAt: &mergedAt}}, - }, false, nil - }, - } - cfg, outR, errR := config.NewTestConfig() - setTestRepo(cfg) - err = unstackAndKeepLocal(cfg, client, sf, s, remote, dir) - output := collectOutput(cfg, outR, errR) - require.NoError(t, err) - if outcome == "already merged" { - assert.Zero(t, calls) - } else { - assert.Equal(t, 1, calls) - } - if outcome == "dissolved" { - assert.Contains(t, output, "Remote stack removed (stack #7)") - assert.NotContains(t, output, "/stack/") - assert.NotContains(t, output, "\x1b]8") - } else { - 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\\" - } - assert.Contains(t, output, "Merged pull requests remain stacked on GitHub (stack "+link+")") - if outcome == "partial" { - assert.Contains(t, output, "Pull requests unstacked on GitHub (stack "+link+")") - } - } - saved, err := stack.Load(dir) - require.NoError(t, err) - require.Len(t, saved.Stacks, 1) - assert.Empty(t, saved.Stacks[0].ID) - assert.Zero(t, saved.Stacks[0].Number) - assert.Equal(t, []string{"b1", "b2"}, saved.Stacks[0].BranchNames()) - if outcome != "dissolved" { - assert.True(t, saved.Stacks[0].Branches[0].PullRequest.Merged) - } - }) - } - } -} - func TestLinkedStackLabel(t *testing.T) { cfg := &config.Config{} setTestRepo(cfg) diff --git a/cmd/view_test.go b/cmd/view_test.go index 198c0552..d8410f8f 100644 --- a/cmd/view_test.go +++ b/cmd/view_test.go @@ -17,11 +17,7 @@ import ( ) func TestViewStackLinks(t *testing.T) { - restore := git.SetOps(&git.MockOps{ - IsAncestorFn: func(string, string) (bool, error) { return true, nil }, - }) - defer restore() - + t.Setenv("GH_STACK_HYPERLINKS", "1") for _, render := range []struct { name string fn func(*config.Config, *stack.Stack, string) error @@ -29,78 +25,20 @@ func TestViewStackLinks(t *testing.T) { {"short", viewShort}, {"static", viewFullStatic}, } { - for _, tt := range []struct { - name, force string - number int - want string - }{ - {"hyperlink", "1", 7, "Stack \x1b]8;;https://github.com/o/r/stack/7\x1b\\#7\x1b]8;;\x1b\\"}, - {"fallback", "0", 7, "Stack #7 (https://github.com/o/r/stack/7)"}, - {"unknown", "1", 0, ""}, - } { - t.Run(render.name+"/"+tt.name, func(t *testing.T) { - t.Setenv("GH_STACK_HYPERLINKS", tt.force) - cfg, outR, errR := config.NewTestConfig() - defer outR.Close() - defer errR.Close() - defer cfg.Err.Close() - defer cfg.Out.Close() - cfg.NonInteractive = true - setTestRepo(cfg) - cfg.GitHubClientOverride = &github.MockClient{} - s := &stack.Stack{ - ID: "6154", Number: tt.number, - Trunk: stack.BranchRef{Branch: "main"}, - Branches: []stack.BranchRef{{ - Branch: "feature", - PullRequest: &stack.PullRequestRef{ - Number: 42, URL: "https://github.com/o/r/pull/42", - }, - }}, - } - require.NoError(t, render.fn(cfg, s, "feature")) - require.NoError(t, cfg.Out.Close()) - raw, err := io.ReadAll(outR) - require.NoError(t, err) - if tt.want == "" { - assert.NotContains(t, string(raw), "Stack #") - assert.NotContains(t, string(raw), "/stack/") - } else { - assert.Contains(t, string(raw), tt.want) - } - assert.NotContains(t, string(raw), "/stack/6154") - assert.NotContains(t, string(raw), "/stack/42") - if tt.force == "0" { - assert.NotContains(t, string(raw), "\x1b]8") - } - }) - } + 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 TestViewJSON_StackLinksDoNotChangeOutput(t *testing.T) { - t.Setenv("GH_STACK_HYPERLINKS", "1") - cfg, outR, errR := config.NewTestConfig() - defer cfg.Out.Close() - defer cfg.Err.Close() - defer outR.Close() - defer errR.Close() - setTestRepo(cfg) - s := &stack.Stack{Number: 7, Trunk: stack.BranchRef{Branch: "main"}} - require.NoError(t, viewJSON(cfg, s, "main")) - require.NoError(t, cfg.Out.Close()) - raw, err := io.ReadAll(outR) - require.NoError(t, err) - var out map[string]json.RawMessage - require.NoError(t, json.Unmarshal(raw, &out)) - assert.Len(t, out, 3) - assert.Contains(t, out, "trunk") - assert.Contains(t, out, "currentBranch") - assert.Contains(t, out, "branches") - assert.NotContains(t, string(raw), "\x1b") - assert.NotContains(t, string(raw), "/stack/") -} - func TestTimeAgo(t *testing.T) { tests := []struct { name string @@ -126,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{ @@ -208,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)