-
Notifications
You must be signed in to change notification settings - Fork 182
Add LSP arguments to selectively disable language services. #1421
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Hi @kv9898! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at [email protected]. Thanks! |
|
I have now signed the CLA, what should I do next? |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
Co-authored-by: kv9898 <[email protected]>
Co-authored-by: kv9898 <[email protected]>
…ervice disabling Co-authored-by: kv9898 <[email protected]>
Co-authored-by: kv9898 <[email protected]>
Co-authored-by: kv9898 <[email protected]>
87cf144 to
723f815
Compare
|
hey, thanks for the contribution - this seems really useful. and sorry about the delay in responding to your initial issue yesterday.
This is great. All that's required now is an approval from us then a merge. I wonder if a better place to put these settings would be in the workspace configuration. This has a few benefits:
What do you think? Are there reasons I'm not seeing for why command-line arguments would be easier for your use case? |
|
Ahh that makes sense! I thought command line arguments were the only way for this PR to be applicable beyond VSCode. It now seems that there's a protocol for transmitting such config? |
Yup. There's a few different way to send it (it can be part of the initialize request or as back/forth) but it's all outlined here. |
…guration (#2) * Initial plan * Implement workspace-based language service disabling Replace command-line arguments with workspace configuration approach. - Add DisabledLanguageServices to LspAnalysisConfig in workspace.rs - Create LanguageService enum to identify different services - Update make_handle_if_enabled to check per-service enablement - Remove command-line arguments from LspArgs - Update all LSP handlers to specify which service they use - Update capabilities() to not depend on disabled services parameter Co-authored-by: kv9898 <[email protected]> * Update documentation and fix tests for workspace config approach - Update selective-language-services.md to document workspace configuration - Remove old command-line argument references from test files - Document dynamic configuration benefits Co-authored-by: kv9898 <[email protected]> * Add example config file and improve documentation based on code review - Add example workspace configuration JSON file - Enhance documentation to explain improvements over command-line approach - Add testing instructions for dynamic configuration changes Co-authored-by: kv9898 <[email protected]> * Add config to package.json * working * remove workspace symbol * remove debugging lines * remove example json * fix documentation
|
@kinto0 Job done! It now works well on my machine, would you give it a go? Here's a simple demonstration with clashing hover support: pyrefly.mp4I do note that |
kinto0
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it! I think we should reuse lsp_types to make sure it's consistent with the LS methods.
another thing we should do is update lsp/readme.md and website/docs/IDE.mdx (the customization section).
otherwise looks great! thanks for making the change :)
…vice enum (#3) * Initial plan * Replace custom LanguageService enum with lsp_types request METHOD strings Co-authored-by: kv9898 <[email protected]> * Apply rustfmt formatting fixes Co-authored-by: kv9898 <[email protected]>
|
@kinto0 all done! |
|
@kinto0 Sorry I forgot to update the docs, and have just done that. Note I haven't included a complete list of all options (which can be accessed via |
oops - I ended up just adding docs already. there were a few merge conflicts and other things I fixed. once I get a second set of eyes on it, we can merge (ill force push the merge conflict resolution here if I can) |
|
@kinto0 haha, don't mind my doc changes then - they are AI generated anyway :) |
stroxler
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review automatically exported from Phabricator review in Meta.
Resolves #1410.
This allows users to choose exactly which IDE features they want enabled, providing flexibility for different workflows and preferences, especially when running multiple Python LSP extensions.
Selective Language Service Disabling
Pyrefly's LSP server supports selective disabling of individual language services via workspace configuration. This allows users to choose exactly which IDE features they want enabled, providing flexibility for different workflows and preferences.
Available Language Services
The following language services can be selectively disabled:
Usage
Workspace Configuration
You can configure which language services to disable through your editor's settings or workspace configuration. The configuration is part of the LSP Analysis settings:
VSCode
Add the following to your
settings.json(workspace or user settings):{ "python.pyrefly.analysis.disabledLanguageServices": { "hover": true, "documentSymbol": true } }General LSP Configuration
For other editors that support LSP workspace configuration, send a configuration with the following structure:
{ "pyrefly": { "analysis": { "disabledLanguageServices": { "hover": true, "completion": true, "signatureHelp": true } } } }Implementation Details
Architecture Decision
The selective disabling is implemented at the workspace configuration level. This design provides several benefits over the previous command-line argument approach:
How It Works
Workspace Configuration: Each workspace can have its own disabled services configuration in the
analysis.disabledLanguageServicesfield of the LSP analysis config.Request Handling: When a language service request is received (e.g., hover, completion), the server checks the workspace configuration for the file being edited.
Service Availability: If a service is disabled for that workspace, the server returns
Noneand logs that the request was skipped.Configuration Changes: When workspace configuration changes, the new settings take effect immediately for subsequent requests.
Use Cases
Scenario 1: Performance Optimization
If you're working on a very large codebase and find that certain language services are slow, you can disable them to improve performance:
{ "python.pyrefly.analysis.disabledLanguageServices": { "references": true, } }Scenario 2: Conflict Resolution
If you're using multiple language servers and want to use Pyrefly for type checking but another server for certain features:
{ "python.pyrefly.analysis.disabledLanguageServices": { "completion": true, "hover": true } }Scenario 3: Minimal Mode
For a minimal setup with only type checking and go-to-definition:
{ "python.pyrefly.analysis.disabledLanguageServices": { "hover": true, "completion": true, "signatureHelp": true, "documentHighlight": true, "documentSymbol": true, "semanticTokens": true, "inlayHint": true } }Scenario 4: Per-Workspace Configuration
You can have different configurations for different workspace folders:
{ "folders": [ { "path": "/path/to/project1", "settings": { "python.pyrefly.analysis.disabledLanguageServices": { "hover": true } } }, { "path": "/path/to/project2", "settings": { "python.pyrefly.analysis.disabledLanguageServices": { "completion": true } } } ] }Testing
The implementation includes tests to ensure:
To test dynamic configuration changes:
Run tests with:
cargo test --package pyrefly --lib