From 25351dcc549f1daddf5e2ae8a242191174342a3e Mon Sep 17 00:00:00 2001 From: Giuseppe D'Angelo Date: Sun, 19 Apr 2020 19:56:18 +0200 Subject: Long live QKeyCombination! C++20 via P1120 is deprecating arithmetic operations between unrelated enumeration types, and GCC 10 is already complaining. Hence, these operations might become illegal in C++23 or C++26 at the latest. A case of this that affects Qt is in key combinations: a QKeySequence can be constructed by summing / ORing modifiers and a key, for instance: Qt::CTRL + Qt::Key_A Qt::SHIFT | Qt::CTRL | Qt::Key_G (recommended, see below) The problem is that the modifiers and the key belong to different enumerations (and there's 2 enumerations for the modifier, and one for the key). To solve this: add a dedicated class to represent a combination of keys, and operators between those enumerations to build instances of this class. I would've simply defined operator|, but again docs and pre-existing code use operator+ as well, so added both to at least tackle simple cases (modifier + key). Multiple modifiers create a problem: operator+ between them yields int, not the corresponding flags type (because operator+ is not overloaded for this use case): Qt::CTRL + Qt::SHIFT + Qt::Key_A \__________________/ / int / \______________/ int Not only this loses track of the datatypes involved, but it would also then "add" the key (with NO warnings, now its int + enum, so it's not mixing enums!) and yielding int again. I don't want to special-case this; the point of the class is that int is the wrong datatype. Everything works just fine when using operator| instead: Qt::CTRL | Qt::SHIFT | Qt::Key_A \__________________/ / Qt::Modifiers / \______________/ QKeyCombination So I'm defining operator+ so that the simple cases still work, but also deprecating it. Port some code around Qt to the new class. In certain cases, it's a huge win for clarity. In some others, I've just added the necessary casts to make it still compile without warnings, without attempting refactorings. [ChangeLog][QtCore][QKeyCombination] New class to represent a combination of a key and zero or more modifiers, to be used when defining shortcuts or similar. [ChangeLog][Potentially Source-Incompatible Changes] A keyboard modifier (such as Qt::CTRL, Qt::AltModifier, etc.) should be combined with a key (such as Qt::Key_A, Qt::Key_F1, etc.) by using operator|, not operator+. The result is now an object of type QKeyCombination, that stores the key and the modifiers. Change-Id: I657a3a328232f059023fff69c5031ee31cc91dd6 Reviewed-by: Volker Hilsheimer --- src/gui/kernel/qevent.cpp | 9 +++++++++ src/gui/kernel/qevent.h | 4 ++++ src/gui/kernel/qguivariant.cpp | 3 +-- src/gui/kernel/qkeymapper.cpp | 25 ++++++++++++------------- src/gui/kernel/qkeysequence.cpp | 25 ++++++++++++++++++------- src/gui/kernel/qkeysequence.h | 8 ++++++-- src/gui/kernel/qkeysequence_p.h | 2 +- src/gui/kernel/qplatformtheme.cpp | 2 +- src/gui/kernel/qshortcutmap.cpp | 14 +++++++------- 9 files changed, 59 insertions(+), 33 deletions(-) (limited to 'src/gui/kernel') diff --git a/src/gui/kernel/qevent.cpp b/src/gui/kernel/qevent.cpp index 1a6a43a4cb..b4880eff53 100644 --- a/src/gui/kernel/qevent.cpp +++ b/src/gui/kernel/qevent.cpp @@ -1285,6 +1285,15 @@ Qt::KeyboardModifiers QKeyEvent::modifiers() const return QInputEvent::modifiers(); } +/*! + \fn QKeyCombination QKeyEvent::keyCombination() const + + Returns a QKeyCombination object containing both the key() and + the modifiers() carried by this event. + + \since 6.0 +*/ + #if QT_CONFIG(shortcut) /*! \fn bool QKeyEvent::matches(QKeySequence::StandardKey key) const diff --git a/src/gui/kernel/qevent.h b/src/gui/kernel/qevent.h index 2c0325296d..c8402b6632 100644 --- a/src/gui/kernel/qevent.h +++ b/src/gui/kernel/qevent.h @@ -480,6 +480,10 @@ public: bool matches(QKeySequence::StandardKey key) const; #endif Qt::KeyboardModifiers modifiers() const; + QKeyCombination keyCombination() const + { + return QKeyCombination(modifiers(), Qt::Key(m_key)); + } inline QString text() const { return m_text; } inline bool isAutoRepeat() const { return m_autoRepeat; } inline int count() const { return int(m_count); } diff --git a/src/gui/kernel/qguivariant.cpp b/src/gui/kernel/qguivariant.cpp index ebb180dc8e..1af0692173 100644 --- a/src/gui/kernel/qguivariant.cpp +++ b/src/gui/kernel/qguivariant.cpp @@ -99,7 +99,6 @@ struct GuiTypesFilter { }; }; - static const struct : QMetaTypeModuleHelper { #define QT_IMPL_METATYPEINTERFACE_GUI_TYPES(MetaTypeName, MetaTypeId, RealName) \ @@ -146,7 +145,7 @@ static const struct : QMetaTypeModuleHelper ); QMETATYPE_CONVERTER(QKeySequence, QString, result = source; return true;); QMETATYPE_CONVERTER(Int, QKeySequence, - result = source.isEmpty() ? 0 : source[0]; + result = source.isEmpty() ? 0 : source[0].toCombined(); return true; ); QMETATYPE_CONVERTER(QKeySequence, Int, result = source; return true;); diff --git a/src/gui/kernel/qkeymapper.cpp b/src/gui/kernel/qkeymapper.cpp index f0ea66dc79..89eacc944a 100644 --- a/src/gui/kernel/qkeymapper.cpp +++ b/src/gui/kernel/qkeymapper.cpp @@ -71,17 +71,20 @@ QKeyMapper::~QKeyMapper() { } -QList QKeyMapper::possibleKeys(QKeyEvent *e) +static QList extractKeyFromEvent(QKeyEvent *e) { QList result; + if (e->key() && (e->key() != Qt::Key_unknown)) + result << e->keyCombination().toCombined(); + else if (!e->text().isEmpty()) + result << int(e->text().at(0).unicode() + (int)e->modifiers()); + return result; +} - if (!e->nativeScanCode()) { - if (e->key() && (e->key() != Qt::Key_unknown)) - result << int(e->key() + e->modifiers()); - else if (!e->text().isEmpty()) - result << int(e->text().at(0).unicode() + e->modifiers()); - return result; - } +QList QKeyMapper::possibleKeys(QKeyEvent *e) +{ + if (!e->nativeScanCode()) + return extractKeyFromEvent(e); return instance()->d_func()->possibleKeys(e); } @@ -132,11 +135,7 @@ QList QKeyMapperPrivate::possibleKeys(QKeyEvent *e) if (!result.isEmpty()) return result; - if (e->key() && (e->key() != Qt::Key_unknown)) - result << int(e->key() + e->modifiers()); - else if (!e->text().isEmpty()) - result << int(e->text().at(0).unicode() + e->modifiers()); - return result; + return extractKeyFromEvent(e); } QT_END_NAMESPACE diff --git a/src/gui/kernel/qkeysequence.cpp b/src/gui/kernel/qkeysequence.cpp index 13e6a058cb..5d52ba139d 100644 --- a/src/gui/kernel/qkeysequence.cpp +++ b/src/gui/kernel/qkeysequence.cpp @@ -869,6 +869,17 @@ QKeySequence::QKeySequence(int k1, int k2, int k3, int k4) d->key[3] = k4; } +/*! + Constructs a key sequence with up to 4 keys \a k1, \a k2, + \a k3 and \a k4. + + \sa QKeyCombination +*/ +QKeySequence::QKeySequence(QKeyCombination k1, QKeyCombination k2, QKeyCombination k3, QKeyCombination k4) + : QKeySequence(k1.toCombined(), k2.toCombined(), k3.toCombined(), k4.toCombined()) +{ +} + /*! Copy constructor. Makes a copy of \a keysequence. */ @@ -908,11 +919,11 @@ QKeySequence::~QKeySequence() delivery. */ -void QKeySequence::setKey(int key, int index) +void QKeySequence::setKey(QKeyCombination key, int index) { Q_ASSERT_X(index >= 0 && index < QKeySequencePrivate::MaxKeyCount, "QKeySequence::setKey", "index out of range"); qAtomicDetach(d); - d->key[index] = key; + d->key[index] = key.toCombined(); } static_assert(QKeySequencePrivate::MaxKeyCount == 4, "Change docs below"); @@ -967,7 +978,7 @@ QKeySequence QKeySequence::mnemonic(const QString &text) if (c.isPrint()) { if (!found) { c = c.toUpper(); - ret = QKeySequence(c.unicode() + Qt::ALT); + ret = QKeySequence(QKeyCombination(Qt::ALT, Qt::Key(c.unicode()))); #ifdef QT_NO_DEBUG return ret; #else @@ -1377,8 +1388,8 @@ QKeySequence::SequenceMatch QKeySequence::matches(const QKeySequence &seq) const SequenceMatch match = (userN == seqN ? ExactMatch : PartialMatch); for (uint i = 0; i < userN; ++i) { - int userKey = (*this)[i], - sequenceKey = seq[i]; + QKeyCombination userKey = (*this)[i], + sequenceKey = seq[i]; if (userKey != sequenceKey) return NoMatch; } @@ -1397,10 +1408,10 @@ QKeySequence::operator QVariant() const Returns a reference to the element at position \a index in the key sequence. This can only be used to read an element. */ -int QKeySequence::operator[](uint index) const +QKeyCombination QKeySequence::operator[](uint index) const { Q_ASSERT_X(index < QKeySequencePrivate::MaxKeyCount, "QKeySequence::operator[]", "index out of range"); - return d->key[index]; + return QKeyCombination::fromCombined(d->key[index]); } diff --git a/src/gui/kernel/qkeysequence.h b/src/gui/kernel/qkeysequence.h index 8b7ff12bed..880d0a581a 100644 --- a/src/gui/kernel/qkeysequence.h +++ b/src/gui/kernel/qkeysequence.h @@ -155,6 +155,10 @@ public: QKeySequence(); QKeySequence(const QString &key, SequenceFormat format = NativeText); QKeySequence(int k1, int k2 = 0, int k3 = 0, int k4 = 0); + QKeySequence(QKeyCombination k1, + QKeyCombination k2 = QKeyCombination::fromCombined(0), + QKeyCombination k3 = QKeyCombination::fromCombined(0), + QKeyCombination k4 = QKeyCombination::fromCombined(0)); QKeySequence(const QKeySequence &ks); QKeySequence(StandardKey key); ~QKeySequence(); @@ -179,7 +183,7 @@ public: static QList keyBindings(StandardKey key); operator QVariant() const; - int operator[](uint i) const; + QKeyCombination operator[](uint i) const; QKeySequence &operator=(const QKeySequence &other); QKeySequence &operator=(QKeySequence &&other) noexcept { swap(other); return *this; } void swap(QKeySequence &other) noexcept { qSwap(d, other.d); } @@ -201,7 +205,7 @@ private: static QString encodeString(int key); int assign(const QString &str); int assign(const QString &str, SequenceFormat format); - void setKey(int key, int index); + void setKey(QKeyCombination key, int index); QKeySequencePrivate *d; diff --git a/src/gui/kernel/qkeysequence_p.h b/src/gui/kernel/qkeysequence_p.h index 8c59505561..99d17e98d1 100644 --- a/src/gui/kernel/qkeysequence_p.h +++ b/src/gui/kernel/qkeysequence_p.h @@ -64,7 +64,7 @@ struct QKeyBinding { QKeySequence::StandardKey standardKey; uchar priority; - uint shortcut; + QKeyCombination shortcut; uint platform; }; diff --git a/src/gui/kernel/qplatformtheme.cpp b/src/gui/kernel/qplatformtheme.cpp index 81c897def7..5de1c98127 100644 --- a/src/gui/kernel/qplatformtheme.cpp +++ b/src/gui/kernel/qplatformtheme.cpp @@ -645,7 +645,7 @@ QList QPlatformTheme::keyBindings(QKeySequence::StandardKey key) c if (!(it->platform & platform)) continue; - uint shortcut = it->shortcut; + uint shortcut = it->shortcut.toCombined(); if (it->priority > 0) list.prepend(QKeySequence(shortcut)); diff --git a/src/gui/kernel/qshortcutmap.cpp b/src/gui/kernel/qshortcutmap.cpp index 4b6368a749..e79aaacd1a 100644 --- a/src/gui/kernel/qshortcutmap.cpp +++ b/src/gui/kernel/qshortcutmap.cpp @@ -543,12 +543,12 @@ void QShortcutMap::createNewSequences(QKeyEvent *e, QList &ksl, in curKsl.setKey(curSeq[2], 2); curKsl.setKey(curSeq[3], 3); } else { - curKsl.setKey(0, 0); - curKsl.setKey(0, 1); - curKsl.setKey(0, 2); - curKsl.setKey(0, 3); + curKsl.setKey(QKeyCombination::fromCombined(0), 0); + curKsl.setKey(QKeyCombination::fromCombined(0), 1); + curKsl.setKey(QKeyCombination::fromCombined(0), 2); + curKsl.setKey(QKeyCombination::fromCombined(0), 3); } - curKsl.setKey(possibleKeys.at(pkNum) & ~ignoredModifiers, index); + curKsl.setKey(QKeyCombination::fromCombined(possibleKeys.at(pkNum) & ~ignoredModifiers), index); } } } @@ -574,8 +574,8 @@ QKeySequence::SequenceMatch QShortcutMap::matches(const QKeySequence &seq1, : QKeySequence::PartialMatch); for (uint i = 0; i < userN; ++i) { - int userKey = seq1[i], - sequenceKey = seq2[i]; + int userKey = seq1[i].toCombined(), + sequenceKey = seq2[i].toCombined(); if ((userKey & Qt::Key_unknown) == Qt::Key_hyphen) userKey = (userKey & Qt::KeyboardModifierMask) | Qt::Key_Minus; if ((sequenceKey & Qt::Key_unknown) == Qt::Key_hyphen) -- cgit v1.2.3