IEP-1047 Running clean commands in new Job #838
Conversation
WalkthroughThe changes primarily focus on improving the responsiveness of the application by executing commands asynchronously in separate jobs. This is achieved by replacing the Changes
TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Files selected for processing (8)
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/handlers/Messages.java (1 hunks)
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/handlers/ProjectCleanCommandHandler.java (1 hunks)
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/handlers/ProjectFullCleanCommandHandler.java (1 hunks)
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/handlers/PythonCleanCommandHandler.java (1 hunks)
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/handlers/messages.properties (1 hunks)
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/update/AbstractToolsHandler.java (4 hunks)
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/update/Messages.java (1 hunks)
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/update/messages.properties (1 hunks)
Files skipped from review due to trivial changes (4)
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/handlers/Messages.java
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/handlers/ProjectCleanCommandHandler.java
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/handlers/ProjectFullCleanCommandHandler.java
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/update/messages.properties
Additional comments (Suppressed): 8
bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/handlers/messages.properties (1)
- 9-11: The new messages added for the job names are clear and descriptive. They will help users understand what process is currently running in the background.
bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/update/Messages.java (1)
- 10-16: The addition of
AbstractToolsHandler_RunningCommandFormatStringis noted. Ensure that this new string is properly initialized in the correspondingmessages.propertiesfile and is used correctly in the context of running commands in theAbstractToolsHandlerclass.bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/handlers/PythonCleanCommandHandler.java (2)
48-51: The
runCommandmethod has been replaced withrunCommandInNewJob, which is expected to run the command in a separate job. This change should improve the responsiveness of the application by offloading the execution to a background job. However, ensure that the new methodrunCommandInNewJobhandles errors and exceptions properly, and that it logs the output and errors for debugging purposes.51-51: The
runCommandInNewJobmethod is now being passed a new argumentMessages.PythonCleanCommandHandler_RunningPythonCleanJobName. Ensure that this message is properly defined in theMessagesclass and the correspondingmessages.propertiesfile, and that it is correctly displayed in the user interface.bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/update/AbstractToolsHandler.java (4)
14-25: The import statements have been updated to include
org.eclipse.core.runtime.IProgressMonitorandorg.eclipse.core.runtime.jobs.Job. These classes are used for creating and managing background jobs in Eclipse, which aligns with the PR's goal of executing commands asynchronously in separate jobs.163-169: The environment variable
PYTHONUNBUFFEREDis set to1. This forces Python to run in unbuffered mode, which can be useful for real-time output. However, it can also increase the I/O load. Ensure that this change does not negatively impact the performance of the application.218-221: The
runCommandmethod now returns anIStatusobject instead of void. This allows the caller to check the status of the command execution. Ensure that all calls to this method have been updated to handle the returned status.299-314: A new method
runCommandInNewJobhas been added. This method creates a new job with the given name, command arguments, project path, and environment map, and schedules it for execution. The command is executed in therunmethod of theJobclass, and the output is redirected to the console. This is a good practice as it improves the responsiveness of the application by offloading the command execution to a separate thread.Consider adding error handling in the
runmethod. If therunCommandmethod throws an exception, it should be caught and logged, and an error status should be returned. This will help in diagnosing issues if the command execution fails.protected IStatus run(IProgressMonitor monitor) { console.println(String.format(Messages.AbstractToolsHandler_RunningCommandFormatString, String.join(" ", commandArgs))); //$NON-NLS-1$ try { console.println((runCommand(commandArgs, pathToProject, envMap))); return Status.OK_STATUS; } catch (Exception e) { Logger.log(e); return new Status(IStatus.ERROR, IDFCorePlugin.PLUGIN_ID, "Error executing command", e); } }
|
hi @sigmaaa ! Tested under: LGTM 👍 there is only problem: some information is not displayed fully: or here during indexer the PATH info absent at all:
probably some buffer problem. |
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