Skip to content

Open the selected item's links with o - #31

Open
than wants to merge 1 commit into
mainfrom
link-fixes
Open

than wants to merge 1 commit into
mainfrom
link-fixes

Conversation

@than

@than than commented Oct 1, 2026

Copy link
Copy Markdown
Owner

With mouse capture on the terminal no longer opens links itself, so the viewer does (merged in #30 for bare and inline URLs, and #24 for markdown links and autolinks). This adds the keyboard route:

  • o opens the selected item's first link, and again for the next one. The status bar says which (2 of 3), and the key hint shows o open link only when the item has a link.
  • A question that contains a link is still found, wrapped, and colored. Word matching now uses the visible text with hyperlink sequences removed (plainText); stripANSI still keeps them so a link whose target alone changed reads as a changed line.

go test ./... passes, including o cycling through an item's links and a question holding a link.

🤖 Generated with Claude Code

o opens the selected item's first link, and again for the next, so a link is
reachable whatever the terminal does with mouse clicks. A question that
contains a link is still found, wrapped, and colored: word matching now looks
at the visible text, with hyperlink sequences removed.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Oct 1, 2026

Copy link
Copy Markdown

Review

The o key itself looks good: it stays small and on-scope, linkIdx resets in selectItem when the selection changes, the wrap-around guards against a stale index after a reload, hint() is safe because fixItemCursor keeps the selection within itemStarts, and openLink's scheme allowlist still applies. The test changes (https://example.com/pull/7 → example.com/pull/7) are a real fix. Before, lineWith used stripANSI, which keeps the OSC 8 sequences, so strings.Index matched the URL inside the escape payload instead of the visible text. go test ./... passes locally. This PR doesn't touch the watcher, the reload/scroll logic, or the colors.

One substantive issue, about the second half of the PR (questions that contain links):

a) A link in a question is redrawn as plain text, so o and clicks can't reach it, and a long URL can overflow the pane

When markBlock matches, rewriteMarks throws away glamour's lines for the question and puts reflow(questionPrefix, plainMark(askText(it)), width, questionStyle) in their place. reflow builds the rows from the raw markdown text, so:

  • The URL loses its OSC 8 hyperlink. linkSpans finds nothing on those rows, so itemLinks can't list it, o skips it (or says "no link on this item" when the question holds the item's only link), and clicking or hovering does nothing. This PR advertises link-bearing questions as supported, so o really should be able to open them.
  • The URL keeps its full https://… length. The inline scheme-drop and the truncation to fit the pane don't apply to it. reflow never breaks a word: when fresh is true it appends the word whatever its width. So a question holding a URL longer than about width-4 columns produces a row wider than the pane, which is the hard requirement this tool exists for. The new test doesn't catch it because its URL (52 cols) fits the 58-col test width. Try a typical GitHub …/pull/N/files#diff-<sha> URL at width 40.
  • [label](url) in an Ask: line comes out as literal [label](url).

Some of this probably predates the PR: the first-line prefix match already succeeded with stripANSI. But by making markBlock match these questions reliably, this PR makes the raw-text redraw the normal path for any question with a link.

Two possible fixes:

  • Reflow the rendered words instead of askText: take the Fields from glamour's lines [f, l], which keep their OSC 8 wrappers and pane-fitted URL text, and measure each one with visibleWidth. Hyperlinks, truncation, and the width limit then come for free.
  • Or, more simply, skip the rewrite when the question contains a link, and leave glamour's own wrapping in place.

Either way, add a test that renders a question with a URL longer than the width and asserts two things: every line's visibleWidth is <= width, and itemLinks on that item includes the URL.

Minor (optional)

  • When openLink refuses or fails, the counter is appended to the error: won't open that kind of link (1 of 2 — o for the next). It reads fine, and advancing past a bad link is the right behavior. Just confirming it's intentional.

Apart from (a), this is ready to merge.

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.

1 participant