Skip to content

Fixes #5270, #5273, #5281. Fix MarkdownView selection overlay bugs#5282

Merged
YourRobotOverlord merged 6 commits into
gui-cs:developfrom
YourRobotOverlord:fix/5270-markdown-selection-overlay-scroll-sync
May 9, 2026
Merged

Fixes #5270, #5273, #5281. Fix MarkdownView selection overlay bugs#5282
YourRobotOverlord merged 6 commits into
gui-cs:developfrom
YourRobotOverlord:fix/5270-markdown-selection-overlay-scroll-sync

Conversation

@YourRobotOverlord
Copy link
Copy Markdown
Collaborator

Copilot Sessions

766860d7-4a69-4edf-b25c-cd362531af5a, cc36ca80-1ae3-4e1c-a9f7-6230152a804e

Fixes #5270, #5273, and #5281.

Summary

Three related bugs in MarkdownView's selection overlay, all fixed in MarkdownView.Drawing.cs and MarkdownView.Selection.cs.

#5270 — Selection overlay out of sync when scrolled

DrawSelectionOverlayOnSubViewRows() called ContentToScreen(new Point(0, drawRow)) where drawRow = lineIdx - Viewport.Y (viewport-relative). ContentToScreen internally subtracts Viewport.Y, so with any scroll offset the overlay read graphemes from the wrong screen row — overwriting body content with header content.

Fix: Pass lineIdx (content-relative) instead of drawRow.

#5273 — Partial code-block selections include unwanted fence delimiters

GetSelectedText() unconditionally injected an opening fence when entering a code block and always appended a closing fence when the selection ended inside a block.

Fix: Added selectionHasNonCodeContent and codeOpenFenceEmitted flags. Opening fence is only emitted when entering a code block after non-code content has been seen. Trailing fence is no longer appended for selections that end inside a block.

#5281 — Selection highlight covers entire row in code blocks and tables

DrawSelectionOverlayOnSubViewRows() applied the selection attribute to every column unconditionally. Plain-text lines already use a per-grapheme IsInSelection() check — the subview overlay path was missing the equivalent.

Fix: Call IsInSelection(lineIdx, col + Viewport.X) per column. For non-selected columns, restore the original Cell.Attribute from ScreenContents so syntax-highlighted styling is preserved.

Tests

6 new regression tests in MarkdownViewSelectionTests.cs:

YourRobotOverlord and others added 5 commits May 8, 2026 18:56
…nce delimiters

When copying a partial selection that starts or ends inside a fenced code
block, the copied text should not include the fence delimiters unless the
selection actually crosses from non-code content into the code block.

Root cause: GetSelectedText() unconditionally injected the opening fence
whenever it first encountered a code-block line, and unconditionally
injected the closing fence at the end of the loop if still inside a code
block. This meant any selection touching a code block line would include
fences, even if the selection was entirely within the code block.

Fix: Replace the unconditional fence injection with two tracking flags:
- selectionHasNonCodeContent: set true when any non-code line is processed.
  Opening fence is only emitted when transitioning from non-code -> code.
- codeOpenFenceEmitted: tracks whether an opening fence was actually
  emitted for the current code block. Closing fence is only emitted when
  the matching opening fence was emitted.

This ensures:
- Selection entirely within a code block -> no fences (regardless of position)
- Selection starting before a code block -> opening fence included
- Selection ending after a code block -> closing fence included

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…nSubViewRows

ContentToScreen expects content-relative coordinates (where 0 = top of
the content area), not viewport-relative coordinates.  The old code
passed drawRow (= lineIdx - Viewport.Y), so ContentToScreen subtracted
Viewport.Y a second time, reading graphemes from the wrong screen row.
With a non-zero scroll offset this caused the selection overlay to copy
characters from an incorrect row (e.g. the table header) over the
correct row (the table body), making body cell values disappear.

Fix: pass lineIdx (content-relative) instead of drawRow.

Added regression test SelectionOverlay_On_Table_Is_Synced_When_Scrolled
that scrolls a Markdown view past introductory text so only a table is
visible, activates a full selection, and asserts the body-row values
(1, 2) remain in the screen buffer.  The test height (5) is intentionally
less than the total content height (7) so Viewport.Y = 2 is not clamped.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…e fence delimiters from partial code-block selections

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
DrawSelectionOverlayOnSubViewRows was applying the selection attribute to
every column of a subview row unconditionally.  On the start/end lines of
a selection this caused unselected columns to be highlighted, making it
look as though the entire line was selected even when only part of it was.

Fix: call IsInSelection(lineIdx, col + Viewport.X) per column, mirroring
the per-grapheme check already in DrawRenderedLine for plain text lines.
For non-selected columns the original Cell.Attribute from ScreenContents
is restored so the subview styling is preserved.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@YourRobotOverlord
Copy link
Copy Markdown
Collaborator Author

Yay!

WindowsTerminal_ar2KdL4olW

DrawSelectionOverlayOnSubViewRows reads graphemes from ScreenContents
(previous frame) and re-draws them with the selection attribute.
Popovers draw before MarkdownView in the application draw loop, so
their menu items are already written to the screen buffer when the
overlay runs. Re-drawing those cells with stale ScreenContents graphemes
silently erases the popover's content.

Fix: compute the active popover's content view screen rect before the
cell loop and skip any cell that falls inside it.  The popover's draws
are preserved; the selection highlight still covers all other cells.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@YourRobotOverlord YourRobotOverlord marked this pull request as ready for review May 9, 2026 03:47
@YourRobotOverlord YourRobotOverlord requested a review from tig as a code owner May 9, 2026 03:47
Copy link
Copy Markdown
Member

@tig tig left a comment

Choose a reason for hiding this comment

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

LGTM. Nice tests.

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.

Markdown text selection overlay is out of sync with the actual selected text

2 participants