-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Remove dead goto-def/peek/ctrl-click EA impls for TS. TS is now entirely on LSP for these experiences. #71098
Conversation
@MariaSolOs FYI. |
var service = document.GetRequiredLanguageService<INavigableItemsService>(); | ||
var service = document.GetLanguageService<INavigableItemsService>(); | ||
if (service is null) | ||
return ImmutableArray<OmniSharpNavigableItem>.Empty; |
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.
just being paranoid. this is omnisharp. so presumably only for C#/VB. But i'm not taking chances.
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.
Note even VB, just C#. But I'm ok with paranoia.
var service = document.GetRequiredLanguageService<INavigableItemsService>(); | ||
var service = document.GetLanguageService<INavigableItemsService>(); | ||
if (service is null) | ||
return null; |
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 paranoid as our LSP server is only for languages that support this. But this just means we're not dependong that. And, for example, if xaml didn't support this, we'd be ok.
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 kind of prefer throwing actually. It isn't expected that we would generally hit this, and if we do we'll get errors that tell us what is going wrong. It won't take down the LSP server or anything.
var service = document.GetRequiredLanguageService<INavigableItemsService>(); | ||
var service = document.GetLanguageService<INavigableItemsService>(); | ||
if (service is null) | ||
return null; |
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 kind of prefer throwing actually. It isn't expected that we would generally hit this, and if we do we'll get errors that tell us what is going wrong. It won't take down the LSP server or anything.
If https://devdiv.visualstudio.com/DevDiv/_git/TypeScript-VS/pullrequest/514833 goes in first, we can remove more here.