DelphiLSP integration: generate .delphilsp.json from the dproj and keep code insight synced with the active project - #8
Conversation
… the IDE registry Embarcadero's DelphiLSP VS Code extension reads a <project>.delphilsp.json settings file that is normally produced only by the RAD Studio IDE, leaving code insight dead for anyone working purely in VS Code. ddk delphilsp-config reconstructs that file from the same sources the IDE uses: the .dproj (evaluated for the effective configuration/platform), the installation's rsvars.bat, and the IDE's global Library Path, Browsing Path and user-defined environment variables read from HKCU\SOFTWARE\Embarcadero\BDS\<version>. The emitted dccOptions mirrors the IDE's output (verified entry-for-entry against IDE-generated files: identical -I/-U/-O/-R search paths, -D defines, -NS namespaces, -A aliases, output dirs and dllname). Targets resolve like ddk compile: project ID, name, or a .dproj path handled ad-hoc with an optional -c compiler; -o overrides the destination. Generated files carry a top-level "generatedBy": "delphi-devkit" marker so automation can tell them apart from IDE-generated ones, which are never touched. Exposed as the delphilsp-config CLI subcommand, the delphi_generate_delphilsp_config MCP tool, and the delphilsp/generate custom LSP method. All logic lives in the isolated core/src/delphilsp module; 26 inline unit tests plus 13 end-to-end tests against synthetic projects.
…lidate open files When Embarcadero's DelphiLSP extension is installed (and only then — the feature is gated on its presence and hot-enables via extensions.onDidChange), DDK now keeps DelphiLSP's settingsFile pointed at the DDK active project: switching projects updates delphiLsp.settingsFile, generating the project's .delphilsp.json on the fly when it is missing, or regenerating it when it is DDK-owned (generatedBy marker) and older than its .dproj. IDE-generated files are used as-is and never rewritten. A Generate DelphiLSP Config context-menu action covers the explicit case. DelphiLSP's server applies a settings change only to files opened afterwards: already-open files keep their stale diagnostics until edited (its own Select project settings command has the same limitation, and — verified via the server's raw LSP logs — even a full server restart replays didOpen before the settings arrive, so neither helps). After the switch DDK therefore re-emits didOpen for every open Delphi document through a language-mode round-trip (objectpascal -> plaintext -> back), which re-validates it under the new project while leaving tabs, focus, cursor, dirty state and undo history untouched. New settings: ddk.delphilsp.autoSync and ddk.delphilsp.revalidateOpenFilesOnSwitch, both enabled by default. All extension logic is isolated in src/delphilsp/ with a one-line hookup in runtime.ts.
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Adds first-class DelphiLSP integration so DDK can generate and maintain .delphilsp.json settings from .dproj + installation/IDE context, and keep DelphiLSP synced with the currently active DDK project.
Changes:
- Introduces a new
core::delphilspmodule to generate.delphilsp.json, plus CLI/MCP/LSP plumbing to invoke it. - Adds a VS Code extension feature that detects DelphiLSP, auto-syncs
delphiLsp.settingsFile, and revalidates open Delphi files on project switch. - Updates documentation and changelog to describe new commands/settings and behavior.
Reviewed changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| vscode_extension/src/runtime.ts | Initializes DelphiLSP feature early so it can hook initial project refresh. |
| vscode_extension/src/delphilsp/feature.ts | Adds optional DelphiLSP integration gated by extension availability + context key. |
| vscode_extension/src/delphilsp/commands.ts | Registers “Generate DelphiLSP Config” command and calls server generator. |
| vscode_extension/src/delphilsp/autoSync.ts | Implements auto-sync of DelphiLSP settings file and open-file revalidation. |
| vscode_extension/src/constants.ts | Adds DelphiLSP-related constants, config keys, context key, and external setting names. |
| vscode_extension/src/client.ts | Adds delphilsp/generate request wrapper + hooks project updates to auto-sync. |
| vscode_extension/package.json | Contributes command/menu items + new ddk.delphilsp.* settings. |
| server/src/main.rs | Exposes delphilsp/generate custom LSP method that wraps core command. |
| mcp/src/handler.rs | Adds MCP tool delphi_generate_delphilsp_config to call the generator. |
| core/tests/delphilsp.rs | Adds end-to-end tests for .delphilsp.json generation. |
| core/tests/commands.rs | Adds display/serialization tests for DelphiLspConfigResult. |
| core/src/lsp_types.rs | Adds request params type for delphilsp/generate. |
| core/src/lib.rs | Exports new delphilsp module. |
| core/src/delphilsp/registry.rs | Reads RAD Studio library/env settings from registry (Windows) with stubs elsewhere. |
| core/src/delphilsp/mod.rs | Implements macro expansion, dccOptions assembly, file URI helpers, and file generation. |
| core/src/commands.rs | Adds cmd_delphilsp_config and request-building logic for managed/ad-hoc targets. |
| core/Cargo.toml | Adds Windows-only winreg dependency for registry reads. |
| cli/src/main.rs | Adds ddk delphilsp-config command to generate .delphilsp.json. |
| README.md | Documents new CLI/MCP/VS Code DelphiLSP integration + auto-sync behavior. |
| CHANGELOG.md | Adds Unreleased changelog entries for DelphiLSP generation and auto-sync. |
| }, | ||
| { | ||
| "command": "ddk.projects.generateDelphiLspConfig", | ||
| "title": "Generate DelphiLSP Config", | ||
| "icon": "$(json)", | ||
| "when": "ddk:delphiLspAvailable == true", | ||
| "category": "DDK" | ||
| } |
There was a problem hiding this comment.
Fixed in 73a15c1 — moved to enablement on the command contribution, and the Command Palette entry is now gated on ddk:delphiLspAvailable instead of hidden, since the handler gained an active-project fallback.
| Ok(DelphiLspOrAmbiguity::Ambiguity(ambiguity)) => { | ||
| Err(jsonrpc::Error::invalid_params(ambiguity.to_string())) | ||
| } | ||
| Err(error) => { | ||
| lsp_error!(self.client, "Failed to generate DelphiLSP settings: {}", error); | ||
| Err(jsonrpc::Error::invalid_params(format!( | ||
| "Failed to generate DelphiLSP settings: {error}" | ||
| ))) | ||
| } |
There was a problem hiding this comment.
Fixed in 73a15c1 — the Err branch now returns ErrorCode::InternalError; invalid_params stays only for the ambiguity case.
| key.enum_values() | ||
| .filter_map(|entry| entry.ok()) | ||
| // Only string values define a usable `$(NAME)` macro. | ||
| .filter(|(_, value)| matches!(value.vtype, REG_SZ | REG_EXPAND_SZ)) | ||
| .filter_map(|(name, value)| { | ||
| let text = value.to_string().trim().to_string(); | ||
| (!name.trim().is_empty() && !text.is_empty()).then_some((name, text)) | ||
| }) | ||
| .collect() |
There was a problem hiding this comment.
Fixed in 73a15c1 — values are now decoded with String::from_reg_value.
| let is_project_file = |value: &str| { | ||
| let lower = value.to_lowercase(); | ||
| lower.ends_with(".dproj") || lower.ends_with(".dpr") || lower.ends_with(".dpk") | ||
| }; |
There was a problem hiding this comment.
Fixed in 73a15c1 — extracted a shared allocation-free is_delphi_project_path helper (based on Path::extension + eq_ignore_ascii_case) and reused it in cmd_run_path too.
| let result = generate(&fixture.request).unwrap(); | ||
| assert!(result.project_uri.starts_with("file:///"), "{}", result.project_uri); | ||
| assert!(result.project_uri.ends_with("/App.dpr"), "{}", result.project_uri); | ||
| let path_part = result.project_uri.trim_start_matches("file:///"); | ||
| assert!(!path_part.contains(':'), "drive colon must be encoded: {}", result.project_uri); | ||
| assert!(path_part.contains("%3A"), "{}", result.project_uri); | ||
| } |
There was a problem hiding this comment.
Fixed in 73a15c1 — the drive-letter assertions are now #[cfg(windows)].
| pub fn path_to_file_uri(path: &Path) -> String { | ||
| const SAFE: &str = "-._~/"; | ||
| let normalized = path.to_string_lossy().replace('\\', "/"); | ||
| let mut encoded = String::with_capacity(normalized.len() + 8); | ||
| for byte in normalized.as_bytes() { | ||
| let ch = *byte as char; | ||
| if ch.is_ascii_alphanumeric() || SAFE.contains(ch) { | ||
| encoded.push(ch); | ||
| } else { | ||
| encoded.push_str(&format!("%{byte:02X}")); | ||
| } | ||
| } | ||
| format!("file:///{}", encoded.trim_start_matches('/')) | ||
| } |
There was a problem hiding this comment.
Fixed in 73a15c1 — UNC paths are special-cased to file://server/share/... (server as URI authority), with a regression test.
| async function ensureSettingsFile(project: Entities.Project): Promise<Option<string>> { | ||
| const filePath = expectedSettingsFilePath(project); | ||
| if (!filePath) return undefined; | ||
|
|
||
| let needsGeneration = !existsSync(filePath); | ||
| if (!needsGeneration) { | ||
| const marker = await readGeneratedByMarker(filePath); | ||
| if (marker === DELPHILSP.GENERATED_BY_MARKER) | ||
| needsGeneration = await isStale(filePath, project); | ||
| // Existing file without our marker was produced by the RAD Studio IDE — leave it untouched. | ||
| } | ||
| if (!needsGeneration) return filePath; |
There was a problem hiding this comment.
Fixed in 73a15c1 — replaced with an async fs.access check.
| private static async generateConfig(item: BaseFileItem): Promise<void> { | ||
| const project = item?.project?.entity; | ||
| if (!assertError(project, 'Could not determine project for the selected item.')) return; | ||
|
|
||
| try { | ||
| const result = await Runtime.client.generateDelphiLspConfig(String(project.id)); | ||
| if (result.warnings.length > 0) |
There was a problem hiding this comment.
Fixed in 73a15c1 — item is now optional and the handler falls back to the active project, so the palette invocation works.
| if let Some(description) = input.description.as_ref().filter(|d| !d.trim().is_empty()) { | ||
| parts.push(format!("--description:\"{description}\"")); | ||
| } |
There was a problem hiding this comment.
Fixed in 73a15c1 — embedded quotes are escaped before formatting, with a regression test.
- package.json: 'when' is not valid on contributes.commands — use 'enablement' instead, and let the command appear in the Command Palette (gated on DelphiLSP being installed) now that it falls back to the active project when invoked without a tree item - server: report generation failures as InternalError instead of invalid_params, which misclassified server-side faults - registry: decode env-var overrides with String::from_reg_value instead of RegValue's Display formatting - commands: extract a shared allocation-free is_delphi_project_path helper instead of duplicating the lowercase ends_with chain - file URIs: special-case UNC paths (file://server/share/... keeps the server as the URI authority) + regression test - dccOptions: escape embedded quotes in --description + regression test - autoSync: async existence check instead of existsSync on the extension host thread - tests: cfg(windows) the drive-letter URI assertions
|
Switched it to "work in progress" because I noticed that adding the support of DelphiLSP made the "problems" view to be populated both from errors coming from the last compilation attempt AND problems found by the live analysis made by DelphiLSP. I am working on a fix for this problem: I will merge the two outputs recognizing duplicates and I will take care to remove errors signaled also by the compilation phase if DelphiLSP signales they have been fixed in the source code (even if it hasn't been compiled yet) |
…alysis With both extensions active the Problems view showed every compile error twice: once from DDK (compiler output, 1-character range) and once from DelphiLSP (live validation, precise token range) - and DDK's copy went stale the moment the user fixed the code, since it only refreshes on the next compile. The server's publishDiagnostics are now routed through the language client's handleDiagnostics middleware into a DiagnosticCollection the extension owns, so individual entries can be deleted later. Whenever DelphiLSP publishes live diagnostics for a file, every DDK entry matching one of them (same line + same Exxxx code, with message/severity fallbacks when a structured code is missing) is deleted - not hidden - so only the precise live entry remains, and when DelphiLSP clears it because the error was fixed in the editor (even without recompiling) no stale compile copy resurfaces. Compile diagnostics DelphiLSP has no opinion on (files it has not validated, link errors) keep their normal lifetime. Without DelphiLSP installed the module is only the rendering route and nothing is ever deleted. New ddk.delphilsp.mergeDiagnostics setting, on by default.
|
Done in a91b361: DDK compile diagnostics are now routed into a diagnostic collection the extension owns, and whenever DelphiLSP publishes live diagnostics for a file, every DDK entry matching a live one (same line + same error code, with sensible fallbacks) is deleted in favor of the live entry with its precise token range. Once DelphiLSP reports the error as fixed in the editor, the compile copy stays gone even without recompiling. Diagnostics DelphiLSP has no opinion on (unvalidated files, link errors) keep their normal lifetime, and without DelphiLSP installed nothing is ever removed. New |
…d via include_str! Same shape as requested in the PR valentin-baron#7 review — the fixture is easier to open in the IDE and to evolve as a real project file than an inline string literal.
|
Heads-up on an overlap with #7: after both PRs land there will be two implementations of the same two concepts.
Neither PR blocks the other — different files, no textual conflict. Once both are on main I'd unify each pair in a small follow-up (best-of-both for the registry reader, one resolution helper). Flagging it now so it doesn't look like accidental duplication during review. |
|
I will review this sometime tomorrow |
What this is, in plain words
This PR adds support for Embarcadero's official DelphiLSP VS Code extension (
EmbarcaderoTechnologies.delphilsp) — a separate extension, from Embarcadero, that the user installs from the Marketplace alongside DDK. DDK does not bundle or replace it: everything here exists to make that extension actually work (and work well) next to DDK, and none of it appears unless it is installed.DelphiLSP is Embarcadero's own language server for Delphi — the same engine that powers code intelligence inside the RAD Studio IDE, shipped to VS Code through that extension. With it installed and correctly configured, the VS Code editor gains Error Insight (real compiler errors underlined as you type, before you ever build), code completion that actually knows your units and components, go to definition / find references across your whole search path, hover tooltips with declarations, and document symbols/outline.
The catch: DelphiLSP only works when it is given a
<project>.delphilsp.jsonsettings file describing the project's search paths, defines and compiler options — and today that file is only produced by the RAD Studio IDE when you open the project there. Anyone working purely in VS Code (which is exactly DDK's audience) gets no code insight at all, or worse, insight bound to a stale/wrong project producing nonsense diagnostics.This PR closes that gap: DDK now generates and maintains those settings files itself, from the same sources the IDE uses, and keeps DelphiLSP automatically pointed at whatever project is active in DDK. RAD Studio never needs to be opened.
What the user sees
ddk delphilsp-config <ID|NAME|PATH>(CLI),delphi_generate_delphilsp_config(MCP) and a Generate DelphiLSP Config context-menu action on projects: write the project's.delphilsp.jsonon demand. Works for managed projects and for ad-hoc.dprojpaths (-cpicks a compiler,-ooverrides the destination).ddk.delphilsp.autoSync, default on): switching the DDK active project re-points DelphiLSP'ssettingsFileat that project, generating the file on the fly when missing — or refreshing it when it is DDK-generated and older than its.dproj. Code insight always matches the project you are actually working on.ddk.delphilsp.revalidateOpenFilesOnSwitch, default on): after a switch, the diagnostics of already-open Delphi files are recomputed under the new project context — something even DelphiLSP's own Select project settings command does not do (it leaves stale squiggles around until each file is edited).ddk.delphilsp.mergeDiagnostics, default on): with both extensions active, the Problems view no longer shows the same error twice (once from DDK's last compile, once from DelphiLSP's live analysis). DDK's compile diagnostics are routed into a collection the extension owns, and every entry matching a live DelphiLSP one (same line + same error code) is deleted in favor of the live entry with its precise token range. Once DelphiLSP reports the error as fixed in the editor, the compile copy stays gone even without recompiling — no stale squiggles. Diagnostics DelphiLSP has no opinion on (files it has not validated, link errors) keep their normal lifetime.Why this also helps AI assistants
Copilot and other assistants read the editor's Problems panel and open-file diagnostics as context. With DelphiLSP correctly configured per project, that context stops being noise (wrong-project phantom errors) and becomes real compiler feedback — and Copilot's inline chat/fix flows get accurate error messages to work from. Assistants driving DDK over MCP can also generate the settings file themselves via the new tool, so an agent can bootstrap full code insight for a checkout that has never seen the IDE.
How it works
core/src/delphilsp/(isolated module, commands.rs only delegates) reconstructs the file from: the.dprojevaluated for the effective configuration/platform (via dproj-rs), the installation'sbin\rsvars.bat, and the IDE's global Library Path, Browsing Path and user-defined environment variables read fromHKCU\SOFTWARE\Embarcadero\BDS\<version>. Unresolvable$(MACRO)path entries are dropped with a warning. The emitteddccOptionswas verified entry-for-entry against IDE-generated files on a large real-world project group: identical-I/-U/-O/-Rsearch paths (212/212 entries),-D,-NS,-A, output dirs,dllnameper platform (dcc64290/dcc32290),browsingPaths,projectFiles.Ownership is explicit: every file DDK writes carries a top-level
"generatedBy": "delphi-devkit"key. RAD Studio never writes that key, so DDK's auto-refresh only ever touches its own files — an IDE-generated or hand-written.delphilsp.jsonis used as-is and never rewritten (the explicit command always rewrites).The re-validation trick deserves a note: DelphiLSP's server applies a settings change only to files opened afterwards. Its raw LSP logs show that neither the configuration push nor even a full server restart re-validates open files (the
didOpenreplay lands before the settings do). The only trigger it honors is a freshdidOpenafter the settings are loaded, so the extension performs an invisible language-mode round-trip (objectpascal → plaintext → back) on each open Delphi document — VS Code re-emitsdidClose/didOpenfor the same text buffer, leaving tabs, focus, cursor, dirty state and undo history untouched.Wiring
Follows the repo contract: logic in
core/src/delphilsp/→cmd_delphilsp_configin commands.rs → CLI subcommand + MCP tool (+tool_box!) +delphilsp/generateLSP method; extension logic isolated invscode_extension/src/delphilsp/with a one-line hookup inruntime.ts. NoChangevariant (the capability mutates no DDK state). No version bump.Testing
.dproj+ fake installation in a tempdir, exercising macro expansion, property-group inheritance, registry fallbacks, file-URI encoding, marker semantics, Win32/Win64 differences).npm run check-types/lint/ full bundle clean.