Skip to content
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

Add activeSignatureHelp to SignatureHelpContext #65440

Merged
merged 7 commits into from
Jan 7, 2019

Conversation

mjbvz
Copy link
Collaborator

@mjbvz mjbvz commented Dec 20, 2018

For #33413

Adds the active SignatureHelp object (which has the correct activeSignature set) to the SignatureHelpContext. This allows signature help providers to use the activeSignature info to take into account if the user has arrowed through signature help results

Todo:

  • Tests are failing. Not part of this change directly but a logic error that this change revealed
  • How can we ensure that custom SignatureHelp objects are not lost when returned through the SignatureHelpContext?

@mjbvz mjbvz self-assigned this Dec 20, 2018
@mjbvz mjbvz requested a review from jrieken December 20, 2018 03:27
@mjbvz mjbvz force-pushed the dev/mjbvz/activeSignatureHelp branch from a75c49f to da752c8 Compare December 20, 2018 05:51
@jrieken
Copy link
Member

jrieken commented Dec 20, 2018

How can we ensure that custom SignatureHelp objects

We have two ways to do this:

  • Implement a custom lifecycle and custom caching, e.g. as done for IntelliSense
  • Use the heap service. That's more convenient but less performant since the main thread walks all objects. This is how it works: On the ext host use ExtHostHeapService#keep which will keep the object and mix-in an id, on the main side use IHeapService#trackRecursive. Once the main side object is GC'ed, the ext-side object will be removed

mjbvz added 5 commits January 7, 2019 12:57
For #33413

Adds the active `SignatureHelp` object (along with the active signature) to the `SignatureHelpContext`.

Todo:

- [ ] Tests are failing. Not part of  this change directly but a logic error that this change revealed
- How can we ensure that custom `SignatureHelp` objects are not lost when returned in the `SignatureHelpContext`
Makes sure custom SignatureHelp objects are correctly saved off in the  `activeSignature` part of the context
@mjbvz mjbvz force-pushed the dev/mjbvz/activeSignatureHelp branch from da752c8 to cddbf58 Compare January 7, 2019 21:47
@mjbvz mjbvz added this to the December/January 2019 milestone Jan 7, 2019
@mjbvz mjbvz merged commit 4a38520 into master Jan 7, 2019
@mjbvz
Copy link
Collaborator Author

mjbvz commented Jan 7, 2019

Merging in to proposed api

@mjbvz mjbvz deleted the dev/mjbvz/activeSignatureHelp branch January 7, 2019 21:56
joaomoreno added a commit that referenced this pull request Jan 8, 2019
mjbvz added a commit that referenced this pull request Jan 8, 2019
@github-actions github-actions bot locked and limited conversation to collaborators Mar 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants