Skip to content

markdown: Add TextViewState::selected_source_range - #3136

Open
tidely wants to merge 3 commits into
longbridge:mainfrom
tidely:selected-source-range
Open

tidely wants to merge 3 commits into
longbridge:mainfrom
tidely:selected-source-range

Conversation

@tidely

@tidely tidely commented Sep 19, 2026

Copy link
Copy Markdown

Description

This pull request adds a new TextViewState::selected_source_range() API. It returns the byte range in the original Markdown source corresponding to the text currently selected in a rendered TextView.

When the user selects text in rendered markdown, we are currently only exposed the text which the user selected, but not which range of bytes this actually is. This means if there are duplicate instances of the selected text in the markdown, we cannot reliably know which the user selected. This API fixes that.

Public API

List every public item this pull request adds, changes or removes, grouped by crate, with its signature and one line on what it is for. Include JavaScript methods and TypeScript declarations. If none, remove this section.

  • gpui_base::text::TextViewState::selected_source_range(&self) -> Option<Range<usize>> — Returns the original Markdown source byte range for the actual rendered selection.

How to Test

Added tests for:

  • Plain Markdown text.
  • Selection fully inside bold text.
  • Selection crossing into or out of a styled span.
  • Selection spanning complete styled sections.
  • Selection across multiple style boundaries.
  • Heading text, excluding heading syntax from partial selections.
  • Escaped Markdown characters.
  • Repeated identical text, verifying the selected occurrence maps to its own source range rather than the first textual match.
  • Fenced code blocks.
  • Indented code blocks.
  • Global source offsets after incremental document parsing.
  • Select-all behavior.
  • Unmappable selections returning None rather than an incorrect guessed range.

Checklist

  • I have read the CONTRIBUTING document and followed the guidelines.
  • Reviewed the changes in this PR and confirmed AI generated code (If any) is accurate.
  • Passed cargo run for story tests related to the changes.
  • Tested macOS, Windows and Linux platforms performance (if the change is platform-specific)

@huacnlee huacnlee left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this. The need is real: selected_text() cannot tell which of two identical runs was selected, and a source byte range is the right shape for hosts that quote, annotate or edit the selection. It also fits the existing MarkdownNode::source_range() contract, and returning None instead of a guessed range is the right policy.

The implementation is not there yet, though. I reproduced each point below with temporary tests on this branch.

Wrong ranges (worse than None)

  • Escaped backslash maps one byte early (format/markdown.rs:196). source_segments tries the literal-char branch before the backslash-escape branch, so \\ is consumed as one byte and the next char absorbs the leftover. Source a\\b renders a\b; selecting the rendered b (2..3) returns Some(2..4) (\b) instead of 3..4. Check remainder.strip_prefix('\\') before remainder.starts_with(rendered_char).
  • The first.source.start -= 1 adjustment fires on the previous node's escape (format/markdown.rs:217). It only checks that the byte before the span is \, so a preceding \\ shifts the following inline math or hard break one byte early (a\\$x$, selecting $ returns 2..4 instead of 3..4) and can make a cross-node selection return None because the segments now overlap. Restrict it to Text nodes whose own first char is the escaped one.
  • Inline images never contribute their span (node.rs:1279), so the result disagrees with selected_source(), which includes ![alt](url) when the selection reaches the image edge. A host that deletes the returned range drops the image the clipboard copy kept. Either give ImageNode a span (markdown.rs:422 discards raw.position) and merge it under the same conditions, or return Unmapped when the selection reaches an image.

Coverage (None for most real content)

  • Inline code, footnote references, MDX expressions and $$ blocks can never map (format/markdown.rs:397, 450, 477, 619). Their spans start with a delimiter that source_segments does not skip, so the segment list is always empty. `code` x: selecting code returns None, selecting x maps. selected_source_range_rejects_unmapped_gap_in_merged_styled_node passes only because inline code is unmappable, not because a gap was detected. code_source_segments already strips fences and could serve these nodes.
  • A soft break with a container prefix, continuation indentation or trailing spaces, or any HTML entity, makes the whole Text node unmappable (format/markdown.rs:192). a\n b, > a\n> b, - a\n b, a \nb all render a b and selecting 0..1 returns None (plain a\nb works). Copyright &copy; 2024: selecting Copyright returns None. Hard-wrapped paragraphs inside lists and blockquotes are the most common shape of LLM and README Markdown. Align line by line and emit a gap segment for the unmatched part instead of bailing on the node.
  • Multi-line indented code blocks and fences nested in a list or blockquote yield no segments (format/markdown.rs:263). markdown-rs strips per-line indentation and container prefixes from value, so match_indices(code) on the raw span never matches; only single-line indented blocks and top-level fences work, which is exactly the tested set. Map line i of value to the i-th raw line in the body instead of searching for the whole body.

Structure

  • Doc comment displaced (node.rs:1216). selected_source_range was inserted between selected_source's doc comment ("Reconstruct the Markdown source for the current selection…") and its body, so that comment now documents the new method and selected_source has none.
  • One 32-byte SourceSegment per rendered character (node.rs:534), rewritten per emphasis/link nesting level and deep-copied with every streaming append. Storing equal-length runs and interpolating on lookup gives identical results with one segment per plain-text run or code block. Fine as a follow-up, but worth doing before this ships in streaming views.

Not blocking: the streaming tail-reparse leaves the last block unselected until the next paint, but that is pre-existing and affects selected_text() too.

Happy to re-review once the two wrong-range bugs and the doc comment are fixed and inline code plus list/blockquote paragraphs map.

@tidely

tidely commented Sep 21, 2026

Copy link
Copy Markdown
Author

Unfortunately the diff grew quite a bit, but I've addressed the bugs and missing coverage

@tidely
tidely requested a review from huacnlee September 21, 2026 11:07
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.

2 participants