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
Declare ActionUpdateThread as EDT for RunFlutterAction #7356
Declare ActionUpdateThread as EDT for RunFlutterAction #7356
Conversation
From Intellij 2024.1, any actions that doesn't override `getActionUpdateThread` triggers a warning. This is because the default behavior `OLD_EDT` will be removed in the future. To suppress the warning, we need to declare this action as EDT, as it's modifying the presentation in `update()`. Fixes flutter#7331
@bc-lee thanks for looking into this. I'm not sure this will fix the full problem, as we have other classes that have shown this error as well. See my comment here: #7330 (comment). Potentially we could implement the same thing you're doing here in other places, but I'm also wondering if this problem should be fixed in the IJ platform. Do you have any insight into this? |
The warning generated by Intellij 2024.1 is intentional. JetBrains is pushing all actions to override the This PR is currently in draft because there are several additional actions to fix. I will request a review of this PR once all the warnings have been addressed. |
@bc-lee Ah thanks so much for clarifying; I didn't realize IJ intended for us to start overriding the function. In that case, yes, supplying overriding threads for all of the affected classes makes sense. Is this going to be an issue for all classes extending I'm also uncertain about how to decide EDT vs BGT here. The IJ doc here suggests that all actions should change |
Yeah, I think it would be reasonable to do one at a time / a few at a time. My main outstanding question is whether this change should be EDT or BGT, as I'm not sure that modifying Presentation counts as changing UI "directly". |
It turns out that in most cases, the
However, I cannot judge easily if each case is okay to run on the BGT or not. It appears that many of Intellij API's are okay to use both EDT and BGT and if there is a specific requirement, that API has annotation like @RequiresEdt or I'm closing this PR for now as I cannot give a clear answer to this problem, but if someone can give me some advice on how to handle this, I would appreciate it. |
Hi from JetBrains,
Blind guess is that pretty much all actions in the Flutter plugin should be updated in |
@alexander-doroshko thanks for the advice! @bc-lee feel free to reopen this PR. Are you set up to test this? |
From Intellij 2024.1, any actions that doesn't override
getActionUpdateThread
triggers a warning. This is because the default behaviorOLD_EDT
will be removed in the future. To suppress the warning, we need to declare this action as EDT, as it's modifying the presentation inupdate()
.Fixes #7331
Pre-launch Checklist
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.