Initial work on signature help context#58135
Conversation
| */ | ||
| 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
mjbvz
force-pushed
the
mjbvz/trigger-context
branch
from
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 |
3 participants
Fixes #54972
Adds
SignatureHelpContext. This tells providers why signature help was requested. See #54972 for more detailsTODO:
retriggerbe an flag instead of atriggerReason?