Conversation
…x workflow Diagnostics and fixes are separate LSP channels: fixes must be requested via textDocument/codeAction with the diagnostics as context. We consumed only publishDiagnostics, so the servers' quickfixes (vtsls implements them fully) never reached CodeInspection's existing fix UI (fixable icons, per-problem fix buttons, Fix All). Server-agnostic by construction - everything lives on the generic LanguageClient prototype and the per-client LintingProvider, gated purely on runtime capabilities, so any future server (PHP etc.) inherits quickfixes with zero extra wiring: - LSPClient: advertise codeAction literal support (quickfix kind; no resolveSupport, so conformant servers inline edits - one round trip). New requestCodeActions (flush -> codeAction with context.diagnostics, gated on the server's codeActionProvider, boolean or object) and resolveCodeAction (only for actions missing `edit` on servers advertising resolveProvider). publishDiagnostics hands the RAW diagnostics (range/code/data intact - the provider flattens them for CodeInspection) to the quickfix layer. - LintingProvider: idle-fetch policy so this is cheap - fetch ~800ms after diagnostics settle, rescheduled by newer publishes (zero codeAction traffic while typing), only for the active server-synced file, deduped by (lastChangeTimestamp, diagnostics signature) so identical re-publishes and file switches to unchanged files cost zero requests; an activeEditorChange hook catches diagnostics that arrived in a background pane. Strategy is runtime-adaptive: one batched whole-file request (vtsls answers it), falling back to per-diagnostic requests capped at 20. Admitted actions: literal quickfix, not disabled, exactly one TextEdit in exactly this file -> converted to CodeInspection's fix contract (replaceText + character offsets via indexFromPos) and attached to the matching cached error, then CodeInspection.requestRun() re-registers them (staleness stays automatic: CodeInspection drops fixes whenever the document changes). Identical re-publishes now also KEEP the existing errors array instead of replacing it - a fresh array would silently wipe the decorated fixes without triggering a re-fetch. Diagnostics additionally carry endPos now, so squiggles span the server-reported range instead of one token. - CodeInspection: correct the stale fix JSDoc (replace -> replaceText, which _isInvalidFix actually requires). End-to-end test in integration:TypeScript LSP: consol.log(1) -> the 2552 diagnostic appears, the idle fetch surfaces Fix All, clicking it rewrites to console.log(1). Fix application needs no editor focus, so it runs in an unfocused test window. Suite green 14/14; verified live (single fix, Fix All, icons reappearing after typing settles).
The QuickView popover centers and shrink-wraps each provider's content (.quick-view-item is a flex column with align-items:center - right for image/color previews). When another provider widens the popover - e.g. the problem row with its Fix button that now accompanies quickfixes - the narrower hover content floated centered in the leftover space (a small signature box in the middle of a wide popup). align-self:stretch on .lsp-hover-quickview makes just the LSP hover fill the available width, left-aligned, while image previews stay centered as designed.
The intermittent CI failure of "should jump to definition" (LegacyInteg: EditorCommandHandlers) was not cold-start latency: the spec's first NAVIGATE_JUMPTO_DEFINITION fired while tsserver was still loading the project, the server answered with no definition, and doJumpToDef's deferred only resolved inside `if (msgObj && msgObj.range)` - an empty result NEVER settled it. The command promise then hung forever, and awaitsFor races each poll invocation against the FULL timeout, so the "retry loop" never retried once and the spec burned its whole budget on one dead promise (the SpecRunner.js:141 stack in the CI logs). Manual jumps worked the whole time - the server was healthy; the spec was pinned. - DefaultProviders.doJumpToDef: settle (reject) when the server returns no definition, and on a failed cross-file FILE_OPEN. Also fixes real users: F12 on a symbol with no definition left a permanently pending command. - src-node/lsp-client: on unexpected server exit, reject all in-flight requests immediately - the dead process can never answer them, and they previously hung callers until the 2-minute per-request timeout (only graceful stops flushed the pending map). - The spec: the one-time tsserver cold start now happens in the suite's beforeAll warm-up (the TypeScript LSP suite's pattern, 90s budget there); the spec itself waits max 15s, polling every 500ms with each jump attempt time-boxed to 3s - a slow or rejected attempt is a retry, never a pin - and a rejected jump no longer aborts awaitsFor via an exception. LegacyInteg:EditorCommandHandlers Integration green 12/12 twice in a row (including the focus-sensitive clipboard spec).
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Diagnostics and fixes are separate LSP channels: fixes must be requested via
textDocument/codeAction with the diagnostics as context. We consumed only
publishDiagnostics, so the servers' quickfixes (vtsls implements them fully)
never reached CodeInspection's existing fix UI (fixable icons, per-problem
fix buttons, Fix All).
Server-agnostic by construction - everything lives on the generic
LanguageClient prototype and the per-client LintingProvider, gated purely on
runtime capabilities, so any future server (PHP etc.) inherits quickfixes
with zero extra wiring:
LSPClient: advertise codeAction literal support (quickfix kind; no
resolveSupport, so conformant servers inline edits - one round trip). New
requestCodeActions (flush -> codeAction with context.diagnostics, gated on
the server's codeActionProvider, boolean or object) and resolveCodeAction
(only for actions missing
editon servers advertising resolveProvider).publishDiagnostics hands the RAW diagnostics (range/code/data intact - the
provider flattens them for CodeInspection) to the quickfix layer.
LintingProvider: idle-fetch policy so this is cheap - fetch ~800ms after
diagnostics settle, rescheduled by newer publishes (zero codeAction
traffic while typing), only for the active server-synced file, deduped by
(lastChangeTimestamp, diagnostics signature) so identical re-publishes and
file switches to unchanged files cost zero requests; an activeEditorChange
hook catches diagnostics that arrived in a background pane. Strategy is
runtime-adaptive: one batched whole-file request (vtsls answers it),
falling back to per-diagnostic requests capped at 20. Admitted actions:
literal quickfix, not disabled, exactly one TextEdit in exactly this file
-> converted to CodeInspection's fix contract (replaceText + character
offsets via indexFromPos) and attached to the matching cached error, then
CodeInspection.requestRun() re-registers them (staleness stays automatic:
CodeInspection drops fixes whenever the document changes).
Identical re-publishes now also KEEP the existing errors array instead of
replacing it - a fresh array would silently wipe the decorated fixes
without triggering a re-fetch. Diagnostics additionally carry endPos now,
so squiggles span the server-reported range instead of one token.
CodeInspection: correct the stale fix JSDoc (replace -> replaceText, which
_isInvalidFix actually requires).
End-to-end test in integration:TypeScript LSP: consol.log(1) -> the 2552
diagnostic appears, the idle fetch surfaces Fix All, clicking it rewrites to
console.log(1). Fix application needs no editor focus, so it runs in an
unfocused test window. Suite green 14/14; verified live (single fix, Fix
All, icons reappearing after typing settles).