Add Go To Implementation Api #18346
Add Go To Implementation Api #18346
Conversation
For microsoft#10806 Adds a new API for supporting `go to implementation` command for languages. Implements an example for TS
|
Removing myself from reviewer, unless there is a very specific reason why my feedback is needed in this area. Please let me know. |
| precondition: ModeContextKeys.hasImplementationProvider, | ||
| kbOpts: { | ||
| kbExpr: EditorContextKeys.TextFocus, | ||
| primary: KeyMod.CtrlCmd | KeyCode.F12 |
There was a problem hiding this comment.
Ok. Visual Studio uses alt+f12 for "go to implementation" but we already have a binding for that to editor.action.previewDeclaration. I don't see any default binding for cmd+f12 though, but am not sure how this would on other platforms. Any suggestions for an unused keybinding that would work on all platforms well?
| * The implementation provider interface defines the contract between extensions and | ||
| * the go to implementation feature. | ||
| */ | ||
| export interface ImplementationProvider { |
There was a problem hiding this comment.
Maybe TypeDefinitionProvider would be a better name?
|
Good stuff - added some nit comments, ignored the actual implementation in the TypeScript extensions. I would definitely go for a separate provider despite them being quite similar. My name proposal is |
|
+1 for the separate provider. Name wise I would go with TypeOfDefintionProvider :-) |
|
Thanks for the review. I've updated the interface names and am looking into the keybindings |
|
@dbaeumer The language service API should probably pick up support for this feature, but I wanted to check with you to see that should block this PR. I think we can either:
What are your thoughts on this? |
|
@mjbvz we always ship new API in VS Code and then update the LSP later. Can you please file n issue here https://github.com/Microsoft/language-server-protocol so I don't forget to do it. |
**bug** In microsoft#18346, I originally called the new go to implementation provider api `ImplementationProvider` which we then decided to rename to `TypeDefinitionProvider`. At the time, I didn't realize that a type definition was actually its own, unrelated concept. **Fix** Rename `TypeDefinitionProvider` to `TypeImplementationProvider` to make it clear what the purpose and use of this api is.
* Rename TypeDefinitionProvider to TypeImeplementationProvider **bug** In #18346, I originally called the new go to implementation provider api `ImplementationProvider` which we then decided to rename to `TypeDefinitionProvider`. At the time, I didn't realize that a type definition was actually its own, unrelated concept. **Fix** Rename `TypeDefinitionProvider` to `TypeImplementationProvider` to make it clear what the purpose and use of this api is. * Fix a few names in comments
Fixes #10806
Adds a new API for supporting
go to implementationtype commands in languages. Implements an example for TSThe current approach added a new public interface called
ImplementationProvider(a suggestions for a better name would be appreciated). This resulted in some duplicate code that I will try to clean up further. An alternate design would be to extend theDefintionProviderwith an optionalprovideImplemtnationmethod. This would allow reusing our existing internal interfaces without as much duplication, but we may want to keep the two concepts separate. Please let me know if you have any thoughts one way or the other.