Skip to content

feat(style): hyperlink PR numbers in submit output (#105)#107

Merged
boneskull merged 2 commits intoboneskull:mainfrom
mvanhorn:feat/hyperlink-pr-numbers-issue-105
May 8, 2026
Merged

feat(style): hyperlink PR numbers in submit output (#105)#107
boneskull merged 2 commits intoboneskull:mainfrom
mvanhorn:feat/hyperlink-pr-numbers-issue-105

Conversation

@mvanhorn
Copy link
Copy Markdown
Contributor

Closes #105.

What changed

`submit` now wraps the `PR #N` token in its progress line with an OSC 8 terminal hyperlink, so iTerm2 / kitty / recent macOS Terminal / GNOME Terminal users can click the PR number to jump to the URL. Terminals without hyperlink support (or sessions where output is piped) get a plain `text (URL)` fallback so the URL is still visible.

Before:

```
Updating PR #3150 for boneskull/additional-locations (base: master)... ok
```

After (TTY with OSC 8): the `PR #3150` is a clickable link to the PR URL.

After (no TTY / NO_COLOR):

```
Updating PR #3150 (https://github.com/boneskull/gh-stack/pull/3150) for boneskull/additional-locations (base: master)... ok
```

How

  • New `internal/style/style.go` method `Hyperlink(text, url)`:
    • Colors/TTY enabled → `\x1b]8;;URL\x1b\TEXT\x1b]8;;\x1b\` (OSC 8).
    • Disabled → `"text (url)"`.
    • Empty URL → returns text alone, no parens.
  • `cmd/submit.go:465` updated to use `s.Hyperlink(fmt.Sprintf("PR #%d", d.prNum), ghClient.PRURL(d.prNum))` in the `Updating ...` line. The `(base: parent)` suffix and `...` / `ok` / `failed` trailers are unchanged so existing test snapshots and TTY-detection logic are unaffected.
  • `internal/style/style_test.go` (new) covers all three branches.

Why this matches the issue

Quoted from the issue (#105):

wherever terminal links are available, the text `PR #nnnn` should be the linked text.

That's the OSC 8 path. The fallback is what the issue showed as the alternate render.

Verification

  • `go build ./...` clean.
  • `go test ./internal/style/...` — 3 new tests pass.
  • `go test ./cmd/...` — `cmd` tests pass (cached, no relevant impl changes there).
  • `go test ./e2e/...` — same set of e2e dry-run tests fail on `main` as on this branch (pre-existing `git push -u origin main` env issue, unrelated to this change). Verified by stashing and re-running.

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 boneskull#105
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds terminal hyperlink support (OSC 8) for the PR #N token in submit’s “Updating …” progress output, with a non-TTY / NO_COLOR fallback that keeps the PR URL visible.

Changes:

  • Introduces (*style.Style).Hyperlink(text, url) to render OSC 8 hyperlinks when styling is enabled, otherwise text (url) (or text when URL is empty).
  • Updates cmd/submit.go to render the PR number in the update progress line using Hyperlink(...).
  • Adds unit tests covering the enabled/disabled/empty-URL branches.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
internal/style/style.go Adds Hyperlink helper for OSC 8 hyperlinks with fallback formatting.
internal/style/style_test.go Adds tests for hyperlink rendering across enabled/disabled/empty URL cases.
cmd/submit.go Uses Style.Hyperlink when printing “Updating PR …” progress output.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/style/style.go Outdated
Comment on lines +202 to +205
if !s.enabled || url == "" {
if url == "" {
return text
}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mvanhorn This seems valid, but also I'm not too terribly concerned about piped use-cases.

Comment thread internal/style/style_test.go Outdated
Comment on lines +24 to +28
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)
Copy link
Copy Markdown
Owner

@boneskull boneskull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mvanhorn Thanks for the PR! In general this looks good, but please address the two review comments left by the clanker.

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.
@mvanhorn
Copy link
Copy Markdown
Contributor Author

mvanhorn commented May 8, 2026

@boneskull addressed both in 05b7f66:

  • Hyperlink now gates OSC 8 on s.enabled && s.isTTY (added isTTY field, populated via getTermState().IsTerminalOutput() in New()). So forced color while piping no longer leaks escape sequences.
  • TestHyperlinkColorsEnabled now asserts the full OSC 8 sequence including the closing \x1b]8;;\x1b\\ terminator instead of substring-checking.

Verified locally: go build ./..., go test ./internal/style/..., go vet, gofmt all clean.

@boneskull boneskull merged commit 32a9534 into boneskull:main May 8, 2026
7 checks passed
@boneskull
Copy link
Copy Markdown
Owner

@mvanhorn TYVM

@boneskull
Copy link
Copy Markdown
Owner

@all-contributors please add @mvanhorn for code

@allcontributors
Copy link
Copy Markdown
Contributor

@boneskull

I've put up a pull request to add @mvanhorn! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

submit should show link to PR, not just PR number, when updating PRs

3 participants