-
Notifications
You must be signed in to change notification settings - Fork 44
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
vscode/src/lsp/clientPromise.ts
Outdated
import { LOGGER } from "../logger"; | ||
import { NbProcessManager } from "./nbProcessManager"; | ||
import { clientInit } from "./initializer"; | ||
import { NbLanguageClient } from "./nbLanguageClient"; | ||
import { globalState } from "../globalState"; | ||
import { jdkDownloaderPrompt } from "../webviews/jdkDownloader/prompt"; |
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.
nit: remove unused import
@@ -62,10 +64,15 @@ export class ClientPromise { | |||
public restartExtension = async (nbProcessManager: NbProcessManager | null, notifyKill: boolean) => { | |||
if (this.activationPending) { | |||
LOGGER.warn("Server activation requested repeatedly, ignoring..."); | |||
return; |
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.
Why this return
statement is removed?
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.
- restartExtension gets triggered whenever the configurations are changed and in that case even if activationPending we would need to restart (as configs have changed).
- BTW this is linked with issue on windows, it gets stuck here because of the return statement sometimes . So to resolve that we need to remove this return and show the reload window to the user if required.
84c1184
to
6ea48bc
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.
6ea48bc
to
d3a16fe
Compare
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.