mirror of
https://github.com/langchain-ai/deepagents.git
synced 2026-08-27 10:51:26 -04:00
fix(code): extend text selection with Shift + click (#5732)
<kbd>Shift</kbd> + <kbd>click</kbd> now extends an existing text selection from its original anchor. **Known limitation:** this only works when the terminal delivers the modified click to the app. Ghostty binds <kbd>Shift</kbd> + <kbd>click</kbd> to its own selection and never forwards the event while mouse reporting is active, so it's a no-op there (users can `keybind = shift+click=unbind` to opt out). Verified working in iTerm2; kitty and WezTerm also forward the shift modifier bit. --- Textual resets selection state on every mouse press. This preserves the prior selection anchor for modified clicks while retaining stock behavior when no selection exists or the target is not selectable. Regression tests cover forward, backward, and multi-widget ranges. The rebuilt selection state can field-match the finished drag selection, which Textual reactives treat as unchanged — so the patch invokes the (patched, now-async) select-state watcher directly to repaint the highlight. The module docstring documents the terminal-delivery limitation for maintainers. Made by [Open SWE](https://openswe.vercel.app/agents/4d90066c-60b9-51ed-b7a3-cce83ee4fd0b) --------- Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>
This commit is contained in:
@@ -1,6 +1,6 @@
|
||||
r"""Runtime patches over Textual internals, imported for side effect.
|
||||
|
||||
This module hosts five independent best-effort patches over private Textual
|
||||
This module hosts six independent best-effort patches over private Textual
|
||||
APIs. Each guards its own import/assignment and degrades to stock Textual
|
||||
behavior (logging a warning) if the targeted internals move, so they have
|
||||
separate lifecycles — do not delete the whole file when only one lands
|
||||
@@ -42,7 +42,23 @@ upstream.
|
||||
drag) to word boundaries. No upstream issue tracks this yet, so it has
|
||||
no removal criterion — it stays until Textual grows native word select.
|
||||
|
||||
4. Detached-widget hit filtering. The compositor keeps reporting a widget as
|
||||
4. Shift-click selection extension. Stock Textual replaces an existing text
|
||||
selection on every mouse press, so Shift+click cannot move the active end
|
||||
of a drag-selected range. The patch preserves the original selection anchor
|
||||
and applies the click as its new end. No upstream issue tracks this yet.
|
||||
|
||||
Known terminal limitation: this only works when the terminal delivers the
|
||||
modified click to the app. Ghostty binds Shift+click to its own selection
|
||||
and never forwards the event while mouse reporting is active, so
|
||||
Shift+click is a no-op there (users can `keybind = shift+click=unbind` to
|
||||
opt out). iTerm2, kitty, and WezTerm forward it with the shift modifier
|
||||
bit set, which is what the patch keys on. If Shift+click "does nothing"
|
||||
for a user, check terminal delivery first — e.g. run
|
||||
`printf '\e[?1003h\e[?1006h'; cat -v` and confirm Shift+click prints a
|
||||
`^[[<;...;4M`-style sequence (the `4` is the shift bit) — before digging
|
||||
into this patch.
|
||||
|
||||
5. Detached-widget hit filtering. The compositor keeps reporting a widget as
|
||||
visible for a few event-loop iterations after it leaves the DOM, which
|
||||
`Markdown.update` (and therefore the `MarkdownStream` that drives every
|
||||
streaming assistant message) does constantly. `Screen._forward_event`
|
||||
@@ -51,7 +67,7 @@ upstream.
|
||||
crashes the app with `AttributeError: 'NoneType' object has no attribute
|
||||
'region'`. Tracked in Textualize/textual#6643; remove when that lands.
|
||||
|
||||
5. Diff-gutter exclusion from selections. Textual paints the selection
|
||||
6. Diff-gutter exclusion from selections. Textual paints the selection
|
||||
highlight from the geometry stored in `Screen.selections`, so excluding
|
||||
a diff row's decorative gutter (line number, `+`/`-` marker) only in
|
||||
`Widget.get_selection` would copy the right text while still highlighting
|
||||
@@ -447,6 +463,97 @@ else:
|
||||
)
|
||||
|
||||
|
||||
try:
|
||||
from textual import events as _shift_events
|
||||
from textual.screen import Screen as _ShiftScreen
|
||||
from textual.selection import SelectEnd, SelectStart, SelectState
|
||||
|
||||
_original_forward_event_with_shift = _ShiftScreen._forward_event
|
||||
except (ImportError, AttributeError) as exc: # pragma: no cover - defensive
|
||||
logger.warning(
|
||||
"Textual Shift+click selection patch skipped (textual %s): %s",
|
||||
_textual_version,
|
||||
exc,
|
||||
)
|
||||
else:
|
||||
|
||||
def _shift_click_anchor(screen: Screen, event: Event) -> SelectState | None:
|
||||
# When this returns None the press falls through to stock Textual,
|
||||
# which starts a fresh selection — indistinguishable from the event
|
||||
# never arriving. Before debugging the branches below, confirm the
|
||||
# terminal forwarded a shift-modified click at all (see the module
|
||||
# docstring's patch 4 note); terminals like Ghostty consume Shift+click
|
||||
# locally, so `event.shift` never becomes True here.
|
||||
if (
|
||||
not isinstance(event, _shift_events.MouseDown)
|
||||
or not event.shift
|
||||
or screen.app.mouse_captured
|
||||
or not screen.selections
|
||||
):
|
||||
return None
|
||||
select_state = screen._select_state
|
||||
if select_state is None or select_state.end is None:
|
||||
return None
|
||||
content_widget = select_state.start.content_widget
|
||||
if content_widget is not None and not content_widget.is_attached:
|
||||
return None
|
||||
return select_state if select_state.is_attached_to_dom() else None
|
||||
|
||||
def _rebase_anchor_scroll(anchor_start: SelectStart) -> SelectStart:
|
||||
# `SelectStart.pointer_start_offset` adds the scroll travelled since
|
||||
# the drag began, which tracks the viewport rather than the anchored
|
||||
# text. That is harmless mid-drag, but a Shift+click can arrive many
|
||||
# rows after the container scrolled — the transcript auto-scrolls while
|
||||
# a message streams — leaving the anchor pointing at whatever text now
|
||||
# occupies its old screen row. Fold the drift into the pointer delta
|
||||
# and re-base against the current scroll offset, so the anchor stays on
|
||||
# its original text.
|
||||
container = anchor_start.container
|
||||
drift = container.scroll_offset - anchor_start.container_initial_scroll_offset
|
||||
return SelectStart(
|
||||
container,
|
||||
anchor_start.container_pointer_delta - drift,
|
||||
anchor_start.container_initial_offset,
|
||||
container.scroll_offset,
|
||||
content_widget=anchor_start.content_widget,
|
||||
content_offset=anchor_start.content_offset,
|
||||
)
|
||||
|
||||
def _extend_selection_to_click(
|
||||
screen: Screen,
|
||||
anchor: SelectState | None,
|
||||
event: _shift_events.MouseDown,
|
||||
) -> None:
|
||||
click_state = screen._select_state
|
||||
if anchor is None or click_state is None:
|
||||
return
|
||||
click = click_state.start
|
||||
# Stock Textual clears the selection when a MouseUp lands on the
|
||||
# offset of its MouseDown. Forget the offset so the Shift+click's own
|
||||
# MouseUp leaves the extension we install below alone.
|
||||
screen._mouse_down_offset = None
|
||||
screen._select_state = SelectState(
|
||||
event.screen_offset,
|
||||
_rebase_anchor_scroll(anchor.start),
|
||||
SelectEnd(click.container, click.content_widget, click.content_offset),
|
||||
)
|
||||
|
||||
def _forward_event_with_shift_select(self: Screen, event: Event) -> None:
|
||||
shift_anchor = _shift_click_anchor(self, event)
|
||||
_original_forward_event_with_shift(self, event)
|
||||
if isinstance(event, _shift_events.MouseDown):
|
||||
_extend_selection_to_click(self, shift_anchor, event)
|
||||
|
||||
try:
|
||||
_ShiftScreen._forward_event = _forward_event_with_shift_select # ty: ignore[invalid-assignment]
|
||||
except (AttributeError, TypeError) as exc: # pragma: no cover - defensive
|
||||
logger.warning(
|
||||
"Textual Shift+click selection patch assignment rejected (textual %s): %s",
|
||||
_textual_version,
|
||||
exc,
|
||||
)
|
||||
|
||||
|
||||
try:
|
||||
from textual.screen import Screen as _HitScreen
|
||||
|
||||
|
||||
@@ -7,6 +7,8 @@ from __future__ import annotations
|
||||
|
||||
import ast
|
||||
import importlib.util
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
@@ -14,7 +16,7 @@ from textual import events
|
||||
from textual._time import get_time
|
||||
from textual._xterm_parser import XTermParser
|
||||
from textual.app import App, ComposeResult
|
||||
from textual.containers import Vertical
|
||||
from textual.containers import Vertical, VerticalScroll
|
||||
from textual.content import Content
|
||||
from textual.geometry import Offset
|
||||
from textual.selection import Selection
|
||||
@@ -57,6 +59,15 @@ class SelectableHistoryApp(App[None]):
|
||||
yield Static("second message", id="second")
|
||||
|
||||
|
||||
class SelectableScrollApp(App[None]):
|
||||
CSS = "VerticalScroll { height: 8; }"
|
||||
|
||||
def compose(self) -> ComposeResult:
|
||||
with VerticalScroll(id="history"):
|
||||
for index in range(1, 31):
|
||||
yield Static(f"line{index:02d} content", id=f"row{index}")
|
||||
|
||||
|
||||
class TestPatchedWordSelection:
|
||||
async def test_double_click_selects_word_not_entire_widget(self) -> None:
|
||||
async with SelectableTextApp().run_test() as pilot:
|
||||
@@ -88,6 +99,84 @@ class TestPatchedWordSelection:
|
||||
|
||||
assert pilot.app.screen.get_selected_text() == "second message"
|
||||
|
||||
async def test_shift_click_extends_drag_selection_from_anchor(self) -> None:
|
||||
async with SelectableTextApp().run_test() as pilot:
|
||||
await pilot.mouse_down("#msg", offset=(0, 0))
|
||||
await pilot.mouse_up("#msg", offset=(4, 0))
|
||||
assert pilot.app.screen.get_selected_text() == "alpha"
|
||||
|
||||
await pilot.click("#msg", offset=(11, 0), shift=True)
|
||||
|
||||
assert pilot.app.screen.get_selected_text() == "alpha beta g"
|
||||
|
||||
async def test_shift_click_preserves_backward_drag_anchor(self) -> None:
|
||||
async with SelectableTextApp().run_test() as pilot:
|
||||
await pilot.mouse_down("#msg", offset=(15, 0))
|
||||
await pilot.mouse_up("#msg", offset=(11, 0))
|
||||
assert pilot.app.screen.get_selected_text() == "gamma"
|
||||
|
||||
await pilot.click("#msg", offset=(0, 0), shift=True)
|
||||
|
||||
assert pilot.app.screen.get_selected_text() == "alpha beta gamma"
|
||||
|
||||
async def test_shift_click_rejects_detached_markdown_anchor(self) -> None:
|
||||
async with SelectableMarkdownApp().run_test() as pilot:
|
||||
screen = pilot.app.screen
|
||||
document = pilot.app.query_one("#msg", Markdown)
|
||||
await pilot.mouse_down("#msg", offset=(15, 0))
|
||||
await pilot.mouse_up("#msg", offset=(11, 0))
|
||||
select_state = screen._select_state
|
||||
assert select_state is not None
|
||||
anchor_widget = select_state.start.content_widget
|
||||
assert anchor_widget is not None
|
||||
|
||||
await document.update("replacement text")
|
||||
assert not anchor_widget.is_attached
|
||||
await pilot.click("#msg", offset=(0, 0), shift=True)
|
||||
|
||||
assert screen.get_selected_text() is None
|
||||
|
||||
async def test_shift_click_extends_from_anchor_after_scroll(self) -> None:
|
||||
async with SelectableScrollApp().run_test(size=(40, 8)) as pilot:
|
||||
await pilot.mouse_down("#row1", offset=(0, 0))
|
||||
await pilot.mouse_up("#row2", offset=(6, 0))
|
||||
history = pilot.app.query_one("#history", VerticalScroll)
|
||||
history.scroll_to(y=10, animate=False)
|
||||
await pilot.pause()
|
||||
|
||||
await pilot.click("#row14", offset=(6, 0), shift=True)
|
||||
|
||||
selected = pilot.app.screen.get_selected_text()
|
||||
assert selected is not None
|
||||
assert selected.startswith("line01 content")
|
||||
assert selected.endswith("line14")
|
||||
|
||||
async def test_shift_click_ignores_unmodified_click(self) -> None:
|
||||
async with SelectableTextApp().run_test() as pilot:
|
||||
await pilot.mouse_down("#msg", offset=(0, 0))
|
||||
await pilot.mouse_up("#msg", offset=(4, 0))
|
||||
assert pilot.app.screen.get_selected_text() == "alpha"
|
||||
|
||||
await pilot.click("#msg", offset=(11, 0))
|
||||
|
||||
assert pilot.app.screen.get_selected_text() is None
|
||||
|
||||
async def test_shift_click_extends_selection_across_widgets(self) -> None:
|
||||
async with SelectableHistoryApp().run_test() as pilot:
|
||||
await pilot.mouse_down("#first", offset=(6, 0))
|
||||
await pilot.mouse_up("#first", offset=(12, 0))
|
||||
assert pilot.app.screen.get_selected_text() == "message"
|
||||
|
||||
await pilot.click("#second", offset=(6, 0), shift=True)
|
||||
|
||||
assert pilot.app.screen.get_selected_text() == "message\nsecond"
|
||||
|
||||
async def test_shift_click_without_selection_remains_unselected(self) -> None:
|
||||
async with SelectableTextApp().run_test() as pilot:
|
||||
await pilot.click("#msg", offset=(7, 0), shift=True)
|
||||
|
||||
assert pilot.app.screen.get_selected_text() is None
|
||||
|
||||
|
||||
class TestDetachedHitGuard:
|
||||
"""Coverage of the Textualize/textual#6643 crash guard."""
|
||||
@@ -136,6 +225,25 @@ class TestDetachedHitGuard:
|
||||
assert hit_offset == Offset(2, 0)
|
||||
|
||||
|
||||
def test_missing_shift_selection_internals_does_not_break_import() -> None:
|
||||
"""Missing private classes must skip only the best-effort Shift patch."""
|
||||
code = (
|
||||
"import textual.selection\n"
|
||||
"del textual.selection.SelectEnd\n"
|
||||
"del textual.selection.SelectState\n"
|
||||
"import deepagents_code._textual_patches\n"
|
||||
)
|
||||
result = subprocess.run(
|
||||
[sys.executable, "-c", code],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=30,
|
||||
check=False,
|
||||
)
|
||||
|
||||
assert result.returncode == 0, result.stderr
|
||||
|
||||
|
||||
class TestPatchedSequenceToKeyEvents:
|
||||
r"""Targeted coverage of the two interventions in the shim."""
|
||||
|
||||
|
||||
Reference in New Issue
Block a user