-
-
Notifications
You must be signed in to change notification settings - Fork 216
Update the protocol to version 3.17 #350
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
Changes from all commits
73138fe
aca797b
440140a
d964460
463f301
77768b8
b26baf6
31abc51
53aba2a
2c1a2d1
7f2614a
215e55b
1227764
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,36 +3,25 @@ | |
| * Licensed under the MIT License. See License.txt in the project root for license information. | ||
| * ------------------------------------------------------------------------------------------ */ | ||
| import { | ||
| BaseLanguageClient, MessageTransports, LanguageClientOptions, | ||
| StaticFeature, DynamicFeature | ||
| } from "vscode-languageclient/lib/common/client"; | ||
| import { TypeDefinitionFeature } from "vscode-languageclient/lib/common/typeDefinition"; | ||
| import { ConfigurationFeature as PullConfigurationFeature } from "vscode-languageclient/lib/common/configuration"; | ||
| import { ImplementationFeature } from "vscode-languageclient/lib/common/implementation"; | ||
| import { ColorProviderFeature } from "vscode-languageclient/lib/common/colorProvider"; | ||
| import { WorkspaceFoldersFeature } from "vscode-languageclient/lib/common/workspaceFolders"; | ||
| import { FoldingRangeFeature } from "vscode-languageclient/lib/common/foldingRange"; | ||
| import { CallHierarchyFeature } from "vscode-languageclient/lib/common/callHierarchy"; | ||
| import { ProgressFeature } from "vscode-languageclient/lib/common/progress"; | ||
| import { SemanticTokensFeature } from "vscode-languageclient/lib/common/semanticTokens"; | ||
| BaseLanguageClient, MessageTransports, LanguageClientOptions | ||
| } from "vscode-languageclient"; | ||
| import * as p2c from 'vscode-languageclient/lib/common/protocolConverter'; | ||
| import * as c2p from 'vscode-languageclient/lib/common/codeConverter'; | ||
| import { IConnectionProvider, IConnection } from './connection'; | ||
| import { DeclarationFeature } from "vscode-languageclient/lib/common/declaration"; | ||
| import { IConnectionProvider } from './connection'; | ||
| import { CompletionParams, WillSaveTextDocumentParams } from './services' | ||
|
|
||
| export * from 'vscode-languageclient/lib/common/client'; | ||
| import type * as vscode from 'vscode' | ||
|
|
||
| export class MonacoLanguageClient extends BaseLanguageClient { | ||
|
|
||
| static bypassConversion = (result: any) => result || undefined; | ||
| static bypassConversion = (result: any, token?: vscode.CancellationToken) => token != null ? Promise.resolve(result || undefined) : (result || undefined); | ||
|
|
||
| protected readonly connectionProvider: IConnectionProvider; | ||
|
|
||
| constructor({ id, name, clientOptions, connectionProvider }: MonacoLanguageClient.Options) { | ||
| super(id || name.toLowerCase(), name, clientOptions); | ||
| this.connectionProvider = connectionProvider; | ||
| (this as any).createConnection = this.doCreateConnection.bind(this); | ||
|
|
||
| // bypass LSP <=> VS Code conversion | ||
| const self: { | ||
|
|
@@ -77,41 +66,8 @@ export class MonacoLanguageClient extends BaseLanguageClient { | |
| }); | ||
| } | ||
|
|
||
| protected doCreateConnection(): Thenable<IConnection> { | ||
| const errorHandler = (this as any).handleConnectionError.bind(this); | ||
| const closeHandler = this.handleConnectionClosed.bind(this); | ||
| return this.connectionProvider.get(errorHandler, closeHandler, this.outputChannel); | ||
| } | ||
|
|
||
| protected createMessageTransports(encoding: string): Promise<MessageTransports> { | ||
| throw new Error('Unsupported'); | ||
| } | ||
|
|
||
| protected registerBuiltinFeatures(): void { | ||
| super.registerBuiltinFeatures(); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does the base implementation now perform this registration and it is therefore no longer needed here?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, I looked at
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't really know, it seems there isn't any consequence of registering a feature twice, so I don't know for how long it was there. I think these features were |
||
| this.registerFeature(new PullConfigurationFeature(this)); | ||
| this.registerFeature(new TypeDefinitionFeature(this)); | ||
| this.registerFeature(new ImplementationFeature(this)); | ||
| this.registerFeature(new ColorProviderFeature(this)); | ||
| this.registerFeature(new WorkspaceFoldersFeature(this)); | ||
| FoldingRangeFeature['asFoldingRanges'] = MonacoLanguageClient.bypassConversion; | ||
| this.registerFeature(new FoldingRangeFeature(this)); | ||
| this.registerFeature(new DeclarationFeature(this)); | ||
| this.registerFeature(new SemanticTokensFeature(this)); | ||
| this.registerFeature(new CallHierarchyFeature(this)); | ||
| this.registerFeature(new ProgressFeature(this)); | ||
|
|
||
| const features = this['_features'] as ((StaticFeature | DynamicFeature<any>)[]); | ||
| for (const feature of features) { | ||
| if (feature instanceof ColorProviderFeature) { | ||
| feature['asColor'] = MonacoLanguageClient.bypassConversion; | ||
| feature['asColorInformations'] = MonacoLanguageClient.bypassConversion; | ||
| feature['asColorPresentations'] = MonacoLanguageClient.bypassConversion; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| public registerProposedFeatures() { | ||
| return this.connectionProvider.get(encoding); | ||
| } | ||
|
|
||
| protected getLocale(): string { | ||
|
|
||
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.
Do you want to have a strict null check here, then change to
!==?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'm not sure to understand what you mean, can you provide the code you want to be there?
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.
Shouldn't
token != nullbetoken !== null?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.
We can replace
!= nullby!== undefinedif you want.I'm just used to using
== nullbecause it allows to check forundefinedandnull, and it's allowed by the standartjs eslint rules.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.
Do you want me to change it?
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'm not sure to understand what you mean,
tokenis a boolean?Are you talking about #351? I don't think it's a new issue, but I'm currently investigating. There is probably no reason for it to block the next release tho
I never understand why so many libraries are using a version number < 1 even after years (even monaco!).
But it's not a big deal, as you wish!
Uh oh!
There was an error while loading. Please reload this page.
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.
You can rewrite this as such (token is undefined => false or defined => true). That's what I meant:
The method signature is identical to:
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.
Let's aim for 1.0.0. webpack example is almost working again.
monacoworkers are not working properly. Will fix this today. Release today or tomorrow.Uh oh!
There was an error while loading. Please reload this page.
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.
That's what I meant when talking about
if on anything else than boolean, relying on the trythiness of an object and the falsiness of undefined seems wrong and dangerous, no big deal tho since the whole project is done this wayThere 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.
Please press the merge button if you like. 👍