added new shortcuts implementation#113
Conversation
|
Warning Review limit reached
Next review available in: 2 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (23)
📝 WalkthroughWalkthroughThis PR renames 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
framework/shortcuts/qml/Muse/Shortcuts/shortcutsmodel.cpp (1)
177-192: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftSplit command shortcuts out of
apply()and reset handling.m_itemscontains both command and action shortcuts, butapply()sends the whole list toshortcutsRegister()->setShortcuts(...), so command edits go through the action register and are lost on reload.resetToDefaultSelectedShortcuts()has the same problem for command items becauseshortcut.actionis empty there.framework/shortcuts/shortcutstypes.h (2)
61-67: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
operator==ignores the newcommand/panelidentity fields.Command shortcuts populate
command/paneland leaveactionempty (seereadFromFileincommandshortcutsregister.cpplines 304-307). With this equality, two distinct command shortcuts that share the same (empty)actionandsequencescompare equal. This breaks the equality-dependent paths inCommandShortcutsRegister: the early-return insetShortcuts(Line 341), andShortcutList::removeinfilterAndUpdateAdditionalShortcuts(Line 261). Includecommandandpanelin the comparison.🐛 Proposed fix
bool operator ==(const Shortcut& sc) const { return action == sc.action + && command == sc.command + && panel == sc.panel && sequences == sc.sequences && standardKey == sc.standardKey && autoRepeat == sc.autoRepeat; }
56-59: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
isValid()keyed onactionexcludes all command shortcuts.
isValid()returns!action.empty(), but command shortcuts only setcommand/panel(neveraction). ConsequentlyCommandShortcutsRegister::shortcut()/defaultShortcut()(which gate onsh.isValid()) will always report command shortcuts as invalid. Consider treating a non-emptycommandas valid too.🐛 Proposed fix
bool isValid() const { - return !action.empty(); + return !action.empty() || !command.empty(); }
🧹 Nitpick comments (2)
framework/shortcuts/qml/Muse/Shortcuts/shortcutsmodel.cpp (1)
100-168: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCommand-shortcut changes won't refresh the model.
load()subscribes only toshortcutsRegister()->shortcutsChanged(). Since rows are now also built fromcommandShortcutsRegister()->shortcuts(), changes to command shortcuts won't trigger a reload. Consider also subscribing tocommandShortcutsRegister()->shortcutsChanged().Also note the subscription is registered inside
load(); it re-registers on every call (safe only becauseMode::SetReplacededups). Moving it to the constructor would be clearer.framework/rcommand/internal/commanddispatcher.cpp (1)
48-48: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider
LOGD()instead ofLOGI()for per-dispatch logging.
dispatch()runs on every command invocation, so an INFO-level log here will be noisy in normal operation and emits the full query string. A debug-level log is more appropriate for this trace.♻️ Proposed tweak
- LOGI() << "try call command query: " << request.query.toString(); + LOGD() << "try call command query: " << request.query.toString();
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3c5d1f11-33a8-4803-aede-dcb215d1aa5d
📒 Files selected for processing (20)
framework/rcommand/icommandsregister.hframework/rcommand/internal/commanddispatcher.cppframework/rcommand/internal/commandsregister.cppframework/rcommand/internal/commandsregister.hframework/rcommand/qml/Muse/RCommand/commandlistmodel.cppframework/rcontrol/mcp/mcpcontroller.cppframework/shortcuts/CMakeLists.txtframework/shortcuts/icommandshortcutsregister.hframework/shortcuts/internal/commandshortcutsregister.cppframework/shortcuts/internal/commandshortcutsregister.hframework/shortcuts/internal/shortcutsconfiguration.cppframework/shortcuts/internal/shortcutsconfiguration.hframework/shortcuts/internal/shortcutscontroller.cppframework/shortcuts/internal/shortcutscontroller.hframework/shortcuts/ishortcutsconfiguration.hframework/shortcuts/qml/Muse/Shortcuts/shortcutsmodel.cppframework/shortcuts/qml/Muse/Shortcuts/shortcutsmodel.hframework/shortcuts/shortcutsmodule.cppframework/shortcuts/shortcutsmodule.hframework/shortcuts/shortcutstypes.h
igorkorsukov
force-pushed
the
w/rcmd/rcmd_step6
branch
from
15cc2d4 to
0af1eaf
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
framework/shortcuts/qml/Muse/Shortcuts/shortcutsmodel.cpp (1)
342-349: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftDo not reset command shortcuts through the action shortcut register.
For command items,
shortcut.actionis empty, soshortcutsRegister()->defaultShortcut(shortcut.action)cannot find a default and the code clears the command shortcut instead. Handle command shortcuts via the command-shortcuts default path or skip selected reset for command items until that contract exists.🤖 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/shortcuts/qml/Muse/Shortcuts/shortcutsmodel.cpp` around lines 342 - 349, The reset logic in shortcutsmodel.cpp is using shortcutsRegister()->defaultShortcut(shortcut.action), which breaks command items because shortcut.action is empty and causes their shortcut to be cleared. Update the selected-reset path in the shortcuts model to distinguish command shortcuts from action shortcuts, and for command items use the command-shortcuts default path instead of the action shortcut register. If that contract is not available yet, skip resetting command items in this branch and only apply the existing defaultShortcut/sequence clearing logic to non-command shortcuts.framework/shortcuts/shortcutstypes.h (1)
56-59: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
operator==needs to compare all identity fields.ShortcutsRegister::setShortcuts()andCommandShortcutsRegister::setShortcuts()both use list equality to skip writes; ignoringcontext,command, andscopelets changed shortcuts compare equal and bypass persistence.🐛 Proposed fix
bool operator ==(const Shortcut& sc) const { return action == sc.action + && context == sc.context + && command == sc.command + && scope == sc.scope && sequences == sc.sequences && standardKey == sc.standardKey && autoRepeat == sc.autoRepeat; }🤖 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/shortcuts/shortcutstypes.h` around lines 56 - 59, The equality check for shortcut identity is incomplete: `Shortcuts::operator==` (or the equivalent comparison used with `isValid()`) must include all identity fields so `ShortcutsRegister::setShortcuts()` and `CommandShortcutsRegister::setShortcuts()` detect changes correctly. Update the comparison logic to account for `context`, `command`, and `scope` in addition to `action`, keeping the comparison consistent with what gets persisted.
♻️ Duplicate comments (1)
framework/shortcuts/internal/commandshortcutsregister.cpp (1)
240-255: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
filterAndUpdateAdditionalShortcutsmatches onaction, but command shortcuts key oncommand.
std::find(shortcuts.begin(), shortcuts.end(), shortcut.action)constructs a temporaryShortcut(action)and compares viaShortcut::operator==. Command shortcuts leaveactionempty and populatecommand(seereadFromFile, Line 294), so this lookup will never match a real command shortcut and may instead spuriously match any shortcut whoseaction/sequences/standardKey/autoRepeathappen to equal the empty-action temporary. The rest of this register keys oncommand(Lines 121, 162-166); this function should too.This is the same
action-vs-commandmismatch class previously flagged for the (now removed)findCommandShortcut.🐛 Proposed fix
for (auto& [context, additionalShortcuts] : m_additionalShortcutsMap) { for (Shortcut& shortcut : additionalShortcuts) { - auto it = std::find(shortcuts.begin(), shortcuts.end(), shortcut.action); + auto it = std::find_if(shortcuts.begin(), shortcuts.end(), [&shortcut](const Shortcut& s) { + return s.command == shortcut.command; + }); if (it != shortcuts.end()) { shortcut = *it; noAdditionalShortcuts.remove(shortcut); } } }🤖 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/shortcuts/internal/commandshortcutsregister.cpp` around lines 240 - 255, `filterAndUpdateAdditionalShortcuts` is matching additional shortcuts against the wrong field, because command shortcuts are identified by `command` rather than `action`. Update the lookup and any related comparison logic in `CommandShortcutsRegister::filterAndUpdateAdditionalShortcuts` to search by `shortcut.command` and keep the `Shortcut` object updated from the matching command shortcut, consistent with `readFromFile` and the rest of `CommandShortcutsRegister` that key off `command`.
🤖 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/shortcuts/internal/commandshortcutsregister.cpp`:
- Around line 370-372: The early return in setShortcuts via Shortcut::operator==
is too broad for command shortcuts because it ignores command and scope, so
update the comparison logic to explicitly include those fields when the shortcut
type is command-related. Use the existing setShortcuts path in
CommandShortcutsRegister and the Shortcut equality check to ensure two lists
only short-circuit when command, scope, and the rest of the shortcut data all
match.
In `@framework/shortcuts/internal/commandshortcutsregister.h`:
- Around line 36-39: Remove the unused XML forward declarations from
commandshortcutsregister.h, since CommandShortcutsRegister and its
implementation do not reference XmlStreamReader or XmlStreamWriter anywhere.
Update the namespace muse block to keep only the declarations that are actually
used by the current JSON-based code path.
In `@framework/shortcuts/internal/shortcutscontroller.cpp`:
- Line 50: The shortcut dispatch path in ShortcutsController still lacks the
active-panel/scope check before running a command shortcut. Update the shortcut
handling logic in ShortcutsController so it verifies the currently active
panel/context before dispatching, and only falls back to command shortcuts when
the scope matches the intended panel. Use the existing shortcut dispatch entry
point in ShortcutsController to add this guard and keep action shortcuts from
being overridden by unrelated command bindings.
In `@framework/shortcuts/qml/Muse/Shortcuts/shortcutsmodel.cpp`:
- Around line 177-208: ShortcutsModel::apply currently writes command shortcuts
via commandShortcutsRegister()->setShortcuts() before writing action shortcuts
via shortcutsRegister()->setShortcuts(), so a failure in the second step leaves
only part of the state saved. Update apply() to avoid partial commits by
validating both ShortcutList collections first and then applying them in a way
that can be rolled back or otherwise guarantees atomicity across the two
register updates; use the existing apply(), commandShortcutsRegister(), and
shortcutsRegister() flow to locate the change.
- Around line 308-310: The shortcut mutation paths are updating Item::shortcut
directly while RoleSequence still returns the cached Item::sequence, so the
displayed sequence can become stale; update the affected handlers in
shortcutsmodel.cpp (the clear/reset/conflict-resolution logic around the
shortcut mutation sites) to keep Item::sequence synchronized whenever
Item::shortcut changes, so RoleSequence always reflects the current shortcut
state without requiring a full reload.
- Line 58: The RoleSearchKey logic is duplicating the shortcut sequence because
item.searchKey already contains item.sequence, so stop concatenating
item.sequence again in the RoleSearchKey branch and use a single source of truth
for the searchable text. Update the corresponding shortcut model code paths in
shortcutsmodel.cpp, including the places around the other RoleSearchKey handling
blocks, so searchKey is rebuilt from the current shortcut data after edits
rather than preserving a stale embedded sequence. Make sure the same fix is
applied consistently in the affected role-return/update methods in the QML
shortcuts model.
- Around line 165-167: The shortcuts model only refreshes on action shortcut
changes, so command shortcut updates can leave QML rows stale. Update
ShortcutsModel::load() subscriptions in shortcutsmodel.cpp to also listen to the
command-shortcut change notification from shortcutsRegister(), using the same
load() refresh path and async::Asyncable::Mode::SetReplace pattern so external
reset/import/update events for command shortcuts trigger a model reload.
---
Outside diff comments:
In `@framework/shortcuts/qml/Muse/Shortcuts/shortcutsmodel.cpp`:
- Around line 342-349: The reset logic in shortcutsmodel.cpp is using
shortcutsRegister()->defaultShortcut(shortcut.action), which breaks command
items because shortcut.action is empty and causes their shortcut to be cleared.
Update the selected-reset path in the shortcuts model to distinguish command
shortcuts from action shortcuts, and for command items use the command-shortcuts
default path instead of the action shortcut register. If that contract is not
available yet, skip resetting command items in this branch and only apply the
existing defaultShortcut/sequence clearing logic to non-command shortcuts.
In `@framework/shortcuts/shortcutstypes.h`:
- Around line 56-59: The equality check for shortcut identity is incomplete:
`Shortcuts::operator==` (or the equivalent comparison used with `isValid()`)
must include all identity fields so `ShortcutsRegister::setShortcuts()` and
`CommandShortcutsRegister::setShortcuts()` detect changes correctly. Update the
comparison logic to account for `context`, `command`, and `scope` in addition to
`action`, keeping the comparison consistent with what gets persisted.
---
Duplicate comments:
In `@framework/shortcuts/internal/commandshortcutsregister.cpp`:
- Around line 240-255: `filterAndUpdateAdditionalShortcuts` is matching
additional shortcuts against the wrong field, because command shortcuts are
identified by `command` rather than `action`. Update the lookup and any related
comparison logic in
`CommandShortcutsRegister::filterAndUpdateAdditionalShortcuts` to search by
`shortcut.command` and keep the `Shortcut` object updated from the matching
command shortcut, consistent with `readFromFile` and the rest of
`CommandShortcutsRegister` that key off `command`.
🪄 Autofix (Beta)
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
Run ID: c4ffa1aa-a6da-44d7-b6e5-81d4929f9cd6
📒 Files selected for processing (21)
framework/rcommand/icommandsregister.hframework/rcommand/internal/commanddispatcher.cppframework/rcommand/internal/commandsregister.cppframework/rcommand/internal/commandsregister.hframework/rcommand/qml/Muse/RCommand/commandlistmodel.cppframework/rcontrol/mcp/mcpcontroller.cppframework/shortcuts/CMakeLists.txtframework/shortcuts/icommandshortcutsregister.hframework/shortcuts/internal/commandshortcutsregister.cppframework/shortcuts/internal/commandshortcutsregister.hframework/shortcuts/internal/shortcutsconfiguration.cppframework/shortcuts/internal/shortcutsconfiguration.hframework/shortcuts/internal/shortcutscontroller.cppframework/shortcuts/internal/shortcutscontroller.hframework/shortcuts/ishortcutsconfiguration.hframework/shortcuts/qml/Muse/Shortcuts/editshortcutmodel.cppframework/shortcuts/qml/Muse/Shortcuts/shortcutsmodel.cppframework/shortcuts/qml/Muse/Shortcuts/shortcutsmodel.hframework/shortcuts/shortcutsmodule.cppframework/shortcuts/shortcutsmodule.hframework/shortcuts/shortcutstypes.h
💤 Files with no reviewable changes (1)
- framework/shortcuts/qml/Muse/Shortcuts/editshortcutmodel.cpp
| if (shortcuts == m_shortcuts) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect Shortcut::operator== and confirm fields compared
fd shortcutstypes.h --exec sed -n '36,90p' {}Repository: musescore/muse_framework
Length of output: 1467
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant register implementation around the early-out.
FILE=$(fd -a '^commandshortcutsregister.cpp$' framework/shortcuts/internal | head -n 1)
sed -n '300,430p' "$FILE"
# Find where m_shortcuts is assigned or compared elsewhere.
rg -n "m_shortcuts|setShortcuts\\(|command|scope" framework/shortcuts -g '!**/build/**' -g '!**/dist/**'Repository: musescore/muse_framework
Length of output: 21981
Early-out should compare command and scope for command shortcuts. Shortcut::operator== ignores those fields, so setShortcuts() can return early when two command-shortcut lists differ only by command/scope, skipping a real update.
🤖 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/shortcuts/internal/commandshortcutsregister.cpp` around lines 370 -
372, The early return in setShortcuts via Shortcut::operator== is too broad for
command shortcuts because it ignores command and scope, so update the comparison
logic to explicitly include those fields when the shortcut type is
command-related. Use the existing setShortcuts path in CommandShortcutsRegister
and the Shortcut equality check to ensure two lists only short-circuit when
command, scope, and the rest of the shortcut data all match.
| for (const Shortcut& sc : commandShortcuts) { | ||
| const Command& command = Command(sc.command); | ||
| if (commandsState()->commandState(command).enabled) { | ||
| //! TODO Add a check to the active panel |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Unresolved TODO: active-panel check before dispatching command shortcut.
Without the panel/scope check, an enabled command bound to the same sequence may dispatch regardless of the active panel, potentially taking precedence over the intended action shortcut. Want me to open an issue to track adding the active-panel check?
🤖 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/shortcuts/internal/shortcutscontroller.cpp` at line 50, The
shortcut dispatch path in ShortcutsController still lacks the active-panel/scope
check before running a command shortcut. Update the shortcut handling logic in
ShortcutsController so it verifies the currently active panel/context before
dispatching, and only falls back to command shortcuts when the scope matches the
intended panel. Use the existing shortcut dispatch entry point in
ShortcutsController to add this guard and keep action shortcuts from being
overridden by unrelated command bindings.
| case RoleTitle: return item.title; | ||
| case RoleIcon: return item.icon; | ||
| case RoleSequence: return item.sequence; | ||
| case RoleSearchKey: return item.searchKey + item.sequence; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Avoid duplicating and staling shortcut sequences in searchKey.
item.searchKey already includes item.sequence, then Line 58 appends item.sequence again. After editing a shortcut, the role returns the old sequence from item.searchKey plus the new sequence from item.sequence.
Proposed fix
- item.searchKey = QString::fromStdString(info.command.toString())
- + info.title.qTranslatedWithoutMnemonic()
- + info.description.qTranslated()
- + item.sequence;
+ item.searchKey = QString::fromStdString(info.command.toString())
+ + info.title.qTranslatedWithoutMnemonic()
+ + info.description.qTranslated();
...
- item.searchKey = QString::fromStdString(action.code)
- + action.title.qTranslatedWithoutMnemonic()
- + action.description.qTranslated()
- + item.sequence;
+ item.searchKey = QString::fromStdString(action.code)
+ + action.title.qTranslatedWithoutMnemonic()
+ + action.description.qTranslated();Also applies to: 130-133, 157-160
🤖 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/shortcuts/qml/Muse/Shortcuts/shortcutsmodel.cpp` at line 58, The
RoleSearchKey logic is duplicating the shortcut sequence because item.searchKey
already contains item.sequence, so stop concatenating item.sequence again in the
RoleSearchKey branch and use a single source of truth for the searchable text.
Update the corresponding shortcut model code paths in shortcutsmodel.cpp,
including the places around the other RoleSearchKey handling blocks, so
searchKey is rebuilt from the current shortcut data after edits rather than
preserving a stale embedded sequence. Make sure the same fix is applied
consistently in the affected role-return/update methods in the QML shortcuts
model.
| bool ShortcutsModel::apply() | ||
| { | ||
| ShortcutList shortcuts; | ||
|
|
||
| for (const Shortcut& shortcut : std::as_const(m_shortcuts)) { | ||
| shortcuts.push_back(shortcut); | ||
| // command shortcuts | ||
| { | ||
| ShortcutList shortcuts; | ||
| for (const Item& item : std::as_const(m_items)) { | ||
| if (item.shortcut.command.empty()) { | ||
| continue; | ||
| } | ||
| shortcuts.push_back(item.shortcut); | ||
| } | ||
| Ret ret = commandShortcutsRegister()->setShortcuts(shortcuts); | ||
| if (!ret) { | ||
| LOGE() << ret.toString(); | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| Ret ret = shortcutsRegister()->setShortcuts(shortcuts); | ||
|
|
||
| if (!ret) { | ||
| LOGE() << ret.toString(); | ||
| { | ||
| ShortcutList shortcuts; | ||
| for (const Item& item : std::as_const(m_items)) { | ||
| if (item.shortcut.action.empty()) { | ||
| continue; | ||
| } | ||
| shortcuts.push_back(item.shortcut); | ||
| } | ||
| Ret ret = shortcutsRegister()->setShortcuts(shortcuts); | ||
| if (!ret) { | ||
| LOGE() << ret.toString(); | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Prevent partial saves across the two shortcut registers.
Line 188 persists command shortcuts before Line 203 persists action shortcuts. If the second write fails, apply() returns false but command shortcuts have already been changed, leaving the UI state partially applied.
🧰 Tools
🪛 Clang (14.0.6)
[warning] 177-177: use a trailing return type for this function
(modernize-use-trailing-return-type)
[warning] 177-177: method 'apply' can be made static
(readability-convert-member-functions-to-static)
[warning] 181-181: variable 'shortcuts' is not initialized
(cppcoreguidelines-init-variables)
[warning] 196-196: variable 'shortcuts' is not initialized
(cppcoreguidelines-init-variables)
🤖 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/shortcuts/qml/Muse/Shortcuts/shortcutsmodel.cpp` around lines 177 -
208, ShortcutsModel::apply currently writes command shortcuts via
commandShortcutsRegister()->setShortcuts() before writing action shortcuts via
shortcutsRegister()->setShortcuts(), so a failure in the second step leaves only
part of the state saved. Update apply() to avoid partial commits by validating
both ShortcutList collections first and then applying them in a way that can be
rolled back or otherwise guarantees atomicity across the two register updates;
use the existing apply(), commandShortcutsRegister(), and shortcutsRegister()
flow to locate the change.
igorkorsukov
force-pushed
the
w/rcmd/rcmd_step6
branch
from
0af1eaf to
e0a1e41
Compare
1 participant
Summary by CodeRabbit
New Features
Bug Fixes