fix: attemp to fix welcome page with showIntro#1396
Conversation
📝 WalkthroughWalkthroughAdds logic to EspressifToolStartup to close and re-show the Eclipse Intro during early startup via PlatformUI and the IntroManager; cleans up a duplicate Changes
Sequence Diagram(s)sequenceDiagram
participant Startup as EspressifToolStartup
participant Workbench as PlatformUI/Workbench
participant IntroMgr as IntroManager
participant Window as ActiveWorkbenchWindow
Startup->>Workbench: call getWorkbench()
Workbench->>IntroMgr: getIntroManager().getIntro()
IntroMgr-->>Startup: IIntroPart (or null)
Startup->>Window: asyncExec on UI thread
alt intro exists
Window->>IntroMgr: closeIntro(IIntroPart)
end
Window->>IntroMgr: showIntro(Window)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/tools/EspressifToolStartup.java`:
- Around line 75-81: The UI calls in earlyStartup() (calls to
PlatformUI.getWorkbench().getIntroManager().getIntro(),
getActiveWorkbenchWindow(), and showIntro()) run off the UI thread and can throw
SWTException or see a null window; wrap the logic in
Display.getDefault().asyncExec(() -> { ... }) and inside the Runnable re-check
that IIntroPart introPart =
PlatformUI.getWorkbench().getIntroManager().getIntro() is non-null and that
PlatformUI.getWorkbench().getActiveWorkbenchWindow() is non-null before calling
showIntro(..., false) to ensure thread-safety and null-safety.
| IIntroPart introPart = PlatformUI.getWorkbench().getIntroManager().getIntro(); | ||
|
|
||
| if (introPart != null) | ||
| { | ||
| PlatformUI.getWorkbench().getIntroManager().showIntro(PlatformUI.getWorkbench().getActiveWorkbenchWindow(), | ||
| false); | ||
| } |
There was a problem hiding this comment.
earlyStartup() runs on a non-UI thread — these UI calls will throw SWTException or get a null window.
Per the Eclipse IStartup contract, earlyStartup() is invoked on a background thread. Directly calling getIntro(), getActiveWorkbenchWindow(), and showIntro() here will either raise an SWTException: Invalid thread access or return null for the active window. Every other UI operation in this class correctly wraps calls in Display.getDefault().asyncExec()/syncExec().
Additionally, even with Display wrapping, the active workbench window may not yet be available at early startup time — a null-check on the window is advisable.
Proposed fix: wrap in Display.asyncExec with null-safety
public void earlyStartup()
{
- IIntroPart introPart = PlatformUI.getWorkbench().getIntroManager().getIntro();
-
- if (introPart != null)
- {
- PlatformUI.getWorkbench().getIntroManager().showIntro(PlatformUI.getWorkbench().getActiveWorkbenchWindow(),
- false);
- }
+ Display.getDefault().asyncExec(() -> {
+ IIntroPart introPart = PlatformUI.getWorkbench().getIntroManager().getIntro();
+ if (introPart != null)
+ {
+ IWorkbenchWindow window = PlatformUI.getWorkbench().getActiveWorkbenchWindow();
+ if (window != null)
+ {
+ PlatformUI.getWorkbench().getIntroManager().showIntro(window, false);
+ }
+ }
+ });
preferences = org.eclipse.core.runtime.preferences.InstanceScope.INSTANCE.getNode(UIPlugin.PLUGIN_ID);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| IIntroPart introPart = PlatformUI.getWorkbench().getIntroManager().getIntro(); | |
| if (introPart != null) | |
| { | |
| PlatformUI.getWorkbench().getIntroManager().showIntro(PlatformUI.getWorkbench().getActiveWorkbenchWindow(), | |
| false); | |
| } | |
| Display.getDefault().asyncExec(() -> { | |
| IIntroPart introPart = PlatformUI.getWorkbench().getIntroManager().getIntro(); | |
| if (introPart != null) | |
| { | |
| IWorkbenchWindow window = PlatformUI.getWorkbench().getActiveWorkbenchWindow(); | |
| if (window != null) | |
| { | |
| PlatformUI.getWorkbench().getIntroManager().showIntro(window, false); | |
| } | |
| } | |
| }); |
🤖 Prompt for AI Agents
In
`@bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/tools/EspressifToolStartup.java`
around lines 75 - 81, The UI calls in earlyStartup() (calls to
PlatformUI.getWorkbench().getIntroManager().getIntro(),
getActiveWorkbenchWindow(), and showIntro()) run off the UI thread and can throw
SWTException or see a null window; wrap the logic in
Display.getDefault().asyncExec(() -> { ... }) and inside the Runnable re-check
that IIntroPart introPart =
PlatformUI.getWorkbench().getIntroManager().getIntro() is non-null and that
PlatformUI.getWorkbench().getActiveWorkbenchWindow() is non-null before calling
showIntro(..., false) to ensure thread-safety and null-safety.
|
Tested: this solution doesn't work |
1 participant
Description
Please include a summary of the change and which issue is fixed.
Fixes # (IEP-XXX)
Type of change
Please delete options that are not relevant.
How has this been tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Test Configuration:
Dependent components impacted by this PR:
Checklist
Summary by CodeRabbit