Skip to content

transform notation actions to commands #34150

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
igorkorsukov merged 1 commit into from
Jul 10, 2026
Merged

transform notation actions to commands #34150

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/notationscene/inotationcommandscontroller.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ class INotationCommandsController : MODULE_CONTEXT_INTERFACE
virtual NoteInputMethod noteInputMethod() const = 0;
virtual DurationType currentDurationType() const = 0;
virtual int currentDotCount() const = 0;
virtual bool currentIsRest() const = 0;
virtual AccidentalType currentAccidentalType() const = 0;
virtual std::set<SymbolId> currentArticulations() const = 0;
virtual voice_idx_t currentVoice() const = 0;
};
}
171 changes: 159 additions & 12 deletions src/notationscene/internal/notationactioncontroller.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,18 @@ void NotationActionController::init()
registerCommand(ADD_TIE_COMMAND, &Controller::addTie);
registerCommand(ADD_SLUR_COMMAND, &Controller::addSlur);
registerCommand(ADD_LV_COMMAND, &Controller::addLaissezVib);
registerCommand(ADD_MARCATO_COMMAND, [this]() { toggleArticulation(SymbolId::articMarcatoAbove); });
registerCommand(ADD_SFORZATO_COMMAND, [this]() { toggleArticulation(SymbolId::articAccentAbove); });
registerCommand(ADD_TENUTO_COMMAND, [this]() { toggleArticulation(SymbolId::articTenutoAbove); });
registerCommand(ADD_STACCATO_COMMAND, [this]() { toggleArticulation(SymbolId::articStaccatoAbove); });

registerCommand(USE_VOICE_1_COMMAND, [this]() { changeVoice(0); });
registerCommand(USE_VOICE_2_COMMAND, [this]() { changeVoice(1); });
registerCommand(USE_VOICE_3_COMMAND, [this]() { changeVoice(2); });
registerCommand(USE_VOICE_4_COMMAND, [this]() { changeVoice(3); });

registerCommand(FLIP_COMMAND, &Interaction::flipSelection);
registerCommand(FLIP_HORIZONTALLY_COMMAND, &Interaction::flipSelectionHorizontally);

registerAction("note-action", &Controller::handleNoteAction);

Expand Down Expand Up @@ -187,11 +199,6 @@ void NotationActionController::init()

registerAction("rest", &Interaction::putRestToSelection);

registerAction("add-marcato", [this]() { toggleArticulation(SymbolId::articMarcatoAbove); });
registerAction("add-sforzato", [this]() { toggleArticulation(SymbolId::articAccentAbove); });
registerAction("add-tenuto", [this]() { toggleArticulation(SymbolId::articTenutoAbove); });
registerAction("add-staccato", [this]() { toggleArticulation(SymbolId::articStaccatoAbove); });

registerAction("duplet", [this]() { putTuplet(2); }, &Controller::noteOrRestSelected);
registerAction("triplet", [this]() { putTuplet(3); }, &Controller::noteOrRestSelected);
registerAction("quadruplet", [this]() { putTuplet(4); }, &Controller::noteOrRestSelected);
Expand Down Expand Up @@ -235,9 +242,6 @@ void NotationActionController::init()
registerAction("notation-paste-special", [this]() { pasteSelection(PastingType::Special); });
registerAction("notation-swap", &Interaction::swapSelection, &Controller::hasSelection);

registerAction("flip", &Interaction::flipSelection, &Controller::hasSelection);
registerAction("flip-horizontally", &Interaction::flipSelectionHorizontally, &Controller::hasSelection);

registerAction("chord-tie", &Controller::chordTie);

registerAction("hammer-on-pull-off", &Controller::addHammerOnPullOff);
Expand Down Expand Up @@ -506,10 +510,6 @@ void NotationActionController::init()
}
}

for (voice_idx_t i = 0; i < mu::engraving::VOICES; ++i) {
registerAction("voice-" + std::to_string(i + 1), [this, i]() { changeVoice(static_cast<int>(i)); });
}

registerAction("voice-assignment-all-in-instrument", &Interaction::changeSelectedElementsVoiceAssignment,
VoiceAssignment::ALL_VOICE_IN_INSTRUMENT);
registerAction("voice-assignment-all-in-staff", &Interaction::changeSelectedElementsVoiceAssignment,
Expand Down Expand Up @@ -1015,6 +1015,40 @@ int NotationActionController::currentDotCount() const
return interaction->noteInput()->state().duration().dots();
}

bool NotationActionController::currentIsRest() const
{
INotationInteractionPtr interaction = currentNotationInteraction();
if (!interaction) {
return false;
}

INotationNoteInputPtr noteInput = interaction->noteInput();
if (!noteInput) {
return false;
}

if (noteInput->isNoteInputMode()) {
return noteInput->state().rest();
}

INotationSelectionPtr selection = interaction->selection();
if (!selection) {
return false;
}

if (selection->isNone() || selection->isRange()) {
return false;
}

for (const EngravingItem* element: selection->elements()) {
if (!element->isRest()) {
return false;
}
}

return true;
}
Comment on lines +1018 to +1050

@coderabbitai coderabbitai Bot Jul 10, 2026

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.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Guard against empty selection in currentIsRest().

If selection->isNone() is false and selection->isRange() is false but selection->elements() is empty, the loop body never executes and the function returns true (vacuous truth). currentVoice() at Line 1152 explicitly guards this case with selectedElements.empty(), but currentIsRest() does not. Add an early return for consistency and correctness.

🛡️ Proposed fix 
     if (selection->isNone() || selection->isRange()) {
         return false;
     }

+    if (selection->elements().empty()) {
+        return false;
+    }
+
     for (const EngravingItem* element: selection->elements()) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
bool NotationActionController::currentIsRest() const
{
INotationInteractionPtr interaction = currentNotationInteraction();
if (!interaction) {
return false;
}
INotationNoteInputPtr noteInput = interaction->noteInput();
if (!noteInput) {
return false;
}
if (noteInput->isNoteInputMode()) {
return noteInput->state().rest();
}
INotationSelectionPtr selection = interaction->selection();
if (!selection) {
return false;
}
if (selection->isNone() || selection->isRange()) {
return false;
}
for (const EngravingItem* element: selection->elements()) {
if (!element->isRest()) {
return false;
}
}
return true;
}
bool NotationActionController::currentIsRest() const
{
INotationInteractionPtr interaction = currentNotationInteraction();
if (!interaction) {
return false;
}
INotationNoteInputPtr noteInput = interaction->noteInput();
if (!noteInput) {
return false;
}
if (noteInput->isNoteInputMode()) {
return noteInput->state().rest();
}
INotationSelectionPtr selection = interaction->selection();
if (!selection) {
return false;
}
if (selection->isNone() || selection->isRange()) {
return false;
}
if (selection->elements().empty()) {
return false;
}
for (const EngravingItem* element: selection->elements()) {
if (!element->isRest()) {
return false;
}
}
return true;
}
🤖 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/internal/notationactioncontroller.cpp` around lines 1018 -
1050, currentIsRest() incorrectly returns true for an empty non-range selection.
After obtaining the selection elements in currentIsRest(), add an early return
false when the collection is empty, before iterating and checking isRest(),
matching the guard used by currentVoice().


AccidentalType NotationActionController::currentAccidentalType() const
{
INotationInteractionPtr interaction = currentNotationInteraction();
Expand All @@ -1025,6 +1059,119 @@ AccidentalType NotationActionController::currentAccidentalType() const
return interaction->noteInput()->state().accidentalType();
}

std::set<SymbolId> NotationActionController::currentArticulations() const
{
INotationInteractionPtr interaction = currentNotationInteraction();
if (!interaction) {
return {};
}

INotationNoteInputPtr noteInput = interaction->noteInput();
if (!noteInput) {
return {};
}

if (noteInput->isNoteInputMode()) {
return mu::engraving::splitArticulations(noteInput->state().articulationIds());
}

INotationSelectionPtr selection = interaction->selection();
if (!selection) {
return {};
}

if (selection->isNone()) {
return {};
}

auto chordArticulations = [](const Chord* chord) {
std::set<SymbolId> result;
for (Articulation* articulation: chord->articulations()) {
result.insert(articulation->symId());
}

result = mu::engraving::flipArticulations(result, mu::engraving::PlacementV::ABOVE);
return mu::engraving::splitArticulations(result);
};

std::set<SymbolId> result;
bool isFirstNote = true;
for (const EngravingItem* element: selection->elements()) {
if (!element->isNote()) {
continue;
}

const Note* note = toNote(element);
if (isFirstNote) {
result = chordArticulations(note->chord());
isFirstNote = false;
} else {
std::set<SymbolId> currentNoteArticulations = chordArticulations(note->chord());
for (auto it = result.begin(); it != result.end();) {
if (std::find(currentNoteArticulations.begin(), currentNoteArticulations.end(),
*it) == currentNoteArticulations.end()) {
it = result.erase(it);
} else {
++it;
}
}
}
}

return result;
}

voice_idx_t NotationActionController::currentVoice() const
{
constexpr voice_idx_t INVALID_VOICE = muse::nidx;

INotationInteractionPtr interaction = currentNotationInteraction();
if (!interaction) {
return INVALID_VOICE;
}

INotationNoteInputPtr noteInput = interaction->noteInput();
if (!noteInput) {
return INVALID_VOICE;
}

if (noteInput->isNoteInputMode()) {
return noteInput->state().voice();
}

INotationSelectionPtr selection = interaction->selection();
if (!selection) {
return INVALID_VOICE;
}

if (selection->isNone()) {
return INVALID_VOICE;
}

const std::vector<EngravingItem*>& selectedElements = selection->elements();
if (selectedElements.empty()) {
return INVALID_VOICE;
}

voice_idx_t voice = INVALID_VOICE;
for (const EngravingItem* element : selectedElements) {
if (element->hasVoiceAssignmentProperties()) {
VoiceAssignment voiceAssignment = element->getProperty(Pid::VOICE_ASSIGNMENT).value<VoiceAssignment>();
if (voiceAssignment == VoiceAssignment::ALL_VOICE_IN_INSTRUMENT || voiceAssignment == VoiceAssignment::ALL_VOICE_IN_STAFF) {
return INVALID_VOICE;
}
}
voice_idx_t elementVoice = element->voice();
if (elementVoice != voice && voice != INVALID_VOICE) {
return INVALID_VOICE;
}

voice = elementVoice;
}

return voice;
}

muse::async::Notification NotationActionController::selectionChanged() const
{
return m_selectionChanged;
Expand Down
3 changes: 3 additions & 0 deletions src/notationscene/internal/notationactioncontroller.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,10 @@ class NotationActionController : public INotationCommandsController, public muse
NoteInputMethod noteInputMethod() const override;
DurationType currentDurationType() const override;
int currentDotCount() const override;
bool currentIsRest() const override;
AccidentalType currentAccidentalType() const override;
std::set<SymbolId> currentArticulations() const override;
voice_idx_t currentVoice() const override;

muse::async::Notification currentNotationChanged() const;

Expand Down
72 changes: 71 additions & 1 deletion src/notationscene/internal/notationcommandsregister.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,77 @@ static const std::vector<CommandInfo> s_commandInfos = {
TranslatableString("notation", "Add laissez vibrer"),
InputSchema(),
Decoration(IconCode::Code::NOTE_LV)
}
},
CommandInfo{
ADD_MARCATO_COMMAND,
TranslatableString("notation", "Marcato"),
TranslatableString("notation", "Add articulation: marcato"),
InputSchema(),
Decoration(IconCode::Code::MARCATO)
},
CommandInfo{
ADD_SFORZATO_COMMAND,
TranslatableString("notation", "Accent"),
TranslatableString("notation", "Add articulation: accent"),
InputSchema(),
Decoration(IconCode::Code::ACCENT)
},
CommandInfo{
ADD_TENUTO_COMMAND,
TranslatableString("notation", "Tenuto"),
TranslatableString("notation", "Add articulation: tenuto"),
InputSchema(),
Decoration(IconCode::Code::TENUTO)
},
CommandInfo{
ADD_STACCATO_COMMAND,
TranslatableString("notation", "Staccato"),
TranslatableString("notation", "Add articulation: staccato"),
InputSchema(),
Decoration(IconCode::Code::STACCATO)
},
CommandInfo{
USE_VOICE_1_COMMAND,
TranslatableString("notation", "Voice 1"),
TranslatableString("notation", "Use voice 1"),
InputSchema(),
Decoration(IconCode::Code::VOICE_1)
},
CommandInfo{
USE_VOICE_2_COMMAND,
TranslatableString("notation", "Voice 2"),
TranslatableString("notation", "Use voice 2"),
InputSchema(),
Decoration(IconCode::Code::VOICE_2)
},
CommandInfo{
USE_VOICE_3_COMMAND,
TranslatableString("notation", "Voice 3"),
TranslatableString("notation", "Use voice 3"),
InputSchema(),
Decoration(IconCode::Code::VOICE_3)
},
CommandInfo{
USE_VOICE_4_COMMAND,
TranslatableString("notation", "Voice 4"),
TranslatableString("notation", "Use voice 4"),
InputSchema(),
Decoration(IconCode::Code::VOICE_4)
},
CommandInfo{
FLIP_COMMAND,
TranslatableString("notation", "Flip direction"),
TranslatableString("notation", "Flip direction"),
InputSchema(),
Decoration(IconCode::Code::NOTE_FLIP)
},
CommandInfo{
FLIP_HORIZONTALLY_COMMAND,
TranslatableString("notation", "Flip horizontally"),
TranslatableString("notation", "Flip horizontally"),
InputSchema(),
Decoration()
},
};

std::string NotationCommandsRegister::moduleName() const
Expand Down
Loading
Loading