Skip to content
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

Merged

Conversation

remcohaszing
Copy link
Contributor

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.

Problems panel table view showing several diagnostics

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.

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.
@dbaeumer
Copy link
Member

dbaeumer commented Mar 8, 2024

Makes sense. When we started we didn't have a table problems view :-)

@remcohaszing
Copy link
Contributor Author

@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.

@dbaeumer
Copy link
Member

@remcohaszing still agree that this is a good a change. Could you make the code adjustment I asked for.

@remcohaszing
Copy link
Contributor Author

I don’t see any review comments. Did you maybe forget to press Submit? 😅

this._diagnostics = this._clientOptions.diagnosticCollectionName
? Languages.createDiagnosticCollection(this._clientOptions.diagnosticCollectionName)
: Languages.createDiagnosticCollection();
this._diagnostics = Languages.createDiagnosticCollection(this._clientOptions.diagnosticCollectionName || this._id);
Copy link
Member

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

@dbaeumer
Copy link
Member

Did not press the button.

This means the name can be set to an empty string.
@dbaeumer dbaeumer enabled auto-merge (squash) June 28, 2024 13:50
@vscodenpa vscodenpa added this to the June 2024 milestone Jun 28, 2024
@dbaeumer dbaeumer merged commit 6d2a392 into microsoft:main Jun 28, 2024
6 checks passed
@remcohaszing remcohaszing deleted the diagnostics-collection-name-fallback branch June 28, 2024 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants