From 886ca55b5a96305e2f4b9b049dfb3c5109356812 Mon Sep 17 00:00:00 2001
From: Marcus Tillmanns <marcus.tillmanns@qt.io>
Date: Tue, 6 Jun 2023 08:23:09 +0200
Subject: [PATCH] Terminal: Fix warnings about re-registered action

Especially on Linux the pointer value of the Terminal might be reused,
leading to warnings about actions being registered for the same context.

Cleaning up the registration fixes this.

Change-Id: Ie1d53bf79581e9f98576e7a4e70420ec63da0f86
Reviewed-by: Eike Ziller <eike.ziller@qt.io>
---
 src/plugins/terminal/terminalwidget.cpp | 49 +++++++++++++++++--------
 src/plugins/terminal/terminalwidget.h   | 15 ++++----
 2 files changed, 41 insertions(+), 23 deletions(-)

diff --git a/src/plugins/terminal/terminalwidget.cpp b/src/plugins/terminal/terminalwidget.cpp
index 12a0521857c..b08038ee29d 100644
--- a/src/plugins/terminal/terminalwidget.cpp
+++ b/src/plugins/terminal/terminalwidget.cpp
@@ -252,23 +252,40 @@ void TerminalWidget::setupColors()
     update();
 }
 
+static RegisteredAction registerAction(Id commandId, const Context &context)
+{
+    QAction *action = new QAction;
+    ActionManager::registerAction(action, commandId, context);
+
+    return RegisteredAction(action, [commandId](QAction *a) {
+        ActionManager::unregisterAction(a, commandId);
+        delete a;
+    });
+}
+
 void TerminalWidget::setupActions()
 {
-    ActionManager::registerAction(&m_copy, Constants::COPY, m_context);
-    ActionManager::registerAction(&m_paste, Constants::PASTE, m_context);
-    ActionManager::registerAction(&m_close, Core::Constants::CLOSE, m_context);
-    ActionManager::registerAction(&m_clearTerminal, Constants::CLEAR_TERMINAL, m_context);
-    ActionManager::registerAction(&m_clearSelection, Constants::CLEARSELECTION, m_context);
-    ActionManager::registerAction(&m_moveCursorWordLeft, Constants::MOVECURSORWORDLEFT, m_context);
-    ActionManager::registerAction(&m_moveCursorWordRight, Constants::MOVECURSORWORDRIGHT, m_context);
+    m_copy = registerAction(Constants::COPY, m_context);
+    m_paste = registerAction(Constants::PASTE, m_context);
+    m_close = registerAction(Core::Constants::CLOSE, m_context);
+    m_clearTerminal = registerAction(Constants::CLEAR_TERMINAL, m_context);
+    m_clearSelection = registerAction(Constants::CLEARSELECTION, m_context);
+    m_moveCursorWordLeft = registerAction(Constants::MOVECURSORWORDLEFT, m_context);
+    m_moveCursorWordRight = registerAction(Constants::MOVECURSORWORDRIGHT, m_context);
 
-    connect(&m_copy, &QAction::triggered, this, &TerminalWidget::copyToClipboard);
-    connect(&m_paste, &QAction::triggered, this, &TerminalWidget::pasteFromClipboard);
-    connect(&m_close, &QAction::triggered, this, &TerminalWidget::closeTerminal);
-    connect(&m_clearTerminal, &QAction::triggered, this, &TerminalWidget::clearContents);
-    connect(&m_clearSelection, &QAction::triggered, this, &TerminalWidget::clearSelection);
-    connect(&m_moveCursorWordLeft, &QAction::triggered, this, &TerminalWidget::moveCursorWordLeft);
-    connect(&m_moveCursorWordRight, &QAction::triggered, this, &TerminalWidget::moveCursorWordRight);
+    connect(m_copy.get(), &QAction::triggered, this, &TerminalWidget::copyToClipboard);
+    connect(m_paste.get(), &QAction::triggered, this, &TerminalWidget::pasteFromClipboard);
+    connect(m_close.get(), &QAction::triggered, this, &TerminalWidget::closeTerminal);
+    connect(m_clearTerminal.get(), &QAction::triggered, this, &TerminalWidget::clearContents);
+    connect(m_clearSelection.get(), &QAction::triggered, this, &TerminalWidget::clearSelection);
+    connect(m_moveCursorWordLeft.get(),
+            &QAction::triggered,
+            this,
+            &TerminalWidget::moveCursorWordLeft);
+    connect(m_moveCursorWordRight.get(),
+            &QAction::triggered,
+            this,
+            &TerminalWidget::moveCursorWordRight);
 
     m_exit = unlockGlobalAction(Core::Constants::EXIT, m_context);
     m_options = unlockGlobalAction(Core::Constants::OPTIONS, m_context);
@@ -401,7 +418,7 @@ void TerminalWidget::updateCopyState()
     if (!hasFocus())
         return;
 
-    m_copy.setEnabled(m_selection.has_value());
+    m_copy->setEnabled(m_selection.has_value());
 }
 
 void TerminalWidget::setFont(const QFont &font)
@@ -1095,7 +1112,7 @@ void TerminalWidget::keyPressEvent(QKeyEvent *event)
         }
 
         if (m_selection)
-            m_clearSelection.trigger();
+            m_clearSelection->trigger();
         else {
             QAction *returnAction = ActionManager::command(Core::Constants::S_RETURNTOEDITOR)
                                         ->actionForContext(Core::Constants::C_GLOBAL);
diff --git a/src/plugins/terminal/terminalwidget.h b/src/plugins/terminal/terminalwidget.h
index 4b82e4355a1..78e50db7ffc 100644
--- a/src/plugins/terminal/terminalwidget.h
+++ b/src/plugins/terminal/terminalwidget.h
@@ -25,6 +25,7 @@
 namespace Terminal {
 
 using UnlockedGlobalAction = std::unique_ptr<QAction, std::function<void(QAction *)>>;
+using RegisteredAction = std::unique_ptr<QAction, std::function<void(QAction *)>>;
 
 class TerminalWidget : public QAbstractScrollArea
 {
@@ -239,13 +240,13 @@ private:
     Aggregation::Aggregate *m_aggregate{nullptr};
     SearchHit m_lastSelectedHit{};
 
-    QAction m_copy;
-    QAction m_paste;
-    QAction m_clearSelection;
-    QAction m_clearTerminal;
-    QAction m_moveCursorWordLeft;
-    QAction m_moveCursorWordRight;
-    QAction m_close;
+    RegisteredAction m_copy;
+    RegisteredAction m_paste;
+    RegisteredAction m_clearSelection;
+    RegisteredAction m_clearTerminal;
+    RegisteredAction m_moveCursorWordLeft;
+    RegisteredAction m_moveCursorWordRight;
+    RegisteredAction m_close;
 
     UnlockedGlobalAction m_findInDocument;
     UnlockedGlobalAction m_exit;