Conversation
Add Language Server Protocol support to the editor, desktop-only (it depends
on the Node layer to spawn server processes). The first server is vtsls,
bundled in src-node so there is zero user setup. The framework is lazy-loaded
so boot stays fast: the Node module is required on demand and the front end is
pulled in after boot. All six features are capability-gated and degrade
gracefully: completion, hover, jump-to-definition, signature help, diagnostics
and find-references.
Architecture
- src-node/lsp-client.js: NodeConnector-backed ("ph-lsp") multi-server manager
speaking JSON-RPC over stdio with Content-Length framing; resolves servers
from node_modules/.bin then PATH. Not required at boot (lazy-loaded).
- src/languageTools/LSPClient.js: browser-side client that exposes the method
surface DefaultProviders expects, centralizes {line,ch}<->{character} and
path<->file:// URI translation (including the /tauri virtual-path prefix),
returns jQuery Deferreds, routes publishDiagnostics into CodeInspection, and
auto-restarts a crashed server. Completion results are cached per context.
- src/languageTools/DocumentSync.js: generic didOpen/didChange(debounced)/
didClose lifecycle. flush() now syncs on actual text difference rather than a
pending debounce timer, fixing a race where a completion request (e.g. on
".") could reach the server before the keystroke, returning globals instead
of members.
- src/languageTools/HoverProvider.js + DocumentHighlight.js: hover docs via
QuickViewManager and cursor-occurrence highlighting via documentHighlight.
- src/extensions/default/TypeScriptSupport: thin per-language config; registered
via DefaultExtensions.json desktopOnly.
Code hints / insertion
- DefaultProviders.insertHint anchors the replacement start on the server's
textEdit.range.start (stable as you type forward; for members it points at the
trigger "." whose dot is part of newText) and ends at the current cursor,
fixing both "console..log" and the stale-cache "consolenso".
- Snippet completions (insertTextFormat 2) are processed properly instead of
inserting literal "$1": new reusable editor/TabstopManager expands the LSP
snippet grammar ($1/$0, ${1:placeholder}, ${1|a,b|}, variables, escapes),
places the caret at the first stop, and starts a Tab/Shift-Tab navigable
session (marker-backed, so stops survive later edits such as an auto-import
line). additionalTextEdits are applied for real auto-imports. The Emmet
expander is left untouched with a pointer comment for future migration.
- CodeHintList/CodeHintManager: scope row queries to li.code-hints-list-item so
documentation-popup markup no longer pollutes the list, clear stale highlights
on keyboard nav, and make session begin/update foolproof against duplicate
lists.
- KeyBindingManager: fall back to event.code when event.key is "Unidentified"/
"Dead"/empty so Ctrl-Space works under IME.
UI
- Theme-matched, opaque scrollbar for the codehint/inlinemenu dropdown; the
app-wide transparent track previously let the editor show through the popup's
scroll gutter.
- Themed hover quickview and doc-popup styling.
The desktop LSP support bundles @vtsls/language-server (MIT) and its bundled typescript (Apache-2.0) in src-node. Add their license files to src/thirdparty/licences/ and register the copy in the gulp thirdparty-lib-copy task so builds keep them in sync.
makeConcatExtensions fails on any default extension missing from its minify lists. Add TypeScriptSupport to minifyableExtensions.
Cover plain text, simple stops ($1/$0/${0}), ordering (positives ascending
with $0 last, mirror dedupe), placeholders incl. nested, choices, variables,
escapes and multi-line offsets. Registered in UnitTestSuite.
Also fix parseSnippet to preserve tab-stops nested inside a placeholder
default (e.g. ${1:a ${2:b} c}), which were previously discarded.
src-node deps aren't installed in the pipeline (the desktop app build installs them separately), so the copy would fail. The license files are already vendored under src/thirdparty/licences/; comment out the copy with a note.
Real-editor (createMockEditor) tests for insertSnippet: single-stop caret placement, placeholder selection, the import-completion snippet, multi-stop Tab/Shift-Tab navigation ending at $0, Esc, markers following edits above the snippet, and range replacement.
abose
changed the title
The Adobe-era NodeDomain transport is fully replaced by the new LSP framework (LSPClient.js + src-node/lsp-client.js) and was no longer referenced by the app. Remove it and its now-dangling references: - delete LanguageTools.js, ClientLoader.js, BracketsToNodeInterface.js, LanguageClientWrapper.js, DefaultEventHandlers.js, ToolingInfo.json, node/RegisterLanguageClientInfo.js and the LanguageClient/ directory - drop the dead boot requires from test/SpecRunner.js (keep PathConverters and DefaultProviders, which the new framework reuses) - delete the disabled LanguageTools-test.js and its exclusive fixtures (clients/, project/, server/); keep css-language-service, still used by Extn-CSSCodeHints-integ-test - remove the disabled-test line from UnitTestSuite.js - drop the orphaned LANGUAGE_TOOLS_PREFERENCES string - drop the stale languageTools entries from cacheManifest.json
languageTools is a core module loaded at boot, so its code-hint styles belong in the core stylesheet rather than a per-extension CSS loaded at runtime. - move all rules from languageTools/styles/default_provider_style.css into src/styles/brackets.less (next to the existing .lsp-hover-quickview / .lsp-hint-doc-popup styles), adapting dark variants to .dark & nesting - delete the css file and the ExtensionUtils.loadStyleSheet call (and the now unused ExtensionUtils require) from DefaultProviders.js
The LSP framework grew the minified bundle to ~10.02 MB, tripping the 10 MB prod limit. Bump to 11 MB to restore the ~1 MB individual-file margin, and document the margin policy (1 MB per file, 5 MB aggregate).
The TypeScriptSupport extension auto-starts a real vtsls language server and registers global providers (hover/QuickView, linting/CodeInspection, hints, jump-to-def) for js/jsx/ts/tsx. In the test runner this polluted pre-existing integration tests that assume nothing else is registered for those languages: - QuickView: a removed JS hover provider still returned a popover (ours). - ESLint: jsx files gained an active inspector, so inspection-disabled was false. - PreferencesManager: real server spawn + doc-sync churn timed the spec out. Strip it in test windows, matching the existing convention for the heavyweight Git extension (excluded for the same reason in ExtensionLoader).
Re-enable the TypeScriptSupport (vtsls) extension in test windows so the integration/LegacyInteg suites reflect the desktop app, which has LSP-backed JavaScript/TypeScript intelligence. This supersedes the test-window exclusion added in 1c4ce4d (only the heavyweight Git extension stays excluded); the failures that exclusion was hiding are now fixed at their root instead. Provider fixes (affect the shipping app, not just tests): - JumpToDefProvider.doJumpToDef now uses the editor that JumpToDefManager passes in, instead of EditorManager.getFocusedEditor(). getFocusedEditor() returns null whenever the editor lacks DOM focus (jump invoked from a menu/command, or under test), which crashed on a null editor and left dirty editor state that leaked into later tests. - LintingProvider no longer triggers CodeInspection.requestRun() when (a) its LSP inspector is not a currently-registered provider for the file, or (b) the server re-publishes diagnostics identical to what was last surfaced. Servers re-publish in waves and on every edit, often unchanged; needlessly re-running inspection rebuilds the Problems panel, wasting work and detaching live DOM (e.g. inline fix buttons mid-click). It also disrupted tests that take manual control of the inspection pipeline with their own mock linters. Tests adjusted for the desktop-LSP reality (branch on Phoenix.isNativeApp where JavaScript itself is under test; isolate from the LSP where it is incidental): - jump-to-definition: vtsls returns the full declaration range, so desktop lands at its start; browser still selects the identifier via Tern. - code hints inside a regex: vtsls returns zero completions, so assert "no hints shown" (list absent, or present but closed and empty) rather than strictly null. - ESLint jsx: the LSP lints .jsx, so on desktop the inspector is active; the browser build (no LSP, ESLint v8 declines react) keeps it disabled. - QuickView per-language register/unregister: switched the example language from JavaScript to HTML (not served by the LSP) so the test stays focused on registration mechanics with no competing provider.
…lems panel - Forward the user's UI locale (brackets.getLocale()) as LSP InitializeParams.locale on initialize, so vtsls/tsserver emit their diagnostics and hover/quick-info text in the user's language. Unknown locales fall back to English, so this is safe everywhere. - Name the CodeInspection provider after the bare server id (e.g. "typescript") instead of "<id> (LSP)". The Problems panel read "0 typescript (LSP) Problems"; to the user this is just built-in language support, and the LSP backing is an internal detail they don't need to see. The same id still drives the registered-inspector guard, so behaviour is unchanged.
A language server's first publishDiagnostics for a file it has nothing to say about is an empty set. The diagnostic-change guard treated "never recorded" as different from "[]", so that initial empty publish counted as a change and fired CodeInspection.requestRun(), rebuilding the Problems panel. When that rebuild landed while a fix button was being clicked, the button detached and the fix never applied (e.g. the ESLint v9 "fix 1 error" integration spec timed out). Treat an unrecorded file as already-empty, so an initial empty publish is not a change. Real diagnostics and diagnostic clears still trigger a re-run.
tsserver type-checks JS too and emits the noImplicitAny family (7005-7034, including 7016 "Could not find a declaration file for module ... implicitly has an 'any' type. Try `npm i --save-dev @types/...`") as suggestions. For a developer who hasn't opted into typed JS these are noise - "go install @types" prompts they never asked for. Suppress those codes for javascript/jsx files unless the project opts into type-checking via compilerOptions.checkJs (tsconfig/jsconfig) or a per-file // @ts-check - matching how VS Code gates JS type diagnostics. Errors, warnings, unused/deprecated hints, and all type intelligence (hover/completion) are untouched; TypeScript/TSX is unaffected. The LSP framework gains a generic per-server `filterDiagnostics` hook applied as diagnostics arrive. TypeScriptSupport supplies the policy and detects checkJs by reading tsconfig/jsconfig at the project root (JSONC-aware), re-evaluating on project switch and when those configs change.
Info-level (META) diagnostics are listed in the panel but weren't counted in the title, so a file with only suggestions read "0 Problems" above a visible info row. Count them separately and append "(N Info)" when present, e.g. "0 typescript Problems - foo.js (2 Info)". Error/warning counts and the existing title formats are unchanged - the suffix only appears when there are infos.
The test runner loads every default extension's unittests.js. TypeScriptSupport
had none, so the request 404'd and the returned HTML error page was parsed as JS
("SyntaxError: Unexpected token '<'"), reported as a suite-load failure. Add an
empty unittests.js, matching the convention used by other extensions that are
loaded in test windows but ship no unit tests (LightTheme, DarkTheme,
HandlebarsSupport). Git avoids this only because it is excluded from test windows.
Cover the desktop LSP end to end against the real vtsls server: - a .ts file reports a real type error, - a JS project that opts into checkJs (jsconfig) shows implicit-any (kept), - a plain JS project does NOT show implicit-any (suppressed) - with a checkJs precondition so the negative assertion reflects gating, not timing. The tests live in the extension's unittests.js (previously an empty shim) and follow the standard createTestWindowAndRun + brackets.test.* integration pattern; they require core modules via brackets.getModule and are gated to the desktop build (Phoenix.isNativeApp), skipped in the browser. main.js sets a _TypeScriptSupportReadyToIntegTest flag so the tests can await server startup. Fixtures are tiny and need no node_modules. Verified 3/3 passing.
…s late At app start a session-restored file is opened before the language server finishes starting, so the per-file setup it normally gets on a file switch never ran - jump-to-definition did nothing and "Find Usages" stayed disabled until the user switched files once. - DocumentSync.openSupportedDocuments now also didOpens the active editor's document (it may not yet be in getAllOpenDocuments() at server-start time), so the file the user is looking at is synced immediately. - FindReferencesManager refreshes the command's enabled state when a provider registers (the state is otherwise only computed on file switch), so a late-registering LSP references provider lights up "Find Usages" right away.
… Find Usages) The hover popup now shows a pinned action footer below the (scrollable) documentation: "Go to Definition" on the left and "Find Usages" on the right, each gated on what the server actually advertises (definitionProvider / referencesProvider). Clicking places the cursor at the hovered symbol, dismisses the popup, and runs the command - so the action targets what you hovered. The keyboard shortcut is shown as a tooltip rather than inline to keep the row tidy. Also rename the commands to match common IDE wording (applies to the context menu too): "Jump to Definition" -> "Go to Definition", "Find All References" -> "Find Usages". Adds QuickViewManager.hideQuickView() so an action can dismiss the popup; hover styling lives in brackets.less (theme-aware).
The Find Usages command's enabled state is computed on file switch via hasReferences() (which needs the server's capabilities). On a project switch the server restarts asynchronously, so that state is computed while capabilities are still unavailable and the command is left disabled - and CommandManager.execute() rejects disabled commands, so both the menu item and the hover quick-action no-op until the user switches files again. After restartLanguageServer brings the server back with its capabilities, refresh the command state for the active file so Find Usages works immediately.
Run the hover's fenced code block (the type/function signature) through the globally available highlight.js so it reads like code instead of flat monospace. The app's hljs theme is locked to github-dark, so the hover scopes its own token colours: a GitHub-light palette in the light editor theme and GitHub-dark in the dark theme (the signature chip background follows the editor theme).
…nd Usages) Cover the hover quick-actions end to end against the real vtsls server, for both JS and TS: the hover comes up with both actions, clicking "Go to Definition" jumps from a call to the declaration, and clicking "Find Usages" opens the references panel. The clicks retry through the hover so they survive the server's post-(re)start indexing window. Fixtures live in their own per-language project folders (hover-ts has a tsconfig so the server treats it as a project, which reference search needs).
|
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.
Add Language Server Protocol support to the editor, desktop-only (it depends on the Node layer to spawn server processes). The first server is vtsls, bundled in src-node so there is zero user setup. The framework is lazy-loaded so boot stays fast: the Node module is required on demand and the front end is pulled in after boot. All six features are capability-gated and degrade gracefully: completion, hover, jump-to-definition, signature help, diagnostics and find-references.
Architecture
Code hints / insertion
UI