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

Log message when overriding client's document selector #1602

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

vinistock
Copy link
Contributor

Based on this discussion #1487 (comment)

This PR starts logging a message when the client's document selector gets overridden for a feature, in hopes to make this behaviour easier to trace when it occurs.

For the test, I redefined the info method so that I could assert that the log is being invoked as expected. Let me know if that pattern is okay.

@@ -552,6 +552,11 @@ export abstract class TextDocumentLanguageFeature<PO, RO extends TextDocumentReg
if (!documentSelector || !capability) {
return undefined;
}

if (documentSelector && capability) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should only log something if the document selectors are different. Specifying capabilities is fine since there are no client default once. In addition we should somehow log for which feature this is happening.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since both documentSelector and capability are objects, I ended up going with comparing their JSON stringified versions. I don't really like that approach, so let me know if you have a preferred way of comparing the selectors.

There are a few possible types for selectors. If you prefer, we can definitely add checks for each possible combination.

I also added the method name through this._registrationType.method.

@vinistock vinistock force-pushed the vs-add-trace-when-server-overrides-selector branch from cc68315 to 3818da9 Compare January 31, 2025 20:57
@vinistock vinistock requested a review from dbaeumer January 31, 2025 20:59
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.

2 participants