Skip to content

feat(preview): Edit jumps to the fragment you right-clicked - #539

Merged
PathGao merged 3 commits into
masterfrom
feat/jump-to-selection
Aug 8, 2026
Merged

feat(preview): Edit jumps to the fragment you right-clicked#539
PathGao merged 3 commits into
masterfrom
feat/jump-to-selection

Conversation

@PathGao

@PathGao PathGao commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Fixes #90.

Selecting text or an image in the preview and choosing Edit now opens the editor scrolled to that fragment, with its source lines selected. ⌘E and the toolbar buttons do the same thing, so the reader does not have to know which entry point carries them there.

Two pre-existing bugs surfaced while testing this by hand and are fixed here too — both live in the paths this change makes people exercise.

The mapping already existed

comrak runs with options.render.sourcepos = true, so every rendered element already carries the source range it came from, and previewAnchor.ts is already the module that reads those ranges (for the tab's reading position and for split-view scroll sync). This is a third consumer of the same attribute, not a third mapping.

Two facts about data-sourcepos that decide the shape of the feature, both measured against the real renderer rather than assumed:

Inline nodes carry a range, not just blocks.

<p data-sourcepos="3:1-3:67">A paragraph with
  <strong data-sourcepos="3:18-3:30">bold text</strong> and an
  <img data-sourcepos="3:39-3:53" src="img.png" alt="alt" /> inline image.</p>

That is what makes "jump to the selected image" land on the image's own line instead of the whole paragraph.

Only the line numbers are usable. comrak parses the output of convert_markdown's preprocessing, and that pipeline is line-preserving, not column-preserving. For the raw line Math $a+b$ then ![alt](img.png) here. the image really starts at column 18, and comrak reports 3:24-3:38 — the math mask substitutes a token longer than $a+b$ and every column after it drifts. So the jump selects whole lines and nothing reads a column. A column-precise highlight would point at the wrong span in any document containing maths.

Extended, not added

  • previewAnchor.ts gains findSourceLineRange (climb to the narrowest annotated ancestor) and mergeSourceLineRanges, on top of the parseSourceposLineRange it already had.
  • Editor.revealHeader already revealed and selected a source line for the outline; its line branch now delegates to a shared revealSourceRange, so the outline and the context menu cannot drift apart.
  • The existing menu.edit context-menu entry changed behaviour rather than gaining a sibling — no new i18n keys, so all 26 locales are already correct.

The highlight is Monaco's own selection: drawn in the theme's colour, no decoration, no CSS, no timer, and it clears itself on the reader's next click or keystroke — exactly when it stops being useful.

Two pre-existing bugs, fixed

1. Every jump landed short in a document with front matter.

renderMarkdownPreview hands comrak getMarkdownBodyWithoutFrontMatter(raw), so every data-sourcepos counts from the first line of the body, while the editor holds the whole file. Nothing in the attribute says which of the two it means, and the difference is invisible in a document without front matter — which was every fixture in the suite.

The outline has had this bug for as long as both have existed. Toc.svelte reads the same attribute and passes it to revealHeader untouched, so clicking a heading in a document with front matter has always landed short. It went unnoticed because revealHeader falls back to a text search when the line is null, and that path is correct. In samples/stress-test.md the error is 11 lines.

Both now go through toBufferRange. The task-checkbox write-back is not affected: it counts lines in the body as well, so both sides of that comparison share one numbering.

2. ⌘E did nothing in split view, and meant something different from "Edit".

They were two implementations of one intent. toggleEditView is now what the chord means, and the hotkey, the toolbar, the title bar and Monaco's own command all route through it:

reading             → editor, on the selected fragment
editor (alone)      → back to reading
split + selection   → jump to it; the layout does not move
split, no selection → nothing

The inert case is deliberate. Split view already grants what ⌘E asks for — the editor is on screen — and with no selection there is no fragment to travel to, so every remaining reading of the chord is a layout change nobody requested. A mistyped ⌘E would cost the reader the preview pane and a keystroke to get it back; doing nothing costs nothing. ⌘\ opens and closes the split and stays the only way.

formatShortcutKeymap.test.ts holds the two layers together and caught the divergence while this was being written.

3. The outline's jump highlight could not be dismissed. A temporary emphasis whose only removal path was a scroll event on the preview — and the jump's own smooth scroll is swallowed by clickLock, so unless the reader scrolled again it stayed until the document was re-rendered. A pointerdown in the preview and a keydown anywhere now end it too, both bypassing clickLock: that lock exists to ignore scrolling the app itself caused, and a deliberate action by the reader is never that.

Selection cases

range across several blocks union of both ends; direction-independent
one end outside the document the other end wins
caret / nothing selected falls through to the element under the pointer
click on an image the image's own line
right-click outside rendered content null — "Edit" behaves exactly as before
already in split view jumps, does not toggle edit mode off

Validation

  • npm test 871 · npm run check 0 errors · cargo test 141
  • Tested by hand on macOS across all four ⌘E states, the context menu, the toolbar and title-bar buttons, the outline jump, and the highlight dismissal.
  • Rebased onto current master, which picked up eca4f1b (media players inherit the source range they replaced) — so right-clicking a video or audio embed now resolves too.

Not verified: no run on Windows or Linux. The two timing assumptions — that the $effect fires once editorPane is bound, and that pendingReveal survives the mount-time view-state restore — are asserted structurally and confirmed by hand on macOS, not observed under test.

🤖 Generated with Claude Code

PathGao and others added 3 commits August 8, 2026 11:46
"Edit" in the preview's context menu opened the editor wherever the tab
was last left. It now opens it on the block, inline element or selection
that was right-clicked, with those lines selected.

No new preview-to-source mapping. comrak already renders with
`options.render.sourcepos = true`, and `previewAnchor.ts` is already the
module that reads those ranges for the tab's reading position and for
split-view scroll sync; this adds two small functions there — the climb
to the narrowest annotated ancestor, and the union of a selection's two
ends — and reuses `parseSourceposLineRange` for the parsing.

Nor a new jump. `revealHeader` already revealed and selected a source
line for the outline; its line branch now delegates to
`revealSourceRange`, which both callers share, so the two cannot scroll
or focus differently. That function also gained the clamp `revealHeader`
never had: `getLineMaxColumn` throws past the end of the model, and both
callers can hand over a line from a render of a buffer that has since
got shorter.

Lines only, never columns. `data-sourcepos` describes the text
`convert_markdown` hands comrak, and only its line numbers are
contractually equal to the raw buffer's — the math mask substitutes a
token of a different length, so columns after it on the line drift.

The selection is the highlight the report asks for: Monaco draws it in
the theme's own colour, and it clears itself on the next click or
keystroke, so no decoration, CSS or timer is needed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…the selection into ⌘E

Two follow-ups from testing the jump by hand.

**The jump landed short.** `renderMarkdownPreview` hands comrak
`getMarkdownBodyWithoutFrontMatter(raw)`, so every `data-sourcepos`
counts from the first line of the BODY. The editor holds the whole
file. In a document with front matter every jump was early by its
height — 11 lines in `samples/stress-test.md`.

Nothing in the attribute says which of the two numberings it means, and
the difference is invisible in a document without front matter, which
was every fixture in the suite.

**The outline has had the same bug for as long as both have existed.**
`Toc.svelte` reads the same attribute and passes it to `revealHeader`
untouched, so clicking a heading in a document with front matter has
always landed short too. It went unnoticed because `revealHeader` falls
back to a text search when the line is null, and that path is correct.

Both now go through `toBufferRange`. Worth noting the task-checkbox
write-back is *not* affected: it counts lines in the body as well, so
both sides of that comparison share one numbering.

**⌘E and the toolbar now carry the selection too.** The context menu's
"Edit" already opened the editor on what you had selected; the entry
points people actually use did not. `toggleEdit` resolves the selection
before it flips `isEditing` — reading after would ask whether the reader
was in the editor rather than in the preview — and arms the jump only if
the switch took, since the read can fail and leave the tab in reading
mode. `editSourceRange` passes `revealSelection: false` because it has
already captured its target: clicking a menu item is how a selection
goes away.

Tests cover the arithmetic (including CRLF, a `---` that is not front
matter, and the real stress document) and, separately, that all three
call sites route through the shift — pinning the arithmetic alone would
pass with a call site still handing over a raw body line.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
**⌘E and the preview's "Edit" are now the same move.** They were two
implementations of one intent, and they disagreed: "Edit" carried the
reader to the fragment they had selected, the chord did not, and in
split view the chord did nothing at all. `toggleEditView` is now what
⌘E means, and the hotkey, the toolbar, the title bar and Monaco's own
command all route through it — so the chord cannot mean one thing with
the caret in the editor and another with it in the preview.
`formatShortcutKeymap.test.ts` holds those two layers together and
caught the divergence while this was being written.

    reading            → editor, on the selected fragment
    editor (alone)     → back to reading
    split + selection  → jump to it; the layout does not move
    split, no selection→ nothing

**The inert case is the considered one.** Split view already grants
what ⌘E asks for — the editor is on screen — and with no selection
there is no fragment to travel to, so every remaining reading of the
chord is a layout change nobody requested. A mistyped ⌘E would cost the
reader the preview pane and a keystroke to get it back; doing nothing
costs nothing. ⌘\ opens and closes the split and stays the only way.

`toggleEdit` goes back to being only a mode switch. Resolving the
selection lives in `toggleEditView`, above every path that flips
`isEditing`, because a selection read after the switch answers for the
editor rather than for the preview the reader was looking at.

**The outline's jump highlight could not be dismissed.** It is a
temporary emphasis, but the only thing that removed it was a scroll
event on the preview — and the jump's own smooth scroll is swallowed by
`clickLock`, so unless the reader scrolled again afterwards it stayed
until the document was re-rendered. A pointerdown in the preview and a
keydown anywhere now end it too, both bypassing `clickLock`: that lock
exists to ignore scrolling the app itself caused, and a deliberate
action by the reader is never that.

Both were found while testing #90 by hand. Neither is from that change
— the ⌘E branch is #421's and the highlight is older — but both are in
the paths it made people exercise.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@PathGao
PathGao merged commit 7c72ed9 into master Aug 8, 2026
4 checks passed
@PathGao
PathGao deleted the feat/jump-to-selection branch August 8, 2026 04:25
PathGao added a commit that referenced this pull request Aug 8, 2026
…ople download (#558)

**The syntax reference.** Nothing in 2.7.3 changed what renders, so the
compatibility table is untouched. What changed is behaviour, and five of
those are demonstrable on the page itself, so they belong in §17 and §15:
the right-click Edit jump (#539), split-view sync by source line (#541),
sticky scroll (#555), the unpinned outline getting out of the way (#545),
and CJK word-wise navigation with the IME space no longer boxed (#546).

The outline line is written from what f29928a actually does — it collapses
when you pick an entry or reach past it — rather than from the shorter
"steps aside" the commit subject suggests.

Checked and left alone: Copy Reference still follows the document's own
spelling (`preferredReferenceStyle`), heading completion and task
checkboxes were already described correctly. Encoding, line endings, the
clipboard and the load-race fixes are not syntax and are not on this page.

**Making it findable.** It was one link in the nav and one clause in a
Features bullet. It now has its own section between Download and
Installation from source, with the raw URL first so the primary action is
downloading the file rather than reading it on GitHub — the document is
built to be opened in Markpad, and reading it here is the fallback. The
section also suggests handing the file to an AI: it is a complete list of
what renders, so an assistant can reformat an existing document or write a
new one that uses the whole range.

"Markdown support" is gone as a heading. In a README that word means the
help desk; `## What Markpad renders` says what the section is, and does
not collide with `## Features`, which is where "what Markpad can do"
already lives.

The release body gets the same section, with the links pinned to the tag.

**The two platform notices.** The Windows SmartScreen note has been pasted
into the release body by hand since it was written — the workflow never
generated it. Both notices are generated now, so the macOS one cannot go
missing the release someone forgets, and the Windows one must no longer be
added by hand or it will appear twice.

The macOS notice covers what #209 costs a new user: an unnotarized `.dmg`
is refused on first launch with only Cancel and Move to Trash, so
Control-click → Open comes first as the one-step way past it and the
System Settings route second. It also names the consequence people report
as a separate bug — unsigned means file access is granted per prompt, so
the dialogs repeat, most visibly with auto-save on and in image-heavy
documents — and links to the self-signing workaround in #209, noting it
has to be redone after every update.

Co-authored-by: PathGao <gaoyanbo@gaoyanbodeMacBook-Air.local>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.

Jump to the selected fragment in editing mode

1 participant