Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughMoves launch-configuration tab wiring from Java into plugin.xml extensions, simplifies tab-group initialization by delegating tab setup to the framework, makes a TabDebugger constructor public, and updates SWTBot tests to activate the Main tab differently. Changes
Sequence Diagram(s)(omitted — changes are configuration and small control-flow adjustments not requiring a multi-component sequence diagram) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bundles/com.espressif.idf.debug.gdbjtag.openocd/plugin.xml`:
- Around line 231-239: There is a duplicate launch-tab contribution for
TabDebugger (class "com.espressif.idf.debug.gdbjtag.openocd.ui.TabDebugger" with
id "com.espressif.idf.debug.gdbjtag.openocd.ui.debuggertab") which causes
duplicate/misordered tabs at runtime; remove the redundant <tab> element so the
plugin.xml contains only a single declaration of TabDebugger (keep the correct
one and delete the other duplicate), ensuring the remaining entry preserves its
group and placement attributes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: add778f6-8dbd-4e24-b301-24c88989ca67
📒 Files selected for processing (5)
bundles/com.espressif.idf.debug.gdbjtag.openocd/plugin.xmlbundles/com.espressif.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/ui/TabDebugger.javabundles/com.espressif.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/ui/TabGroupLaunchConfiguration.javabundles/com.espressif.idf.launch.serial.ui/plugin.xmlbundles/com.espressif.idf.launch.serial.ui/src/com/espressif/idf/launch/serial/ui/internal/SerialFlashLaunchConfigTabGroup.java
| <tab | ||
| class="com.espressif.idf.debug.gdbjtag.openocd.ui.TabDebugger" | ||
| group="com.espressif.idf.debug.gdbjtag.openocd.launchConfigurationTabGroup" | ||
| id="com.espressif.idf.debug.gdbjtag.openocd.ui.debuggertab" | ||
| name="Debugger"> | ||
| <placement | ||
| after="org.eclipse.cdt.cdi.launch.mainTab"> | ||
| </placement> | ||
| </tab> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="bundles/com.espressif.idf.debug.gdbjtag.openocd/plugin.xml"
python - <<'PY'
import xml.etree.ElementTree as ET
from collections import Counter
file = "bundles/com.espressif.idf.debug.gdbjtag.openocd/plugin.xml"
root = ET.parse(file).getroot()
tab_ids = []
for ext in root.findall("extension"):
if ext.attrib.get("point") == "org.eclipse.debug.ui.launchConfigurationTabs":
for tab in ext.findall("tab"):
tab_ids.append(tab.attrib.get("id"))
dups = {k: v for k, v in Counter(tab_ids).items() if k and v > 1}
print("Duplicate tab ids:", dups)
PY
rg -n 'com\.espressif\.idf\.debug\.gdbjtag\.openocd\.ui\.debuggertab|class="com\.espressif\.idf\.debug\.gdbjtag\.openocd\.ui\.TabDebugger"' "$FILE"Repository: espressif/idf-eclipse-plugin
Length of output: 535
Remove duplicated TabDebugger tab contribution.
TabDebugger is declared twice with the same id (com.espressif.idf.debug.gdbjtag.openocd.ui.debuggertab) at lines 187–189 and lines 232–234. This duplicate registry entry can produce duplicate or misordered tabs at runtime.
🛠️ Proposed fix
- <tab
- class="com.espressif.idf.debug.gdbjtag.openocd.ui.TabDebugger"
- group="com.espressif.idf.debug.gdbjtag.openocd.launchConfigurationTabGroup"
- id="com.espressif.idf.debug.gdbjtag.openocd.ui.debuggertab"
- name="Debugger">
- <placement
- after="org.eclipse.cdt.cdi.launch.mainTab">
- </placement>
- </tab>📝 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.
| <tab | |
| class="com.espressif.idf.debug.gdbjtag.openocd.ui.TabDebugger" | |
| group="com.espressif.idf.debug.gdbjtag.openocd.launchConfigurationTabGroup" | |
| id="com.espressif.idf.debug.gdbjtag.openocd.ui.debuggertab" | |
| name="Debugger"> | |
| <placement | |
| after="org.eclipse.cdt.cdi.launch.mainTab"> | |
| </placement> | |
| </tab> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bundles/com.espressif.idf.debug.gdbjtag.openocd/plugin.xml` around lines 231
- 239, There is a duplicate launch-tab contribution for TabDebugger (class
"com.espressif.idf.debug.gdbjtag.openocd.ui.TabDebugger" with id
"com.espressif.idf.debug.gdbjtag.openocd.ui.debuggertab") which causes
duplicate/misordered tabs at runtime; remove the redundant <tab> element so the
plugin.xml contains only a single declaration of TabDebugger (keep the correct
one and delete the other duplicate), ensuring the remaining entry preserves its
group and placement attributes.
kolipakakondal
left a comment
kolipakakondal
left a comment
There was a problem hiding this comment.
Thanks for the PR @sigmaaa LGTM.
How do you ensure (in an automated way) that this does not create any regression issues? Do we already have tests for this? If not, can you think of how we could implement them?
|
@sigmaaa hi ! Tested under: Launch / Debug config: all expected tabs are visible, tab order is correct, tab titles are correct, no tab is missing, no duplicate tabs appear ✅ |
3 participants
Description
Migrated the tab declarations from the Java class to plugin.xml using the org.eclipse.debug.ui.launchConfigurationTabs extension point. This aligns with modern Eclipse declarative UI patterns and makes the process of merging Launch and Debug configurations easier in the future.
Fixes # (IEP-1721)
Type of change
Please delete options that are not relevant.
How has this been tested?
Test Configuration:
Dependent components impacted by this PR:
Checklist
Summary by CodeRabbit
New Features
Refactor
Tests