-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Use a single language server instance when type set to Node #11315
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
Kudos, SonarCloud Quality Gate passed!
|
Codecov Report
@@ Coverage Diff @@
## master #11315 +/- ##
==========================================
+ Coverage 61.12% 61.46% +0.33%
==========================================
Files 601 606 +5
Lines 33128 33812 +684
Branches 4680 4790 +110
==========================================
+ Hits 20249 20781 +532
- Misses 11861 12579 +718
+ Partials 1018 452 -566
Continue to review full report at Codecov.
|
const configurationService = this.serviceContainer.get<IConfigurationService>(IConfigurationService); | ||
const serverType = configurationService.getSettings(this.resource).languageServer; | ||
if (serverType === LanguageServerType.Node) { | ||
return 'shared-ls'; |
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.
This ensures only a single instance is used.
import { ILanguageServerOutputChannel } from '../types'; | ||
|
||
@injectable() | ||
export class NodeLanguageServerAnalysisOptions extends LanguageServerAnalysisOptionsBase { |
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.
Empty class; no options are needed to configure this other than the defaults.
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.
This PR splits apart
analysisOptions.ts
into two implementations, switching between them depending on the language server type setting. It also makes the LS key a static string to ensure only a single instance is spawned.This means that the LS can be started without any document selectors (eventually leading to handling of #5132), and only have a single instance to handle all workspaces at once.
Also, while moving this code, delete some ancient deprecated options which no recent LS has used.
While working on this, I discovered the reason for #5132 (comment); I'll file a new issue about that as it's out of scope for this PR.
Has a news entry file (remember to thank yourself!).Test plan is updated as appropriate.package-lock.json
has been regenerated by runningnpm install
(if dependencies have changed).The wiki is updated with any design decisions/details.