-
Notifications
You must be signed in to change notification settings - Fork 744
Implement support for Code Lenses #2145
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
base: main
Are you sure you want to change the base?
Conversation
|
Thankfully this is opt-in, otherwise I'd be even more worried about merging this given this also uses the checker. |
| return ls.ProvideCodeLenses(ctx, params.TextDocument.Uri) | ||
| } | ||
|
|
||
| func (s *Server) handleCodeLensResolve(ctx context.Context, codeLens *lsproto.CodeLens, reqMsg *lsproto.RequestMessage) (*lsproto.CodeLens, error) { |
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.
This is likely to conflict with Sheetal's PR, but probably in a good way given the codelens needs to do cross-project too.
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.
I'm noticing that that PR (#1991) doesn't touch go-to-implementations, which in theory is also multi-project. I think that that should be a follow-up to this so the two have a common way to walk references across related projects.
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.
I can do that part as a separate PR but strada didn’t do it so I didn’t port it in my earlier pr
…ly namespaced command in the extension.
This PR implements support for
textDocument/codeLensandcodeLens/resolveon the server side, alongside a custom client command calledtypescript.codeLens.showLocations.CodeLensobjects are produced bytextDocument/codeLensas indicators that an editor client can get more information "later on". WhencodeLens/resolveis called for a givenCodeLensobject, theTitle,Command, and commandArgumentsare populated appropriately so that a user can see some high-level information and potentially click on the UI decoration to trigger a command.We produce two types of
CodeLensobjects: "references" code lens, and "implementations" code lens. These act as markers/indicators so that when an editor like VS Code scrolls into the view of a code lens, it will call on the server to populate each lens with a title and list of locations to preview upon clicking. This amounts to a find-all-references or go-to-implementation call when scrolling into the view of a given code lens.The VS Code extension wires this up with a minimal client-side command which just converts the LSP objects into VS Code ones before redirecting the values to the editor's "peek" pane.
Much of the logic is roughly ported over from VS Code's Code Lens implementations, though it is not one-to-one. On an implementation level, instead of two separate providers/walks like the client did, this implementation does one walk and may produce up to two
CodeLensobjects, encoding how theCodeLensshould be resolved later. More importantly, we have full syntactic and semantic information, so can make better judgments on what kinds of syntax should be decorated with aCodeLensif we ever want to modify the behavior.That said, we have some differences - some that I think are desirable, and some that are just bugs.
One thing I implemented here was support for
includeDeclarationintextDocument/references(aside: I find this behavior iffy for callers who "forget" to pass inincludeDeclaration). I used this as a shortcut to avoid filtering out declarations in find-all-references; however, as you can see above, this gets a different "References" count because it drops overrides/implementations. I actually think that this might not be the right move, and that maybe we should only discount references containing the source position.Current TODOs:
Datais handled given Jake's PR at Refine LSP with our own types, generate more stuff #2141codeLens/resolve.Fixes #1897