Add markdown anchor links: <a href> + #fragment heading resolution (phase 1)#13962
Open
fbartho wants to merge 18 commits into
Open
Add markdown anchor links: <a href> + #fragment heading resolution (phase 1)#13962fbartho wants to merge 18 commits into
fbartho wants to merge 18 commits into
Conversation
…hors Warp's inline parser has no HTML-tag concept at all beyond the <u> special case, so raw <a href>/<a id> tags render as literal text. Separately, and more importantly, a markdown-native [text](#fragment) link already parses today but resolves as a plain URL — there's no anchor/slug concept anywhere in the content model, so neither heading-based nor explicit <a id> fragment targets can be resolved. This adds the product + tech spec for tier-zero tag warpdotdev#1 of warpdotdev#13652 (split into warpdotdev#13725): parse <a href> as a Hyperlink (small, reuses the existing link styling/click path), give headings implicit GitHub-style slugs and let <a id>/<a name> register explicit anchors (medium — a genuinely new content- model concept, not an extension of an existing type), and wire fragment clicks to the scroll-to-character-offset autoscroll API that already exists (request_autoscroll_to_exact_vertical). Phases the work so headings + <a href> ship first, since that slice also fixes markdown-native fragment links and is the highest-value piece; arbitrary <a id> targets are phase 2. Nested under specs/GH13725/ (per the specs/APP-4319/<slug>/ chain precedent).
The prior draft claimed no #-fragment click branch existed and that the fix site was FormattedTextElement::register_default_click_handlers. On master the Markdown viewer already routes #-fragment clicks through maybe_open_url -> scroll_to_matching_header -> find_matching_header, which walks outline_blocks() live per click and scrolls via request_autoscroll_to. The real gap is that find_matching_header compares against exact lowercased heading text, not a GitHub slug. Rescope phase 1 to slug-normalize inside find_matching_header (no anchor index, no cache, no invalidation lifecycle), refresh drifted line refs, add render-surface (GUI vs TUI) and user-content-prefix notes, and flag the untracked html_tag_support_tests.rs probes that must be inverted.
Restore the cut cross-document non-goal as an in-scope phase, sized MEDIUM after confirming a fragment-less relative link already opens and focuses the target Markdown-viewer tab today. The remaining work is carrying the #section through the existing file-open flow and draining it as a deferred scroll once the destination document parses, reusing the same find_matching_header slug resolver as the same-document jump.
- Specify a genuinely GitHub-compatible slug normalizer that preserves Unicode word characters (accented Latin, CJK) instead of stripping to ASCII, so non-English README anchors keep working. - Specify phase 2's anchor lookup: extend the same live click-time walk find_matching_header already uses (no cache), reading fragment anchor_id styles alongside the header-outline pass, with a defined explicit-anchor-wins precedence rule over colliding heading slugs. - Replace the untracked-probe-file testing plan with committed test cases in markdown_parser_tests.rs and the editor resolution tests, including Unicode slug cases and the phase 2 precedence cases; frame the existing probe file as historical context only.
…down-anchor-links-impl
…tdev#13725) Recognize inline <a href="…">text</a> in the markdown inline grammar and apply the existing Hyperlink::Url styling to its body, mirroring the <u>/<link> delimiter machinery. The href is carried on a new HtmlAnchorStart delimiter and applied when </a> is parsed. Other attributes (title, target, class, …) are parsed and discarded; malformed or unclosed tags degrade to literal text. A bare <a id> anchor target (no href) is intentionally left for phase 2. Phase 1 of the anchor-links feature.
…gs (warpdotdev#13725) Replace the exact-lowercased-text comparison in find_matching_header with a GitHub-compatible slug normalizer applied to both the incoming fragment and each heading, so a hyphenated #target-section resolves against a spaced heading "Target Section". The normalizer is Unicode-aware: it lowercases and strips punctuation/symbols while preserving accented Latin, CJK, and other word characters, so non-English anchors resolve too. This fixes both raw <a href="#slug"> clicks and markdown-native [text](#slug) links, which share the same #-fragment click path. Also make an in-document #fragment miss a deliberate no-op instead of falling through to the file/URL resolver, which would surface a spurious broken-link tooltip. Phase 1 of the anchor-links feature.
github-actions
Bot
added
the
external-contributor
2 tasks
…3725) Clicking `[text](other-file.md#section)` now opens (or focuses) the target file's Markdown-viewer tab and scrolls to the matching heading once it loads, reusing the same GitHub-style slug resolver as the same-document jump. The anchor rides through LinkTarget::LocalFile -> LinkEvent::OpenFileNotebook -> pane_group::Event::OpenFileInWarp -> open_file_notebook -> FilePane::new, mirroring the code editor's existing line_and_column plumbing. A freshly opened notebook has no on-load hook, so the fragment is stashed as a pending_anchor on the editor model and drained once on the first LayoutUpdated; an already-open tab scrolls immediately. A fragment miss is a silent no-op, matching same-document miss semantics. Delivering this surfaced three latent resolution defects in NotebookLinks::resolve that had to be repaired first, since they block even a fragment-less bare relative link: - ccTLD misclassification: a bare `README.md` (no `./`, no `/`) was classified as a domain by the bare-domain heuristic, because `.md` is Moldova's ccTLD (verified with addr 0.15.6), and opened in the browser. Now a scheme-less target that resolves to an existing local file is treated as a file before the heuristic runs; a genuine bare domain with no matching file (warp.dev) still opens the browser. - fragment stripping: a `#fragment` was included in the on-disk stat, so `file.md#section` missed. It is now split off before file resolution, while a `#L100` line-number suffix is preserved for CleanPathResult. - viewer-tab base dir: a standalone viewer tab (opened with no session) could not resolve its own relative links. SessionSource::Active now carries the document's own parent directory as a base-dir fallback. Tests: classification matrix, fragment-split (multiple #, empty, #L guard), anchor threading to the open event, and pending-anchor drain hit/miss.
Amend the GH13725 phasing: cross-document fragment navigation (was "phase 3, later") is delivered in this PR alongside phase 1, because implementation found its assumed baseline — "a fragment-less relative link already opens the target today" — false in three ways. Document those as a new Resolution repairs section (item 6b), including the ccTLD collision evidence (.md is Moldova's ccTLD, verified with addr 0.15.6). Specify every link-form's behavior so there are no undefined interim states (bare/./-prefixed/fragment-suffixed/miss/genuine-domain), and record the landed tests. Arbitrary <a id>/<a name> explicit anchor markers remain the follow-up.
…tdev#13725) A relative link that resolves to the currently-open document (this-doc.md, ./this-doc.md, with or without #fragment) must focus the same tab and scroll within it, not open a duplicate. The open/focus dedup in open_file_notebook compared the resolved link path against the notebook's stored path, but the stored path is canonical (recorded on load via CanonicalizedPath) while the link resolves to base_directory.join(relative) — keeping ./.. components and, on macOS, the /tmp vs /private/tmp symlink alias. So a self-link failed to match its own pane. Canonicalize the resolved local target with the same dunce::canonicalize before the dedup comparison (extracted as canonicalize_local_path_for_dedup). A self-link with a fragment then hits the already-open branch and scrolls immediately; one without just refocuses. This also dedups any two spellings of the same file. Unit test covers the ./, .., symlink-alias, and non-existent-path-passthrough shapes.
Add product invariant 12: a relative link resolving to the open document focuses the same tab (fragment scrolls immediately, no fragment just refocuses), reusing the cross-document dedup with canonicalization for path equality. Document the canonicalization requirement in tech item 6a (self-reference and macOS /tmp symlink aliasing). Renumber the "fully specified" invariant to 13.
…v#13725) Add multi-segment `.md` guards (docs/guide.md, ./docs/guide.md) to the ccTLD classification test. These shapes were already safe — any `/` makes the pre-slash substring a non-suffix string, so the bare-domain heuristic can't fire — but pinning them asserts the repair's file-first ordering change doesn't regress the multi-segment case while fixing the single-segment one.
…v#13725) These two tests asserted the superseded pre-anchor-links behavior for an in-document `#fragment`, and phase 1 changed exactly the code they exercise (`maybe_open_url`'s `#`-branch) without running them — the phase-1 gate used a scoped test selection that never executed this module, so they landed failing-but-unnoticed. - test_editable_markdown_anchor_click_opens_link_tooltip asserted a plain click on `#goal` in an editable editor shows an editable link tooltip. Under the anchor work a `#fragment` is resolved entirely in the viewer and never surfaces a tooltip; a non-cmd click in an editable editor is a non-activating no-op. Renamed to test_editable_markdown_anchor_click_is_silent_no_op and asserts open_link stays None. - test_cmd_click_missing_markdown_anchor_falls_back_to_link_resolution asserted a `#fragment` miss falls through to the link resolver and emits OpenFileWithTarget for a literal `#missing.png` file. That directly violates product invariant 7 (a fragment miss does nothing observable — "no attempt to open it as an external URL"). Renamed to test_cmd_click_missing_markdown_anchor_is_silent_no_op and asserts no event is emitted and no tooltip is shown, citing invariant 7. Removed the now-unused assert_eventually import. Module passes 14/14 in isolation; full-parallel failure count drops from 14 to 12 (the two anchor tests), remaining 12 are pre-existing test-isolation flakiness.
…tdev#13725) Relative and cross-document file links were silent no-ops in a standalone Markdown viewer tab (opened from Finder / `open -a Warp file.md`), while in-document `#fragment` links kept working. NotebookLinks::resolve was session-gated at the tail: after correctly classifying a bare `file.md` as a local file (ccTLD repair) and finding the document's own directory as the base directory (document_dir fallback), it matched on the active session and returned Err(MissingContext) from the `None` arm — even with a valid base directory and a confirmed-existing file. A standalone viewer has no terminal session, so every file link died there; resolve_and_open drops the Err, producing the silent no-op. `#fragment` links are handled entirely in-viewer and never touch the resolver, so they were unaffected — matching the observed split. Make the session optional through the resolve→open chain (LinkTarget::LocalFile, resolve_file, LinkEvent::OpenFileNotebook, pane_group::Event::OpenFileInWarp). When there is no session but a base directory is present, resolve the relative path against it and carry a session-less LinkTarget. Downstream was already Option-ready: open_file_notebook takes an Option<Session> and FileNotebookView has a no-session open path. The existing link tests always installed a TEST_SESSION even on the SessionSource::active path, so the no-session-with-base-directory condition — the real standalone-viewer GUI state — was never exercised. Add an init_link_model_no_session harness and two tests covering resolve() and the end-to-end resolve_and_open → OpenFileNotebook path without a session.
…3725) Extend find_matching_header so a #fragment click also matches an explicit <a id="x">/<a name="x"> anchor tag, not just a heading slug. The phase-1 inline parser only recognizes <a href> as a delimiter pair (it requires an href attribute to match), so a bare <a id>/<a name> tag falls through to literal text and its raw markup survives verbatim in the buffer. Resolution reuses that fact directly: a regex scan over the buffer's plain text finds anchor tags and converts their byte-offset match position to a CharOffset via CharCounter, the same byte-to-char bridge global_search already uses for match highlighting. No new content-model field, no cached index — same zero-cache, live-walk pattern the heading matcher already established. Anchor ids match verbatim (as authored), never through the heading slug normalizer — there is no id rewriting and no user-content- prefix. When an anchor and a heading slug collide, first occurrence in document order wins (a shared namespace, not "anchors always win"). Phase 2 of the anchor-links feature, pulled forward from a deferred follow-up.
…(GH13725) Update the phasing section to reflect that arbitrary anchor targets shipped in this PR rather than deferring to a follow-up. Replace the tech spec's zero-width-fragment-marker design (item ii) with the mechanism actually implemented: a live text scan over the buffer, since characterization of the phase-1 parser showed a bare <a id>/<a name> tag's raw markup already survives verbatim as literal text with no href to gate it. Correct product invariant 6, which had said explicit anchors always win a collision with a heading slug — the shipped rule is first-occurrence-in-document-order, matching GitHub's single shared id namespace.
…rpdotdev#13982 Correct invariant 5 and the tech spec's phase-2 section, which both still described the pre-implementation zero-width-marker design (renders nothing). What shipped instead resolves anchors via a live text scan, which only works because the tag survives as visible literal text — so the tag is visible, not hidden, as an honest statement of shipped behavior rather than the originally planned one. Hiding it requires a first-class, save-round-trippable content-model representation (70-130+ call sites across the buffer/editor layer), deliberately deferred to a design-discussion ticket, warpdotdev#13982, rather than building an unreviewed representation now.
fbartho
marked this pull request as ready for review
July 19, 2026 23:09
Contributor
|
Every PR must be linked to a same-repo issue before Oz can review it. This PR is linked to #13725, but no linked issue is marked See the contribution guidelines for the full readiness model. Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Every PR must be linked to a same-repo issue before Oz can review it.
This PR is linked to #13725, but no linked issue is marked ready-to-implement yet. Only repository maintainers apply that label, so please wait for a maintainer to mark the issue. Once it is marked, push a new commit or comment /oz-review to re-trigger review.
See the contribution guidelines for the full readiness model.
Powered by Oz
…nchor-links-impl # Conflicts: # app/src/notebooks/editor/view_tests.rs # app/src/notebooks/link.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #13725
Phase 1 of markdown-viewer anchor links: raw
<a href>hyperlinks plus GitHub-style#fragmentresolution against heading slugs. This fixes the issue's headline case — an<a href="#target-section">or markdown-native[Jump](#target-section)link scrolling to a heading "Target Section" — and repairs markdown-native fragment links along the way (they already parsed but had nothing to resolve against).What's in this PR (phase 1 per the spec's sequencing):
<a href="…">text</a>as aHyperlink::Url(other attributes discarded; malformed tags degrade to literal text). Displays in the TUI renderer for free via the shared parser.#fragmentmisses are now a deliberate no-op instead of surfacing a broken-link tooltip.Also in this PR (scope expanded per review of the delivered behavior):
other-file.md#sectionopens (or focuses) the target document in the viewer and scrolls to the fragment once laid out, reusing the same slug resolution. Misses open the document with no scroll and no error.file.mdtargets were misclassified as domains (.mdis Moldova's ccTLD in the public suffix list — local-file existence now outranks the domain heuristic, while genuine domains likewarp.devstill open the browser);#fragmentsuffixes are now split off before file lookup (previously the literal fragment-included filename was stat'd); standalone viewer tabs now resolve relative links against the document's own directory instead of requiring a terminal session.NotebookLinks::resolvepreviously required a terminal session and silently erred for ANY file link in a standalone viewer tab (the session becameOptionthrough the resolve→open chain, which was alreadyOption-ready downstream). A new no-session test harness covers the standalone-viewer condition the prior harness structurally couldn't express../this-doc.md#section) dedup to the already-open tab via canonicalized path comparison (handles./,.., and macOS/tmpsymlink aliasing) and scroll immediately.Anchor targets (scope expanded per review discussion): Extends the anchor-links feature so a
#fragmentclick also resolves against explicit<a id="x">/<a name="x">targets, not just heading slugs, completing the hand-built-table-of-contents case from the original issue. Resolution reuses the same zero-cache, click-time buffer walk the heading resolver already established — a bare anchor tag's markup survives verbatim as literal text, so no new content-model field or cached index was needed. Anchor ids match exactly as authored (no rewriting); an anchor and a colliding heading resolve by first occurrence in document order, matching GitHub's shared namespace. Known limitation, shipped deliberately: the anchor tag itself still renders as visible literal text rather than being hidden — hiding needs a first-class, save-round-trippable content-model representation, tracked separately as #13982 for maintainer design input.Not in this PR (tracked follow-up):
<a id>/<a name>anchor tags (currently rendered as literal text) #13982.This PR contains the approved spec (
specs/GH13725/), merged from #13907's branch, which this PR supersedes. Manually verified end-to-end in a local build: both fragment-link forms scroll centered to their headings (including accented and CJK targets), misses are silent no-ops, and external links open normally.