Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

feat: workspace support#408

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
psteinroe merged 31 commits intomainfromfeat/workspace-support
Jun 1, 2025
Merged

feat: workspace support#408

psteinroe merged 31 commits intomainfromfeat/workspace-support
Jun 1, 2025

Conversation

psteinroe
Copy link
Collaborator

@psteinroepsteinroe commentedMay 26, 2025
edited
Loading

adds workspace support as well as support forextends in 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:

  • we now store both the schema cache and the db connection per connection and not globally
  • the connections have a idle timeout of a few minutes after which we drop the connection from the cache
  • we select the current project when the user opens a file. this is the same pattern biome uses and I think it makes sense.
  • EDIT: I want to check if we even need to track the current project since all requests should include the path from which we can find the correct one -> we cannot easily do that. I would go with the current approach for now and maybe do it in a follow up with a better analysis.
  • EDIT: Will fix windows path issues for tests 🥲 -> done
  • EDIT: Will remove all the windows debug logs 🥲 -> done

ToDo:

  • add methods to workspace api
  • migrate schema cache manager to store per-connection
  • integration ls
  • integration cli
  • make test green
  • implementtry_from_payload with extends
  • add test for extends config

closes#334

mavcook reacted with heart emoji
@psteinroepsteinroe marked this pull request as ready for reviewMay 29, 2025 15:25
Ok(SchemaCacheHandle::new(&self.inner))
}
}
let schema_cache = Arc::new(run_async(async move { SchemaCache::load(&pool).await })??);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

won't there be race conditions ifload is called multiple times in parallel and there is no schema_cache in cache? We wouldSchemaCache::load a couple of times, right?

Copy link
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

good catch!

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");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

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 toexpect – we do expect the settings to be present for a working LSP right?

Copy link
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

yeah, makes sense. added that.

Copy link
Contributor

@CopilotCopilotAI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Pull Request Overview

This PR adds workspace folder management andextends support in configuration loading, ensuring per-connection caches and idle timeouts, and updates LSP/CLI integrations.

  • Introduceregister_project_folder/unregister_project_folder API and per-project data storage
  • Implementextends logic inPartialConfiguration with 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
FileDescription
crates/pgt_workspace/src/workspace.rsAdd project registration/unregistration and slotmap-based storage
crates/pgt_workspace/src/settings.rsManage per-project settings and current-project logic
crates/pgt_workspace/src/lib.rsRe-export newPartialConfigurationExt trait
crates/pgt_workspace/src/diagnostics.rsBridgeCantLoadExtendFile intoWorkspaceError
crates/pgt_workspace/src/configuration.rsReplace payload conversion withtry_from_payload and implementextends merging
crates/pgt_workspace/Cargo.tomlAddslotmap dependency for project keys
crates/pgt_lsp/src/session.rsIntroduce client info, callregister_project_folder during initialization
crates/pgt_lsp/src/server.rsHandle workspace folder changes and register/unregister methods
crates/pgt_fs/src/fs/os.rsIntegrateoxc_resolver for config resolution
crates/pgt_fs/src/fs/memory.rsAdd stub forresolve_configuration in memory FS
crates/pgt_fs/src/fs.rsExtendFileSystem trait withresolve_configuration
crates/pgt_fs/Cargo.tomlAddoxc_resolver dependency
crates/pgt_diagnostics/src/adapters.rsWrap resolver errors into diagnostics
crates/pgt_diagnostics/Cargo.tomlAddoxc_resolver dependency
crates/pgt_configuration/src/lib.rsAddextends field and default in partial config
crates/pgt_configuration/src/diagnostics.rsNew diagnostics for extend resolution and load errors
crates/pgt_configuration/Cargo.tomlAddoxc_resolver dependency
crates/pgt_cli/src/diagnostics.rsUpdate expected diagnostic size
crates/pgt_cli/src/commands/mod.rsRegister workspace folder in CLI commands
Cargo.tomlAddoxc_resolver andslotmap to workspace
Comments suppressed due to low confidence (2)

crates/pgt_workspace/src/configuration.rs:349

  • The detection logic treats only.jsonc as local files;.json entries will be resolved externally. Consider includingb"json" in the match to handle.json extends 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.await is inside a synchronous function, which will fail to compile. Either makeinitialize async or spawn a separate async task to log the message.
self.client.log_message(MessageType::ERROR, &error).await;

Copy link
Collaborator

@juleswritescodejuleswritescode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Beautiful!!!

@psteinroepsteinroe merged commit3698af5 intomainJun 1, 2025
7 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

Copilot code reviewCopilotCopilot left review comments

@juleswritescodejuleswritescodejuleswritescode approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Workspace support
2 participants
@psteinroe@juleswritescode

[8]ページ先頭

©2009-2025 Movatter.jp