Merge pull request #24744 from rpatters1/rpatters1/undo-history-phase-2

Implement Undo/Redo naming and history

src/appshell/view/appmenumodel.cpp

@@ -119,6 +119,17 @@ void AppMenuModel::setupConnections()
         MenuItem& pluginsItem = findMenu("menu-plugins");
         pluginsItem.setSubitems(makePluginsMenuSubitems());
     });
+
+    globalContext()->currentNotationChanged().onNotify(this, [this]() {
+        auto stack = undoStack();
+        if (stack) {
+            stack->stackChanged().onNotify(this, [this]() {
+                updateUndoRedoItems();
+            });
+        }
+
+        updateUndoRedoItems();
+    });
 }
 
 MenuItem* AppMenuModel::makeMenuItem(const ActionCode& actionCode, MenuItemRole menuRole)
@@ -169,6 +180,7 @@ MenuItem* AppMenuModel::makeEditMenu()
     MenuItemList editItems {
         makeMenuItem("undo"),
         makeMenuItem("redo"),
+        makeMenuItem("undo-history"),
         makeSeparator(),
         makeMenuItem("notation-cut"),
         makeMenuItem("notation-copy"),
@@ -188,6 +200,29 @@ MenuItem* AppMenuModel::makeEditMenu()
     return makeMenu(TranslatableString("appshell/menu/edit", "&Edit"), editItems, "menu-edit");
 }
 
+mu::notation::INotationUndoStackPtr AppMenuModel::undoStack() const
+{
+    mu::notation::INotationPtr notation = globalContext()->currentNotation();
+    return notation ? notation->undoStack() : nullptr;
+}
+
+void AppMenuModel::updateUndoRedoItems()
+{
+    auto stack = undoStack();
+
+    MenuItem& undoItem = findItem(ActionCode("undo"));
+    const TranslatableString undoActionName = stack ? stack->topMostUndoActionName() : TranslatableString();
+    undoItem.setTitle(undoActionName.isEmpty()
+                      ? TranslatableString("action", "Undo")
+                      : TranslatableString("action", "Undo ‘%1’").arg(undoActionName));
+
+    MenuItem& redoItem = findItem(ActionCode("redo"));
+    const TranslatableString redoActionName = stack ? stack->topMostRedoActionName() : TranslatableString();
+    redoItem.setTitle(redoActionName.isEmpty()
+                      ? TranslatableString("action", "Redo")
+                      : TranslatableString("action", "Redo ‘%1’").arg(redoActionName));
+}
+
 MenuItem* AppMenuModel::makeViewMenu()
 {
     MenuItemList viewItems {

src/appshell/view/appmenumodel.h

@@ -22,6 +22,7 @@
 #ifndef MU_APPSHELL_APPMENUMODEL_H
 #define MU_APPSHELL_APPMENUMODEL_H
 
+#include "context/iglobalcontext.h"
 #include "uicomponents/view/abstractmenumodel.h"
 
 #include "modularity/ioc.h"
@@ -58,6 +59,7 @@ class AppMenuModel : public muse::uicomponents::AbstractMenuModel
     muse::Inject<muse::update::IUpdateConfiguration> updateConfiguration = { this };
     muse::Inject<muse::IGlobalConfiguration> globalConfiguration = { this };
     muse::Inject<project::IProjectConfiguration> projectConfiguration = { this };
+    muse::Inject<mu::context::IGlobalContext> globalContext = { this };
 
 public:
     explicit AppMenuModel(QObject* parent = nullptr);
@@ -96,6 +98,9 @@ class AppMenuModel : public muse::uicomponents::AbstractMenuModel
     muse::uicomponents::MenuItemList makeWorkspacesItems();
     muse::uicomponents::MenuItemList makeShowItems();
     muse::uicomponents::MenuItemList makePluginsItems();
+
+    mu::notation::INotationUndoStackPtr undoStack() const;
+    void updateUndoRedoItems();
 };
 }
 

src/braille/internal/notationbraille.cpp

@@ -699,7 +699,7 @@ bool NotationBraille::addTie()
         return false;
     }
 
-    score()->startCmd();
+    score()->startCmd(TranslatableString("undoableAction", "Add tie"));
     Note* note = toNote(currentEngravingItem());
 
     Tie* tie = Factory::createTie(score()->dummy());
@@ -725,7 +725,7 @@ bool NotationBraille::addSlur()
             ChordRest* firstChordRest = toChordRest(note1->parent());
             ChordRest* secondChordRest = toChordRest(note2->parent());
 
-            score()->startCmd();
+            score()->startCmd(TranslatableString("undoableAction", "Add slur"));
 
             Slur* slur = Factory::createSlur(firstChordRest->measure()->system());
             slur->setScore(firstChordRest->score());
@@ -770,7 +770,7 @@ bool NotationBraille::addLongSlur()
             ChordRest* firstChordRest = toChordRest(note1->parent());
             ChordRest* secondChordRest = toChordRest(note2->parent());
 
-            score()->startCmd();
+            score()->startCmd(TranslatableString("undoableAction", "Add long slur"));
 
             Slur* slur = Factory::createSlur(firstChordRest->measure()->system());
             slur->setScore(firstChordRest->score());

src/engraving/api/v1/qmlpluginapi.cpp

@@ -278,7 +278,7 @@ apiv1::Score* PluginAPI::newScore(const QString& /*name*/, const QString& part,
 
     qApp->processEvents();
     Q_ASSERT(currentScore() == score);
-    score->startCmd();
+    score->startCmd(TranslatableString("undoableAction", "New score"));
     return wrap<Score>(score, Ownership::SCORE);
 }
 

src/engraving/api/v1/score.cpp

@@ -237,13 +237,17 @@ QQmlListProperty<Staff> Score::staves()
 //   Score::startCmd
 //---------------------------------------------------------
 
-void Score::startCmd()
+void Score::startCmd(const QString& qActionName)
 {
     IF_ASSERT_FAILED(undoStack()) {
         return;
     }
 
-    undoStack()->prepareChanges();
+    muse::TranslatableString actionName = qActionName.isEmpty()
+                                          ? TranslatableString("undoableAction", "Plugin edit")
+                                          : TranslatableString::untranslatable(qActionName);
+
+    undoStack()->prepareChanges(actionName);
 }
 
 void Score::endCmd(bool rollback)

src/engraving/api/v1/score.h

@@ -225,8 +225,10 @@ class Score : public apiv1::ScoreElement, public muse::Injectable
      * a corresponding endCmd() call. Should be used at
      * least once by "dock" type plugins in case they
      * modify the score.
+     * \param qActionName - Optional action name that appears in Undo/Redo
+     * menus, palettes, and lists.
      */
-    Q_INVOKABLE void startCmd();
+    Q_INVOKABLE void startCmd(const QString& qActionName = {});
     /**
      * For "dock" type plugins: to be used after score
      * modifications to make them undoable.

src/engraving/api/v1/selection.cpp

@@ -136,7 +136,7 @@ bool Selection::selectRange(int startTick, int endTick, int startStaff, int endS
         return false;
     }
 
-    if (segEnd && _select->score()->undoStack()->active()) {
+    if (segEnd && _select->score()->undoStack()->hasActiveCommand()) {
         _select->setRangeTicks(segStart->tick(), segEnd->tick(), startStaff, endStaff);
     } else {
         _select->setRange(segStart, segEnd, startStaff, endStaff);

src/engraving/devtools/corruptscoredevtoolsmodel.cpp

@@ -40,7 +40,8 @@ void CorruptScoreDevToolsModel::corruptOpenScore()
 
     mu::notation::INotationPtr notation = project->masterNotation()->notation();
 
-    notation->undoStack()->prepareChanges();
+    //: "Corrupt" is used as a verb here, i.e. "Make the current score corrupted" (for testing purposes).
+    notation->undoStack()->prepareChanges(TranslatableString("undoableAction", "Corrupt score"));
 
     for (engraving::System* system : notation->elements()->msScore()->systems()) {
         for (engraving::MeasureBase* measureBase : system->measures()) {

src/engraving/dom/cmd.cpp

@@ -96,7 +96,7 @@ static UndoMacro::ChangesInfo changesInfo(const UndoStack* stack)
         return empty;
     }
 
-    const UndoMacro* actualMacro = stack->current();
+    const UndoMacro* actualMacro = stack->activeCommand();
 
     if (!actualMacro) {
         actualMacro = stack->last();
@@ -319,9 +319,9 @@ void CmdState::setUpdateMode(UpdateMode m)
 ///   and starting a user-visible undo.
 //---------------------------------------------------------
 
-void Score::startCmd()
+void Score::startCmd(const TranslatableString& actionName)
 {
-    if (undoStack()->locked()) {
+    if (undoStack()->isLocked()) {
         return;
     }
 
@@ -335,11 +335,11 @@ void Score::startCmd()
 
     // Start collecting low-level undo operations for a
     // user-visible undo action.
-    if (undoStack()->active()) {
+    if (undoStack()->hasActiveCommand()) {
         LOGD("Score::startCmd(): cmd already active");
         return;
     }
-    undoStack()->beginMacro(this);
+    undoStack()->beginMacro(this, actionName);
 }
 
 //---------------------------------------------------------
@@ -396,11 +396,11 @@ void Score::undoRedo(bool undo, EditData* ed)
 
 void Score::endCmd(bool rollback, bool layoutAllParts)
 {
-    if (undoStack()->locked()) {
+    if (undoStack()->isLocked()) {
         return;
     }
 
-    if (!undoStack()->active()) {
+    if (!undoStack()->hasActiveCommand()) {
         LOGW() << "no command active";
         update();
         return;
@@ -411,25 +411,25 @@ void Score::endCmd(bool rollback, bool layoutAllParts)
     }
 
     if (rollback) {
-        undoStack()->current()->unwind();
+        undoStack()->activeCommand()->unwind();
     }
 
     update(false, layoutAllParts);
 
     ScoreChangesRange range = changesRange();
 
-    LOGD() << "Undo stack current macro child count: " << undoStack()->current()->childCount();
+    LOGD() << "Undo stack current macro child count: " << undoStack()->activeCommand()->childCount();
 
-    const bool noUndo = undoStack()->current()->empty(); // nothing to undo?
-    undoStack()->endMacro(noUndo);
+    const bool isCurrentCommandEmpty = undoStack()->activeCommand()->empty(); // nothing to undo?
+    undoStack()->endMacro(isCurrentCommandEmpty);
 
     if (dirty()) {
         masterScore()->setPlaylistDirty(); // TODO: flag individual operations
     }
 
     cmdState().reset();
 
-    if (!rollback) {
+    if (!isCurrentCommandEmpty && !rollback) {
         changesChannel().send(range);
     }
 }
@@ -2790,7 +2790,6 @@ void Score::cmdResetNoteAndRestGroupings()
     staff_idx_t sStaff = selection().staffStart();
     staff_idx_t eStaff = selection().staffEnd();
 
-    startCmd();
     for (staff_idx_t staff = sStaff; staff < eStaff; staff++) {
         track_idx_t sTrack = staff * VOICES;
         track_idx_t eTrack = sTrack + VOICES;
@@ -2800,7 +2799,7 @@ void Score::cmdResetNoteAndRestGroupings()
             }
         }
     }
-    endCmd();
+
     if (noSelection) {
         deselectAll();
     }
@@ -2822,7 +2821,7 @@ void Score::cmdResetAllPositions(bool undoable)
     TRACEFUNC;
 
     if (undoable) {
-        startCmd();
+        startCmd(TranslatableString("undoableAction", "Reset all positions"));
     }
     resetAutoplace();
     if (undoable) {

src/engraving/dom/durationelement.cpp

@@ -122,7 +122,7 @@ Fraction DurationElement::actualTicks() const
 void DurationElement::readAddTuplet(Tuplet* t)
 {
     setTuplet(t);
-    if (!score()->undoStack()->active()) {     // HACK, also added in Undo::AddElement()
+    if (!score()->undoStack()->hasActiveCommand()) { // HACK, also added in Undo::AddElement()
         t->add(this);
     }
 }

src/engraving/dom/edit.cpp

@@ -1421,7 +1421,7 @@ void Score::cmdAddTimeSig(Measure* fm, staff_idx_t staffIdx, TimeSig* ts, bool l
                 for (size_t i = 0; i < nstaves(); ++i) {
                     if (staff(i)->timeSig(tick) && staff(i)->timeSig(tick)->isLocal()) {
                         if (!mScore->rewriteMeasures(mf, ns, i)) {
-                            undoStack()->current()->unwind();
+                            undoStack()->activeCommand()->unwind();
                             return;
                         }
                     }
@@ -1435,7 +1435,7 @@ void Score::cmdAddTimeSig(Measure* fm, staff_idx_t staffIdx, TimeSig* ts, bool l
         // this means, however, that the rewrite cannot depend on the time signatures being in place
         if (mf) {
             if (!mScore->rewriteMeasures(mf, ns, local ? staffIdx : muse::nidx)) {
-                undoStack()->current()->unwind();
+                undoStack()->activeCommand()->unwind();
                 return;
             }
         }
@@ -1527,7 +1527,7 @@ void Score::cmdRemoveTimeSig(TimeSig* ts)
     Fraction ns(pm ? pm->timesig() : Fraction(4, 4));
 
     if (!rScore->rewriteMeasures(rm, ns, muse::nidx)) {
-        undoStack()->current()->unwind();
+        undoStack()->activeCommand()->unwind();
     } else {
         m = tick2measure(tick);           // old m may have been replaced
         // hack: fix measure rest durations for staves with local time signatures
@@ -1876,7 +1876,7 @@ void Score::cmdAddTie(bool addToChord)
         return;
     }
 
-    startCmd();
+    startCmd(TranslatableString("undoableAction", "Add tie"));
     Chord* lastAddedChord = 0;
     for (Note* note : noteList) {
         if (note->tieFor()) {
@@ -2010,7 +2010,11 @@ void Score::cmdToggleTie()
         }
     }
 
-    startCmd();
+    const TranslatableString actionName = canAddTies
+                                          ? TranslatableString("undoableAction", "Add tie")
+                                          : TranslatableString("undoableAction", "Remove tie");
+
+    startCmd(actionName);
 
     if (canAddTies) {
         for (size_t i = 0; i < notes; ++i) {
@@ -3998,7 +4002,7 @@ void Score::cmdEnterRest(const TDuration& d)
         LOGD("cmdEnterRest: track invalid");
         return;
     }
-    startCmd();
+    startCmd(TranslatableString("undoableAction", "Enter rest"));
     enterRest(d);
     endCmd();
 }
@@ -4928,7 +4932,7 @@ bool Score::undoPropertyChanged(EngravingItem* item, Pid propId, const PropertyV
 
     if ((currentPropValue != propValue) || (currentPropFlags != propFlags)) {
         item->setPropertyFlags(propId, propFlags);
-        undoStack()->push1(new ChangeProperty(item, propId, propValue, propFlags));
+        undoStack()->pushWithoutPerforming(new ChangeProperty(item, propId, propValue, propFlags));
         changed = true;
     }
 
@@ -4942,7 +4946,7 @@ bool Score::undoPropertyChanged(EngravingItem* item, Pid propId, const PropertyV
         switch (propertyPropagate) {
         case PropertyPropagation::PROPAGATE:
             if (linkedItem->getProperty(propId) != currentPropValue) {
-                undoStack()->push(new ChangeProperty(linkedItem, propId, currentPropValue, propFlags), nullptr);
+                undoStack()->pushAndPerform(new ChangeProperty(linkedItem, propId, currentPropValue, propFlags), nullptr);
                 changed = true;
             }
             break;
@@ -4960,7 +4964,7 @@ bool Score::undoPropertyChanged(EngravingItem* item, Pid propId, const PropertyV
 void Score::undoPropertyChanged(EngravingObject* e, Pid t, const PropertyValue& st, PropertyFlags ps)
 {
     if (e->getProperty(t) != st) {
-        undoStack()->push1(new ChangeProperty(e, t, st, ps));
+        undoStack()->pushWithoutPerforming(new ChangeProperty(e, t, st, ps));
     }
 }
 
@@ -5098,7 +5102,7 @@ void Score::undoChangePitch(Note* note, int pitch, int tpc1, int tpc2)
 {
     for (EngravingObject* e : note->linkList()) {
         Note* n = toNote(e);
-        undoStack()->push(new ChangePitch(n, pitch, tpc1, tpc2), 0);
+        undoStack()->pushAndPerform(new ChangePitch(n, pitch, tpc1, tpc2), 0);
     }
 }
 

src/engraving/dom/engravingobject.cpp

@@ -528,7 +528,7 @@ void EngravingObject::undoChangeProperty(Pid id, const PropertyValue& v, Propert
 void EngravingObject::undoPushProperty(Pid id)
 {
     PropertyValue val = getProperty(id);
-    score()->undoStack()->push1(new ChangeProperty(this, id, val));
+    score()->undoStack()->pushWithoutPerforming(new ChangeProperty(this, id, val));
 }
 
 //---------------------------------------------------------