Uh oh!
There was an error while loading. Please reload this page.
Make the URLs a world prints clickable, wrapped or not - #23
Conversation
The terminal's own URL detection works across the terminal *row*, and an output pane is narrower than one. A long URL wrapped inside a pane is `https://exa` on one row and `mple.com/page` on the next, with a divider and possibly another pane's output in between: neither half is a URL, neither is clickable, and nothing says so. `UrlDetector` (Core) marks the span itself, which moves the decision to the layer that knows where the line really ends — MarkupParser splits a `[link=…]` across every row it wraps onto and MarkupControl hit-tests each row. It runs over the whole line rather than span by span, because a server may change colour mid-URL and matching per span produces two links to two truncated targets: the same defect one layer down. It runs after the emoji substitution so a link's target is exactly the text under it, and skips any run overlapping a span the server already marked up. `ProcessOutputLine` now computes the shown line once and hands the same line to `SpawnLine` and to `Print`. A capture used to receive `result.Line` while the main window received the substituted one, so one line read differently in the two panes it landed in. Clicking an http(s) link in a pane opens the desktop's browser; the built-in web view keeps its own anchors and `/web <url>`. The destination is decided by the surface the click came from, not by the payload — otherwise the built-in browser would eject you on its first in-page link. The scheme is gated at the moment of opening rather than in the detector, because that path also carries what a *server* marked up: `file://`, `javascript:` and the schemes a desktop registers to applications would otherwise let the world choose which program runs. The launcher is caller-supplied and null by default, like save/logRoot/ restore, so a snapshot and a test start no browser. F7 gains `detect links in output`, default on, read per line — the same ingest-time family as `strip incoming colour` and `emoji substitution`, so unticking it stops the next line rather than rewriting history. New `--view links` renders the defect: a split, so the pane is narrower than the terminal, with one underlined span running to the pane's edge and continuing on the next row. Build clean, 0 warnings. Core 869, Tui 1550, Graphics 83, Scripting 42, Web 37 — all passing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN
WalkthroughAutomatic HTTP(S) URL detection now applies to styled output, spawned lines, and demo output. Users can control detection from Text & ANSI settings. Pane links route internally in the web view and externally elsewhere through validated browser launching. ChangesURL Detection and Link Routing
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Session as WorldSession
participant Detector as UrlDetector
participant App as SharpMUTermApp
participant Browser as ExternalBrowser
Session->>Detector: ApplyToLine displayed output
Detector-->>Session: Styled line with link interactions
Session->>App: Render linked pane output
App->>Browser: Validate clicked HTTP(S) target
Browser-->>App: Return canonical URI
App->>Browser: Open validated URI
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/SharpMUTerm.Tui/ExternalBrowser.cs`:
- Around line 53-54: Update ExternalBrowser.Open to validate and canonicalize
the supplied URL internally before launching it, rather than passing the raw
string to ProcessStartInfo. Reuse the existing URL validation scheme gate,
launch only the resulting Uri.AbsoluteUri, and preserve disposal of the started
process.
In `@tests/SharpMUTerm.Tui.Tests/AutoLinkTests.cs`:
- Around line 318-326: Update the XML summary for LinksOn to state that
MarkupParser.Parse parses a single buffered line without pane-width wrapping and
returns its link spans. Distinguish it from PaneRowLinks, which applies the pane
width and returns the rows actually painted.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c170f5a6-712d-4424-8532-7b19e8101c17
📒 Files selected for processing (14)
CLAUDE.mdsrc/SharpMUTerm.Core/Configuration/PreferenceSettings.cssrc/SharpMUTerm.Core/Session/WorldSession.cssrc/SharpMUTerm.Core/Text/UrlDetector.cssrc/SharpMUTerm.Tui/ExternalBrowser.cssrc/SharpMUTerm.Tui/OptionsScreenRenderer.cssrc/SharpMUTerm.Tui/Program.cssrc/SharpMUTerm.Tui/SharpMUTermApp.cstests/SharpMUTerm.Core.Tests/Text/UrlDetectorTests.cstests/SharpMUTerm.Tui.Tests/AutoLinkTests.cstests/SharpMUTerm.Tui.Tests/LinkSchemeSecurityTests.cstests/SharpMUTerm.Tui.Tests/ScreenCursorTests.cstests/SharpMUTerm.Tui.Tests/ScreenModelTests.cstests/SharpMUTerm.Tui.Tests/SnapshotViewEmptyConfigTests.cs
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
`ExternalBrowser.Open` took whatever it was handed and gave it to ProcessStartInfo. The one caller validates first, so nothing was reachable — but a scheme gate one function away from the process launch is a gate the next caller walks around without noticing. It is now a fact about the function: Open re-parses, launches only the resulting AbsoluteUri, and throws otherwise, which the app already catches and reports. Also corrects the AutoLinkTests helper comment: `LinksOn` parses a buffered line at no particular width, so it answers "is this clickable at all"; `PaneRowLinks` parses at the pane's width and answers "can a click reach it". Conflating those two is precisely the bug the wrapped-URL test exists to catch, and the comment claimed the helper did the second one. Both from CodeRabbit on #23. Core 869, Tui 1554, Graphics 83, Scripting 42, Web 37 — all passing, build clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN
HarryCordewener
commented
Aug 11, 2026
Both addressed in 0ed553b.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/SharpMUTerm.Tui.Tests/AutoLinkTests.cs (1)
236-283: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDefault the harness to a null browser action.
Connectedinjectsopened.Addwhen callers omitwithOpener. Test applications must receive a null browser action by default. SetwithOpenertofalse. PasswithOpener: trueonly in tests that assert a successful external launch.Proposed fix
- bool withOpener = true,+ bool withOpener = false,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/SharpMUTerm.Tui.Tests/AutoLinkTests.cs` around lines 236 - 283, Update the Connected test helper’s withOpener default to false so omitted callers inject a null browser action. Preserve the opened.Add callback only when withOpener is explicitly true, and update successful external-launch tests to pass withOpener: true.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@tests/SharpMUTerm.Tui.Tests/AutoLinkTests.cs`:
- Around line 236-283: Update the Connected test helper’s withOpener default to
false so omitted callers inject a null browser action. Preserve the opened.Add
callback only when withOpener is explicitly true, and update successful
external-launch tests to pass withOpener: true.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: dce52e7e-8db5-4a02-99e9-b7eb9e3d5d29
📒 Files selected for processing (2)
src/SharpMUTerm.Tui/ExternalBrowser.cstests/SharpMUTerm.Tui.Tests/AutoLinkTests.cs
HarryCordewener
commented
Aug 11, 2026
Checked, and declining this one with a reason. The premise is that a test app could launch a real browser. It cannot: the only wiring of the real launcher in the repository is The property being asked for does already hold where it matters, and is pinned: |
A game prints
https://example.org/very/long/path. The terminal emulator finds it and makes it clickable — across the terminal row. An output pane is narrower than the row, so the URL wraps and the emulator seeshttps://exaon one row andmple.com/pageon the next, with a divider and possibly another pane's output between them. Neither half is a URL, neither is clickable, and nothing says so.This client marks the span itself, which moves the decision to the layer that knows where the line really ends:
MarkupParsersplits a[link=…]across every row it wraps onto andMarkupControlhit-tests each row. OSC 8 was not an option — the compositor's cells carry no such attribute.What is in it
UrlDetector(Core). Findshttp:///https://in aStyledLineand gives those characters aSpanInteraction. The text is untouched; only span boundaries move.EmojiSubstitutor.ApplyToLine.www., no bare host, nomailto:: the output is eventually handed to the desktop, so what this can name is a security property. Trailing punctuation and unbalanced closers go back to the sentence (a wiki URL keeps its parens); a scheme inside a URL does not start a second link; there is a length cap.ProcessOutputLinecomputes it once and hands the same line toSpawnLineandPrint. A capture used to receiveresult.Linewhile the main window received the substituted one, so one line read differently in the two panes it landed in — fixed on the way past.ExternalBrowser). The built-in web view keeps its own anchors and/web <url>:LinkAction.Webis routed by the surface the click came from, not by the payload, or the built-in browser would eject you to Firefox on its first in-page link. The window id is a trusted parameter set where the handler is subscribed, never server text.file:///…,javascript:,ms-msdt:. Handing those toxdg-open/ShellExecuteis letting the world choose which program runs. Launched asProcessStartInfo.FileName(never a shell string), and what is launched isUri.AbsoluteUri— what .NET parsed, not a second reading of the same bytes.save:/logRoot:/restore:family: a snapshot and a test start no browser, and an app with no opener refuses out loud.detect links in output, default on, read per line. Ingest-time likestrip incoming colourandemoji substitution: unticking it stops the next line rather than rewriting history. Documented as that rather than pretended otherwise — it is not the timestamp gutter's situation, because a pane's history is markup by then.Verification
dotnet build SharpMUTerm.slnx— 0 warnings, 0 errors. All five suites, run directly: Core 869, Tui 1550, Graphics 83, Scripting 42, Web 37 — 0 failed.New
--view linksrenders the defect: a split, so the pane is narrower than the terminal. Decoded from the frame, the underline runs to the pane's edge and continues on the next row —Two existing tests in
LinkSchemeSecurityTestschanged destination rather than behaviour (a hyperlink now opens the browser; a forgedmux:send:payload is now refused by the http(s) gate instead of failing inside the web view), and the three F7 screen tests moved by one row.🤖 Generated with Claude Code
https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN
Summary by CodeRabbit
New Features
linkssnapshot view for reviewing detected links.Improvements