Skip to content

LanguageService: add getSupportedCodeFixes to make it proxyable #29010

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

Closed
wants to merge 3 commits into from

Conversation

ajafff
Copy link
Contributor

@ajafff ajafff commented Dec 13, 2018

Fixes: #28966

I'd highly appreciate some pointers on how to test this change. (@sheetalkamat?)

Is the change to the protocol what @DanielRosenwasser meant in #28966 (comment)?

Copy link
Member

@sheetalkamat sheetalkamat left a comment

Choose a reason for hiding this comment

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

#28106 has example of how to add test that configures plugin and adds proxy

@ajafff
Copy link
Contributor Author

ajafff commented Dec 14, 2018

@sheetalkamat I changed getSupportedCodeFixes to take the fileName as parameter, although that means the client would need to call getSupportedCodeFixes once for each file instead of once per project.
I'm wondering if this API still makes sense as a means for reducing communication to the service. In addition it would need to be refreshed once a plugin's configuration changed in any way. Wouldn't it be easier to just call getCodeFixes?

Deprecating this API would also resolve #28990, microsoft/vscode#64848 and microsoft/vscode#64872


Regarding tests: as far as I understand, fourslash only tests Project and LanguageService but not Session where the actual change of this PR is. If Session doesn't have tests, do we even need a test for the new method in LanguageService (given that every method in LanguageService can be intercepted by a plugin)?

@sheetalkamat
Copy link
Member

sheetalkamat commented Dec 14, 2018

@ajafff for test you can look at #28385 to see how to test this through session.
I am not very familiar with getSupportedCodeFixes api and reasoning behind it. @mjbvz might know more about it.

Edit:
Forgot to mention that from what I saw it seems like instead of calling getCodeFixes for each error, I think vscode calls getCodeFixes only on these list from supported error codes. So could this be done to avoid too many interaction on eg parser errors etc?

@sheetalkamat
Copy link
Member

Closing this since #29051 makes more sense than this change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Backlog Bug PRs that fix a backlog bug
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

LanguageService Plugin cannot intercept 'getSupportedCodeFixes'
4 participants