IEP-1049: Add a validation for esp-idf path in the install tools dialog #986
Conversation
|
Warning Rate limit exceeded@alirana01 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 20 minutes and 59 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe updates significantly enhance the validation and error handling for the ESP-IDF tool installation in the Espressif IDE. Key improvements include new methods for retrieving and validating the IDF version, clearer error messages for better user feedback, and a more streamlined logic for handling spaces in paths, contributing to an overall more robust installation experience. Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/IDFDownloadPage.java (1 hunks)
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/Messages.java (1 hunks)
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/messages.properties (1 hunks)
Files skipped from review due to trivial changes (1)
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/Messages.java
Additional comments not posted (2)
bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/messages.properties (1)
5-5: The updated error message is clear and aligns with the PR objectives to enhance user feedback during the installation process.bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/IDFDownloadPage.java (1)
400-403: The validation logic correctly checks for the presence ofidf.pyandidf_tools.pyin the specified directory, aligning with the PR objectives. This ensures that the user has selected a valid ESP-IDF directory.
| String idfToolsPyPth = idfPath + File.separator + "tools" + File.separator + "idf_tools.py"; //$NON-NLS-1$ //$NON-NLS-2$ | ||
| if (!new File (idfPyPath).exists() && !new File (idfToolsPyPth).exists()) | ||
| { | ||
| setErrorMessage(MessageFormat.format(Messages.IDFDownloadPage_CantfindIDFpy, idfPath)); | ||
| setErrorMessage(MessageFormat.format(Messages.IDFDownloadPage_CantfindProperEspIDFDirectory, idfPath)); |
There was a problem hiding this comment.
Consider using a constant for the repeated path segment "tools" to enhance maintainability.
- String idfPyPath = idfPath + File.separator + "tools" + File.separator + "idf.py";
- String idfToolsPyPth = idfPath + File.separator + "tools" + File.separator + "idf_tools.py";
+ final String TOOLS_DIR = "tools";
+ String idfPyPath = idfPath + File.separator + TOOLS_DIR + File.separator + "idf.py";
+ String idfToolsPyPth = idfPath + File.separator + TOOLS_DIR + File.separator + "idf_tools.py";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.
| String idfToolsPyPth = idfPath + File.separator + "tools" + File.separator + "idf_tools.py"; //$NON-NLS-1$ //$NON-NLS-2$ | |
| if (!new File (idfPyPath).exists() && !new File (idfToolsPyPth).exists()) | |
| { | |
| setErrorMessage(MessageFormat.format(Messages.IDFDownloadPage_CantfindIDFpy, idfPath)); | |
| setErrorMessage(MessageFormat.format(Messages.IDFDownloadPage_CantfindProperEspIDFDirectory, idfPath)); | |
| final String TOOLS_DIR = "tools"; | |
| String idfPyPath = idfPath + File.separator + TOOLS_DIR + File.separator + "idf.py"; | |
| String idfToolsPyPth = idfPath + File.separator + TOOLS_DIR + File.separator + "idf_tools.py"; //$NON-NLS-1$ //$NON-NLS-2$ | |
| if (!new File (idfPyPath).exists() && !new File (idfToolsPyPth).exists()) | |
| { | |
| setErrorMessage(MessageFormat.format(Messages.IDFDownloadPage_CantfindProperEspIDFDirectory, idfPath)); |
| @@ -397,9 +397,10 @@ private void validate() | |||
| } | |||
|
|
|||
| String idfPyPath = idfPath + File.separator + "tools" + File.separator + "idf.py"; //$NON-NLS-1$ //$NON-NLS-2$ | |||
| if (!new File (idfPyPath).exists()) | |||
| String idfToolsPyPth = idfPath + File.separator + "tools" + File.separator + "idf_tools.py"; //$NON-NLS-1$ //$NON-NLS-2$ | |||
| if (!new File (idfPyPath).exists() && !new File (idfToolsPyPth).exists()) | |||
| { | |||
There was a problem hiding this comment.
Use constants for idf.py and idf_tools.py from the IDFConstants file.
| @@ -397,9 +397,10 @@ private void validate() | |||
| } | |||
|
|
|||
| String idfPyPath = idfPath + File.separator + "tools" + File.separator + "idf.py"; //$NON-NLS-1$ //$NON-NLS-2$ | |||
| if (!new File (idfPyPath).exists()) | |||
| String idfToolsPyPth = idfPath + File.separator + "tools" + File.separator + "idf_tools.py"; //$NON-NLS-1$ //$NON-NLS-2$ | |||
| if (!new File (idfPyPath).exists() && !new File (idfToolsPyPth).exists()) | |||
There was a problem hiding this comment.
We need to show an error message even if one of the files does not exist.
if (!new File(idfPyPath).exists() || !new File(idfToolsPyPath).exists())
{
// show error message
}
There was a problem hiding this comment.
@kolipakakondal Thanks for the feedback but we are already setting error message on wizard do you think we should show another popup message to user on an already opened popup wizard?
There was a problem hiding this comment.
@kolipakakondal Done
|
@AndriiFilippov please verify once the PR comments are addressed! |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/IDFDownloadPage.java (2 hunks)
Additional comments not posted (3)
bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/IDFDownloadPage.java (3)
41-41: Good practice: Use of constants for path segments.The import of
IDFConstantsis a good practice for maintainability and consistency in path handling.
400-401: Improvement: Use of constants for path segments.The use of
IDFConstants.TOOLS_FOLDER,IDFConstants.IDF_PYTHON_SCRIPT, andIDFConstants.IDF_TOOLS_SCRIPTimproves maintainability and consistency.
402-404: Validation logic: Ensure both files exist.The updated validation logic correctly checks for the existence of both
idf.pyandidf_tools.pyfiles. The error message is appropriately updated.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/IDFDownloadPage.java (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/IDFDownloadPage.java
|
@alirana01 hi ! Tested under: select any folder with white space. The white space validation was executed. Then, the absence of the esp-idf folder was not validated, allowing the process to proceed with the incorrect folder.
THEN: |
@alirana01 Can you please look into this? Can we also avoid showing error message popups on the wizard? Instead, we could show error messages in the wizard's message area at the top.? |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/IDFDownloadPage.java (6 hunks)
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/Messages.java (2 hunks)
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/messages.properties (1 hunks)
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/tools/ToolsJob.java (1 hunks)
Additional comments not posted (10)
bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/Messages.java (2)
12-12: Updated message constant name is clear and descriptive.The new name
IDFDownloadPage_CantfindProperEspIDFDirectoryis more descriptive and aligns with the purpose of the message.
25-25: New message constant added for version space error.The new constant
IDFDownloadPage_VersionSpaceErrorprovides clarity for the specific error condition related to ESP-IDF version and path spaces.bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/messages.properties (2)
5-5: Updated message provides clearer instructions.The updated message now clearly instructs users to select a valid ESP-IDF directory.
13-13: New message added for version space error.The new message
ESP-IDF build system supports spaces in paths after v5.0. Please choose a different directory or continue if your ESP-IDF version supports spaces.provides clear guidance to users regarding path spaces and version compatibility.bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/tools/ToolsJob.java (2)
110-112: Added environment variables for IDF component manager and maintainer.The addition of these environment variables (
IDF_COMPONENT_MANAGERandIDF_MAINTAINER) ensures that the necessary settings are applied for the build process.
119-143: Simplified version retrieval logic.The refactored code simplifies the process of retrieving and setting the IDF version from
idf.py. The previous commented-out code has been removed, and the new logic is more straightforward.bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/IDFDownloadPage.java (4)
400-412: Added version validation for paths with spaces.The new logic checks if the ESP-IDF version supports spaces in paths and sets appropriate error messages if not. This ensures better user feedback and prevents issues with unsupported versions.
415-419: Improved error handling for missingidf.pyandidf_tools.py.The updated code uses constants from
IDFConstantsand provides a more descriptive error message if the required files are not found.
475-518: New methodgetIdfVersionUsingGitfor version retrieval.This method uses
git describeto retrieve the ESP-IDF version, improving the accuracy of version detection.
520-561: New methodisVersionGreaterOrEqualfor version comparison.This method compares the current version with a minimum required version, ensuring compatibility checks are accurate.
|
@alirana01 hi ! Tested under: It now let you to select ESP-IDF with spaces in path like this: BUT downloading validation a bit broken. and now switch version to >=5.0 - bug ⛔ The same works for opposite way: select >=5.0 - it pass validation and then select v4.4 - no validation |
|
Probably we should delete this part of info message. It does not make sense. |
|
@AndriiFilippov please verify after the builds and also make sure to test on mac and linix |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/IDFDownloadPage.java (8 hunks)
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/messages.properties (1 hunks)
Additional comments not posted (8)
bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/messages.properties (2)
5-5: LGTM! Improved clarity in the error message.The updated message provides clearer guidance by specifying the issue and emphasizing the importance of selecting a valid directory.
13-13: LGTM! Clear and necessary guidance for users.The new message addresses potential issues with directory paths containing spaces for versions of ESP-IDF after 5.0.
bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/IDFDownloadPage.java (6)
408-420: Ensure version check logic is correct.The version check logic has been added to handle paths containing spaces.
Ensure that the
getIdfVersionUsingGitmethod returns the correct version format and that theisVersionGreaterOrEqualmethod accurately compares versions.
423-427: Good use of constants for paths.The use of constants from
IDFConstantsimproves maintainability.
483-526: Ensure command execution and output parsing is correct.The method retrieves the IDF version using Git commands and parses the output.
Ensure that the command execution and output parsing handle all possible scenarios and edge cases.
528-569: Ensure version comparison logic is correct.The method compares versions using regular expressions.
Ensure that the version comparison logic handles all possible scenarios and edge cases.
465-465: Ensure error messages are set correctly.The error message for directory paths containing spaces has been updated.
Ensure that the error message is set correctly based on the validation results.
421-421: Ensure page completion status is set correctly.The page completion status is set based on the validation results.
Ensure that the page completion status is set correctly based on the validation results.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/IDFDownloadPage.java (8 hunks)
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/Messages.java (2 hunks)
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/messages.properties (1 hunks)
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/tools/ToolsJob.java (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/messages.properties
Additional comments not posted (11)
bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/Messages.java (2)
12-12: Renamed variable improves clarity.The variable
IDFDownloadPage_CantfindIDFpyhas been renamed toIDFDownloadPage_CantfindProperEspIDFDirectory, which enhances clarity regarding the purpose of the string.
25-25: New variable for version space error.The new variable
IDFDownloadPage_VersionSpaceErrorhas been added to handle error messaging related to version space issues. This is a good addition for enhancing user feedback.bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/tools/ToolsJob.java (3)
110-112: Added environment variable settings.The environment variables
IDF_COMPONENT_MANAGERandIDF_MAINTAINERare being set directly. This is a useful addition for ensuring the correct environment setup.
119-135: Removed conditional logic for setting IDF version.The conditional logic for setting the IDF version has been removed, simplifying the process. This change improves code readability and maintainability.
135-143: Simplified IDF version retrieval logic.The logic for retrieving the IDF version has been simplified by always using the output from
idfPy. This ensures consistency in setting the IDF version.bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/IDFDownloadPage.java (6)
11-13: Added imports for new functionality.The imports for
ArrayList,HashMap, andListhave been added to support the new functionality introduced in the file.
115-122: Added selection listener for version combo.The
versionCombonow has a selection listener that triggers validation when a version is selected. This ensures that the selected version is always validated.
408-420: Added version validation for IDF path with spaces.The
validatemethod now includes a check for the IDF version if the path contains spaces. This ensures that only valid versions are accepted.
423-427: Improved error handling for missing files.The error handling for missing
idf.pyandidf_tools.pyfiles has been improved. This ensures that users are informed when required files are missing.
483-526: New method for retrieving IDF version using Git.The
getIdfVersionUsingGitmethod retrieves the IDF version using Git commands. This is a useful addition for ensuring compatibility with the specified version requirements.
528-569: New method for comparing version strings.The
isVersionGreaterOrEqualmethod compares two version strings to determine if one is greater than or equal to the other. This is a useful addition for version validation.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/tools/manager/pages/ESPIDFMainTablePage.java (1 hunks)
Files skipped from review due to trivial changes (1)
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/tools/manager/pages/ESPIDFMainTablePage.java
|
@alirana01 hi ! Tested under: Well, since we have removed I don't like only this behave- when we select just empty folder(with whitespaces) the validation starts for "release <5.0" not the validation for esp-idf folder.
So, my point is: the validation for release <v5.0 should be just in case if the v4.4 selected in a folder with whitespace like here: But if the empty folder selected, this message does not have any sense:
instead should be: |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/IDFDownloadPage.java (8 hunks)
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/Messages.java (2 hunks)
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/messages.properties (1 hunks)
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/tools/ToolsJob.java (1 hunks)
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/tools/manager/pages/ESPIDFMainTablePage.java (1 hunks)
Files skipped from review due to trivial changes (1)
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/tools/manager/pages/ESPIDFMainTablePage.java
Additional comments not posted (12)
bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/Messages.java (2)
12-12: Clarify message identifier.The identifier
IDFDownloadPage_CantfindProperEspIDFDirectoryprovides a clearer context than the previous name. This change enhances the semantic clarity of the message.
25-25: Add new error message for version space issues.The new string
IDFDownloadPage_VersionSpaceErroris introduced to handle errors related to spaces in paths for ESP-IDF versions. This addition improves user feedback during the installation process.bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/messages.properties (2)
5-5: Clarify error message for directory validation.The updated message
IDFDownloadPage_CantfindProperEspIDFDirectoryprovides clearer guidance by specifying the issue with the ESP-IDF directory. This change enhances user understanding.
13-13: Introduce error message for version space issues.The new entry
IDFDownloadPage_VersionSpaceErrorinforms users about the limitations on spaces in paths for ESP-IDF versions. This addition improves user guidance during the installation process.bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/tools/ToolsJob.java (3)
110-112: Set default environment variables.The addition of
IDF_COMPONENT_MANAGERandIDF_MAINTAINERenvironment variables is a good practice for ensuring consistent build environments.
119-134: Streamline IDF version retrieval logic.The commented-out conditional block simplifies the logic by always retrieving the IDF version using
getIdfVersionFromIdfPy(). This change improves maintainability by removing unnecessary checks.
135-143: Ensure consistent IDF version setting.The refactored logic ensures that the IDF version is consistently retrieved and set, which is crucial for maintaining accurate environment configurations.
bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/IDFDownloadPage.java (5)
408-420: Ensure consistent error handling for version checks.The logic for handling version checks when spaces are present in the path is clear. However, ensure that the error message is consistently set and cleared to avoid user confusion.
Verification successful
Consistent error handling for version checks is confirmed.
The logic for setting and clearing the error message
Messages.IDFDownloadPage_VersionSpaceErroris consistent and correctly implemented. The error message is set when the version check fails and cleared appropriately when the conditions are met. No further issues were found.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the error message handling logic is consistent. # Test: Check for consistent setting and clearing of error messages related to version checks. rg --type java -A 5 $'setErrorMessage' | rg 'VersionSpaceError'Length of output: 547
483-526: Verify command execution and error handling.The method correctly constructs and executes the Git command to retrieve the IDF version. Ensure that the command execution and error handling are robust, especially in diverse environments.
Verification successful
Command Execution and Error Handling Verified
The method
getIdfVersionUsingGituses therunInBackgroundmethod fromProcessBuilderFactory, which is consistently used across the codebase for executing commands. This indicates that the command execution and error handling are robust and consistent with other parts of the system. No issues found.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the robustness of command execution and error handling. # Test: Check for correct usage of ProcessBuilder and error handling in command execution. rg --type java -A 5 $'ProcessBuilderFactory' | rg 'runInBackground'Length of output: 3167
528-569: Verify version comparison logic.The method accurately compares version strings using regex and numerical comparison. Ensure that the regex pattern matches the expected version format and handles edge cases correctly.
Verification successful
Version comparison logic is correct.
The regex pattern
\\d+\\.\\d+\\.\\d+accurately matches version strings in the formatX.Y.Z, and the method correctly compares these versions numerically. No issues found with the implementation.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of the version comparison logic. # Test: Check for correct regex pattern and comparison logic in version comparison. rg --type java -A 5 $'isVersionGreaterOrEqual' | rg 'Pattern.compile'Length of output: 189
423-424: Verify usage of constants for file paths.The usage of constants for file paths from
IDFConstantsis appropriate. Ensure that all relevant paths are using constants to improve maintainability.Verification successful
Usage of Constants for File Paths Verified
The usage of constants from
IDFConstantsfor file paths inIDFDownloadPage.javais consistent with best practices and is correctly implemented. All relevant paths in the snippet provided are using constants, which improves maintainability. No further changes are necessary.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify usage of constants for file paths. # Test: Check for usage of constants from IDFConstants for file paths. rg --type java -A 5 $'IDFConstants' | rg 'TOOL'Length of output: 5503
Line range hint
370-374:
Ensure comprehensive validation for executables.The method checks for the presence of Git and Python executables. Ensure that the validation logic is comprehensive and error messages are clear and informative.
Verification successful
Validation logic for executables is comprehensive and informative.
The
validateGitAndPython()method provides clear and specific error messages for missing Git and Python executables, ensuring that users receive informative feedback when validation fails. This addresses the review comment's concern effectively.
- Error messages such as "Git executable not found" and "Python executable not found" are set, indicating comprehensive validation logic.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify comprehensive validation for Git and Python executables. # Test: Check for validation logic and error messages related to executable paths. rg --type java -A 5 $'validateGitAndPython' | rg 'setErrorMessage'Length of output: 283
Script:
#!/bin/bash # Description: Search for all uses of setErrorMessage in the file to check for specific error messages. # Test: Find all instances of setErrorMessage in the file. rg --type java 'setErrorMessage' bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/IDFDownloadPage.javaLength of output: 732
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/IDFDownloadPage.java (8 hunks)
Files skipped from review as they are similar to previous changes (1)
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/IDFDownloadPage.java
| Logger.log(IDFCorePlugin.getPlugin(), | ||
| IDFCorePlugin.errorStatus("Unable to get the process status.", null)); //$NON-NLS-1$ | ||
| } | ||
| String cmdOutput = status.getMessage(); |
There was a problem hiding this comment.
we have check for status == null but we are not using break in that condition, so here is possible NPE
| // if (StringUtil.isEmpty(idfToolSet.getEnvVars().get(IDFEnvironmentVariables.ESP_IDF_VERSION))) | ||
| // { | ||
| // IStatus status = getIdfVersionFromIdfPy(); | ||
| // String cmdOutput = status.getMessage(); | ||
| // Pattern pattern = Pattern.compile("v(\\d+\\.\\d+\\.\\d+)"); | ||
| // Matcher matcher = pattern.matcher(cmdOutput.toLowerCase()); | ||
| // if (matcher.find()) | ||
| // { | ||
| // idfToolSet.setIdfVersion(matcher.group(1)); | ||
| // } | ||
| // idfToolSet.getEnvVars().put(IDFEnvironmentVariables.ESP_IDF_VERSION, idfToolSet.getIdfVersion()); | ||
| // } | ||
| // else | ||
| // { | ||
| // idfToolSet.setIdfVersion(idfToolSet.getEnvVars().get(IDFEnvironmentVariables.ESP_IDF_VERSION)); | ||
| // } |
There was a problem hiding this comment.
can we remove the commented code?
|
@alirana01 hi ! Tested under: LGTM 👍 |
Description
Added validation in tools installation to check for idf directory
Fixes # (IEP-1049)
Type of change
Please delete options that are not relevant.
How has this been tested?
Select a directory that is not ESP-IDF the installation dialog should reflect proper error message and valid idf directory should allow installation of tools
Test Configuration:
Checklist
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Documentation