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

Fix references cancellation and preview #11056

Merged
merged 19 commits into from
Jun 16, 2023
Merged

Conversation

michelleangela
Copy link
Contributor

@michelleangela michelleangela commented Jun 7, 2023

This changeset simplifies how preview works for "find all references". It also makes cancellation more straightforward and sequential for "find all references", "rename" and "call hierarchy callers of" by changing their messages to the language server to a RequestType object. This avoids race conditions in reading cancellation states among the different requests and also avoids the chance of not resolving a request from reading/setting incorrect states due to race conditions.

This change also has a corresponding changeset on the language server code base.

Changes for Preview and Cancel for Find All References

  • When doing a preview, the partial or preview results is now shown in the "Other References" view. This no longer resolves a current request to show any confirmed reference type from partial results in the VS Code References view.
  • When cancelling, any in progress results are now shown in the "Other References" view. This is because cancelling the request will return an empty result object from the request message returned from the language server.
  • A find all references request will now only complete when the language server has finished processing the request or when the processing gets cancelled.

Request Changes

  • The typescript message sent to the language server for getting "find all references", "rename", and "call hierarchy callers of" is now a RequestType object. This is to ensure synchronization of a language server response to its typescript sender. This also removes the requirement of having to manually manage cancellation of requests.
  • Cancellation is now done by calling cancel on a local cancellation token passed to the request to the language server rather than by sending a separate cancellation notification to the language server.
  • A new request sends a cancellation event to cancel a current request by calling ReferencesManager.cancelCurrentReferenceRequest.

Other Changes

  • Refactored some objects and variables to the appropriate classes/files that use them exclusively.

@michelleangela michelleangela linked an issue Jun 7, 2023 that may be closed by this pull request
@michelleangela michelleangela force-pushed the mimatias/cancellation branch from 6536d7b to c99d0e9 Compare June 14, 2023 02:16
Colengms
Colengms previously approved these changes Jun 14, 2023
Colengms
Colengms previously approved these changes Jun 14, 2023
Copy link
Contributor

@sean-mcmanus sean-mcmanus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see Find All References cancels Call Hierarchy, but Call Hierarchy isn't canceling Find All References -- can you add cancel handling to prepareCallHierarchy be make that cancel Find All References (and Call Hierarchy)?

@michelleangela
Copy link
Contributor Author

I see Find All References cancels Call Hierarchy, but Call Hierarchy isn't canceling Find All References -- can you add cancel handling to prepareCallHierarchy be make that cancel Find All References (and Call Hierarchy)?

Added.

@michelleangela michelleangela force-pushed the mimatias/cancellation branch from 1a9698d to 42bbea7 Compare June 15, 2023 22:05
@michelleangela michelleangela merged commit 491456e into main Jun 16, 2023
@michelleangela michelleangela deleted the mimatias/cancellation branch June 16, 2023 00:02
@github-actions github-actions bot locked and limited conversation to collaborators Jul 31, 2023
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.

Fix Cancellation for References/Rename/Call Hierarchy
3 participants