Skip to content
This repository was archived by the owner on Mar 5, 2026. It is now read-only.

Upgrade to use new IJ ActionUpdateThread API#875

Merged
pkukielka merged 3 commits into
mainfrom
pkukielka/fix-edt-in-in2024
Feb 28, 2024
Merged

Upgrade to use new IJ ActionUpdateThread API#875
pkukielka merged 3 commits into
mainfrom
pkukielka/fix-edt-in-in2024

Conversation

@pkukielka

Copy link
Copy Markdown
Contributor

Changes

Fixes #610
ActionUpdateThread.OLD_EDT is deprecated and going to be removed soon

Why we get that issue?
https://plugins.jetbrains.com/docs/intellij/basic-action-system.html#principal-implementation-overrides
https://github.com/JetBrains/intellij-community/blob/idea/233.14475.28/platform/editor-ui-api/src/com/intellij/openapi/actionSystem/ActionUpdateThread.java

New API was become required in 2024 EAP, and every IntelliJ Action must define if update should be running on EDT or BGT thread. Unfortunately that API was not present before 2022.2, so we need to upgrade a bit.

Test plan

This change requires full manual QA

Comment thread src/main/kotlin/com/sourcegraph/common/ui/DumbAwareBGTAction.kt Outdated
Comment thread src/main/kotlin/com/sourcegraph/common/UpgradeToCodyProNotification.kt Outdated
import com.intellij.openapi.actionSystem.KeyboardShortcut
import com.intellij.openapi.actionSystem.ShortcutSet
import com.intellij.openapi.project.DumbAwareAction
import com.intellij.openapi.actionSystem.*

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I assume your IntelliJ settings changed it. I do not like wildcard imports (StackOverflow)

Comment thread src/main/kotlin/com/sourcegraph/cody/ui/AutoGrowingTextArea.kt Outdated
class ReportCodyBugAction : DumbAwareAction("Open GitHub To Report Cody Issue") {
class ReportCodyBugAction : DumbAwareBGTAction("Open GitHub To Report Cody Issue") {
override fun actionPerformed(p0: AnActionEvent) {
BrowserUtil.open("https://github.com/sourcegraph/jetbrains/issues/new?template=bug_report.yml")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

where is it used? this action? don't we need to put the label param btw?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

In the CodyStatusBarActionGroup.
What label param?

}

@Suppress("MissingRecentApi")
override fun getActionUpdateThread(): ActionUpdateThread = ActionUpdateThread.BGT

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

IIUC this is the other case (next to DumbAwareAction) where we need to define getActionUpdateThread, right?

  1. can we have a similar solution - NotificationBGTAction?
  2. can you add a CI check that verifies that we DO not use NotificationAction or DumbAwareAction in any other place than the corresponding abstract classes?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Turns out for NotificationAction it is not really needed, I will revert this.
In fact it looks it is needed only on classes which overrides update method, so I will do a few more reverts.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As for CI it's possible to run IJ inspection but I do not see an easy way to verify this in CI.

Comment thread gradle.properties Outdated

@mkondratek mkondratek left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

comments 🗯️

@pkukielka pkukielka force-pushed the pkukielka/fix-edt-in-in2024 branch from 2674f14 to 2e55b24 Compare February 27, 2024 11:48

@mkondratek mkondratek left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lepiej nie będzie

@pkukielka pkukielka merged commit 60caea4 into main Feb 28, 2024
@pkukielka pkukielka deleted the pkukielka/fix-edt-in-in2024 branch February 28, 2024 09:33
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.

Event Dispatch Thread errors

2 participants