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

Use the DumbAwareBGTAction interface to silence the update thread warning. #1332

Merged
merged 2 commits into from
Apr 24, 2024

Conversation

odisseus
Copy link
Contributor

@odisseus odisseus commented Apr 17, 2024

This should fix #1327.

Test plan

I have clicked around a little bit in IntelliJ IDEA 2024.1.1 RC, and I haven't seen any OLD_EDT warnings in the log.

@odisseus odisseus requested a review from pkukielka April 17, 2024 22:02
Copy link
Contributor

@pkukielka pkukielka left a comment

Choose a reason for hiding this comment

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

LGTM

@odisseus odisseus force-pushed the mg/issue-1327 branch 2 times, most recently from 6ffeb82 to d6c9822 Compare April 22, 2024 14:52
@odisseus odisseus requested a review from pkukielka April 22, 2024 14:53
@odisseus odisseus marked this pull request as ready for review April 22, 2024 14:54
@odisseus odisseus requested a review from a team April 23, 2024 01:11
didUseShortcut(shortcut)
}
}
SimpleDumbAwareBGTAction { didUseShortcut(shortcut) }
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@@ -124,7 +124,7 @@ class RemoteRepoPopupController(val project: Project) {
.createPopup()

val okAction =
object : DumbAwareAction() {
object : DumbAwareBGTAction() {
Copy link
Contributor

Choose a reason for hiding this comment

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

SimpleDumbAwareBGTAction? :)

Copy link
Contributor Author

@odisseus odisseus Apr 23, 2024

Choose a reason for hiding this comment

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

The SimpleDumbAwareBGTAction doesn't give the lambda access to the action event class. In its current form, it isn't applicable when the event is actually used, though I may consider refactoring it to remove this restriction.

P.S. This action also uses AnAction#unregisterCustomShortcutSet, so the simple wrapper is definitely not applicable here.

@@ -19,7 +20,7 @@ private constructor(title: String, content: String, shouldShowUpgradeOption: Boo
init {
icon = Icons.CodyLogo
val learnMoreAction: AnAction =
object : DumbAwareAction("Learn more") {
object : DumbAwareBGTAction("Learn more") {
Copy link
Contributor

Choose a reason for hiding this comment

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

SimpleDumbAwareBGTAction

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Refactored


if (shouldShowUpgradeOption) {
val upgradeAction: AnAction =
object : DumbAwareAction("Upgrade") {
object : DumbAwareBGTAction("Upgrade") {
Copy link
Contributor

Choose a reason for hiding this comment

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

SimpleDumbAwareBGTAction

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Refactored

class SimpleDumbAwareBGTAction(
text: @NlsActions.ActionText String? = null,
private val action: () -> Unit
) : DumbAwareAction(text) {
Copy link
Contributor

Choose a reason for hiding this comment

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

How is that supposed to work? Shouldn't it inherit from DumbAwareBGTAction?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch!

@pkukielka pkukielka merged commit 1b45779 into main Apr 24, 2024
9 checks passed
@pkukielka pkukielka deleted the mg/issue-1327 branch April 24, 2024 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants