From f5ff6c65bea6d633b07bf119b7668a1b75f15190 Mon Sep 17 00:00:00 2001 From: Matt Van Horn Date: Thu, 30 Apr 2026 03:20:57 -0700 Subject: [PATCH 1/2] feat(style): hyperlink PR numbers in submit output (#105) submit prints "Updating PR #N for branch (base: parent)..." with no URL. Per the issue, the PR number should be the linked text when the terminal supports OSC 8 hyperlinks; otherwise fall back to "text (URL)" so the URL remains visible. Added Hyperlink(text, url) to internal/style/style.go: - Color/TTY enabled: emits OSC 8 escape sequence (\x1b]8;;URL\x1b\\TEXT\x1b]8;;\x1b\\), which renders as a clickable link in iTerm2, kitty, recent macOS Terminal, GNOME Terminal, etc. - Color/TTY disabled (NO_COLOR, piped output, dumb terminal): falls back to "text (url)". - Empty URL: returns text alone, no parens. Updated cmd/submit.go:465 to wrap "PR #N" with Hyperlink() and ghClient.PRURL(d.prNum). The (base: parent) suffix and trailing "... ok" / "failed" are unchanged. Added internal/style/style_test.go covering the three branches (enabled, disabled, empty URL). Closes #105 --- cmd/submit.go | 2 +- internal/style/style.go | 17 +++++++++++++++ internal/style/style_test.go | 40 ++++++++++++++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 1 deletion(-) create mode 100644 internal/style/style_test.go diff --git a/cmd/submit.go b/cmd/submit.go index 4063c80..a183af1 100644 --- a/cmd/submit.go +++ b/cmd/submit.go @@ -462,7 +462,7 @@ func executePRDecisions(g *git.Git, cfg *config.Config, root *tree.Node, decisio if opts.DryRun { fmt.Printf("%s Would update PR #%d base to %s\n", s.Muted("dry-run:"), d.prNum, s.Branch(parent)) } else { - fmt.Printf("Updating PR #%d for %s (base: %s)... ", d.prNum, s.Branch(b.Name), s.Branch(parent)) + fmt.Printf("Updating %s for %s (base: %s)... ", s.Hyperlink(fmt.Sprintf("PR #%d", d.prNum), ghClient.PRURL(d.prNum)), s.Branch(b.Name), s.Branch(parent)) if err := ghClient.UpdatePRBase(d.prNum, parent); err != nil { fmt.Println(s.Error("failed")) fmt.Printf("%s failed to update PR #%d base: %v\n", s.WarningIcon(), d.prNum, err) diff --git a/internal/style/style.go b/internal/style/style.go index e4c5083..d5cd51b 100644 --- a/internal/style/style.go +++ b/internal/style/style.go @@ -191,3 +191,20 @@ func (s *Style) WarningMessage(msg string) string { func (s *Style) FailureMessage(msg string) string { return fmt.Sprintf("%s %s", s.FailureIcon(), s.Error(msg)) } + +// Hyperlink renders text as a terminal hyperlink to url using the OSC 8 +// escape sequence when colors/TTY are enabled. When the terminal can't +// support it (NO_COLOR, piped output, dumb terminal), it falls back to +// "text (url)" so the URL stays visible to the user. +// +// Reference: https://gist.github.com/egmontkob/eb114294efbcd5adb1944c9f3cb5feda +func (s *Style) Hyperlink(text, url string) string { + if !s.enabled || url == "" { + if url == "" { + return text + } + return fmt.Sprintf("%s (%s)", text, url) + } + // OSC 8 hyperlink: ESC]8;;URLESC\TEXTESC]8;;ESC\ + return fmt.Sprintf("\x1b]8;;%s\x1b\\%s\x1b]8;;\x1b\\", url, text) +} diff --git a/internal/style/style_test.go b/internal/style/style_test.go new file mode 100644 index 0000000..859fa6c --- /dev/null +++ b/internal/style/style_test.go @@ -0,0 +1,40 @@ +package style + +import ( + "strings" + "testing" +) + +func TestHyperlinkColorsDisabled(t *testing.T) { + s := NewWithColor(false) + got := s.Hyperlink("PR #42", "https://github.com/owner/repo/pull/42") + want := "PR #42 (https://github.com/owner/repo/pull/42)" + if got != want { + t.Errorf("Hyperlink fallback: got %q, want %q", got, want) + } +} + +func TestHyperlinkColorsEnabled(t *testing.T) { + s := NewWithColor(true) + got := s.Hyperlink("PR #42", "https://github.com/owner/repo/pull/42") + // OSC 8 sequence wraps the visible text + if !strings.Contains(got, "PR #42") { + t.Errorf("Hyperlink should contain visible text; got %q", got) + } + if !strings.Contains(got, "https://github.com/owner/repo/pull/42") { + t.Errorf("Hyperlink should contain URL; got %q", got) + } + if !strings.HasPrefix(got, "\x1b]8;;") { + t.Errorf("Hyperlink should start with OSC 8 sequence; got %q", got) + } +} + +func TestHyperlinkEmptyURL(t *testing.T) { + for _, enabled := range []bool{false, true} { + s := NewWithColor(enabled) + got := s.Hyperlink("PR #42", "") + if got != "PR #42" { + t.Errorf("Hyperlink with empty URL (enabled=%v): got %q, want %q", enabled, got, "PR #42") + } + } +} From 05b7f66e42b5e9e489167096efcebba6d2b3a100 Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Fri, 8 May 2026 01:41:08 -0700 Subject: [PATCH 2/2] fix(style): gate OSC 8 hyperlinks on TTY and assert full sequence Address review feedback from copilot-pull-request-reviewer[bot]: - Hyperlink now requires both s.enabled and s.isTTY before emitting OSC 8, so CLICOLOR_FORCE / GH_FORCE_TTY no longer leaks escape sequences into piped output. NewWithColor(enabled) sets isTTY to match enabled so existing test behavior is preserved. - TestHyperlinkColorsEnabled now asserts the full OSC 8 sequence including the closing terminator, catching malformed or truncated output that the prior substring checks would have missed. --- internal/style/style.go | 13 ++++++++++--- internal/style/style_test.go | 17 ++++------------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/internal/style/style.go b/internal/style/style.go index d5cd51b..6fc3d44 100644 --- a/internal/style/style.go +++ b/internal/style/style.go @@ -15,6 +15,7 @@ import ( // All methods return plain text when colors are disabled. type Style struct { enabled bool + isTTY bool } var ( @@ -54,13 +55,19 @@ func isColorEnabled() bool { // New creates a new Style instance. // Colors are automatically enabled/disabled based on terminal capabilities. func New() *Style { - return &Style{enabled: isColorEnabled()} + return &Style{ + enabled: isColorEnabled(), + isTTY: getTermState().IsTerminalOutput(), + } } // NewWithColor creates a Style with explicit color setting. // Useful for testing or forcing color on/off. func NewWithColor(enabled bool) *Style { - return &Style{enabled: enabled} + return &Style{ + enabled: enabled, + isTTY: enabled, + } } // Enabled returns whether colors are enabled. @@ -199,7 +206,7 @@ func (s *Style) FailureMessage(msg string) string { // // Reference: https://gist.github.com/egmontkob/eb114294efbcd5adb1944c9f3cb5feda func (s *Style) Hyperlink(text, url string) string { - if !s.enabled || url == "" { + if !s.enabled || !s.isTTY || url == "" { if url == "" { return text } diff --git a/internal/style/style_test.go b/internal/style/style_test.go index 859fa6c..576d0ce 100644 --- a/internal/style/style_test.go +++ b/internal/style/style_test.go @@ -1,9 +1,6 @@ package style -import ( - "strings" - "testing" -) +import "testing" func TestHyperlinkColorsDisabled(t *testing.T) { s := NewWithColor(false) @@ -17,15 +14,9 @@ func TestHyperlinkColorsDisabled(t *testing.T) { func TestHyperlinkColorsEnabled(t *testing.T) { s := NewWithColor(true) got := s.Hyperlink("PR #42", "https://github.com/owner/repo/pull/42") - // OSC 8 sequence wraps the visible text - if !strings.Contains(got, "PR #42") { - t.Errorf("Hyperlink should contain visible text; got %q", got) - } - if !strings.Contains(got, "https://github.com/owner/repo/pull/42") { - t.Errorf("Hyperlink should contain URL; got %q", got) - } - if !strings.HasPrefix(got, "\x1b]8;;") { - t.Errorf("Hyperlink should start with OSC 8 sequence; got %q", got) + want := "\x1b]8;;https://github.com/owner/repo/pull/42\x1b\\PR #42\x1b]8;;\x1b\\" + if got != want { + t.Errorf("Hyperlink OSC 8: got %q, want %q", got, want) } }