IEP-1787 Mixed / and \ cause incorrect OPENOCD path

[sigmaaa]

Description

Changed approach to getting bin path from the openocd_scripts variable

Fixes # (IEP-1787)

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)

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 A
• Test B

Test Configuration:

• ESP-IDF Version:
• OS (Windows,Linux and macOS):

Dependent components impacted by this PR:

• Component 1
• Component 2

Checklist

• PR Self Reviewed
• Applied Code formatting
• Added Documentation
• Added Unit Test
• Verified on all platforms - Windows,Linux and macOS

Summary by CodeRabbit

Summary

• Bug Fixes

  • Improved OpenOCD script location handling to prevent incorrect path resolution when script paths are empty or invalid.
  • Ensures the OpenOCD binary directory is derived more reliably from the configured script location.

• Refactor

  • Updated the internal logic for determining the OpenOCD location to be more robust and consistent.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d4cbffa4-cb4a-4eaf-88bd-a0b24ce2d854

📥 Commits

Reviewing files that changed from the base of the PR and between 23eb722 and c6b2705.

📒 Files selected for processing (1)

• bundles/com.espressif.idf.core/src/com/espressif/idf/core/util/IDFUtil.java

🚧 Files skipped from review as they are similar to previous changes (1)

• bundles/com.espressif.idf.core/src/com/espressif/idf/core/util/IDFUtil.java

---

📝 Walkthrough

Walkthrough

getOpenOCDLocation() in IDFUtil.java replaces manual string substitution of the share/openocd/scripts path segment with a portable relative path resolution using Paths.get(...).resolve("../../../bin").normalize(). An explicit early return of StringUtil.EMPTY is added when OPENOCD_SCRIPTS is empty.

Changes

OpenOCD Path Resolution Refactor

Layer / File(s)	Summary
Replace string-based path derivation with Paths API bundles/com.espressif.idf.core/src/com/espressif/idf/core/util/IDFUtil.java	getOpenOCDLocation() now uses Paths.get(openocdScriptsPath).resolve("../../../bin").normalize() to compute the OpenOCD bin directory, replacing the previous replace("share/openocd/scripts", "bin") string operation. Adds an explicit StringUtil.EMPTY return when the scripts path is empty.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Poem

A rabbit hopped past paths of string,
Where replace() once did its thing,
Now Paths.get leads the way,
With ../../../ at play —
Normalized trails for a tidy spring! 🐇✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main bug being fixed: mixed forward and backslashes causing incorrect OpenOCD path resolution, with an issue reference (IEP-1787).
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch IEP-1787

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

bundles/com.espressif.idf.core/src/com/espressif/idf/core/util/IDFUtil.java (1)

349-350: ⚡ Quick win

Use block braces for the changed if body

The new one-line if return does not follow the Java block brace rule used by this repository for these paths.

As per coding guidelines, bundles/*/src/**/*.java: “Place opening brace on the next line for types, methods, constructors, blocks, and switch statements; use end-of-line brace for lambda bodies and array initializers”.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bundles/com.espressif.idf.core/src/com/espressif/idf/core/util/IDFUtil.java`
around lines 349 - 350, The if statement checking
StringUtil.isEmpty(openOCDScriptPath) does not follow the repository's Java
formatting guidelines for block braces. Modify the single-line if statement to
use explicit block braces by placing the opening brace on the next line after
the condition and the closing brace after the return statement, following the
repository's style rule that requires opening braces on the next line for blocks
and statements.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@bundles/com.espressif.idf.core/src/com/espressif/idf/core/util/IDFUtil.java`:
- Line 352: In the getOpenOCDLocation() method, wrap the
Paths.get(openOCDScriptPath).resolve("../../../bin").normalize().toString() call
in a try-catch block to handle the InvalidPathException that can be thrown when
OPENOCD_SCRIPTS contains malformed path characters. Return null or an
appropriate default value in the catch block to allow callers like
getOpenocdVersion() to fail gracefully. Additionally, fix the bracing style of
the if statement at line 349 by moving the opening brace to the next line to
follow the project's code style guidelines.

---

Nitpick comments:
In `@bundles/com.espressif.idf.core/src/com/espressif/idf/core/util/IDFUtil.java`:
- Around line 349-350: The if statement checking
StringUtil.isEmpty(openOCDScriptPath) does not follow the repository's Java
formatting guidelines for block braces. Modify the single-line if statement to
use explicit block braces by placing the opening brace on the next line after
the condition and the closing brace after the return statement, following the
repository's style rule that requires opening braces on the next line for blocks
and statements.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: df9647b5-ea91-496b-96df-8bab0059e487

📥 Commits

Reviewing files that changed from the base of the PR and between f2f8aef and 23eb722.

📒 Files selected for processing (1)

• bundles/com.espressif.idf.core/src/com/espressif/idf/core/util/IDFUtil.java

[AndriiFilippov]

LGTM