transform dock actions to commands - #212
Conversation
|
Warning Review limit reached
Next review available in: 35 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughBoth dock window variants now define four dock commands and provide command metadata and state services. Module and context import resolution registers these services with the command registries. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@framework/dockwindow/internal/dockwindowactionscontroller.cpp`:
- Around line 77-109: Validate CommandQuery parameter types before conversion in
both DockWindowActionsController implementations:
framework/dockwindow/internal/dockwindowactionscontroller.cpp lines 77-109 and
framework/dockwindow_v2/internal/dockwindowactionscontroller.cpp lines 77-109.
In setDockOpen, require dock_name to have Val::String type and open to have
Val::Bool type; in toggleOpened and toggleFloating, require dock_name to be
Val::String. Return BadArgs for missing or non-conforming parameters before
calling QString conversion or window actions.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: bb7babc3-62fe-452f-abfe-3cc195fb513a
📒 Files selected for processing (20)
framework/dockwindow/CMakeLists.txtframework/dockwindow/dockcommands.hframework/dockwindow/dockmodule.cppframework/dockwindow/dockmodule.hframework/dockwindow/internal/dockcommandsregister.cppframework/dockwindow/internal/dockcommandsregister.hframework/dockwindow/internal/dockcommandsstate.cppframework/dockwindow/internal/dockcommandsstate.hframework/dockwindow/internal/dockwindowactionscontroller.cppframework/dockwindow/internal/dockwindowactionscontroller.hframework/dockwindow_v2/CMakeLists.txtframework/dockwindow_v2/dockcommands.hframework/dockwindow_v2/dockmodule.cppframework/dockwindow_v2/dockmodule.hframework/dockwindow_v2/internal/dockcommandsregister.cppframework/dockwindow_v2/internal/dockcommandsregister.hframework/dockwindow_v2/internal/dockcommandsstate.cppframework/dockwindow_v2/internal/dockcommandsstate.hframework/dockwindow_v2/internal/dockwindowactionscontroller.cppframework/dockwindow_v2/internal/dockwindowactionscontroller.h
| muse::Ret DockWindowActionsController::setDockOpen(const CommandQuery& query) | ||
| { | ||
| if (args.count() < 2) { | ||
| return; | ||
| if (!(query.contains("dock_name") && query.contains("open"))) { | ||
| return muse::make_ret(Ret::Code::BadArgs); | ||
| } | ||
|
|
||
| QString dockName = args.arg<QString>(0); | ||
| bool open = args.arg<bool>(1); | ||
| QString dockName = QString::fromStdString(query.param("dock_name").toString()); | ||
| bool open = query.param("open").toBool(); | ||
|
|
||
| window()->setDockOpen(dockName, open); | ||
| return muse::make_ok(); | ||
| } | ||
|
|
||
| void DockWindowActionsController::toggleOpened(const ActionData& args) | ||
| muse::Ret DockWindowActionsController::toggleOpened(const CommandQuery& query) | ||
| { | ||
| QString dockName = dockNameFromArgs(args); | ||
| if (!query.contains("dock_name")) { | ||
| return muse::make_ret(Ret::Code::BadArgs); | ||
| } | ||
|
|
||
| QString dockName = QString::fromStdString(query.param("dock_name").toString()); | ||
| window()->toggleDock(dockName); | ||
| return muse::make_ok(); | ||
| } | ||
|
|
||
| void DockWindowActionsController::toggleFloating(const ActionData& args) | ||
| muse::Ret DockWindowActionsController::toggleFloating(const CommandQuery& query) | ||
| { | ||
| QString dockName = dockNameFromArgs(args); | ||
| if (!query.contains("dock_name")) { | ||
| return muse::make_ret(Ret::Code::BadArgs); | ||
| } | ||
|
|
||
| QString dockName = QString::fromStdString(query.param("dock_name").toString()); | ||
| window()->toggleDockFloating(dockName); | ||
| return muse::make_ok(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the CommandQuery and Val conversion contracts.
rg -n -C 5 \
'class CommandQuery|struct CommandQuery|CommandQuery::param|class Val|struct Val|Val::toBool|Val::toString|toBool\(\) const|toString\(\) const' \
.
# Locate command-dispatch tests and direct dock command requests.
rg -n -C 4 \
'DOCK_SET_OPEN_COMMAND|DOCK_TOGGLE_COMMAND|DOCK_TOGGLE_FLOATING_COMMAND|CommandQuery' \
frameworkRepository: musescore/muse_framework
Length of output: 46609
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Val implementation ---'
sed -n '1,240p' framework/global/types/val.cpp
printf '%s\n' '--- Val type API and UriQuery API ---'
sed -n '44,125p' framework/global/types/val.h
sed -n '80,140p' framework/global/types/uri.h
printf '%s\n' '--- UriQuery implementation ---'
rg -n -C 8 'UriQuery::(param|addParam|contains)|Val&|const Val&' framework/global/types/uri.cpp framework/global/types/uri.h
printf '%s\n' '--- Command schema and dispatch validation ---'
rg -n -C 8 'InputSchema|DataType|validate|schema|Arg' framework/rcommand framework/global | head -n 400Repository: musescore/muse_framework
Length of output: 43478
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Val conversions ---'
sed -n '90,225p' framework/global/types/val.cpp
printf '%s\n' '--- UriQuery declarations and implementation ---'
sed -n '95,125p' framework/global/types/uri.h
rg -n -C 12 'UriQuery::(param|addParam|contains)' framework/global/types/uri.cpp
printf '%s\n' '--- InputSchema and dispatcher paths ---'
rg -n -C 10 'InputSchema|validate|DataType|schema' framework/rcommand framework/global/typesRepository: musescore/muse_framework
Length of output: 13073
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Command dispatch ---'
sed -n '50,95p' framework/rcommand/internal/commanddispatcher.cpp
printf '%s\n' '--- InputSchema consumers ---'
rg -n -C 8 'inputSchema|InputSchema|DataType::Boolean|DataType::String' framework --glob '*.{cpp,h}'
printf '%s\n' '--- UriQuery parsing ---'
sed -n '140,265p' framework/global/types/uri.cppRepository: musescore/muse_framework
Length of output: 35626
Reject non-conforming CommandQuery parameter types.
InputSchema is not enforced by CommandDispatcher. Val::toString() converts numeric and boolean values, while Val::toBool() converts numeric, string, list, and map values. Validate Val::type() before dispatch and return BadArgs unless dock_name is String and open is Bool in both controllers.
🧰 Tools
🪛 Clang (14.0.6)
[warning] 77-77: use a trailing return type for this function
(modernize-use-trailing-return-type)
[warning] 90-90: use a trailing return type for this function
(modernize-use-trailing-return-type)
[warning] 101-101: use a trailing return type for this function
(modernize-use-trailing-return-type)
🪛 Cppcheck (2.21.0)
[error] 107-107: failed to evaluate #if condition, undefined function-like macro invocation
(syntaxError)
📍 Affects 2 files
framework/dockwindow/internal/dockwindowactionscontroller.cpp#L77-L109(this comment)framework/dockwindow_v2/internal/dockwindowactionscontroller.cpp#L77-L109
🤖 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 `@framework/dockwindow/internal/dockwindowactionscontroller.cpp` around lines
77 - 109, Validate CommandQuery parameter types before conversion in both
DockWindowActionsController implementations:
framework/dockwindow/internal/dockwindowactionscontroller.cpp lines 77-109 and
framework/dockwindow_v2/internal/dockwindowactionscontroller.cpp lines 77-109.
In setDockOpen, require dock_name to have Val::String type and open to have
Val::Bool type; in toggleOpened and toggleFloating, require dock_name to be
Val::String. Return BadArgs for missing or non-conforming parameters before
calling QString conversion or window actions.
3d19fa4 to
ebbaaad
Compare
ebbaaad to
09f3c6d
Compare
2 participants
No description provided.