-
Notifications
You must be signed in to change notification settings - Fork 335
[chore][setup]: misleading idf.py not found error (VSC-1761) #1642
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
Conversation
The original error message posting `/tools/idf.py` not found, meanwhile the extension don't search this directory, which is misleading. This PR corrects those messages. --- BTW, I was ought to find out why this extension cannot detect my esp-idf installed with this [ArchLinux AUR repo](https://aur.archlinux.org/packages/esp-idf-git). The repo installs esp-idf to `/opt/esp-idf`, surely there is `./tools/idf.py` or `./tools/export.sh`. Even I was sure the existense of `/opt/esp-idf/tools/idf.py`, the extension still keeps yelling `/tools/idf.py` not found. Which I don't reproduce on the master branch. So it's assumed to be fixed and I'll just update your error message here. Signed-off-by: 蔡略 <[email protected]>
brianignacio5
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR corrects misleading error messages in the ESP-IDF setup flow by updating error messages to display the actual path being checked for validation instead of a generic "/tools/idf.py" message.
- Updates error messages in setup store and main to show the specific path that failed validation
- Refactors file access check logic to ensure proper path handling and prevent unnecessary checks
- Improves accuracy in error reporting by displaying the full path to
tools/idf.pyin responses
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/views/setup/store.ts | Updates error message to display the actual path checked instead of hardcoded "/tools/idf.py" |
| src/views/setup/main.ts | Updates error message to use the specific path from the response message |
| src/setup/SetupPanel.ts | Refactors file access check logic and ensures consistent path reporting in responses |
|
Pull request has been marked as |
|
/cc @brianignacio5 merge it if you feel this PR is fine. |
brianignacio5
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Fabricio-ESP
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update.
The original error message posting
/tools/idf.pynot found, meanwhile the extension don't search this directory, which is misleading.This PR corrects those messages.
BTW, I ought to find out why this extension cannot detect my esp-idf installed with this ArchLinux AUR repo. The repo installs esp-idf to
/opt/esp-idf, surely there is./tools/idf.pyor./tools/export.sh.Even I was sure of the existence of
/opt/esp-idf/tools/idf.py, the extension still keeps yelling/tools/idf.pynot found. Which I don't reproduce on the master branch. So it's assumed to be fixed and I'll just update the error message here.Feel free to edit this PR, busy working recently.
Description
Please include a summary of the change and which issue is fixed.
Fixes... I hate summaries, here is what Copilot concludes:
This pull request improves the handling and reporting of ESP-IDF path validation in the setup flow. The main changes focus on more accurate error messaging and streamlining file access checks.
Error Messaging Improvements:
main.tsandstore.tsto display the actual path checked for ESP-IDF validity, making it clearer to users which path is problematic. [1] [2]File Access Check Logic:
SetupPanel.tsto ensure the file access check only proceeds when a path is provided, and to consistently use the full path totools/idf.pyin responses. This prevents unnecessary checks and improves accuracy in reporting. [1] [2]Code Formatting:
SetupPanel.tsfor consistency.Type of change
Please delete options that are not relevant.
Steps to test this pull request
Provide a list of steps to test changes in this PR and required output
Expected behaviour:
Expected output:
How has this been tested?
Just opened
configure extension, selectexisting toolchains, and type in an incorrect ESP-IDF's path like/booger/aidsto see if it reminds path/booger/aids/tools/idf.pynot found.Dependent components impacted by this PR:
None.
Checklist