Skip to content

transform notation actions to commands#34138

Merged
igorkorsukov merged 1 commit into
musescore:mainfrom
igorkorsukov:w/rcmd/rcmd_step10
Jul 10, 2026
Merged

transform notation actions to commands#34138
igorkorsukov merged 1 commit into
musescore:mainfrom
igorkorsukov:w/rcmd/rcmd_step10

Conversation

@igorkorsukov

Copy link
Copy Markdown
Member

No description provided.

@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: eb06e456-9963-42b5-b452-207addb1f483

📥 Commits

Reviewing files that changed from the base of the PR and between 2c93a62 and 77490b7.

📒 Files selected for processing (24)
  • src/app/configs/data/shortcuts.json
  • src/app/configs/data/shortcuts.xml
  • src/appshell/qml/MuseScore/AppShell/appmenumodel.cpp
  • src/appshell/qml/MuseScore/AppShell/navigableappmenumodel.cpp
  • src/braille/internal/notationbraille.cpp
  • src/context/internal/shortcutresolver.cpp
  • src/context/internal/shortcutresolver.h
  • src/notation/internal/notationinteraction.cpp
  • src/notation/internal/notationmidiinput.cpp
  • src/notationscene/inotationcommandscontroller.h
  • src/notationscene/internal/notationactioncontroller.cpp
  • src/notationscene/internal/notationactioncontroller.h
  • src/notationscene/internal/notationcommandsregister.cpp
  • src/notationscene/internal/notationcommandsstate.cpp
  • src/notationscene/internal/notationcommandsstate.h
  • src/notationscene/internal/notationuiactions.cpp
  • src/notationscene/internal/notationuiactions.h
  • src/notationscene/notationcommands.h
  • src/notationscene/qml/MuseScore/NotationScene/notationviewinputcontroller.cpp
  • src/notationscene/qml/MuseScore/NotationScene/noteinputbarmodel.cpp
  • src/notationscene/qml/MuseScore/NotationScene/noteinputbarmodel.h
  • src/preferences/qml/MuseScore/Preferences/noteinputpreferencesmodel.cpp
  • src/web/appshell/view/appmenumodel.cpp
  • src/web/appshell/view/navigableappmenumodel.cpp
💤 Files with no reviewable changes (3)
  • src/app/configs/data/shortcuts.xml
  • src/notationscene/qml/MuseScore/NotationScene/noteinputbarmodel.h
  • src/notationscene/internal/notationuiactions.h
🚧 Files skipped from review as they are similar to previous changes (18)
  • src/notationscene/qml/MuseScore/NotationScene/notationviewinputcontroller.cpp
  • src/notationscene/internal/notationcommandsregister.cpp
  • src/notationscene/internal/notationcommandsstate.h
  • src/context/internal/shortcutresolver.h
  • src/notationscene/notationcommands.h
  • src/preferences/qml/MuseScore/Preferences/noteinputpreferencesmodel.cpp
  • src/context/internal/shortcutresolver.cpp
  • src/web/appshell/view/appmenumodel.cpp
  • src/notation/internal/notationmidiinput.cpp
  • src/app/configs/data/shortcuts.json
  • src/braille/internal/notationbraille.cpp
  • src/appshell/qml/MuseScore/AppShell/appmenumodel.cpp
  • src/notationscene/inotationcommandscontroller.h
  • src/notation/internal/notationinteraction.cpp
  • src/notationscene/internal/notationcommandsstate.cpp
  • src/notationscene/qml/MuseScore/NotationScene/noteinputbarmodel.cpp
  • src/notationscene/internal/notationuiactions.cpp
  • src/notationscene/internal/notationactioncontroller.cpp

📝 Walkthrough

Walkthrough

The PR adds command URIs for notation note input, durations, dots, accidentals, rests, and notation additions. It expands controller APIs for note-input state, selection properties, and current notation parameters. Command registration and command-state computation are refactored around these APIs. Legacy notation UI actions and state helpers are removed, while menus, preferences, shortcuts, MIDI, braille, and mouse input dispatches are migrated to command URIs. Navigation dispatches also adopt command URIs.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning No PR description was provided, so the required issue reference, summary, and checklist items are missing. Add a filled-out description using the template: issue link or number, change summary, CLA username, checklist items, and testing notes.
Docstring Coverage ⚠️ Warning Docstring coverage is 1.39% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and directly matches the PR's main change from notation actions to commands.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

Review ran into problems

🔥 Problems

Linked repositories: Public OSS repositories can only analyze public repositories installed in this organization. No linked repositories were analyzed; skipped musescore/muse_framework.git.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
src/notationscene/qml/MuseScore/NotationScene/notationviewinputcontroller.cpp (1)

1377-1377: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider using the TOGGLE_NOTE_INPUT_COMMAND constant instead of a hardcoded string.

This file is in the notationscene module and could include notationcommands.h to use the TOGGLE_NOTE_INPUT_COMMAND constant, preventing typos and easing future refactoring. The command constants are defined in src/notationscene/notationcommands.h and already used by NotationActionController.

🤖 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/notationscene/qml/MuseScore/NotationScene/notationviewinputcontroller.cpp`
at line 1377, Replace the hardcoded command string in
notationviewinputcontroller::dispatch with the existing
TOGGLE_NOTE_INPUT_COMMAND constant. Include notationcommands.h in
notationviewinputcontroller.cpp if needed, and update the call site so the
command stays consistent with NotationActionController and avoids typo-prone
string literals.
🤖 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 @.gitmodules:
- Line 3: The muse_framework submodule URL was changed to an SSH fork URL, which
breaks the expected repo contract and CI checks. Update the .gitmodules entry
for submodule.muse_framework back to the upstream HTTPS URL so fresh clones and
.github/workflows/check_submodules.yml continue to work without private SSH
access.

In `@src/notationscene/internal/notationcommandsstate.cpp`:
- Around line 197-199: The checked-state logic in notationcommandsstate.cpp for
TOGGLE_NOTE_INPUT_COMMAND is incorrect because it compares noteInputMethod()
against NoteInputMethod::UNKNOWN, so the main note-input toggle never becomes
checked. Update the CommandState returned from the NOTE_INPUT_COMMANDS branch in
notationcommandsstate::stateForCommand (or equivalent lookup path) so the toggle
command uses only controller()->isNoteInputMode() for its checked state, while
keeping the existing method-specific comparison for the other note-input
commands.

---

Nitpick comments:
In
`@src/notationscene/qml/MuseScore/NotationScene/notationviewinputcontroller.cpp`:
- Line 1377: Replace the hardcoded command string in
notationviewinputcontroller::dispatch with the existing
TOGGLE_NOTE_INPUT_COMMAND constant. Include notationcommands.h in
notationviewinputcontroller.cpp if needed, and update the call site so the
command stays consistent with NotationActionController and avoids typo-prone
string literals.
🪄 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: CHILL

Plan: Pro

Run ID: 55422158-971b-441f-bd89-f4de6a73d53e

📥 Commits

Reviewing files that changed from the base of the PR and between 15584ce and a9643a6.

📒 Files selected for processing (24)
  • .gitmodules
  • muse
  • src/app/configs/data/shortcuts.json
  • src/app/configs/data/shortcuts.xml
  • src/appshell/qml/MuseScore/AppShell/appmenumodel.cpp
  • src/braille/internal/notationbraille.cpp
  • src/context/internal/shortcutresolver.cpp
  • src/context/internal/shortcutresolver.h
  • src/notation/internal/notationinteraction.cpp
  • src/notation/internal/notationmidiinput.cpp
  • src/notationscene/inotationcommandscontroller.h
  • src/notationscene/internal/notationactioncontroller.cpp
  • src/notationscene/internal/notationactioncontroller.h
  • src/notationscene/internal/notationcommandsregister.cpp
  • src/notationscene/internal/notationcommandsstate.cpp
  • src/notationscene/internal/notationcommandsstate.h
  • src/notationscene/internal/notationuiactions.cpp
  • src/notationscene/internal/notationuiactions.h
  • src/notationscene/notationcommands.h
  • src/notationscene/qml/MuseScore/NotationScene/notationviewinputcontroller.cpp
  • src/notationscene/qml/MuseScore/NotationScene/noteinputbarmodel.cpp
  • src/notationscene/qml/MuseScore/NotationScene/noteinputbarmodel.h
  • src/preferences/qml/MuseScore/Preferences/noteinputpreferencesmodel.cpp
  • src/web/appshell/view/appmenumodel.cpp
💤 Files with no reviewable changes (3)
  • src/notationscene/internal/notationuiactions.h
  • src/app/configs/data/shortcuts.xml
  • src/notationscene/qml/MuseScore/NotationScene/noteinputbarmodel.h

Comment thread .gitmodules Outdated
Comment thread src/notationscene/internal/notationcommandsstate.cpp
Tie* newTie = mu::engraving::EditTie::cmdToggleTie(score());

notifyAboutNotationChanged();
m_selectionChanged.notify();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use notifyAboutSelectionChangedIfNeed() instead

@igorkorsukov
igorkorsukov force-pushed the w/rcmd/rcmd_step10 branch 2 times, most recently from 5214a32 to 2c93a62 Compare July 10, 2026 07:43
@igorkorsukov
igorkorsukov merged commit f63b3d1 into musescore:main Jul 10, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants