ADR 0030: LSP completion + hover (Phase 17)¶
Status¶
Accepted
Context¶
Phase 16 (ADR 0029) wired diagnostics end to end but left completion
and hover as core-only stubs — ase_lsp_client_request_completion
existed but nothing in the GUI ever called it, and hover didn't exist
in the client at all. This phase closes both, with two deliberate,
user-chosen departures from this project's usual "manual trigger"
philosophy (:compile's Ctrl+B, the command line's Ctrl+;): completion
fires automatically while typing, and hover fires from the mouse
pausing over a symbol, rather than either needing a keybinding. Both
were an explicit choice — the alternative (manual triggers, e.g.
Ctrl+Space and Ctrl+K) was offered and declined.
Decision¶
Core: textDocument/hover¶
ase_lsp_client_request_hover(client, uri, position, callback,
user_data) — same request/response shape as completion/definition,
the raw JSON result handed back unparsed (a hover response's
contents varies by server: a plain string, a MarkupContent
{kind, value}, a single legacy MarkedString, or an array of them —
parsing that variety belongs at the call site, not duplicated as a
core-level union type). fake_lsp_server.c and test_lsp_client.c
got a matching canned-response/assertion pair, following Phase 16's
pattern exactly.
No request-sequence tracking — a deliberate v1 simplification¶
Neither completion nor hover requests carry a sequence token to
discard stale, out-of-order responses. AseLspResultCallback's
user_data is fixed at registration time (always this), so
disambiguating would need a small heap-allocated context per request —
and ase_lsp_client_stop() never fires a still-pending request's
callback (the pending-request table is simply freed, not drained), so
a request in flight when the client stops (e.g. openFile() switching
documents) would leak that context. Avoiding the allocation entirely
avoids the leak. In practice a request is local and fast (well under
typing speed), nothing is ever auto-applied without the user pressing
Enter/Tab, and a hover response never clobbers an already-good tooltip
(see applyLspHover's doc comment) — so the worst case is a one-
keystroke-stale completion list that the very next response corrects,
not a correctness bug.
Completion: gated automatic trigger, not "after every keystroke"¶
requestCompletionIfAppropriate() runs from the end of
refreshCache() — the same choke point sendLspDidChange() already
uses — but only actually sends a request when exactly one cursor with
no active selection sits right after an identifier character or a
member-access trigger (., or the > of ->; C has no ::).
Anything else (a space, a newline, punctuation) dismisses whatever's
open instead of firing a request. Without this gate, "automatic"
would mean a wall of irrelevant global-symbol completions after every
space — every real editor's automatic mode gates the same way client-
side, LSP servers don't do it for you.
Accepting (acceptCompletion(), wired to Enter and Tab while the
popup is showing) replaces [m_completionPrefixStart, cursor) with
the selected item's insertText by giving insertText() a temporary
single-cursor selection over that range — it already knows how to
replace an active selection (docs/adr/0019), so this reuses that path
instead of duplicating replace-range logic. m_completionPrefixStart
is recomputed on every request by scanning backward from the cursor
while bytes are identifier characters (reusing the existing
isWordChar predicate) — the same byte-level "ASCII v1
simplification" this codebase already applies to LSP character offsets
(docs/adr/0029).
A real, fixed rough edge found during live verification: accepting
inserts an identifier, which itself typically still ends in a word
character, so the very insert that follows (insertText() calling
refreshCache() calling requestCompletionIfAppropriate() again)
would immediately reopen a fresh popup showing the item just
accepted — not what any editor actually does. Fixed with
m_suppressNextCompletionTrigger, a one-shot flag set right before
acceptCompletion()'s insertText() call and consumed by the very
next requestCompletionIfAppropriate().
Hover: pause-then-request, word-range aware dismissal¶
EditorViewport::mouseMoveEvent now also runs with no mouse button
held (setMouseTracking(true), new in this phase — previously the
widget only got move events during an active drag). A qualifying move
calls scheduleHoverRequest(), which restarts a 500ms single-shot
timer (m_hoverTimer) unless the pointer is still within the buffer
range the currently shown tooltip covers — avoiding a flicker/re-
request loop from ordinary small hand jitter while reading a tooltip.
That range comes from the response's own range field when the
server sends one (clangd does), falling back to a client-side
identifier-run scan around the requested offset otherwise. The
tooltip also dismisses on: any key press, a click, a wheel scroll (the
screen point it was anchored to is now wrong), the mouse leaving the
widget, and the widget losing focus (leaveEvent/focusOutEvent,
both new overrides).
No markdown rendering — extractHoverText() pulls plain text out of
whichever contents shape the server sent (string / {value} object
/ array of either, joined with blank lines) and shows it as-is in a
word-wrapped label. clangd's plaintext hover (type, value, containing
scope, the declaration line) reads perfectly well unstyled and matches
this app's austere aesthetic; full markdown rendering was cut as
scope not worth it for v1.
Two new widgets, deliberately not FloatingPanel¶
CompletionPopup and HoverPanel (gui/src/completion_popup.{h,cpp},
gui/src/hover_panel.{h,cpp}) share FloatingPanel's flat/translucent
visual language (same panelBackgroundColor()/panelBorderColor()
accessors) but don't derive from it and don't use its host-centered
anchor system or snapshot-based scale+fade animation
(docs/adr/0022, docs/adr/0024). Both track a moving point (the caret,
the mouse) and refresh content far more often than a glance-act-
dismiss chrome window like Find/Replace — replaying a scale-pop
animation on every keystroke would read as busy, not smooth, so each
only fades (faster, 90ms vs. FloatingPanel's 110ms) and only on the
hidden→visible edge; an already-open widget just updates content and
geometry instantly. Positioning is a plain move()/setGeometry()
clamped to stay inside the viewport, flipping above the anchor point
instead of below when there's no room.
CompletionPopup paints its own rows via QPainter rather than using
QListWidget — this codebase already hit a real class of "geometry
wrong until some later Qt-internal layout pass" bugs with
QListWidget in FileBrowserPanel (docs/adr/0024), and this widget
resizes on nearly every keystroke, far more often than that one ever
did. HoverPanel sizes its wrapped text the same way, using
QFontMetrics::boundingRect(..., Qt::TextWordWrap, text) directly
instead of reading a size back from QLabel after the fact — the same
"measure explicitly, don't trust a widget's own lazy layout pass"
reasoning, echoing xForColumn's approach to caret positioning
(docs/adr/0013).
Both are wired into main.cpp exactly like every other panel:
constructed as children of viewport, handed back to it via a setter
(setCompletionPopup/setHoverPanel), theme-refreshed on config
hot-reload alongside the rest.
Consequences¶
All 9 ctest suites pass, including the new
ase_lsp_client_tests hover round-trip. Verified live against real
clangd: typing pri inside main() produced a live-updating,
correctly-positioned completion popup with real macro completions
from <stdio.h>'s preamble (clangd itself prefixes macro-kind labels
with • — confirmed as the server's own convention, not a parsing
bug, by inspecting the rendered rows at high zoom); navigating with
arrow keys and accepting with Enter correctly replaced the typed
prefix (pri → PRId64) with no leftover duplicate text and no
immediate popup reopen; hovering over value after a ~500ms pause
showed clangd's real hover payload (variable value, Type: int,
Value = 42 (0x2a), the declaration line) in a correctly word-wrapped
panel; moving the mouse away dismissed it.
Not attempted: markdown rendering of hover content (plain text only —
see above); completion item kind-based icons/badges (label + detail
only, no per-kind glyph); incremental/fuzzy client-side filtering of
an already-fetched list as the user keeps typing past what the last
response covers (every keystroke sends a fresh request instead,
acceptable at local-clangd latency); go-to-definition
(ase_lsp_client_request_definition) still has no GUI wiring — it
remains, like completion was before this phase, implemented at the
core layer only.