transform palette actions to commands - #34469
Conversation
📝 WalkthroughWalkthroughThe palette module adds command declarations, metadata registration, and project-dependent command state. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsLinked repositories: Public OSS repositories can only analyze public repositories installed in this organization. No linked repositories were analyzed; skipped 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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/palette/internal/palettecommandsstate.cpp`:
- Around line 38-81: Update PaletteCommandsState::commandState to return each
palette setting command’s actual checked value from IPaletteConfiguration, while
preserving the existing disabled state when no project is open. In init(),
subscribe to the three configuration change channels for
PALETTE_TOGGLE_SINGLE_CLICK_TO_OPEN_COMMAND,
PALETTE_TOGGLE_SINGLE_PALETTE_COMMAND, and PALETTE_TOGGLE_DRAG_ENABLED_COMMAND,
and refresh the affected command state after each update; disconnect these
subscriptions in deinit().
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 50778af2-5f4f-42cf-ae4a-0bdd17390007
📒 Files selected for processing (14)
src/palette/CMakeLists.txtsrc/palette/internal/ipalettecommandscontroller.hsrc/palette/internal/paletteactionscontroller.cppsrc/palette/internal/paletteactionscontroller.hsrc/palette/internal/palettecommandsregister.cppsrc/palette/internal/palettecommandsregister.hsrc/palette/internal/palettecommandsstate.cppsrc/palette/internal/palettecommandsstate.hsrc/palette/palettecommands.hsrc/palette/palettemodule.cppsrc/palette/qml/MuseScore/Palette/internal/paletterootmodel.cppsrc/palette/qml/MuseScore/Palette/internal/paletterootmodel.hsrc/palette/qml/MuseScore/Palette/internal/palettespanelcontextmenumodel.cppsrc/palette/qml/MuseScore/Palette/internal/palettespanelcontextmenumodel.h
6f86639 to
cf963a0
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/palette/internal/palettecommandsstate.h (1)
25-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd direct includes for
std::stringandstd::vector.
PaletteCommandsStateusesstd::stringat Line 46 andstd::vectorat Line 56, but this header includes only<map>. Add the direct standard-library headers so the header does not depend on transitive includes.Proposed include fix
`#include` <map> +#include <string> +#include <vector>Also applies to: 46-56
🤖 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 `@src/palette/internal/palettecommandsstate.h` around lines 25 - 33, Update the includes in PaletteCommandsState to directly include the standard-library headers declaring std::string and std::vector, alongside the existing <map> include. Leave the surrounding module and interface includes unchanged.
🤖 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 `@src/palette/internal/palettecommandsstate.h`:
- Around line 25-33: Update the includes in PaletteCommandsState to directly
include the standard-library headers declaring std::string and std::vector,
alongside the existing <map> include. Leave the surrounding module and interface
includes unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ea877f26-ae36-40b5-88ce-cd09e6b92d5b
📒 Files selected for processing (15)
src/appshell/qml/MuseScore/AppShell/appmenumodel.cppsrc/palette/CMakeLists.txtsrc/palette/internal/ipalettecommandscontroller.hsrc/palette/internal/paletteactionscontroller.cppsrc/palette/internal/paletteactionscontroller.hsrc/palette/internal/palettecommandsregister.cppsrc/palette/internal/palettecommandsregister.hsrc/palette/internal/palettecommandsstate.cppsrc/palette/internal/palettecommandsstate.hsrc/palette/palettecommands.hsrc/palette/palettemodule.cppsrc/palette/qml/MuseScore/Palette/internal/paletterootmodel.cppsrc/palette/qml/MuseScore/Palette/internal/paletterootmodel.hsrc/palette/qml/MuseScore/Palette/internal/palettespanelcontextmenumodel.cppsrc/palette/qml/MuseScore/Palette/internal/palettespanelcontextmenumodel.h
🚧 Files skipped from review as they are similar to previous changes (12)
- src/palette/qml/MuseScore/Palette/internal/paletterootmodel.cpp
- src/palette/qml/MuseScore/Palette/internal/paletterootmodel.h
- src/palette/qml/MuseScore/Palette/internal/palettespanelcontextmenumodel.h
- src/palette/internal/ipalettecommandscontroller.h
- src/palette/internal/palettecommandsregister.cpp
- src/palette/internal/palettecommandsstate.cpp
- src/palette/internal/paletteactionscontroller.cpp
- src/palette/qml/MuseScore/Palette/internal/palettespanelcontextmenumodel.cpp
- src/palette/CMakeLists.txt
- src/palette/internal/paletteactionscontroller.h
- src/palette/palettemodule.cpp
- src/palette/internal/palettecommandsregister.h
2 participants
No description provided.