-
Notifications
You must be signed in to change notification settings - Fork 49
Extension restart fix #433
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
base: main
Are you sure you want to change the base?
Conversation
6ea48bc
to
d3a16fe
Compare
Moved the reload window pop up from jdk downloader to configuration change listener(restartExtension) triggered whenever any configuration is changed and extension is in bad state.
d3a16fe
to
d3721f8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please rebase the branch to main
in order to remove unrelated changes in l10n files. Thanks.
--- Update: Sorry, please ignore. I mistook the highlight due to the added ,
before the necessary changes.
While checking for the consistency I find that the export function deactivate(): Thenable<void> {
Telemetry.enqueueCloseEvent();
const process = globalState.getNbProcessManager()?.getProcess();
if (process != null) {
process?.kill();
}
return globalState.getClientPromise().stopClient();
} This function is called by vscode only (when closing window) and is for cleaning up . So we can do the following export function deactivate(): Thenable<void> {
Telemetry.enqueueCloseEvent();
const clientStopped = globalState.getClientPromise().stopClient();
const process = globalState.getNbProcessManager()?.getProcess();
if (process != null) {
process?.kill();
}
return clientStopped;
} Note
|
As per the LSP protocol the client manages the server life cycle . So as per the protocol the client sends a Shutdown Request to the server indicating the server to prepare for shutdown . The sever must respond with errors if any that happen during the shutdown . On receiving the Shutdown request's response from the server the client sends an Exit notification which when the server receives must kill itself i.e exit the process .
Implementing the |
Issue
Post activating the extension one of the two cases happen
1)
JDK Found
2)
No JDK Found
In case of "No JDK Found" we enter a bad state where even if the user changes the jdk path to some valid path we cannot restart unless the user reloads .
In case initially JDK found but later user changes the path to an invalid jdk home then also we enter
No JDK Found
state.Fix
Moved the reload window pop up from jdk downloader to configuration change listener(restartExtension) triggered whenever any configuration is changed and extension is in bad state.
This gives user a prompt to reload window so that extension can come out of the bad state.