feature: Use espressif modified clangd tool#476
Conversation
…475) The pioarduino IDE extension now exports PLATFORMIO_IDE_INTELLISENSE_ENGINE as an environment variable. When the value is 'clangd', the platform automatically installs tool-clangd-esp during configure_default_packages(). Espressif's clangd has native Xtensa and ESP RISC-V ISA extension support (xespv, xesploop, xespdsp, etc.) that the upstream clangd lacks, which eliminates unknown-flag errors and improves IntelliSense for ESP32 targets. Amp-Thread-ID: https://ampcode.com/threads/T-019d8c25-963a-728c-8166-76efd9747d60 Co-authored-by: Amp <amp@ampcode.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis pull request adds support for conditional installation of the clangd-esp tool package. A new optional tool package entry is introduced in the platform definition, with corresponding logic to automatically install it when the IDE's IntelliSense engine is configured to use clangd. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
platform.py (1)
808-811: Normalize engine value and log install failure for better resilience.Line 808 currently depends on an exact env string match and Line 811 ignores install result. Normalizing and checking the return value would make this path more robust.
Proposed improvement
- engine = os.environ.get("PLATFORMIO_IDE_INTELLISENSE_ENGINE", "") - if engine == "clangd" and "tool-clangd-esp" in self.packages: + engine = os.environ.get("PLATFORMIO_IDE_INTELLISENSE_ENGINE", "").strip().lower() + if engine == "clangd" and "tool-clangd-esp" in self.packages: logger.info("clangd IntelliSense engine detected, installing tool-clangd-esp") - self.install_tool("tool-clangd-esp") + if not self.install_tool("tool-clangd-esp"): + logger.warning("Failed to install tool-clangd-esp for clangd IntelliSense engine")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platform.py` around lines 808 - 811, The code currently checks engine via exact match and ignores install result; update the engine check to normalize the env var (strip and lower) before comparing to "clangd", keep the existing package presence check ("tool-clangd-esp" in self.packages), then call self.install_tool("tool-clangd-esp") and capture its return value; if installation fails, emit a logger.error with a clear message including the engine value and package name so failures are visible. Use the existing symbols engine, self.packages, self.install_tool, and logger in the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@platform.py`:
- Around line 808-811: The code currently checks engine via exact match and
ignores install result; update the engine check to normalize the env var (strip
and lower) before comparing to "clangd", keep the existing package presence
check ("tool-clangd-esp" in self.packages), then call
self.install_tool("tool-clangd-esp") and capture its return value; if
installation fails, emit a logger.error with a clear message including the
engine value and package name so failures are visible. Use the existing symbols
engine, self.packages, self.install_tool, and logger in the change.
1 participant
Description:
install use espressif clangd when pioarduino env var intellisense is set to
clangd.Needs not yet released pioarduino VSC extension v1.3.8
Checklist:
Summary by CodeRabbit
Release Notes