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

[Gradle JDK Automanagement][Intellij plugin] Background tool window #405

Merged
merged 13 commits into from
Sep 11, 2024

Conversation

crogoz
Copy link
Contributor

@crogoz crogoz commented Sep 5, 2024

Before this PR

After this PR

Fixes: #395

  • Gradle JDK setup tool window will be minimized by default
  • The Gradle JDK setup would look as if it were a background process (similar to the Gradle Sync workflow)
  • If the Gradle JDK setup fails, then we will show a pop-up Notification with a link to the logs.

==COMMIT_MSG==
[Gradle JDK Automanagement][Intellij plugin] Background tool window
==COMMIT_MSG==

Note: I've tried using the declarative setup for the Gradle JDK setup toolWindow (https://plugins.jetbrains.com/docs/intellij/tool-windows.html#declarative-setup) but the workflow had 2 issues:

Possible downsides?

@@ -90,7 +110,28 @@ public void maybeSetupGradleJdks() {
"Skipping setupGradleJdks because gradle JDK setup is not found %s", gradleSetupScript));
return;
}
setupGradleJdks();
TasksKt.withBackgroundProgress(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment on lines +113 to +134
TasksKt.withBackgroundProgress(
project,
"Gradle JDK Setup",
(_coroutineScope, continuation) -> {
StepsKt.withProgressText(
"`Gradle JDK Setup` is running. Logs in the `Gradle JDK Setup` window ...",
(_cor, conti) -> {
setupGradleJdks();
return conti;
},
continuation);
return continuation;
},
new Continuation<>() {
@Override
public @NotNull CoroutineContext getContext() {
return EmptyCoroutineContext.INSTANCE;
}

@Override
public void resumeWith(@NotNull Object _object) {}
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the Gradle JDK setup process would look similar to the Gradle Sync background process. The logs can be seen in Gradle JDK Setup window tab.

Screenshot 2024-09-06 at 12 21 50 PM

@@ -109,6 +151,21 @@ public void processTerminated(@NotNull ProcessEvent _event) {
consoleView.get().attachToProcess(handler);
ProcessTerminatedListener.attach(handler, project, "Gradle JDK setup finished with exit code $EXIT_CODE$");
handler.waitFor();
if (handler.getExitCode() != 0) {
Notification notification = NotificationGroupManager.getInstance()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

if the process has failed for any reason, a pop-up will appear:

Screenshot 2024-09-06 at 12 28 17 PM

I could also focus the Gradle JDK setup window tool by default. wdyt ?

Choose a reason for hiding this comment

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

I think it makes sense to focus the window on failure. The notification windows are quite quiet. I for some reason barely ever notice the "required plugins" notification lolol and I think it's quite common.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed in: aceac47

@@ -72,13 +83,22 @@ private ConsoleView initConsoleView() {
ContentFactory contentFactory = ContentFactory.getInstance();
Content content = contentFactory.createContent(newConsoleView.getComponent(), "", false);
toolWindow.getContentManager().addContent(content);
// TODO(crogoz): Focus only when error or takes a long time?
toolWindow.activate(null);
Disposer.register(project, newConsoleView);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe this fixes: #394 I wasn't able to repro the issue

Choose a reason for hiding this comment

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

🙏

@crogoz crogoz marked this pull request as ready for review September 6, 2024 12:07
@bulldozer-bot bulldozer-bot bot merged commit 9dbc638 into develop Sep 11, 2024
6 checks passed
@bulldozer-bot bulldozer-bot bot deleted the cr/window-manager-in-plugin branch September 11, 2024 14:34
@autorelease3
Copy link

autorelease3 bot commented Sep 11, 2024

Released 0.47.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Intellij plugin: Can the popup not steal focus
4 participants