-
Notifications
You must be signed in to change notification settings - Fork 115
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
feat(server): support code action #1723
Conversation
ce3f9d8
to
d32ae3b
Compare
@ivanwonder Would you be able to rebase this PR? Let me know if it's time for review on this as well. Thanks! |
d32ae3b
to
9a41161
Compare
@dylhunn Done. You can review it now. |
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.
LGTM, with a few nits and requests for additional comments. Great work!
In ts service, the `codeFixes`([getCodeFixesAtPosition](https://github.com/microsoft/TypeScript/blob/7584e6aad6b21f7334562bfd9d8c3c80aafed064/src/services/services.ts#L2689)) and refactors([getApplicableRefactors](https://github.com/microsoft/TypeScript/blob/7584e6aad6b21f7334562bfd9d8c3c80aafed064/src/services/services.ts#L2699)) are resolved in different request. But in LSP they are resolved in the `CodeAction` request. Now, this PR only handles the `codeFixes` because the `@angular/language-service` only supports it.
9a41161
to
1674a30
Compare
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
In ts service, the
codeFixes
(getCodeFixesAtPosition)and
refactors(getApplicableRefactors)
are resolved in different request. But in LSP they are resolved in the
CodeAction
request.Now, this PR only handles the
codeFixes
because the@angular/language-service
only supports it.