Skip to content

IEP-1754 Run ESP-IDF version detection asynchronously with progress reporting #1445

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.

Sign up for GitHub

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

Merged
kolipakakondal merged 2 commits into from
Apr 28, 2026
Merged

IEP-1754 Run ESP-IDF version detection asynchronously with progress reporting #1445

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9b18216
feat: use Job and virtual threads for refresh
sigmaaa Apr 22, 2026
c974b75
feat: internalizing strings in the ESPIDFMainTablePage
sigmaaa Apr 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/tools/Messages.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,22 @@ public class Messages extends NLS

public static String ESPIDFMainTablePage_ActiveLbl;

public static String ESPIDFMainTablePage_DetectingEspIdfSubTaskName;

public static String ESPIDFMainTablePage_EIMButtonTooltip;

public static String ESPIDFMainTablePage_FailderRefreshMsg;

public static String ESPIDFMainTablePage_MainContentGroupLbl;

public static String ESPIDFMainTablePage_RefreshEnvBtnName;

public static String ESPIDFMainTablePage_RefreshEnvBtnTooltip;

public static String ESPIDFMainTablePage_RefreshingIdfJobName;

public static String ESPIDFMainTablePage_ScanningProcessTaskName;

public static String ESPIDFMainTablePage_SettingUpLbl;

public static String ESPIDFMainTablePage_StatusColumnName;
Expand Down
95 changes: 58 additions & 37 deletions ...om.espressif.idf.ui/src/com/espressif/idf/ui/tools/manager/pages/ESPIDFMainTablePage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,12 @@

import java.util.List;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.Executors;

import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.core.runtime.IStatus;
import org.eclipse.core.runtime.Status;
import org.eclipse.core.runtime.jobs.Job;
import org.eclipse.core.runtime.preferences.IEclipsePreferences;
import org.eclipse.core.runtime.preferences.InstanceScope;
import org.eclipse.jface.layout.TableColumnLayout;
Expand Down Expand Up @@ -81,16 +86,14 @@ private record IdfRow(IdfInstalled original, boolean isActive, String version, S
private EimJson eimJson;

private final EimIdfConfiguratinParser configParser;
private final ToolInitializer toolInitializer;

private final IDFConsole idfConsole = new IDFConsole();
private IdfInstalled currentInstallingNode = null;

public ESPIDFMainTablePage(EimJson eimJson)
{
this.eimJson = eimJson;
this.configParser = new EimIdfConfiguratinParser();
this.toolInitializer = new ToolInitializer(InstanceScope.INSTANCE.getNode(UIPlugin.PLUGIN_ID));
new ToolInitializer(InstanceScope.INSTANCE.getNode(UIPlugin.PLUGIN_ID));
}

public Composite createPage(Composite parent)
Expand Down Expand Up @@ -423,44 +426,62 @@ public void refreshEditorUI()
if (container == null || container.isDisposed())
return;

CompletableFuture.supplyAsync(() -> {
try
Job refreshJob = new Job(Messages.ESPIDFMainTablePage_RefreshingIdfJobName)
{
@Override
protected IStatus run(IProgressMonitor monitor)
{
var newJson = configParser.getEimJson(true);
if (newJson != null && newJson.getIdfInstalled() != null)
monitor.beginTask(Messages.ESPIDFMainTablePage_ScanningProcessTaskName, IProgressMonitor.UNKNOWN);

try
{
var gitPath = newJson.getGitPath();
return newJson.getIdfInstalled().stream()
.map(idf -> new IdfRow(idf, ToolsUtility.isIdfInstalledActive(idf),
ToolsUtility.getIdfVersion(idf, gitPath), idf.getName(), idf.getPath()))
.toList();
var newJson = configParser.getEimJson(true);
List<IdfRow> rows = List.of();

if (newJson != null && newJson.getIdfInstalled() != null)
{
var gitPath = newJson.getGitPath();

monitor.subTask(Messages.ESPIDFMainTablePage_DetectingEspIdfSubTaskName);

try (var executor = Executors.newVirtualThreadPerTaskExecutor())
{
var futures = newJson.getIdfInstalled().stream().map(idf -> CompletableFuture.supplyAsync(
() -> new IdfRow(idf, ToolsUtility.isIdfInstalledActive(idf),
ToolsUtility.getIdfVersion(idf, gitPath), idf.getName(), idf.getPath()),
executor)).toList();

rows = futures.stream().map(CompletableFuture::join).toList();
}
}

final List<IdfRow> finalRows = rows;

Display.getDefault().asyncExec(() -> {
if (container.isDisposed())
return;

var currentSelection = tableViewer.getSelection();
currentInstallingNode = null;

tableViewer.setInput(finalRows);
Comment on lines +438 to +467
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 22, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Keep eimJson synchronized with the refreshed configuration.

Rows are rebuilt from newJson, but performToolsSetup() still uses the stale eimJson field when creating SetupToolsInIde. Refresh the field on the UI thread before exposing the new rows.

🐛 Proposed fix 
 					var newJson = configParser.getEimJson(true);
 					List<IdfRow> rows = List.of();
@@
 					}
 
+					final EimJson finalJson = newJson;
 					final List<IdfRow> finalRows = rows;
 
 					Display.getDefault().asyncExec(() -> {
 						if (container.isDisposed())
 							return;
 
 						var currentSelection = tableViewer.getSelection();
+						this.eimJson = finalJson;
 						currentInstallingNode = null;
 
 						tableViewer.setInput(finalRows);
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/tools/manager/pages/ESPIDFMainTablePage.java`
around lines 438 - 467, Rows are built from the local newJson but the shared
field eimJson remains stale, causing performToolsSetup() (and SetupToolsInIde
construction) to operate on old data; inside the
Display.getDefault().asyncExec(...) block, assign the freshly parsed newJson to
the eimJson field (e.g., eimJson = newJson) before calling
tableViewer.setInput(finalRows) or any code that may trigger
performToolsSetup()/new SetupToolsInIde, ensuring UI-thread synchronization of
the configuration state.

tableViewer.setSelection(currentSelection);
updateButtonState();
});

return Status.OK_STATUS;
}
catch (Exception e)
{
Logger.log(e);
return new Status(IStatus.ERROR, UIPlugin.PLUGIN_ID, Messages.ESPIDFMainTablePage_FailderRefreshMsg, e);
} finally
{
monitor.done();
Comment on lines +478 to +480
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 22, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Match the project brace style for finally.

The repository formatter requires block braces on the next line; split finally from the preceding closing brace. As per coding guidelines, “Place opening brace on the next line for types, methods, constructors, blocks, and switch statements.”

🎨 Proposed formatting fix 
-				} finally
+				}
+				finally
 				{
 					monitor.done();
📝 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.

Suggested change
} finally
{
monitor.done();
}
finally
{
monitor.done();
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/tools/manager/pages/ESPIDFMainTablePage.java`
around lines 478 - 480, The finally block formatting must follow the project
brace style: move the `finally` keyword to its own line so the closing brace of
the try/catch ends on one line and `finally` begins the next, e.g. change the
sequence that currently reads like `} finally {` (around the block that calls
`monitor.done()`) to `}\nfinally\n{`, ensuring the block that contains
`monitor.done()` retains its contents unchanged.

}
return List.<IdfRow>of();
}
catch (Exception e)
{
throw new RuntimeException(e);
}
}).thenAcceptAsync(rows -> {
if (container.isDisposed())
return;
var currentSelection = tableViewer.getSelection();
this.currentInstallingNode = null;
try
{
this.eimJson = configParser.getEimJson(false);
}
catch (Exception e)
{
Logger.log(e.toString());
}
tableViewer.setInput(rows);
tableViewer.setSelection(currentSelection);
updateButtonState();
}, Display.getDefault()::asyncExec).exceptionally(ex -> {
Logger.log(ex.getCause() != null ? ex.getCause().toString() : ex.toString());
return null;
});
};
refreshJob.schedule();
}

public void setupInitialEspIdf()
Expand Down
4 changes: 4 additions & 0 deletions bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/tools/messages.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,14 @@ EimJsonStateChangedMsgDetail=It looks like the ESP-IDF installation was modified
ESPIDFMainTablePage_ActiveBtnName=Activate Selected
ESPIDFMainTablePage_ActiveBtnTooltip=Set this version as the active ESP-IDF
ESPIDFMainTablePage_ActiveLbl=\u2713 Active
ESPIDFMainTablePage_DetectingEspIdfSubTaskName=Detecting ESP-IDF versions...
ESPIDFMainTablePage_EIMButtonTooltip=Open the ESP-IDF Installation Manager (EIM) to install, update, or remove ESP-IDF versions.
ESPIDFMainTablePage_FailderRefreshMsg=Failed to refresh ESP-IDF environments
ESPIDFMainTablePage_MainContentGroupLbl=Installed IDF Versions
ESPIDFMainTablePage_RefreshEnvBtnName=Refresh Environment
ESPIDFMainTablePage_RefreshEnvBtnTooltip=Refresh toolchains, Python virtual environment, and IDE settings for the CURRENTLY ACTIVE version
ESPIDFMainTablePage_RefreshingIdfJobName=Refreshing ESP-IDF Environments
ESPIDFMainTablePage_ScanningProcessTaskName=Scanning configurations...
ESPIDFMainTablePage_SettingUpLbl=Setting up...
ESPIDFMainTablePage_StatusColumnName=Status
ESPIDFMainTablePage_VersionDetectionFailedMsg=Detection Failed
Expand Down
Loading