Conversation
| Ok(SchemaCacheHandle::new(&self.inner)) | ||
| } | ||
| } | ||
| let schema_cache = Arc::new(run_async(async move { SchemaCache::load(&pool).await })??); |
There was a problem hiding this comment.
won't there be race conditions if load is called multiple times in parallel and there is no schema_cache in cache? We would SchemaCache::load a couple of times, right?
| let settings = self.workspaces(); | ||
| // TODO: not sure how we should handle this | ||
| // maybe fallback to default settings? or return an error? | ||
| let settings = settings.settings().expect("Settings should be initialized"); |
There was a problem hiding this comment.
Is there state where a user requests diagnostics before or while settings are set up, for example when the file opens?
If so, I think we should return an emtpy response. But if not, it's fine to expect – we do expect the settings to be present for a working LSP right?
There was a problem hiding this comment.
yeah, makes sense. added that.
There was a problem hiding this comment.
Pull Request Overview
This PR adds workspace folder management and extends support in configuration loading, ensuring per-connection caches and idle timeouts, and updates LSP/CLI integrations.
- Introduce
register_project_folder/unregister_project_folderAPI and per-project data storage - Implement
extendslogic inPartialConfigurationwith file resolution and merging - Wire workspace registration in LSP and CLI sessions and update file system resolver integration
Reviewed Changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| crates/pgt_workspace/src/workspace.rs | Add project registration/unregistration and slotmap-based storage |
| crates/pgt_workspace/src/settings.rs | Manage per-project settings and current-project logic |
| crates/pgt_workspace/src/lib.rs | Re-export new PartialConfigurationExt trait |
| crates/pgt_workspace/src/diagnostics.rs | Bridge CantLoadExtendFile into WorkspaceError |
| crates/pgt_workspace/src/configuration.rs | Replace payload conversion with try_from_payload and implement extends merging |
| crates/pgt_workspace/Cargo.toml | Add slotmap dependency for project keys |
| crates/pgt_lsp/src/session.rs | Introduce client info, call register_project_folder during initialization |
| crates/pgt_lsp/src/server.rs | Handle workspace folder changes and register/unregister methods |
| crates/pgt_fs/src/fs/os.rs | Integrate oxc_resolver for config resolution |
| crates/pgt_fs/src/fs/memory.rs | Add stub for resolve_configuration in memory FS |
| crates/pgt_fs/src/fs.rs | Extend FileSystem trait with resolve_configuration |
| crates/pgt_fs/Cargo.toml | Add oxc_resolver dependency |
| crates/pgt_diagnostics/src/adapters.rs | Wrap resolver errors into diagnostics |
| crates/pgt_diagnostics/Cargo.toml | Add oxc_resolver dependency |
| crates/pgt_configuration/src/lib.rs | Add extends field and default in partial config |
| crates/pgt_configuration/src/diagnostics.rs | New diagnostics for extend resolution and load errors |
| crates/pgt_configuration/Cargo.toml | Add oxc_resolver dependency |
| crates/pgt_cli/src/diagnostics.rs | Update expected diagnostic size |
| crates/pgt_cli/src/commands/mod.rs | Register workspace folder in CLI commands |
| Cargo.toml | Add oxc_resolver and slotmap to workspace |
Comments suppressed due to low confidence (2)
crates/pgt_workspace/src/configuration.rs:349
- The detection logic treats only
.jsoncas local files;.jsonentries will be resolved externally. Consider includingb"json"in the match to handle.jsonextends as local paths.
if extend_entry_as_path.starts_with(".") || matches!(extend_entry_as_path.extension().map(OsStr::as_encoded_bytes), Some(b"jsonc")) {
crates/pgt_lsp/src/session.rs:492
- This call to
.awaitis inside a synchronous function, which will fail to compile. Either makeinitializeasync or spawn a separate async task to log the message.
self.client.log_message(MessageType::ERROR, &error).await;
adds workspace support as well as support for
extendsin the config. This now works:./postgrestools.jsonc{ // ...my linter config }./project_one/postgrestools.jsonc{ "extends": "../postgrestools.jsonc" // e.g. add specific db config for this project }Will need to check if client-side changes are required to enable workspace support there.
A few internal changes:
ToDo:
try_from_payloadwith extendscloses #334