Keep wrapped URLs clickable (#15)

[than]

Overview

A bare URL is the board's own convention for something a person should be able to click straight from the pane. That convention broke down the moment a URL outgrew the render width: glamour's word‑wrap doesn't know a URL is a single, indivisible token, so it force‑breaks it mid‑character wherever the width boundary happens to fall — and the terminal's native link matcher, which this project deliberately relies on instead of capturing the mouse, sees two dead fragments instead of one link.

Approach

Emit OSC 8 hyperlinks around bare URLs, with the visible text pre‑shortened so it never gets a chance to wrap — a hybrid of the two candidates in the issue.

Pure OSC 8 (option a) doesn't survive this pipeline unmodified: muesli/reflow's ANSI‑aware word‑wrap and width counter only recognize a narrow CSI terminator range ([0-9;]*[A-Za-z]), with no notion of OSC sequences. Feed it an OSC 8 escape carrying a URL and the first ordinary lowercase letter inside that URL reads as a premature sequence terminator — wrapping and width math both corrupt.

So the fix does the shrink first: any bare‑URL‑only line (the existing board convention — a URL alone on its own list item or line) that's wider than the render width gets truncated to fit, ellipsis included, before it reaches glamour. That guarantees glamour's own word‑wrap never has to force‑break it — it's already short enough. Only after rendering and tidying is the OSC 8 hyperlink stitched back on, wrapping the truncated visible text and pointing at the untouched, full URL. In a terminal that honors OSC 8 (Ghostty included), the whole truncated line is clickable and opens the real target regardless of what's visually shown. In one that doesn't, the line simply reads as short, plain, ellipsis‑terminated text — no worse than a truncated bookmark title.

visibleWidth and stripANSI were both taught to strip OSC 8 sequences before measuring or diffing, so the hyperlink's URL payload can never leak into a width check or a change comparison. Short URLs that already fit are left completely alone — no new styling, no risk to the existing autolink path, and the pre‑existing tests for the fixture's short URLs pass unmodified.

Trade-offs

• Truncation is visual, not just theoretical. A very long URL now shows as https://github.com/… rather than in full — some pre‑click context is lost, though the OSC 8 target underneath is always the untruncated URL, so nothing is ever unreachable.
• Scoped to bare-URL-only lines, matching the board convention called out in the issue. A URL embedded mid‑sentence still wraps the old way; broadening the fix to inline links is a reasonable follow‑up if it turns out to matter in practice.
• BEL-terminated OSC 8, not ST. ESC \ never gets recognized as a terminator by this codebase's ANSI helpers (they only match [A-Za-z]), so an ST‑terminated sequence would appear to run forever. BEL is a single byte and terminates cleanly under the same logic already used for CSI codes.

Testing

• New regression tests in render_test.go: TestLongBareURLNeverWraps (a URL wider than the render width stays on one line across widths 24/40/78, and its OSC 8 target carries the full untruncated URL) and TestOSC8IsInvisible (the hyperlink escape never counts toward visibleWidth or survives stripANSI).
• New unit tests in linkify_test.go covering truncateBareURLs (short URLs left alone, inline URLs left alone, long ones shortened to fit) and findPlainRange (ANSI-aware substring location, including the no-match case).
• Full suite: go test ./... and go vet ./... both clean.

Closes #15

🤖 Generated with Claude Code

[claude]

Review: keep wrapped URLs clickable

The approach is right — shrink before glamour sees it, stitch the OSC 8 back on after. The reasoning about reflow not understanding OSC sequences is correct, and teaching visibleWidth/stripANSI to skip OSC 8 is the right call: applyLineBg padding and changedLines both go through those, so the flash and the change-pointer stay correct. Nothing here touches the watcher, the reload/scroll path, or p.Send, and the link styling stays truecolor off colorLink. Three things to fix before merge.

a. Two truncations with the same display text mislink (real bug)

linkifyTruncations searches rendered from offset 0 on every iteration, and the first replacement leaves the display runes visible (the escapes go around them, not inside). So when two truncations share a display string, the second iteration re-finds the first occurrence and nests a second OSC 8 inside the first. Net effect: the first line links to the second URL, and the second line gets no hyperlink at all. Visible width is unchanged, so no existing test notices.

Sharing a display string is the common case, not a corner — two URLs collide whenever they agree on their first budget-1 runes. testdata/REVIEW.md already does it: .../pull/412 and .../pull/415 are both 39 runes, so at width 40 (a width TestNeverWiderThanWidth already exercises) budget is 38, keep is 37, and both truncate to the identical https://github.com/example/app/pull/4….

Truncations are produced in document order and render in document order, so a cursor fixes it:

cursor := 0
for _, t := range truncations {
    start, end, ok := findPlainRange(rendered[cursor:], t.display)
    if !ok {
        continue
    }
    start, end = start+cursor, end+cursor
    // ... build styled ...
    rendered = rendered[:start] + styled + rendered[end:]
    cursor = start + len(styled)
}

Worth a regression test: two bare URLs sharing a long prefix, asserting both full targets appear as OSC 8 opens, in order.

b. truncateBareURLs rewrites inside fenced code blocks

bareURLLineRE runs over every raw line with no fence tracking, so a lone URL inside a fenced block gets ellipsized and then hyperlinked. That silently alters content the user typed verbatim, and code blocks are not word-wrapped by glamour anyway, so there is nothing to fix there in the first place. Track fence state (a line of three backticks or three tildes toggling in/out) and skip lines inside. Indented code blocks are already safe, since the regex anchors the URL at column 0 when there is no bullet prefix.

c. reserve = 2 only holds for a top-level bullet

reserve is hard-coded to 2 for the • prefix, but List.LevelIndent is 2 as well, so a nested item ( - https://...) carries that indent on top of the bullet. The budget is then too generous by the indent, the rendered line goes over width, and glamour force-breaks the truncated text — the exact bug this PR exists to fix. Worse, once it breaks, findPlainRange cannot match across the break, so it takes the silent continue and the link is gone too: truncated text pointing nowhere, strictly worse than before. Deriving reserve from the matched prefix (leading whitespace length + 2) plus a nested-bullet case in TestLongBareURLNeverWraps at ~30 columns would cover it.

More generally, that silent continue is the one swallowed path here. If the display text cannot be located, the text has already been shortened and the target is now unreachable — better to not truncate at all than to truncate without attaching a link.

Smaller notes

• > https://... and - [ ] https://... do not match bareURLLineRE, so those still force-break. Fine as scope; just noting the fix is narrower than it reads.
• The README rendering-style bullet still says bare URLs stay intact and clickable. Worth a clause that over-width ones are shortened, with the full URL carried as the OSC 8 target.
• indexRunes, the empty-needle branch is unreachable — findPlainRange already rejects an empty plain. Collapse it.
• budget < 8 cannot trigger: width is clamped to 10 just above and reserve maxes at 2.
• findPlainRange builds three full-document slices per truncation. Fine at board sizes; the cursor fix above also shrinks each scan to the remainder.

[claude]

Read the whole diff plus diff.go, ui.go, main.go. go test ./... and go vet ./... are clean here too. The approach is sound and the code is unusually careful — the reasoning in the comments about why reflow can't see OSC 8, and why the shrink has to happen before glamour, is correct and worth having written down.

What holds up. The watcher, scroll preservation, and the p.Send path are untouched. The never-wider-than-the-pane requirement survives: visibleWidth and stripANSI both neutralize OSC 8 before measuring, and tidy still runs before linkifyTruncations, so no trailing-space padding is reintroduced. Colors stay hex/truecolor. The forward-only cursor in linkifyTruncations is the right design and the collision test pins it. The unresolved-to-untruncated fallback is a good safety valve.

Real problems below, most important first.

a. Change detection goes blind on exactly the lines this PR creates

changedLines diffs on stripANSI(l), and stripANSI now strips the OSC 8 wrapper including the target. So the only surviving representation of a long URL in the compared text is the truncated display string — and two URLs sharing a prefix longer than the budget produce the identical display string.

Concretely, at a 40-column pane: renderWidth 38, reserve 2, budget 36. https://github.com/than/sidecar/pull/17 is 39 chars, so it truncates to https://github.com/than/sidecar/pul…. Edit the board to point at /pull/18 and the rendered visible text is byte-identical — no ▸ marker, no line flash. The README sells "points at what changed"; this silently disables it for precisely the long-URL lines the PR targets, and only in the narrow panes this tool is built for.

Fix is small: give the diff a comparison key that retains the hyperlink target, e.g. in changedLines use ansiRE.ReplaceAllString(l, "") (SGR only) instead of stripANSI(l). stripANSI itself should keep stripping OSC 8 — isBulletLine and the width path want that.

b. --static into a pipe loses the URL outright

runStatic is documented as "handy for piping, CI", and falls back to width 80 when stdout isn't a TTY. Truncation now applies there too, so a redirected board renders https://github.com/… with the only copy of the full URL buried inside an escape sequence that grep, an editor, or a CI log viewer won't surface. In the TUI the OSC 8 target makes truncation lossless; in a pipe it doesn't. Consider skipping truncateBareURLs when stdout isn't a terminal — runStatic already knows, since it calls term.GetSize.

c. Control bytes in a URL escape the OSC 8 sequence

bareURLLineRE captures https?://\S+, and \S happily matches a raw BEL or ESC. osc8Open emits t.full verbatim, so a board line containing https://x.test/<BEL>evil terminates the OSC early and hands the remaining bytes to the terminal as commands. osc8RE has the mirror problem — [^\x07]* stops at the embedded BEL, so visibleWidth and stripANSI both mis-measure that line. The board is written by an agent, so the content is effectively arbitrary. One-line guard: skip truncation when the URL contains any byte outside 0x20–0x7e, which is what OSC 8 requires of the target anyway.

d. Budget is counted in runes, not cells (nit)

len([]rune(url)) <= budget and the rune-slice cut both assume one cell per rune. An IDN or otherwise non-ASCII bare URL measures short and renders wide, glamour wraps it anyway, findPlainRange misses, and the fallback quietly reverts to the pre-fix broken behavior. Using visibleWidth for the check and cutting on accumulated cell width would make it consistent with the rest of the file.

e. Fence tracking ignores the fence character (nit)

fenceRE toggles inFence on any ``` or ~~~ line regardless of which opened the block, so a ~~~ line inside a backtick fence flips the state and the rest of that code block becomes eligible for truncation. Recording the opening fence's character and requiring a match to close would fix it.

f. Coverage gap, for the record (nit)

The PR documents the bare-URL-only scoping as a trade-off, which is fair. Worth naming the shapes that still fall through: 1. https://…, > https://…, and - [ ] https://…. The task-list form is the likeliest to bite on a to-do board, since Task is a styled block in styleConfig.

One thing to eyeball

No test exercises the full TUI path, only renderMarkdown directly. Bubble Tea's renderer and the viewport do their own line-width work on the final frame, and whether those helpers understand OSC 8 (charmbracelet/x/ansi does; muesli/reflow does not) decides whether a hyperlinked line survives the last hop intact. Worth one manual look in Ghostty at roughly a 40-column pane with a long URL before merging — if the wrong helper is in that path, the symptom is a line cut mid-escape and the rest of the frame swallowed, which no unit test here would catch.

Item (a) is the one I'd want fixed before merge; (b) and (c) are cheap. The rest are nits.

[claude]

Reviewed at c950fa5. go build, go vet, and go test ./... are all clean locally.

Overall: solid

The core move — shrink the line before glamour, stitch the OSC 8 back on after — is the right shape for this pipeline, and the diagnosis is correct: reflow's width counter and wordwrap only recognise CSI, so an OSC 8 escape fed through them corrupts on the first lowercase letter in the URL. Working around that instead of through it was the right call.

The invariants I care about all hold:

• Width. Nothing renders wider than the pane. visibleWidth strips OSC 8 before measuring, and the failure mode when the reserve estimate is wrong is a wrap — which glamour bounds to the width — so findPlainRange misses and the untruncated fallback kicks in. Degrading into "the old bug" rather than "an over-wide line" is the correct direction to fail, and renderMarkdown's retry-with-skip makes that explicit rather than silent.
• Compact spacing / no trailing padding. linkifyTruncations runs after tidy, and only substitutes bytes within a line — the replacement contains no newline and no trailing space, so neither invariant can be disturbed. Ordering here matters and it's right.
• Colors. The injected style is truecolor derived from colorLink; no palette indexes.
• Bare URLs. Short URLs are left completely alone — no new escapes on the existing autolink path. That's the important part.
• Watcher / scroll / concurrency. Untouched, and correctly so. Everything new in linkify.go is pure functions over strings; the package-level vars are compiled regexps, so the watcher goroutine's p.Send path gains no shared mutable state.

The changedLines split from stripANSI is subtle but justified, and the comment earns its length. Test coverage is genuinely good — the colliding-display-text and nested-indent regressions are exactly the ones I'd have gone looking for.

Worth fixing

a) The osc8Safe skip doesn't advance the cursor in linkifyTruncations.

The forward-only cursor is what keeps colliding display strings from stacking onto the first occurrence — but the unsafe-URL branch continues before the search, leaving the cursor where it was. So if an unsafe URL and a later safe one truncate to the same display text:

- https://example.test/aaaa…<BEL>x
- https://example.test/aaaa…y

...the second truncation searches from the stale cursor, finds the first line's text, and hyperlinks it to the second URL — the wrong line gets the link, and the intended line gets none. This is the same failure the cursor was added to prevent, just reached through the other early exit. Needs a control byte in a URL to trigger, so it's low severity, but the fix is small: on the unsafe skip, still run findPlainRange and advance cursor past the match before continuing.

b) The ellipsis is assumed to be one cell wide.

cutToCellWidth(url, budget-1) hardcodes 1 for the …. U+2026 is East Asian Ambiguous — go-runewidth's default condition picks up RUNEWIDTH_EASTASIAN, and failing that infers from LANG/LC_CTYPE, so under a CJK locale RuneWidth('…') is 2. The display text then comes out one cell over budget, glamour wraps it, findPlainRange misses, and every long URL silently falls through to the untruncated path — issue 15 quietly unfixed for those users, with no width violation to show for it. Use runewidth.RuneWidth('…') instead of the literal 1 (or measure the assembled string). Same argument you already make for cell-width vs rune-count, one layer up.

c) osc8RE's [^\x07]* crosses newlines.

Safe today because open and close are always emitted as a pair inside one line. But stripANSI is a general helper and tests already hand it whole documents — if an unpaired open ever reaches it, the match runs to the next BEL anywhere downstream and eats visible text along the way. [^\x07\n]* costs nothing and makes the bound structural rather than incidental.

One design note, not a blocker

In a terminal that ignores OSC 8, the visible https://github.com/exa… is still shaped like a URL, so that terminal's own matcher will likely linkify the valid-looking prefix. That's arguably worse than the dead fragments it replaces: a clickable link that quietly goes somewhere else beats one that goes nowhere, for the wrong reasons. The trade-offs section frames this as "no worse than a truncated bookmark title," which undersells it slightly. Not something to change here — OSC 8 support is the common case and Ghostty is the stated target — but worth naming honestly in the README line if you touch it again.

[claude]

Review

Overall this is careful work. The core insight — shrink before glamour, hyperlink after tidy — is right, the fenced-code and cursor-collision handling are correct, and the failure mode when the reserve estimate is wrong (fall back to untruncated) degrades to the pre-fix behavior rather than to something worse. visibleWidth/stripANSI correctly treat OSC 8 as invisible, so the "never wider than the pane" invariant and the ▸/flash pipeline (isBulletLine, applyLineBg, composeMarked) all still hold. Scroll preservation and the watcher are untouched. Three things below.

---

a. Non-ASCII URLs lose their tail with no way to recover it

osc8Safe rejects every byte outside 0x20–0x7e, which rejects all of UTF-8. But truncateBareURLs truncates regardless of osc8Safe — so for a URL containing any non-ASCII rune (IDN host, unencoded path segment, a Wikipedia link with an accent), a long line gets:

• visibly truncated to https://ex…, and
• no OSC 8 target attached (linkifyTruncations skips it and does not report it unresolved, so the fallback retry never fires).

The full URL is then unrecoverable from the rendered output. That is a regression against the pre-PR behavior, where the URL wrapped but every character was at least on screen. TestWideRuneURLNeverWraps exercises exactly this shape and only asserts width, so it passes while the URL is silently destroyed.

The right fix is to percent-encode rather than reject. OSC 8 wants a URI; percent-encoding non-ASCII is the standard transform and is what the target should carry anyway:

// osc8Target returns url encoded for use as an OSC 8 target: bytes outside
// printable ASCII are percent-encoded. Returns false only for bytes that
// cannot appear in a URI at all (C0 controls, DEL) — those would terminate
// or restart our own escape.
func osc8Target(url string) (string, bool) {
    var b strings.Builder
    for i := 0; i < len(url); i++ {
        c := url[i]
        switch {
        case c < 0x20 || c == 0x7f:
            return "", false
        case c > 0x7e:
            fmt.Fprintf(&b, "%%%02X", c)
        default:
            b.WriteByte(c)
        }
    }
    return b.String(), true
}

The BEL/ESC injection guard — the part that actually matters — is preserved, and legitimate content stops being collateral damage. If you would rather keep the scope tight, the minimum acceptable alternative is to skip truncation too when osc8Safe fails, so those URLs keep the old wrap-but-intact behavior instead of becoming inert truncated text.

---

b. Please verify bubbletea does not re-truncate these lines with reflow

This is the one I could not check from this environment, and it is the highest-consequence item if it holds.

visibleWidth was taught to strip OSC 8 before handing anything to reflow — correct, and the PR description explains exactly why (reflow's ansi.IsTerminator matches [A-Za-z], so the first lowercase letter of the URL inside the escape ends the sequence early and the rest of the URL counts as visible text). But that fix only covers this repo's calls into reflow.

bubbletea's standardRenderer.flush() has historically truncated every line it writes to the terminal width using github.com/muesli/reflow/truncate, which has the same non-OSC-aware scanner. If v1.3.10 still does that, a linkified line measures as roughly width + len(url) - 6 cells to reflow, gets cut inside the escape's URL payload, and the actual visible text — which comes after the escape — is dropped entirely, leaving an unterminated OSC 8 sequence on the wire. Every test here operates on the return value of renderMarkdown, so none would catch it; it would only show up in the real TUI.

Worth a direct check before merge, e.g.:

out, _ := renderMarkdown("- "+longURL+"\n", 38) // pane 40
for _, line := range strings.Split(out, "\n") {
    if got := stripANSI(truncate.String(line, 40)); got != stripANSI(line) {
        t.Errorf("renderer-level truncation ate the line: %q -> %q", stripANSI(line), got)
    }
}

If bubbletea v1.3.x has moved to charmbracelet/x/ansi (which has a real OSC parser), this is a non-issue and the test above documents that. If it has not, the OSC 8 approach needs a guard.

---

c. Minor: visibleWidth now runs a regexp on every line of every frame

osc8RE.ReplaceAllString is unconditional, and visibleWidth is called per line in tidy, in the owner pass of composeMarked, and again in applyLineBg. On a 300-line board that is roughly a thousand regexp scans per recompose, and the flash toggles recompose twice per change — all to serve the handful of lines that ever contain an OSC 8 escape. A cheap guard keeps the common path allocation-free:

func visibleWidth(line string) int {
    if strings.Contains(line, "\x1b]8") {
        line = osc8RE.ReplaceAllString(line, "")
    }
    return reflowansi.PrintableRuneWidth(line)
}

---

Smaller notes, not blocking

• The changedLines split from stripANSI is well reasoned and the comment earns its length. Good catch on the collision case.
• In a terminal without OSC 8 support, the visible https://github.com/exa… will be regex-linkified to a wrong URL rather than to no URL. Roughly the same badness as the dead fragments it replaces, but worth a sentence in the README trade-off note, since the current text implies the shortened text always opens the right place.
• A bare URL that is a lazy-continuation line of a multi-line paragraph matches bareURLLineRE and gets truncated even though the reflowed paragraph might have had room. Harmless (it either resolves or falls back), just not what the "own line" convention means.
• TestTruncateBareURLsAccountsForWideEllipsis mutates the package-global runewidth.DefaultCondition. Fine today since nothing runs parallel, but it will bite whoever adds t.Parallel().