From 31cc9b8e69305c4aa59e9d690a4841d7100086e7 Mon Sep 17 00:00:00 2001 From: Eike Ziller Date: Wed, 4 Mar 2020 16:37:45 +0100 Subject: [PATCH] Make it possible to set more than one shortcut per action Multiple shortcuts per action make it possible to configure friendlier behavior. E.g. on macOS decreasing/increasing font sizes, and deleting elements usually have two shortcuts each. But also custom configurations that assign a different key to a common function without removing the default can be useful. In this patch the functionality is still pretty much hidden from the user, even though there is a "secret" way to enter such multiple shortcuts in the settings dialog, by separating shortcuts with " | ". Task-number: QTCREATORBUG-72 Change-Id: I16bec0a71aaf4abf50335b0fd7da620c73b31777 Reviewed-by: David Schulz --- .../actionmanager/actionmanager.cpp | 48 ++++- .../coreplugin/actionmanager/command.cpp | 24 ++- .../coreplugin/actionmanager/command.h | 6 +- .../coreplugin/actionmanager/command_p.h | 8 +- .../coreplugin/actionmanager/commandsfile.cpp | 24 +-- .../coreplugin/actionmanager/commandsfile.h | 2 +- .../coreplugin/dialogs/shortcutsettings.cpp | 174 +++++++++++++----- .../coreplugin/dialogs/shortcutsettings.h | 2 +- 8 files changed, 204 insertions(+), 84 deletions(-) diff --git a/src/plugins/coreplugin/actionmanager/actionmanager.cpp b/src/plugins/coreplugin/actionmanager/actionmanager.cpp index 449a0040bcb..eff51a816b7 100644 --- a/src/plugins/coreplugin/actionmanager/actionmanager.cpp +++ b/src/plugins/coreplugin/actionmanager/actionmanager.cpp @@ -30,6 +30,7 @@ #include #include +#include #include #include @@ -46,6 +47,7 @@ namespace { } static const char kKeyboardSettingsKey[] = "KeyboardShortcuts"; +static const char kKeyboardSettingsKeyV2[] = "KeyboardShortcutsV2"; using namespace Core; using namespace Core::Internal; @@ -495,22 +497,50 @@ Action *ActionManagerPrivate::overridableAction(Id id) void ActionManagerPrivate::readUserSettings(Id id, Action *cmd) { + // TODO Settings V2 were introduced in Qt Creator 4.13, remove old settings at some point QSettings *settings = ICore::settings(); - settings->beginGroup(QLatin1String(kKeyboardSettingsKey)); - if (settings->contains(id.toString())) - cmd->setKeySequence(QKeySequence(settings->value(id.toString()).toString())); + // transfer from old settings if not done before + const QString group = settings->childGroups().contains(kKeyboardSettingsKeyV2) + ? QString(kKeyboardSettingsKeyV2) + : QString(kKeyboardSettingsKey); + settings->beginGroup(group); + if (settings->contains(id.toString())) { + const QVariant v = settings->value(id.toString()); + if (QMetaType::Type(v.type()) == QMetaType::QStringList) { + cmd->setKeySequences(Utils::transform(v.toStringList(), [](const QString &s) { + return QKeySequence::fromString(s); + })); + } else { + cmd->setKeySequences({QKeySequence::fromString(v.toString())}); + } + } settings->endGroup(); } void ActionManagerPrivate::saveSettings(Action *cmd) { - const QString settingsKey = QLatin1String(kKeyboardSettingsKey) + QLatin1Char('/') - + cmd->id().toString(); - QKeySequence key = cmd->keySequence(); - if (key != cmd->defaultKeySequence()) - ICore::settings()->setValue(settingsKey, key.toString()); - else + const QString id = cmd->id().toString(); + const QString settingsKey = QLatin1String(kKeyboardSettingsKeyV2) + '/' + id; + const QString compatSettingsKey = QLatin1String(kKeyboardSettingsKey) + '/' + id; + const QList keys = cmd->keySequences(); + const QList defaultKeys = cmd->defaultKeySequences(); + if (keys != defaultKeys) { + if (keys.isEmpty()) { + ICore::settings()->setValue(settingsKey, QString()); + ICore::settings()->setValue(compatSettingsKey, QString()); + } else if (keys.size() == 1) { + ICore::settings()->setValue(settingsKey, keys.first().toString()); + ICore::settings()->setValue(compatSettingsKey, keys.first().toString()); + } else { + ICore::settings()->setValue(settingsKey, + Utils::transform(keys, + [](const QKeySequence &k) { + return k.toString(); + })); + } + } else { ICore::settings()->remove(settingsKey); + } } void ActionManagerPrivate::saveSettings() diff --git a/src/plugins/coreplugin/actionmanager/command.cpp b/src/plugins/coreplugin/actionmanager/command.cpp index cd801c1ec2f..7109c15bd94 100644 --- a/src/plugins/coreplugin/actionmanager/command.cpp +++ b/src/plugins/coreplugin/actionmanager/command.cpp @@ -253,13 +253,20 @@ Id Action::id() const void Action::setDefaultKeySequence(const QKeySequence &key) { if (!m_isKeyInitialized) - setKeySequence(key); - m_defaultKey = key; + setKeySequences({key}); + m_defaultKeys = {key}; } -QKeySequence Action::defaultKeySequence() const +void Action::setDefaultKeySequences(const QList &keys) { - return m_defaultKey; + if (!m_isKeyInitialized) + setKeySequences(keys); + m_defaultKeys = keys; +} + +QList Action::defaultKeySequences() const +{ + return m_defaultKeys; } QAction *Action::action() const @@ -277,13 +284,18 @@ Context Action::context() const return m_context; } -void Action::setKeySequence(const QKeySequence &key) +void Action::setKeySequences(const QList &keys) { m_isKeyInitialized = true; - m_action->setShortcut(key); + m_action->setShortcuts(keys); emit keySequenceChanged(); } +QList Action::keySequences() const +{ + return m_action->shortcuts(); +} + QKeySequence Action::keySequence() const { return m_action->shortcut(); diff --git a/src/plugins/coreplugin/actionmanager/command.h b/src/plugins/coreplugin/actionmanager/command.h index 355507de121..d17ef0d2661 100644 --- a/src/plugins/coreplugin/actionmanager/command.h +++ b/src/plugins/coreplugin/actionmanager/command.h @@ -58,7 +58,9 @@ public: Q_DECLARE_FLAGS(CommandAttributes, CommandAttribute) virtual void setDefaultKeySequence(const QKeySequence &key) = 0; - virtual QKeySequence defaultKeySequence() const = 0; + virtual void setDefaultKeySequences(const QList &keys) = 0; + virtual QList defaultKeySequences() const = 0; + virtual QList keySequences() const = 0; virtual QKeySequence keySequence() const = 0; // explicitly set the description (used e.g. in shortcut settings) // default is to use the action text for actions, or the whatsThis for shortcuts, @@ -78,7 +80,7 @@ public: virtual bool isActive() const = 0; - virtual void setKeySequence(const QKeySequence &key) = 0; + virtual void setKeySequences(const QList &keys) = 0; virtual QString stringWithAppendedShortcut(const QString &str) const = 0; void augmentActionWithShortcutToolTip(QAction *action) const; static QToolButton *toolButtonWithAppendedShortcut(QAction *action, Command *cmd); diff --git a/src/plugins/coreplugin/actionmanager/command_p.h b/src/plugins/coreplugin/actionmanager/command_p.h index 9811c88fb66..de722d41cf5 100644 --- a/src/plugins/coreplugin/actionmanager/command_p.h +++ b/src/plugins/coreplugin/actionmanager/command_p.h @@ -52,9 +52,11 @@ public: Id id() const override; void setDefaultKeySequence(const QKeySequence &key) override; - QKeySequence defaultKeySequence() const override; + void setDefaultKeySequences(const QList &key) override; + QList defaultKeySequences() const override; - void setKeySequence(const QKeySequence &key) override; + void setKeySequences(const QList &keys) override; + QList keySequences() const override; QKeySequence keySequence() const override; void setDescription(const QString &text) override; @@ -92,7 +94,7 @@ private: Context m_context; CommandAttributes m_attributes; Id m_id; - QKeySequence m_defaultKey; + QList m_defaultKeys; QString m_defaultText; QString m_touchBarText; QIcon m_touchBarIcon; diff --git a/src/plugins/coreplugin/actionmanager/commandsfile.cpp b/src/plugins/coreplugin/actionmanager/commandsfile.cpp index faeed00e07e..8b03fee9e57 100644 --- a/src/plugins/coreplugin/actionmanager/commandsfile.cpp +++ b/src/plugins/coreplugin/actionmanager/commandsfile.cpp @@ -83,9 +83,9 @@ CommandsFile::CommandsFile(const QString &filename) /*! \internal */ -QMap CommandsFile::importCommands() const +QMap> CommandsFile::importCommands() const { - QMap result; + QMap> result; QFile file(m_filename); if (!file.open(QIODevice::ReadOnly|QIODevice::Text)) @@ -101,19 +101,17 @@ QMap CommandsFile::importCommands() const case QXmlStreamReader::StartElement: { const QStringRef name = r.name(); if (name == ctx.shortCutElement) { - if (!currentId.isEmpty()) // shortcut element without key element == empty shortcut - result.insert(currentId, QKeySequence()); currentId = r.attributes().value(ctx.idAttribute).toString(); + if (!result.contains(currentId)) + result.insert(currentId, {}); } else if (name == ctx.keyElement) { - QTC_ASSERT(!currentId.isEmpty(), return result); + QTC_ASSERT(!currentId.isEmpty(), continue); const QXmlStreamAttributes attributes = r.attributes(); if (attributes.hasAttribute(ctx.valueAttribute)) { const QString keyString = attributes.value(ctx.valueAttribute).toString(); - result.insert(currentId, QKeySequence(keyString)); - } else { - result.insert(currentId, QKeySequence()); + QList keys = result.value(currentId); + result.insert(currentId, keys << QKeySequence(keyString)); } - currentId.clear(); } // if key element } // case QXmlStreamReader::StartElement default: @@ -144,14 +142,16 @@ bool CommandsFile::exportCommands(const QList &items) w.writeStartElement(ctx.mappingElement); foreach (const ShortcutItem *item, items) { const Id id = item->m_cmd->id(); - if (item->m_key.isEmpty()) { + if (item->m_keys.isEmpty() || item->m_keys.first().isEmpty()) { w.writeEmptyElement(ctx.shortCutElement); w.writeAttribute(ctx.idAttribute, id.toString()); } else { w.writeStartElement(ctx.shortCutElement); w.writeAttribute(ctx.idAttribute, id.toString()); - w.writeEmptyElement(ctx.keyElement); - w.writeAttribute(ctx.valueAttribute, item->m_key.toString()); + for (const QKeySequence &k : item->m_keys) { + w.writeEmptyElement(ctx.keyElement); + w.writeAttribute(ctx.valueAttribute, k.toString()); + } w.writeEndElement(); // Shortcut } } diff --git a/src/plugins/coreplugin/actionmanager/commandsfile.h b/src/plugins/coreplugin/actionmanager/commandsfile.h index 891856383fa..cd4bceeeeb6 100644 --- a/src/plugins/coreplugin/actionmanager/commandsfile.h +++ b/src/plugins/coreplugin/actionmanager/commandsfile.h @@ -43,7 +43,7 @@ class CommandsFile : public QObject public: CommandsFile(const QString &filename); - QMap importCommands() const; + QMap > importCommands() const; bool exportCommands(const QList &items); private: diff --git a/src/plugins/coreplugin/dialogs/shortcutsettings.cpp b/src/plugins/coreplugin/dialogs/shortcutsettings.cpp index f75c215bf1d..bf83d112d67 100644 --- a/src/plugins/coreplugin/dialogs/shortcutsettings.cpp +++ b/src/plugins/coreplugin/dialogs/shortcutsettings.cpp @@ -33,6 +33,7 @@ #include #include +#include #include #include #include @@ -50,6 +51,8 @@ Q_DECLARE_METATYPE(Core::Internal::ShortcutItem*) +const char kSeparator[] = " | "; + static int translateModifiers(Qt::KeyboardModifiers state, const QString &text) { @@ -82,9 +85,23 @@ static QString keySequenceToEditString(const QKeySequence &sequence) return text; } +static QString keySequencesToEditString(const QList &sequence) +{ + return Utils::transform(sequence, keySequenceToEditString).join(kSeparator); +} + +static QString keySequencesToNativeString(const QList &sequence) +{ + return Utils::transform(sequence, + [](const QKeySequence &k) { + return k.toString(QKeySequence::NativeText); + }) + .join(kSeparator); +} + static QKeySequence keySequenceFromEditString(const QString &editString) { - QString text = editString; + QString text = editString.trimmed(); if (Utils::HostOsInfo::isMacHost()) { // adapt the modifier names text.replace(QLatin1String("Opt"), QLatin1String("Alt"), Qt::CaseInsensitive); @@ -94,6 +111,21 @@ static QKeySequence keySequenceFromEditString(const QString &editString) return QKeySequence::fromString(text, QKeySequence::PortableText); } +struct ParsedKey +{ + QString text; + QKeySequence key; +}; + +static QList keySequencesFromEditString(const QString &editString) +{ + if (editString.trimmed().isEmpty()) + return {}; + return Utils::transform(editString.split(kSeparator), [](const QString &str) { + return ParsedKey{str, keySequenceFromEditString(str)}; + }); +} + static bool keySequenceIsValid(const QKeySequence &sequence) { if (sequence.isEmpty()) @@ -242,7 +274,7 @@ private: void resetToDefault(); bool validateShortcutEdit() const; bool markCollisions(ShortcutItem *); - void setKeySequence(const QKeySequence &key); + void setKeySequences(const QList &keys); void showConflicts(); void clear(); @@ -292,8 +324,10 @@ ShortcutSettingsWidget::ShortcutSettingsWidget() "enter \"Ctrl+Shift+Escape,A\".") + QLatin1String("")); auto shortcutButton = new ShortcutButton(m_shortcutBox); - connect(shortcutButton, &ShortcutButton::keySequenceChanged, - this, &ShortcutSettingsWidget::setKeySequence); + connect(shortcutButton, + &ShortcutButton::keySequenceChanged, + this, + [this](const QKeySequence &k) { setKeySequences({k}); }); auto resetButton = new QPushButton(tr("Reset"), m_shortcutBox); resetButton->setToolTip(tr("Reset to default.")); connect(resetButton, &QPushButton::clicked, @@ -343,7 +377,7 @@ QWidget *ShortcutSettings::widget() void ShortcutSettingsWidget::apply() { foreach (ShortcutItem *item, m_scitems) - item->m_cmd->setKeySequence(item->m_key); + item->m_cmd->setKeySequences(item->m_keys); } void ShortcutSettings::apply() @@ -372,12 +406,32 @@ void ShortcutSettingsWidget::handleCurrentCommandChanged(QTreeWidgetItem *curren m_warningLabel->clear(); m_shortcutBox->setEnabled(false); } else { - setKeySequence(scitem->m_key); + setKeySequences(scitem->m_keys); markCollisions(scitem); m_shortcutBox->setEnabled(true); } } +static bool checkValidity(const QList &keys, QString *warningMessage) +{ + QTC_ASSERT(warningMessage, return true); + for (const ParsedKey &k : keys) { + if (!keySequenceIsValid(k.key)) { + *warningMessage = ShortcutSettingsWidget::tr("Invalid key sequence \"%1\".").arg(k.text); + return false; + } + } + for (const ParsedKey &k : keys) { + if (textKeySequence(k.key)) { + *warningMessage = ShortcutSettingsWidget::tr( + "Key sequence \"%1\" will not work in editor.") + .arg(k.text); + break; + } + } + return true; +} + bool ShortcutSettingsWidget::validateShortcutEdit() const { m_warningLabel->clear(); @@ -385,43 +439,57 @@ bool ShortcutSettingsWidget::validateShortcutEdit() const ShortcutItem *item = shortcutItem(current); if (!item) return true; - bool valid = false; const QString text = m_shortcutEdit->text().trimmed(); - const QKeySequence currentKey = keySequenceFromEditString(text); + const QList currentKeys = keySequencesFromEditString(text); + QString warningMessage; + bool isValid = checkValidity(currentKeys, &warningMessage); - if (keySequenceIsValid(currentKey) || text.isEmpty()) { - item->m_key = currentKey; + if (isValid) { + item->m_keys = Utils::transform(currentKeys, &ParsedKey::key); auto that = const_cast(this); - if (item->m_cmd->defaultKeySequence() != item->m_key) + if (item->m_keys != item->m_cmd->defaultKeySequences()) that->setModified(current, true); else that->setModified(current, false); - current->setText(2, item->m_key.toString(QKeySequence::NativeText)); - valid = !that->markCollisions(item); - if (!valid) { + current->setText(2, keySequencesToNativeString(item->m_keys)); + isValid = !that->markCollisions(item); + if (!isValid) { m_warningLabel->setText( tr("Key sequence has potential conflicts. Show.")); - } else if (textKeySequence(currentKey)) { - m_warningLabel->setText(tr("Key sequence will not work in editor.")); } - } else { - m_warningLabel->setText(m_warningLabel->text() + tr("Invalid key sequence.")); } - return valid; + if (!warningMessage.isEmpty()) { + if (m_warningLabel->text().isEmpty()) + m_warningLabel->setText(warningMessage); + else + m_warningLabel->setText(m_warningLabel->text() + " " + warningMessage); + } + return isValid; } bool ShortcutSettingsWidget::filterColumn(const QString &filterString, QTreeWidgetItem *item, int column) const { - QString text; - ShortcutItem *scitem = shortcutItem(item); + const ShortcutItem *scitem = shortcutItem(item); if (column == item->columnCount() - 1) { // shortcut // filter on the shortcut edit text if (!scitem) return true; - text = keySequenceToEditString(scitem->m_key); - } else if (column == 0 && scitem) { // command id + const QStringList filters = Utils::transform(filterString.split(kSeparator), + [](const QString &s) { return s.trimmed(); }); + for (const QKeySequence &k : scitem->m_keys) { + const QString &keyString = keySequenceToEditString(k); + const bool found = Utils::anyOf(filters, [keyString](const QString &f) { + return keyString.contains(f, Qt::CaseInsensitive); + }); + if (found) + return false; + } + return true; + } + QString text; + if (column == 0 && scitem) { // command id text = scitem->m_cmd->id().toString(); } else { text = item->text(column); @@ -429,9 +497,9 @@ bool ShortcutSettingsWidget::filterColumn(const QString &filterString, QTreeWidg return !text.contains(filterString, Qt::CaseInsensitive); } -void ShortcutSettingsWidget::setKeySequence(const QKeySequence &key) +void ShortcutSettingsWidget::setKeySequences(const QList &keys) { - m_shortcutEdit->setText(keySequenceToEditString(key)); + m_shortcutEdit->setText(keySequencesToEditString(keys)); } void ShortcutSettingsWidget::showConflicts() @@ -439,7 +507,7 @@ void ShortcutSettingsWidget::showConflicts() QTreeWidgetItem *current = commandList()->currentItem(); ShortcutItem *scitem = shortcutItem(current); if (scitem) - setFilterText(keySequenceToEditString(scitem->m_key)); + setFilterText(keySequencesToEditString(scitem->m_keys)); } void ShortcutSettingsWidget::resetToDefault() @@ -447,7 +515,7 @@ void ShortcutSettingsWidget::resetToDefault() QTreeWidgetItem *current = commandList()->currentItem(); ShortcutItem *scitem = shortcutItem(current); if (scitem) { - setKeySequence(scitem->m_cmd->defaultKeySequence()); + setKeySequences(scitem->m_cmd->defaultKeySequences()); foreach (ShortcutItem *item, m_scitems) markCollisions(item); } @@ -461,17 +529,16 @@ void ShortcutSettingsWidget::importAction() if (!fileName.isEmpty()) { CommandsFile cf(fileName); - QMap mapping = cf.importCommands(); - - foreach (ShortcutItem *item, m_scitems) { + QMap> mapping = cf.importCommands(); + for (ShortcutItem *item : qAsConst(m_scitems)) { QString sid = item->m_cmd->id().toString(); if (mapping.contains(sid)) { - item->m_key = mapping.value(sid); - item->m_item->setText(2, item->m_key.toString(QKeySequence::NativeText)); + item->m_keys = mapping.value(sid); + item->m_item->setText(2, keySequencesToNativeString(item->m_keys)); if (item->m_item == commandList()->currentItem()) emit currentCommandChanged(item->m_item); - if (item->m_cmd->defaultKeySequence() != item->m_key) + if (item->m_keys != item->m_cmd->defaultKeySequences()) setModified(item->m_item, true); else setModified(item->m_item, false); @@ -486,8 +553,8 @@ void ShortcutSettingsWidget::importAction() void ShortcutSettingsWidget::defaultAction() { foreach (ShortcutItem *item, m_scitems) { - item->m_key = item->m_cmd->defaultKeySequence(); - item->m_item->setText(2, item->m_key.toString(QKeySequence::NativeText)); + item->m_keys = item->m_cmd->defaultKeySequences(); + item->m_item->setText(2, keySequencesToNativeString(item->m_keys)); setModified(item->m_item, false); if (item->m_item == commandList()->currentItem()) emit currentCommandChanged(item->m_item); @@ -551,11 +618,11 @@ void ShortcutSettingsWidget::initialize() } sections[section]->addChild(item); - s->m_key = c->keySequence(); + s->m_keys = c->keySequences(); item->setText(0, subId); item->setText(1, c->description()); - item->setText(2, s->m_key.toString(QKeySequence::NativeText)); - if (s->m_cmd->defaultKeySequence() != s->m_key) + item->setText(2, keySequencesToNativeString(s->m_keys)); + if (s->m_keys != s->m_cmd->defaultKeySequences()) setModified(item, true); item->setData(0, Qt::UserRole, QVariant::fromValue(s)); @@ -568,35 +635,42 @@ void ShortcutSettingsWidget::initialize() bool ShortcutSettingsWidget::markCollisions(ShortcutItem *item) { bool hasCollision = false; - if (!item->m_key.isEmpty()) { + if (!item->m_keys.isEmpty()) { Id globalId(Constants::C_GLOBAL); const Context itemContext = item->m_cmd->context(); const bool itemHasGlobalContext = itemContext.contains(globalId); - foreach (ShortcutItem *currentItem, m_scitems) { - if (currentItem->m_key.isEmpty() || item == currentItem - || item->m_key != currentItem->m_key) + for (ShortcutItem *currentItem : qAsConst(m_scitems)) { + if (item == currentItem) continue; + const bool containsSameShortcut = Utils::anyOf(currentItem->m_keys, + [item](const QKeySequence &k) { + return item->m_keys.contains(k); + }); + if (!containsSameShortcut) + continue; + // check if contexts might conflict const Context currentContext = currentItem->m_cmd->context(); bool currentIsConflicting = (itemHasGlobalContext && !currentContext.isEmpty()); if (!currentIsConflicting) { - foreach (const Id &id, currentContext) { - if ((id == globalId && !itemContext.isEmpty()) - || itemContext.contains(id)) { + for (const Id &id : currentContext) { + if ((id == globalId && !itemContext.isEmpty()) || itemContext.contains(id)) { currentIsConflicting = true; break; } } } if (currentIsConflicting) { - currentItem->m_item->setForeground( - 2, Utils::creatorTheme()->color(Utils::Theme::TextColorError)); + currentItem->m_item->setForeground(2, + Utils::creatorTheme()->color( + Utils::Theme::TextColorError)); hasCollision = true; } } } - item->m_item->setForeground(2, hasCollision - ? Utils::creatorTheme()->color(Utils::Theme::TextColorError) - : commandList()->palette().windowText()); + item->m_item->setForeground(2, + hasCollision + ? Utils::creatorTheme()->color(Utils::Theme::TextColorError) + : commandList()->palette().windowText()); return hasCollision; } diff --git a/src/plugins/coreplugin/dialogs/shortcutsettings.h b/src/plugins/coreplugin/dialogs/shortcutsettings.h index a49d45a6159..14d56ae8f0d 100644 --- a/src/plugins/coreplugin/dialogs/shortcutsettings.h +++ b/src/plugins/coreplugin/dialogs/shortcutsettings.h @@ -51,7 +51,7 @@ class ShortcutSettingsWidget; struct ShortcutItem { Command *m_cmd; - QKeySequence m_key; + QList m_keys; QTreeWidgetItem *m_item; };