diff --git a/lib/lsp-devtools/changes/247.fix.md b/lib/lsp-devtools/changes/247.fix.md new file mode 100644 index 0000000..f1e6757 --- /dev/null +++ b/lib/lsp-devtools/changes/247.fix.md @@ -0,0 +1 @@ +Preserve the currently selected message in `lsp-devtools inspect` (and the client inspector) when new traffic arrives, instead of always jumping to the latest row. Auto-follow still happens when the cursor is already on the last message. diff --git a/lib/lsp-devtools/lsp_devtools/cli/client/__init__.py b/lib/lsp-devtools/lsp_devtools/cli/client/__init__.py index 44a4e60..d764468 100644 --- a/lib/lsp-devtools/lsp_devtools/cli/client/__init__.py +++ b/lib/lsp-devtools/lsp_devtools/cli/client/__init__.py @@ -12,7 +12,6 @@ from textual import on from textual.app import App from textual.containers import Vertical -from textual.message import Message from textual.widgets import Button from textual.widgets import Footer @@ -153,7 +152,9 @@ async def on_ready(self, event: events.Ready): @on(LiveSqlHandler.MessageReceived) def on_message_received(self, event: LiveSqlHandler.MessageReceived): browser = self.query_one(MessageBrowser) - browser.reload(follow=True) + # Only jump to the latest message when the user is already following the + # tail; otherwise preserve their selected row. See #247. + browser.reload(follow=browser.is_following_tail()) def on_button_pressed(self, event: Button.Pressed): if event.button.id == "open-settings-btn": @@ -203,7 +204,7 @@ async def start_server(self) -> LanguageClient | None: await client.start_io(*server_config.command) - result = await client.initialize_async( + await client.initialize_async( types.InitializeParams( capabilities=types.ClientCapabilities(), process_id=os.getpid(), diff --git a/lib/lsp-devtools/lsp_devtools/cli/inspector.py b/lib/lsp-devtools/lsp_devtools/cli/inspector.py index 81cceda..11a21ee 100644 --- a/lib/lsp-devtools/lsp_devtools/cli/inspector.py +++ b/lib/lsp-devtools/lsp_devtools/cli/inspector.py @@ -58,7 +58,9 @@ def compose(self) -> ComposeResult: @on(LiveSqlHandler.MessageReceived) def on_message_received(self, event: LiveSqlHandler.MessageReceived): browser = self.query_one(MessageBrowser) - browser.reload(follow=True) + # Only jump to the latest message when the user is already following the + # tail; otherwise preserve their selected row. See #247. + browser.reload(follow=browser.is_following_tail()) async def on_ready(self, event: Ready): browser = self.query_one(MessageBrowser) diff --git a/lib/lsp-devtools/lsp_devtools/inspector/message_browser.py b/lib/lsp-devtools/lsp_devtools/inspector/message_browser.py index f37afcb..703f81f 100644 --- a/lib/lsp-devtools/lsp_devtools/inspector/message_browser.py +++ b/lib/lsp-devtools/lsp_devtools/inspector/message_browser.py @@ -2,6 +2,7 @@ import typing from datetime import datetime +from datetime import timezone from textual.containers import Container from textual.containers import Horizontal @@ -141,17 +142,33 @@ def reset(self): self.clear() self._last_rowid = -1 + def is_following_tail(self) -> bool: + """Return ``True`` if the cursor is on (or past) the latest message. + + Used to decide whether newly arrived messages should steal focus. When the + user has selected an older row, their selection is preserved. + """ + table = self.query_one(DataTable) + if table.row_count == 0: + return True + return table.cursor_row >= table.row_count - 1 + def reload(self, follow: bool = False): """Reload messages. Parameters ---------- follow - If ``True``, move the cursor so that the most recent message is selected + If ``True``, move the cursor so that the most recent message is selected. + If ``False``, keep the currently selected row (if it still exists). """ table = self.query_one(DataTable) + selected_key: RowKey | None = None + if not follow and table.row_count > 0: + selected_key = table.coordinate_to_cell_key(table.cursor_coordinate).row_key + for rowid, message in self.app.db.find_messages(after=self._last_rowid): # TODO: Convert the filter into a SQL query so we can take advantage of # the fact we're using SQLite! @@ -162,7 +179,7 @@ def reload(self, follow: bool = False): if (msg_source := message.source) is not None: source = format_message_source(msg_source) - timestamp = message.timestamp or datetime.now() + timestamp = message.timestamp or datetime.now(timezone.utc) key = table.add_row( f"{timestamp:%H:%M:%S.%f}", source, @@ -175,3 +192,5 @@ def reload(self, follow: bool = False): if follow: table.action_scroll_bottom() + elif selected_key is not None and selected_key in self._messages: + table.move_cursor(row=table.get_row_index(selected_key), animate=False) diff --git a/lib/lsp-devtools/tests/inspector/test_message_browser.py b/lib/lsp-devtools/tests/inspector/test_message_browser.py new file mode 100644 index 0000000..af3e88f --- /dev/null +++ b/lib/lsp-devtools/tests/inspector/test_message_browser.py @@ -0,0 +1,128 @@ +from __future__ import annotations + +from datetime import datetime +from datetime import timezone + +import pytest +from textual.app import App +from textual.app import ComposeResult +from textual.widgets import DataTable + +from lsp_devtools.handlers.jsonrpc import JsonRPCMessage +from lsp_devtools.inspector.message_browser import MessageBrowser + + +def _message(method: str, msg_id: int) -> JsonRPCMessage: + return JsonRPCMessage.client( + { + "jsonrpc": "2.0", + "id": msg_id, + "method": method, + "params": {}, + } + ) + + +class _FakeDb: + def __init__(self) -> None: + self._rows: list[tuple[int, JsonRPCMessage]] = [] + + def add(self, message: JsonRPCMessage) -> None: + rowid = len(self._rows) + 1 + message.metadata["timestamp"] = datetime.now(timezone.utc).isoformat(" ") + self._rows.append((rowid, message)) + + def find_messages(self, after: int = -1): + for rowid, message in self._rows: + if rowid > after: + yield rowid, message + + def get_method_names(self) -> list[str]: + return sorted( + {m.method for _, m in self._rows if m.method is not None}, + ) + + +class BrowserApp(App[None]): + def __init__(self, db: _FakeDb, *args, **kwargs): + super().__init__(*args, **kwargs) + self.db = db + + def compose(self) -> ComposeResult: + yield MessageBrowser() + + +@pytest.mark.asyncio +async def test_reload_preserves_selection_when_not_following(): + """New messages must not steal the selected row when follow=False (#247).""" + db = _FakeDb() + for i in range(3): + db.add(_message(f"method/{i}", i)) + + app = BrowserApp(db) + async with app.run_test() as pilot: + browser = app.query_one(MessageBrowser) + table = browser.query_one(DataTable) + + browser.reload() + await pilot.pause() + assert table.row_count == 3 + + table.move_cursor(row=1, animate=False) + await pilot.pause() + selected = table.coordinate_to_cell_key(table.cursor_coordinate).row_key + + db.add(_message("method/new", 99)) + browser.reload(follow=False) + await pilot.pause() + + assert table.row_count == 4 + assert table.coordinate_to_cell_key(table.cursor_coordinate).row_key == selected + assert table.cursor_row == 1 + + +@pytest.mark.asyncio +async def test_reload_follows_tail_when_requested(): + """follow=True should select the newest message.""" + db = _FakeDb() + for i in range(2): + db.add(_message(f"method/{i}", i)) + + app = BrowserApp(db) + async with app.run_test() as pilot: + browser = app.query_one(MessageBrowser) + table = browser.query_one(DataTable) + + browser.reload() + await pilot.pause() + table.move_cursor(row=0, animate=False) + await pilot.pause() + + db.add(_message("method/new", 99)) + browser.reload(follow=True) + await pilot.pause() + + assert table.cursor_row == table.row_count - 1 + + +@pytest.mark.asyncio +async def test_is_following_tail(): + db = _FakeDb() + for i in range(3): + db.add(_message(f"method/{i}", i)) + + app = BrowserApp(db) + async with app.run_test() as pilot: + browser = app.query_one(MessageBrowser) + table = browser.query_one(DataTable) + + browser.reload() + await pilot.pause() + + table.move_cursor(row=table.row_count - 1, animate=False) + await pilot.pause() + assert browser.is_following_tail() is True + + table.move_cursor(row=0, animate=False) + await pilot.pause() + assert browser.is_following_tail() is False