Migrated UID Viewer from bottom‑panel dock to editor tool (Repull)#1249
Migrated UID Viewer from bottom‑panel dock to editor tool (Repull)#1249TheAenema wants to merge 1 commit into
Conversation
WalkthroughThe PR converts ChangesUID Viewer Window Refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
editor/file_system/uid_viewer.cpp (1)
180-195: 💤 Low valueConsider using named constants for context menu IDs.
The menu item identification uses magic numbers (0 = Copy UID, 1 = Copy Path). While this works, using named constants or an enum would improve readability and maintainability.
♻️ Example refactor using enum
Add to the class private section in the header:
enum ContextMenuOption { CONTEXT_MENU_COPY_UID = 0, CONTEXT_MENU_COPY_PATH = 1, };Then update the implementation:
void UIDViewer::_on_context_menu_id_pressed(int id) { if (!last_selected_item) { return; } String text_to_copy; - if (id == 0) { // Copy UID + if (id == CONTEXT_MENU_COPY_UID) { text_to_copy = last_selected_item->get_text(0); - } else if (id == 1) { // Copy Path + } else if (id == CONTEXT_MENU_COPY_PATH) { text_to_copy = last_selected_item->get_text(1); }🤖 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 `@editor/file_system/uid_viewer.cpp` around lines 180 - 195, Replace the magic numeric menu IDs in UIDViewer::_on_context_menu_id_pressed with named constants: add an enum ContextMenuOption { CONTEXT_MENU_COPY_UID = 0, CONTEXT_MENU_COPY_PATH = 1 }; to the UIDViewer class (private section) and update the if/else to compare id against CONTEXT_MENU_COPY_UID and CONTEXT_MENU_COPY_PATH; keep the rest of the logic (text selection and clipboard_set) unchanged so behavior is identical but the intent is clear.
🤖 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.
Nitpick comments:
In `@editor/file_system/uid_viewer.cpp`:
- Around line 180-195: Replace the magic numeric menu IDs in
UIDViewer::_on_context_menu_id_pressed with named constants: add an enum
ContextMenuOption { CONTEXT_MENU_COPY_UID = 0, CONTEXT_MENU_COPY_PATH = 1 }; to
the UIDViewer class (private section) and update the if/else to compare id
against CONTEXT_MENU_COPY_UID and CONTEXT_MENU_COPY_PATH; keep the rest of the
logic (text selection and clipboard_set) unchanged so behavior is identical but
the intent is clear.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1830322e-80cd-407f-ac0b-3a7ebb7ace04
📒 Files selected for processing (4)
editor/editor_node.cppeditor/editor_node.heditor/file_system/uid_viewer.cppeditor/file_system/uid_viewer.h
Repulled and Retargeted in #1252