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

Return the API before importing the projects #2905

Merged

Conversation

datho7561
Copy link
Contributor

@datho7561 datho7561 commented Jan 27, 2023

This also has the side effect that extensions that depend on vscode-java can start up sooner.

Fixes #2900

Signed-off-by: David Thompson davthomp@redhat.com

@datho7561 datho7561 force-pushed the 2900-return-extension-api-earlier branch from 3e25759 to ccb426c Compare January 27, 2023 17:06
@datho7561 datho7561 requested review from rgrunber and fbricon January 27, 2023 17:18
@datho7561 datho7561 force-pushed the 2900-return-extension-api-earlier branch from ccb426c to 5b52f36 Compare January 27, 2023 17:26
@rgrunber
Copy link
Member

rgrunber commented Jan 27, 2023

Yup, the underlying issue here was definitely a startup issue. Wouldn't be surprised if we hit it now because VS Code started properly waiting for extension activation. The early resolve() after the apiManager is initialized fixed it for me.

@datho7561 datho7561 force-pushed the 2900-return-extension-api-earlier branch 3 times, most recently from 761a9a2 to b8162c2 Compare January 27, 2023 18:49
This also has the side effect that extensions that depend on vscode-java
can start up sooner.

Fixes redhat-developer#2900

Signed-off-by: David Thompson <davthomp@redhat.com>
@datho7561 datho7561 force-pushed the 2900-return-extension-api-earlier branch from b8162c2 to f7d75e7 Compare January 27, 2023 19:03
Copy link
Member

@rgrunber rgrunber left a comment

Choose a reason for hiding this comment

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

Change works for me.

@testforstephen testforstephen requested a review from jdneo January 30, 2023 05:51
Copy link
Collaborator

@jdneo jdneo left a comment

Choose a reason for hiding this comment

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

LGTM.

Though the API instance is resolved early, plugins of JDT-LS can leverage and wait serverReady() during activation.

@rgrunber
Copy link
Member

@jdneo I'll merge this. I would just keep an eye on any extension tests that may depend on this, or any places that make any assumptions about the state of the language server through the api manager immediately on startup.

@rgrunber rgrunber merged commit 6765a6e into redhat-developer:master Jan 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Check details" link ineffective on startup, until the server actually finished initialization
3 participants