Initial work on signature help context #58135
Conversation
| @@ -458,7 +470,7 @@ export interface SignatureHelpProvider { | |||
| /** | |||
| * Provide help for the signature at the given position and document. | |||
| */ | |||
| provideSignatureHelp(model: model.ITextModel, position: Position, token: CancellationToken): ProviderResult<SignatureHelp>; | |||
| provideSignatureHelp(model: model.ITextModel, position: Position, token: CancellationToken, context: SignatureHelpContext): ProviderResult<SignatureHelp>; | |||
| } | |||
There was a problem hiding this comment.
Put token at the end where we can
There was a problem hiding this comment.
Agreed but we ran into this case with CompletionItemProvider too and decided it was better to put the context at the end rather than deal with the complexity of the function overload. We can discuss this more at the API sync
|
Implementation looks good, didn't look at the API too much yet |
|
So as I understand it right, I think the way that you have it now for TypeScript - where everything is a retrigger as long as signature help is present - is actually fine for the most part. The only time I could imagine this being a problem is when you'd prefer for certain characters to actually dismiss signature help. I can't currently think of any cases though. |
Fixes #54972 Adds `SignatureHelpContext`. This tells providers why signature help was requested TODO: - [ ] Better understand semantics of retrigger. Should `retrigger` be an flag instead of a `triggerReason`? - [ ] Fix skipped test - [ ] Add more tests for trigger reasons / trigger characters
4593dc2 to
71a755e
Compare
|
Merging in this initial work to get more testing on the JS/TS part of it. We'll discuss the API and review the design more at the api sync. No commitment to get it finalized this month. If anyone has additional api feedback please let me know |
Fixes #54972
Adds
SignatureHelpContext. This tells providers why signature help was requested. See #54972 for more detailsTODO:
retriggerbe an flag instead of atriggerReason?