From 84ba3c58c20cbb3c197ff4e684c52f03dea977c8 Mon Sep 17 00:00:00 2001
From: Marcus Tillmanns <marcus.tillmanns@qt.io>
Date: Wed, 20 Sep 2023 09:06:13 +0200
Subject: [PATCH] Utils: Replace BaseAspect::pushUndo

Moving the logic into Undoable::set() reduces allocations.

Change-Id: Ib4a10d1e41da8d42d66bfe9ef24a667c5bfd8c0c
Reviewed-by: hjk <hjk@qt.io>
---
 src/libs/utils/aspects.cpp | 25 ++++++-------------------
 src/libs/utils/aspects.h   | 10 ++++++----
 2 files changed, 12 insertions(+), 23 deletions(-)

diff --git a/src/libs/utils/aspects.cpp b/src/libs/utils/aspects.cpp
index ba6c7b69cd9..c31d7b615e2 100644
--- a/src/libs/utils/aspects.cpp
+++ b/src/libs/utils/aspects.cpp
@@ -322,19 +322,6 @@ QUndoStack *BaseAspect::undoStack() const
     return d->m_undoStack;
 }
 
-void BaseAspect::pushUndo(QUndoCommand *cmd)
-{
-    if (!cmd)
-        return;
-
-    if (d->m_undoStack)
-        d->m_undoStack->push(cmd);
-    else {
-        cmd->redo();
-        delete cmd;
-    }
-}
-
 bool BaseAspect::isEnabled() const
 {
     return d->m_enabled;
@@ -1739,7 +1726,7 @@ void BoolAspect::addToLayoutHelper(Layouting::LayoutItem &parent, QAbstractButto
     }
 
     connect(button, &QAbstractButton::clicked, this, [button, this] {
-        pushUndo(d->m_undoable.set(button->isChecked()));
+        d->m_undoable.set(undoStack(), button->isChecked());
     });
 
     connect(&d->m_undoable.m_signal, &UndoSignaller::changed, button, [button, this] {
@@ -1780,7 +1767,7 @@ std::function<void (QObject *)> BoolAspect::groupChecker()
         groupBox->setChecked(value());
 
         connect(groupBox, &QGroupBox::clicked, this, [groupBox, this] {
-            pushUndo(d->m_undoable.set(groupBox->isChecked()));
+            d->m_undoable.set(undoStack(), groupBox->isChecked());
         });
 
         connect(&d->m_undoable.m_signal, &UndoSignaller::changed, groupBox, [groupBox, this] {
@@ -2475,7 +2462,7 @@ void FilePathListAspect::addToLayout(LayoutItem &parent)
     PathListEditor *editor = new PathListEditor;
     editor->setPathList(value());
     connect(editor, &PathListEditor::changed, this, [this, editor] {
-        pushUndo(d->undoable.set(editor->pathList()));
+        d->undoable.set(undoStack(), editor->pathList());
     });
     connect(&d->undoable.m_signal, &UndoSignaller::changed, this, [this, editor] {
         if (editor->pathList() != d->undoable.get())
@@ -3101,7 +3088,7 @@ QList<std::shared_ptr<BaseAspect>> AspectList::volatileItems() const
 std::shared_ptr<BaseAspect> AspectList::addItem(const std::shared_ptr<BaseAspect> &item)
 {
     if (undoStack())
-        pushUndo(new AddItemCommand(this, item));
+        undoStack()->push(new AddItemCommand(this, item));
     else
         return actualAddItem(item);
 
@@ -3121,7 +3108,7 @@ void AspectList::actualRemoveItem(const std::shared_ptr<BaseAspect> &item)
 void AspectList::removeItem(const std::shared_ptr<BaseAspect> &item)
 {
     if (undoStack())
-        pushUndo(new RemoveItemCommand(this, item));
+        undoStack()->push(new RemoveItemCommand(this, item));
     else
         actualRemoveItem(item);
 }
@@ -3343,7 +3330,7 @@ void StringSelectionAspect::addToLayout(Layouting::LayoutItem &parent)
         if (newValue.isEmpty())
             return;
 
-        pushUndo(m_undoable.set(newValue));
+        m_undoable.set(undoStack(), newValue);
         bufferToGui();
     });
 
diff --git a/src/libs/utils/aspects.h b/src/libs/utils/aspects.h
index c83dbe23405..cd589aa75fe 100644
--- a/src/libs/utils/aspects.h
+++ b/src/libs/utils/aspects.h
@@ -93,7 +93,6 @@ public:
 
     virtual void setUndoStack(QUndoStack *undoStack);
     QUndoStack *undoStack() const;
-    void pushUndo(QUndoCommand *cmd);
 
     bool isEnabled() const;
     void setEnabled(bool enabled);
@@ -978,12 +977,15 @@ public:
         T m_newValue;
     };
 
-    QUndoCommand *set(const T &value)
+    void set(QUndoStack *stack, const T &value)
     {
         if (m_value == value)
-            return nullptr;
+            return;
 
-        return new UndoCmd(this, m_value, value);
+        if (stack)
+            stack->push(new UndoCmd(this, m_value, value));
+        else
+            setInternal(value);
     }
 
     void setSilently(const T &value) { m_value = value; }