From 351f0301def9524804bc1ba5736b652cbf7c1a83 Mon Sep 17 00:00:00 2001 From: Eike Ziller Date: Tue, 31 May 2022 09:43:56 +0200 Subject: [PATCH] Un-virtualize Command and remove an inheritance This was a left-over from times where we had a QShortcut variant of Command, which was removed in Qt Creator 3.2. Change-Id: I62a728f3af352c61bc137461232fc1ed8debccf2 Reviewed-by: hjk Reviewed-by: Qt CI Bot --- .../actionmanager/actionmanager.cpp | 63 +++---- .../actionmanager/actionmanager_p.h | 13 +- .../coreplugin/actionmanager/command.cpp | 177 ++++++++++-------- .../coreplugin/actionmanager/command.h | 61 +++--- .../coreplugin/actionmanager/command_p.h | 40 +--- .../coreplugin/actionmanager/commandsfile.cpp | 2 +- .../coreplugin/dialogs/shortcutsettings.cpp | 8 +- src/plugins/coreplugin/mainwindow.cpp | 2 + 8 files changed, 180 insertions(+), 186 deletions(-) diff --git a/src/plugins/coreplugin/actionmanager/actionmanager.cpp b/src/plugins/coreplugin/actionmanager/actionmanager.cpp index 3d1b5e696d8..b328bc3afd4 100644 --- a/src/plugins/coreplugin/actionmanager/actionmanager.cpp +++ b/src/plugins/coreplugin/actionmanager/actionmanager.cpp @@ -243,13 +243,13 @@ ActionContainer *ActionManager::createTouchBar(Id id, const QIcon &icon, const Q */ Command *ActionManager::registerAction(QAction *action, Id id, const Context &context, bool scriptable) { - Action *a = d->overridableAction(id); - if (a) { - a->addOverrideAction(action, context, scriptable); + Command *cmd = d->overridableAction(id); + if (cmd) { + cmd->d->addOverrideAction(action, context, scriptable); emit m_instance->commandListChanged(); emit m_instance->commandAdded(id); } - return a; + return cmd; } /*! @@ -301,11 +301,7 @@ ActionContainer *ActionManager::actionContainer(Id id) */ QList ActionManager::commands() { - // transform list of Action into list of Command - QList result; - for (Command *cmd : qAsConst(d->m_idCmdMap)) - result << cmd; - return result; + return d->m_idCmdMap.values(); } /*! @@ -318,21 +314,21 @@ QList ActionManager::commands() */ void ActionManager::unregisterAction(QAction *action, Id id) { - Action *a = d->m_idCmdMap.value(id, nullptr); - if (!a) { + Command *cmd = d->m_idCmdMap.value(id, nullptr); + if (!cmd) { qWarning() << "unregisterAction: id" << id.name() << "is registered with a different command type."; return; } - a->removeOverrideAction(action); - if (a->isEmpty()) { + cmd->d->removeOverrideAction(action); + if (cmd->d->isEmpty()) { // clean up - ActionManagerPrivate::saveSettings(a); - ICore::mainWindow()->removeAction(a->action()); + ActionManagerPrivate::saveSettings(cmd); + ICore::mainWindow()->removeAction(cmd->action()); // ActionContainers listen to the commands' destroyed signals - delete a->action(); + delete cmd->action(); d->m_idCmdMap.remove(id); - delete a; + delete cmd; } emit m_instance->commandListChanged(); } @@ -421,7 +417,7 @@ void ActionManagerPrivate::setContext(const Context &context) m_context = context; const IdCmdMap::const_iterator cmdcend = m_idCmdMap.constEnd(); for (IdCmdMap::const_iterator it = m_idCmdMap.constBegin(); it != cmdcend; ++it) - it.value()->setCurrentContext(m_context); + it.value()->d->setCurrentContext(m_context); } bool ActionManagerPrivate::hasContext(const Context &context) const @@ -463,26 +459,29 @@ void ActionManagerPrivate::showShortcutPopup(const QString &shortcut) Utils::FadingIndicator::showText(window, shortcut); } -Action *ActionManagerPrivate::overridableAction(Id id) +Command *ActionManagerPrivate::overridableAction(Id id) { - Action *a = m_idCmdMap.value(id, nullptr); - if (!a) { - a = new Action(id); - m_idCmdMap.insert(id, a); - readUserSettings(id, a); - ICore::mainWindow()->addAction(a->action()); - a->action()->setObjectName(id.toString()); - a->action()->setShortcutContext(Qt::ApplicationShortcut); - a->setCurrentContext(m_context); + Command *cmd = m_idCmdMap.value(id, nullptr); + if (!cmd) { + cmd = new Command(id); + m_idCmdMap.insert(id, cmd); + readUserSettings(id, cmd); + ICore::mainWindow()->addAction(cmd->action()); + cmd->action()->setObjectName(id.toString()); + cmd->action()->setShortcutContext(Qt::ApplicationShortcut); + cmd->d->setCurrentContext(m_context); if (ActionManager::isPresentationModeEnabled()) - connect(a->action(), &QAction::triggered, this, &ActionManagerPrivate::actionTriggered); + connect(cmd->action(), + &QAction::triggered, + this, + &ActionManagerPrivate::actionTriggered); } - return a; + return cmd; } -void ActionManagerPrivate::readUserSettings(Id id, Action *cmd) +void ActionManagerPrivate::readUserSettings(Id id, Command *cmd) { QSettings *settings = ICore::settings(); settings->beginGroup(kKeyboardSettingsKeyV2); @@ -499,7 +498,7 @@ void ActionManagerPrivate::readUserSettings(Id id, Action *cmd) settings->endGroup(); } -void ActionManagerPrivate::saveSettings(Action *cmd) +void ActionManagerPrivate::saveSettings(Command *cmd) { const QString id = cmd->id().toString(); const QString settingsKey = QLatin1String(kKeyboardSettingsKeyV2) + '/' + id; diff --git a/src/plugins/coreplugin/actionmanager/actionmanager_p.h b/src/plugins/coreplugin/actionmanager/actionmanager_p.h index 9fdc96aef1a..9226b0380ad 100644 --- a/src/plugins/coreplugin/actionmanager/actionmanager_p.h +++ b/src/plugins/coreplugin/actionmanager/actionmanager_p.h @@ -25,8 +25,6 @@ #pragma once -#include -#include #include #include @@ -36,9 +34,10 @@ namespace Core { +class Command; + namespace Internal { -class Action; class ActionContainerPrivate; class ActionManagerPrivate : public QObject @@ -46,7 +45,7 @@ class ActionManagerPrivate : public QObject Q_OBJECT public: - using IdCmdMap = QHash; + using IdCmdMap = QHash; using IdContainerMap = QHash; ~ActionManagerPrivate() override; @@ -55,13 +54,13 @@ public: bool hasContext(int context) const; void saveSettings(); - static void saveSettings(Action *cmd); + static void saveSettings(Command *cmd); static void showShortcutPopup(const QString &shortcut); bool hasContext(const Context &context) const; - Action *overridableAction(Utils::Id id); + Command *overridableAction(Utils::Id id); - static void readUserSettings(Utils::Id id, Action *cmd); + static void readUserSettings(Utils::Id id, Command *cmd); void containerDestroyed(); void actionTriggered(); diff --git a/src/plugins/coreplugin/actionmanager/command.cpp b/src/plugins/coreplugin/actionmanager/command.cpp index d41637eaba9..fe2b1c31e95 100644 --- a/src/plugins/coreplugin/actionmanager/command.cpp +++ b/src/plugins/coreplugin/actionmanager/command.cpp @@ -23,6 +23,7 @@ ** ****************************************************************************/ +#include "command.h" #include "command_p.h" #include @@ -260,82 +261,92 @@ using namespace Utils; namespace Core { -namespace Internal { -Action::Action(Id id) - : m_attributes({}), - m_id(id), - m_action(new Utils::ProxyAction(this)) +Command::Command(Utils::Id id) + : d(new Internal::CommandPrivate(this)) +{ + d->m_id = id; +} + +Command::~Command() +{ + delete d; +} + +Internal::CommandPrivate::CommandPrivate(Command *parent) + : m_q(parent) + , m_attributes({}) + , m_action(new Utils::ProxyAction(this)) { m_action->setShortcutVisibleInToolTip(true); - connect(m_action, &QAction::changed, this, &Action::updateActiveState); + connect(m_action, &QAction::changed, this, &CommandPrivate::updateActiveState); } -Id Action::id() const +Id Command::id() const { - return m_id; + return d->m_id; } -void Action::setDefaultKeySequence(const QKeySequence &key) +void Command::setDefaultKeySequence(const QKeySequence &key) { - if (!m_isKeyInitialized) + if (!d->m_isKeyInitialized) setKeySequences({key}); - m_defaultKeys = {key}; + d->m_defaultKeys = {key}; } -void Action::setDefaultKeySequences(const QList &keys) +void Command::setDefaultKeySequences(const QList &keys) { - if (!m_isKeyInitialized) + if (!d->m_isKeyInitialized) setKeySequences(keys); - m_defaultKeys = keys; + d->m_defaultKeys = keys; } -QList Action::defaultKeySequences() const +QList Command::defaultKeySequences() const { - return m_defaultKeys; + return d->m_defaultKeys; } -QAction *Action::action() const +QAction *Command::action() const { - return m_action; + return d->m_action; } -QString Action::stringWithAppendedShortcut(const QString &str) const +QString Command::stringWithAppendedShortcut(const QString &str) const { return Utils::ProxyAction::stringWithAppendedShortcut(str, keySequence()); } -Context Action::context() const +Context Command::context() const { - return m_context; + return d->m_context; } -void Action::setKeySequences(const QList &keys) +void Command::setKeySequences(const QList &keys) { - m_isKeyInitialized = true; - m_action->setShortcuts(keys); + d->m_isKeyInitialized = true; + d->m_action->setShortcuts(keys); emit keySequenceChanged(); } -QList Action::keySequences() const +QList Command::keySequences() const { - return m_action->shortcuts(); + return d->m_action->shortcuts(); } -QKeySequence Action::keySequence() const +QKeySequence Command::keySequence() const { - return m_action->shortcut(); + return d->m_action->shortcut(); } -void Action::setDescription(const QString &text) +void Command::setDescription(const QString &text) { - m_defaultText = text; + d->m_defaultText = text; } -QString Action::description() const +QString Command::description() const { - if (!m_defaultText.isEmpty()) - return m_defaultText; + if (!d->m_defaultText.isEmpty()) + return d->m_defaultText; if (QAction *act = action()) { const QString text = Utils::stripAccelerator(act->text()); if (!text.isEmpty()) @@ -344,7 +355,7 @@ QString Action::description() const return id().toString(); } -void Action::setCurrentContext(const Context &context) +void Internal::CommandPrivate::setCurrentContext(const Context &context) { m_context = context; @@ -360,7 +371,7 @@ void Action::setCurrentContext(const Context &context) updateActiveState(); } -void Action::updateActiveState() +void Internal::CommandPrivate::updateActiveState() { setActive(m_action->isEnabled() && m_action->isVisible() && !m_action->isSeparator()); } @@ -377,7 +388,9 @@ static QString msgActionWarning(QAction *newAction, Id id, QAction *oldAction) return msg; } -void Action::addOverrideAction(QAction *action, const Context &context, bool scriptable) +void Internal::CommandPrivate::addOverrideAction(QAction *action, + const Context &context, + bool scriptable) { // disallow TextHeuristic menu role, because it doesn't work with translations, // e.g. QTCREATORBUG-13101 @@ -398,7 +411,7 @@ void Action::addOverrideAction(QAction *action, const Context &context, bool scr setCurrentContext(m_context); } -void Action::removeOverrideAction(QAction *action) +void Internal::CommandPrivate::removeOverrideAction(QAction *action) { QList toRemove; for (auto it = m_contextActionMap.cbegin(), end = m_contextActionMap.cend(); it != end; ++it) { @@ -410,125 +423,123 @@ void Action::removeOverrideAction(QAction *action) setCurrentContext(m_context); } -bool Action::isActive() const +bool Command::isActive() const { - return m_active; + return d->m_active; } -void Action::setActive(bool state) +void Internal::CommandPrivate::setActive(bool state) { if (state != m_active) { m_active = state; - emit activeStateChanged(); + emit m_q->activeStateChanged(); } } -bool Action::isEmpty() const +bool Internal::CommandPrivate::isEmpty() const { return m_contextActionMap.isEmpty(); } -bool Action::isScriptable() const +bool Command::isScriptable() const { - return std::find(m_scriptableMap.cbegin(), m_scriptableMap.cend(), true) != - m_scriptableMap.cend(); + return std::find(d->m_scriptableMap.cbegin(), d->m_scriptableMap.cend(), true) + != d->m_scriptableMap.cend(); } -bool Action::isScriptable(const Context &context) const +bool Command::isScriptable(const Context &context) const { - if (context == m_context && m_scriptableMap.contains(m_action->action())) - return m_scriptableMap.value(m_action->action()); + if (context == d->m_context && d->m_scriptableMap.contains(d->m_action->action())) + return d->m_scriptableMap.value(d->m_action->action()); for (int i = 0; i < context.size(); ++i) { - if (QAction *a = m_contextActionMap.value(context.at(i), nullptr)) { - if (m_scriptableMap.contains(a) && m_scriptableMap.value(a)) + if (QAction *a = d->m_contextActionMap.value(context.at(i), nullptr)) { + if (d->m_scriptableMap.contains(a) && d->m_scriptableMap.value(a)) return true; } } return false; } -void Action::setAttribute(CommandAttribute attr) +void Command::setAttribute(CommandAttribute attr) { - m_attributes |= attr; + d->m_attributes |= attr; switch (attr) { case Command::CA_Hide: - m_action->setAttribute(Utils::ProxyAction::Hide); + d->m_action->setAttribute(Utils::ProxyAction::Hide); break; case Command::CA_UpdateText: - m_action->setAttribute(Utils::ProxyAction::UpdateText); + d->m_action->setAttribute(Utils::ProxyAction::UpdateText); break; case Command::CA_UpdateIcon: - m_action->setAttribute(Utils::ProxyAction::UpdateIcon); + d->m_action->setAttribute(Utils::ProxyAction::UpdateIcon); break; case Command::CA_NonConfigurable: break; } } -void Action::removeAttribute(CommandAttribute attr) +void Command::removeAttribute(CommandAttribute attr) { - m_attributes &= ~attr; + d->m_attributes &= ~attr; switch (attr) { case Command::CA_Hide: - m_action->removeAttribute(Utils::ProxyAction::Hide); + d->m_action->removeAttribute(Utils::ProxyAction::Hide); break; case Command::CA_UpdateText: - m_action->removeAttribute(Utils::ProxyAction::UpdateText); + d->m_action->removeAttribute(Utils::ProxyAction::UpdateText); break; case Command::CA_UpdateIcon: - m_action->removeAttribute(Utils::ProxyAction::UpdateIcon); + d->m_action->removeAttribute(Utils::ProxyAction::UpdateIcon); break; case Command::CA_NonConfigurable: break; } } -bool Action::hasAttribute(Command::CommandAttribute attr) const +bool Command::hasAttribute(CommandAttribute attr) const { - return (m_attributes & attr); + return (d->m_attributes & attr); } -void Action::setTouchBarText(const QString &text) +void Command::setTouchBarText(const QString &text) { - m_touchBarText = text; + d->m_touchBarText = text; } -QString Action::touchBarText() const +QString Command::touchBarText() const { - return m_touchBarText; + return d->m_touchBarText; } -void Action::setTouchBarIcon(const QIcon &icon) +void Command::setTouchBarIcon(const QIcon &icon) { - m_touchBarIcon = icon; + d->m_touchBarIcon = icon; } -QIcon Action::touchBarIcon() const +QIcon Command::touchBarIcon() const { - return m_touchBarIcon; + return d->m_touchBarIcon; } -QAction *Action::touchBarAction() const +QAction *Command::touchBarAction() const { - if (!m_touchBarAction) { - m_touchBarAction = std::make_unique(); - m_touchBarAction->initialize(m_action); - m_touchBarAction->setIcon(m_touchBarIcon); - m_touchBarAction->setText(m_touchBarText); + if (!d->m_touchBarAction) { + d->m_touchBarAction = std::make_unique(); + d->m_touchBarAction->initialize(d->m_action); + d->m_touchBarAction->setIcon(d->m_touchBarIcon); + d->m_touchBarAction->setText(d->m_touchBarText); // the touch bar action should be hidden if the command is not valid for the context - m_touchBarAction->setAttribute(Utils::ProxyAction::Hide); - m_touchBarAction->setAction(m_action->action()); - connect(m_action, + d->m_touchBarAction->setAttribute(Utils::ProxyAction::Hide); + d->m_touchBarAction->setAction(d->m_action->action()); + connect(d->m_action, &Utils::ProxyAction::currentActionChanged, - m_touchBarAction.get(), + d->m_touchBarAction.get(), &Utils::ProxyAction::setAction); } - return m_touchBarAction.get(); + return d->m_touchBarAction.get(); } -} // namespace Internal - /*! Appends the main keyboard shortcut that is currently assigned to the action \a a to its tool tip. diff --git a/src/plugins/coreplugin/actionmanager/command.h b/src/plugins/coreplugin/actionmanager/command.h index 677a1308185..f2c89469f34 100644 --- a/src/plugins/coreplugin/actionmanager/command.h +++ b/src/plugins/coreplugin/actionmanager/command.h @@ -40,7 +40,12 @@ QT_END_NAMESPACE namespace Core { +namespace Internal { +class ActionManagerPrivate; +class CommandPrivate; +} // namespace Internal +class ActionManager; class Context; constexpr bool useMacShortcuts = Utils::HostOsInfo::isMacHost(); @@ -57,46 +62,56 @@ public: }; Q_DECLARE_FLAGS(CommandAttributes, CommandAttribute) - virtual void setDefaultKeySequence(const QKeySequence &key) = 0; - virtual void setDefaultKeySequences(const QList &keys) = 0; - virtual QList defaultKeySequences() const = 0; - virtual QList keySequences() const = 0; - virtual QKeySequence keySequence() const = 0; + ~Command(); + + void setDefaultKeySequence(const QKeySequence &key); + void setDefaultKeySequences(const QList &keys); + QList defaultKeySequences() const; + QList keySequences() const; + QKeySequence keySequence() const; // explicitly set the description (used e.g. in shortcut settings) // default is to use the action text for actions, or the whatsThis for shortcuts, // or, as a last fall back if these are empty, the command ID string // override the default e.g. if the text is context dependent and contains file names etc - virtual void setDescription(const QString &text) = 0; - virtual QString description() const = 0; + void setDescription(const QString &text); + QString description() const; - virtual Utils::Id id() const = 0; + Utils::Id id() const; - virtual QAction *action() const = 0; - virtual Context context() const = 0; + QAction *action() const; + Context context() const; - virtual void setAttribute(CommandAttribute attr) = 0; - virtual void removeAttribute(CommandAttribute attr) = 0; - virtual bool hasAttribute(CommandAttribute attr) const = 0; + void setAttribute(CommandAttribute attr); + void removeAttribute(CommandAttribute attr); + bool hasAttribute(CommandAttribute attr) const; - virtual bool isActive() const = 0; + bool isActive() const; - virtual void setKeySequences(const QList &keys) = 0; - virtual QString stringWithAppendedShortcut(const QString &str) const = 0; + void setKeySequences(const QList &keys); + QString stringWithAppendedShortcut(const QString &str) const; void augmentActionWithShortcutToolTip(QAction *action) const; static QToolButton *toolButtonWithAppendedShortcut(QAction *action, Command *cmd); - virtual bool isScriptable() const = 0; - virtual bool isScriptable(const Context &) const = 0; + bool isScriptable() const; + bool isScriptable(const Context &) const; - virtual void setTouchBarText(const QString &text) = 0; - virtual QString touchBarText() const = 0; - virtual void setTouchBarIcon(const QIcon &icon) = 0; - virtual QIcon touchBarIcon() const = 0; - virtual QAction *touchBarAction() const = 0; + void setTouchBarText(const QString &text); + QString touchBarText() const; + void setTouchBarIcon(const QIcon &icon); + QIcon touchBarIcon() const; + QAction *touchBarAction() const; signals: void keySequenceChanged(); void activeStateChanged(); + +private: + friend class ActionManager; + friend class Internal::ActionManagerPrivate; + + Command(Utils::Id id); + + Internal::CommandPrivate *d; }; } // namespace Core diff --git a/src/plugins/coreplugin/actionmanager/command_p.h b/src/plugins/coreplugin/actionmanager/command_p.h index 94f72ddfa1e..a0b412c1ee9 100644 --- a/src/plugins/coreplugin/actionmanager/command_p.h +++ b/src/plugins/coreplugin/actionmanager/command_p.h @@ -43,56 +43,24 @@ namespace Core { namespace Internal { -class Action : public Command +class CommandPrivate : public QObject { Q_OBJECT public: - Action(Utils::Id id); + CommandPrivate(Command *parent); - Utils::Id id() const override; - - void setDefaultKeySequence(const QKeySequence &key) override; - void setDefaultKeySequences(const QList &key) override; - QList defaultKeySequences() const override; - - void setKeySequences(const QList &keys) override; - QList keySequences() const override; - QKeySequence keySequence() const override; - - void setDescription(const QString &text) override; - QString description() const override; - - QAction *action() const override; - - QString stringWithAppendedShortcut(const QString &str) const override; - - Context context() const override; void setCurrentContext(const Context &context); - bool isActive() const override; void addOverrideAction(QAction *action, const Context &context, bool scriptable); void removeOverrideAction(QAction *action); bool isEmpty() const; - bool isScriptable() const override; - bool isScriptable(const Context &context) const override; - - void setAttribute(CommandAttribute attr) override; - void removeAttribute(CommandAttribute attr) override; - bool hasAttribute(CommandAttribute attr) const override; - - void setTouchBarText(const QString &text) override; - QString touchBarText() const override; - void setTouchBarIcon(const QIcon &icon) override; - QIcon touchBarIcon() const override; - QAction *touchBarAction() const override; - -private: void updateActiveState(); void setActive(bool state); + Command *m_q = nullptr; Context m_context; - CommandAttributes m_attributes; + Command::CommandAttributes m_attributes; Utils::Id m_id; QList m_defaultKeys; QString m_defaultText; diff --git a/src/plugins/coreplugin/actionmanager/commandsfile.cpp b/src/plugins/coreplugin/actionmanager/commandsfile.cpp index 340eb969de0..8d4964999be 100644 --- a/src/plugins/coreplugin/actionmanager/commandsfile.cpp +++ b/src/plugins/coreplugin/actionmanager/commandsfile.cpp @@ -24,7 +24,7 @@ ****************************************************************************/ #include "commandsfile.h" -#include "command_p.h" +#include "command.h" #include #include diff --git a/src/plugins/coreplugin/dialogs/shortcutsettings.cpp b/src/plugins/coreplugin/dialogs/shortcutsettings.cpp index 35e1deb2880..37f860926c4 100644 --- a/src/plugins/coreplugin/dialogs/shortcutsettings.cpp +++ b/src/plugins/coreplugin/dialogs/shortcutsettings.cpp @@ -29,7 +29,6 @@ #include #include #include -#include #include #include @@ -38,15 +37,16 @@ #include #include -#include +#include +#include +#include #include #include #include +#include #include #include #include -#include -#include using namespace Utils; diff --git a/src/plugins/coreplugin/mainwindow.cpp b/src/plugins/coreplugin/mainwindow.cpp index 7495949271c..1fa6d4be141 100644 --- a/src/plugins/coreplugin/mainwindow.cpp +++ b/src/plugins/coreplugin/mainwindow.cpp @@ -71,10 +71,12 @@ #include #include #include +#include #include #include #include #include +#include #include #include