-
Notifications
You must be signed in to change notification settings - Fork 324
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
Use client ID as fallback for diagnostics collection #1445
Use client ID as fallback for diagnostics collection #1445
Conversation
This sets the language client ID as a fallback for the diagnostics collection name. If no collection name is provided, VSCode generates one. This name is visible in the diagnostics table view of the problems panel.
Makes sense. When we started we didn't have a table problems view :-) |
@dbaeumer Friendly ping. I read your comment as you finding this a good idea. I think it makes sense to include this in v10, which is in alpha now. |
@remcohaszing still agree that this is a good a change. Could you make the code adjustment I asked for. |
I don’t see any review comments. Did you maybe forget to press Submit? 😅 |
client/src/common/client.ts
Outdated
this._diagnostics = this._clientOptions.diagnosticCollectionName | ||
? Languages.createDiagnosticCollection(this._clientOptions.diagnosticCollectionName) | ||
: Languages.createDiagnosticCollection(); | ||
this._diagnostics = Languages.createDiagnosticCollection(this._clientOptions.diagnosticCollectionName || this._id); |
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.
Can we make this this._clientOptions.diagnosticCollectionName ?? this._id
Did not press the button. |
This means the name can be set to an empty string.
This sets the language client ID as a fallback for the diagnostics collection name. If no collection name is provided, VSCode generates one. This name is visible in the diagnostics table view of the problems panel.
This can be solved for every language server integration separately, but IMO it’s nicer if this package provides a better default. An alternative is to use the name instead of the ID. I went with ID, because
typescript
in the screenshot is lower case.