QShortcutMap: Rename variables to clarify their use

The state machinery of QShortcutMap is complicated enough as it is,
so let's use better variable names. In particular, let's distinguish
the registered shortcuts (QShortcutEntries), from the candidate
QKeySequence sequences that we compute based on the incoming events.

Task-number: QTBUG-116873
Change-Id: I9bd59097e786ecfb9d241c2eb7b871a4bba9b44f
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
This commit is contained in:
Tor Arne Vestbø 2023-09-15 21:25:19 +02:00
parent 2496882ea7
commit 709c93083e

View File

@ -29,23 +29,23 @@ Q_LOGGING_CATEGORY(lcShortcutMap, "qt.gui.shortcutmap")
struct QShortcutEntry struct QShortcutEntry
{ {
QShortcutEntry() QShortcutEntry()
: keyseq(0), context(Qt::WindowShortcut), enabled(false), autorepeat(1), id(0), owner(nullptr), contextMatcher(nullptr) : keySequence(0), context(Qt::WindowShortcut), enabled(false), autorepeat(1), id(0), owner(nullptr), contextMatcher(nullptr)
{} {}
QShortcutEntry(const QKeySequence &k) QShortcutEntry(const QKeySequence &k)
: keyseq(k), context(Qt::WindowShortcut), enabled(false), autorepeat(1), id(0), owner(nullptr), contextMatcher(nullptr) : keySequence(k), context(Qt::WindowShortcut), enabled(false), autorepeat(1), id(0), owner(nullptr), contextMatcher(nullptr)
{} {}
QShortcutEntry(QObject *o, const QKeySequence &k, Qt::ShortcutContext c, int i, bool a, QShortcutMap::ContextMatcher m) QShortcutEntry(QObject *o, const QKeySequence &k, Qt::ShortcutContext c, int i, bool a, QShortcutMap::ContextMatcher m)
: keyseq(k), context(c), enabled(true), autorepeat(a), id(i), owner(o), contextMatcher(m) : keySequence(k), context(c), enabled(true), autorepeat(a), id(i), owner(o), contextMatcher(m)
{} {}
bool correctContext() const { return contextMatcher(owner, context); } bool correctContext() const { return contextMatcher(owner, context); }
bool operator<(const QShortcutEntry &f) const bool operator<(const QShortcutEntry &f) const
{ return keyseq < f.keyseq; } { return keySequence < f.keySequence; }
QKeySequence keyseq; QKeySequence keySequence;
Qt::ShortcutContext context; Qt::ShortcutContext context;
bool enabled : 1; bool enabled : 1;
bool autorepeat : 1; bool autorepeat : 1;
@ -88,7 +88,7 @@ public:
} }
QShortcutMap *q_ptr; // Private's parent QShortcutMap *q_ptr; // Private's parent
QList<QShortcutEntry> sequences; // All sequences! QList<QShortcutEntry> shortcuts; // All shortcuts!
int currentId; // Global shortcut ID number int currentId; // Global shortcut ID number
int ambigCount; // Index of last enabled ambiguous dispatch int ambigCount; // Index of last enabled ambiguous dispatch
@ -120,18 +120,18 @@ QShortcutMap::~QShortcutMap()
Adds a shortcut to the global map. Adds a shortcut to the global map.
Returns the id of the newly added shortcut. Returns the id of the newly added shortcut.
*/ */
int QShortcutMap::addShortcut(QObject *owner, const QKeySequence &key, Qt::ShortcutContext context, ContextMatcher matcher) int QShortcutMap::addShortcut(QObject *owner, const QKeySequence &keySequence, Qt::ShortcutContext context, ContextMatcher matcher)
{ {
Q_ASSERT_X(owner, "QShortcutMap::addShortcut", "All shortcuts need an owner"); Q_ASSERT_X(owner, "QShortcutMap::addShortcut", "All shortcuts need an owner");
Q_ASSERT_X(!key.isEmpty(), "QShortcutMap::addShortcut", "Cannot add keyless shortcuts to map"); Q_ASSERT_X(!keySequence.isEmpty(), "QShortcutMap::addShortcut", "Cannot add keyless shortcuts to map");
Q_D(QShortcutMap); Q_D(QShortcutMap);
QShortcutEntry newEntry(owner, key, context, --(d->currentId), true, matcher); QShortcutEntry newEntry(owner, keySequence, context, --(d->currentId), true, matcher);
const auto it = std::upper_bound(d->sequences.begin(), d->sequences.end(), newEntry); const auto it = std::upper_bound(d->shortcuts.begin(), d->shortcuts.end(), newEntry);
d->sequences.insert(it, newEntry); // Insert sorted d->shortcuts.insert(it, newEntry); // Insert sorted
qCDebug(lcShortcutMap).nospace() qCDebug(lcShortcutMap).nospace()
<< "QShortcutMap::addShortcut(" << owner << ", " << "QShortcutMap::addShortcut(" << owner << ", "
<< key << ", " << context << ") added shortcut with ID " << d->currentId; << keySequence << ", " << context << ") added shortcut with ID " << d->currentId;
return d->currentId; return d->currentId;
} }
@ -144,36 +144,36 @@ int QShortcutMap::addShortcut(QObject *owner, const QKeySequence &key, Qt::Short
Returns the number of sequences removed from the map. Returns the number of sequences removed from the map.
*/ */
int QShortcutMap::removeShortcut(int id, QObject *owner, const QKeySequence &key) int QShortcutMap::removeShortcut(int id, QObject *owner, const QKeySequence &keySequence)
{ {
Q_D(QShortcutMap); Q_D(QShortcutMap);
int itemsRemoved = 0; int itemsRemoved = 0;
bool allOwners = (owner == nullptr); bool allOwners = (owner == nullptr);
bool allKeys = key.isEmpty(); bool allKeys = keySequence.isEmpty();
bool allIds = id == 0; bool allIds = id == 0;
auto debug = qScopeGuard([&](){ auto debug = qScopeGuard([&](){
qCDebug(lcShortcutMap).nospace() qCDebug(lcShortcutMap).nospace()
<< "QShortcutMap::removeShortcut(" << id << ", " << owner << ", " << "QShortcutMap::removeShortcut(" << id << ", " << owner << ", "
<< key << ") removed " << itemsRemoved << " shortcuts(s)"; << keySequence << ") removed " << itemsRemoved << " shortcuts(s)";
}); });
// Special case, remove everything // Special case, remove everything
if (allOwners && allKeys && allIds) { if (allOwners && allKeys && allIds) {
itemsRemoved = d->sequences.size(); itemsRemoved = d->shortcuts.size();
d->sequences.clear(); d->shortcuts.clear();
return itemsRemoved; return itemsRemoved;
} }
int i = d->sequences.size()-1; int i = d->shortcuts.size()-1;
while (i>=0) while (i>=0)
{ {
const QShortcutEntry &entry = d->sequences.at(i); const QShortcutEntry &entry = d->shortcuts.at(i);
int entryId = entry.id; int entryId = entry.id;
if ((allOwners || entry.owner == owner) if ((allOwners || entry.owner == owner)
&& (allIds || entry.id == id) && (allIds || entry.id == id)
&& (allKeys || entry.keyseq == key)) { && (allKeys || entry.keySequence == keySequence)) {
d->sequences.removeAt(i); d->shortcuts.removeAt(i);
++itemsRemoved; ++itemsRemoved;
} }
if (id == entryId) if (id == entryId)
@ -191,22 +191,22 @@ int QShortcutMap::removeShortcut(int id, QObject *owner, const QKeySequence &key
are changed. are changed.
Returns the number of sequences which are matched in the map. Returns the number of sequences which are matched in the map.
*/ */
int QShortcutMap::setShortcutEnabled(bool enable, int id, QObject *owner, const QKeySequence &key) int QShortcutMap::setShortcutEnabled(bool enable, int id, QObject *owner, const QKeySequence &keySequence)
{ {
Q_D(QShortcutMap); Q_D(QShortcutMap);
int itemsChanged = 0; int itemsChanged = 0;
bool allOwners = (owner == nullptr); bool allOwners = (owner == nullptr);
bool allKeys = key.isEmpty(); bool allKeys = keySequence.isEmpty();
bool allIds = id == 0; bool allIds = id == 0;
int i = d->sequences.size()-1; int i = d->shortcuts.size()-1;
while (i>=0) while (i>=0)
{ {
QShortcutEntry entry = d->sequences.at(i); QShortcutEntry entry = d->shortcuts.at(i);
if ((allOwners || entry.owner == owner) if ((allOwners || entry.owner == owner)
&& (allIds || entry.id == id) && (allIds || entry.id == id)
&& (allKeys || entry.keyseq == key)) { && (allKeys || entry.keySequence == keySequence)) {
d->sequences[i].enabled = enable; d->shortcuts[i].enabled = enable;
++itemsChanged; ++itemsChanged;
} }
if (id == entry.id) if (id == entry.id)
@ -215,7 +215,7 @@ int QShortcutMap::setShortcutEnabled(bool enable, int id, QObject *owner, const
} }
qCDebug(lcShortcutMap).nospace() qCDebug(lcShortcutMap).nospace()
<< "QShortcutMap::setShortcutEnabled(" << enable << ", " << id << ", " << "QShortcutMap::setShortcutEnabled(" << enable << ", " << id << ", "
<< owner << ", " << key << ") = " << itemsChanged; << owner << ", " << keySequence << ") = " << itemsChanged;
return itemsChanged; return itemsChanged;
} }
@ -227,22 +227,22 @@ int QShortcutMap::setShortcutEnabled(bool enable, int id, QObject *owner, const
are changed. are changed.
Returns the number of sequences which are matched in the map. Returns the number of sequences which are matched in the map.
*/ */
int QShortcutMap::setShortcutAutoRepeat(bool on, int id, QObject *owner, const QKeySequence &key) int QShortcutMap::setShortcutAutoRepeat(bool on, int id, QObject *owner, const QKeySequence &keySequence)
{ {
Q_D(QShortcutMap); Q_D(QShortcutMap);
int itemsChanged = 0; int itemsChanged = 0;
bool allOwners = (owner == nullptr); bool allOwners = (owner == nullptr);
bool allKeys = key.isEmpty(); bool allKeys = keySequence.isEmpty();
bool allIds = id == 0; bool allIds = id == 0;
int i = d->sequences.size()-1; int i = d->shortcuts.size()-1;
while (i>=0) while (i>=0)
{ {
QShortcutEntry entry = d->sequences.at(i); QShortcutEntry entry = d->shortcuts.at(i);
if ((allOwners || entry.owner == owner) if ((allOwners || entry.owner == owner)
&& (allIds || entry.id == id) && (allIds || entry.id == id)
&& (allKeys || entry.keyseq == key)) { && (allKeys || entry.keySequence == keySequence)) {
d->sequences[i].autorepeat = on; d->shortcuts[i].autorepeat = on;
++itemsChanged; ++itemsChanged;
} }
if (id == entry.id) if (id == entry.id)
@ -251,7 +251,7 @@ int QShortcutMap::setShortcutAutoRepeat(bool on, int id, QObject *owner, const Q
} }
qCDebug(lcShortcutMap).nospace() qCDebug(lcShortcutMap).nospace()
<< "QShortcutMap::setShortcutAutoRepeat(" << on << ", " << id << ", " << "QShortcutMap::setShortcutAutoRepeat(" << on << ", " << id << ", "
<< owner << ", " << key << ") = " << itemsChanged; << owner << ", " << keySequence << ") = " << itemsChanged;
return itemsChanged; return itemsChanged;
} }
@ -365,11 +365,12 @@ bool QShortcutMap::hasShortcutForKeySequence(const QKeySequence &seq) const
{ {
Q_D(const QShortcutMap); Q_D(const QShortcutMap);
QShortcutEntry entry(seq); // needed for searching QShortcutEntry entry(seq); // needed for searching
const auto itEnd = d->sequences.cend(); const auto itEnd = d->shortcuts.cend();
auto it = std::lower_bound(d->sequences.cbegin(), itEnd, entry); auto it = std::lower_bound(d->shortcuts.cbegin(), itEnd, entry);
for (;it != itEnd; ++it) { for (;it != itEnd; ++it) {
if (matches(entry.keyseq, (*it).keyseq) == QKeySequence::ExactMatch && (*it).correctContext() && (*it).enabled) { if (matches(entry.keySequence, (*it).keySequence) == QKeySequence::ExactMatch
&& (*it).correctContext() && (*it).enabled) {
return true; return true;
} }
} }
@ -388,7 +389,7 @@ bool QShortcutMap::hasShortcutForKeySequence(const QKeySequence &seq) const
QKeySequence::SequenceMatch QShortcutMap::find(QKeyEvent *e, int ignoredModifiers) QKeySequence::SequenceMatch QShortcutMap::find(QKeyEvent *e, int ignoredModifiers)
{ {
Q_D(QShortcutMap); Q_D(QShortcutMap);
if (!d->sequences.size()) if (!d->shortcuts.size())
return QKeySequence::NoMatch; return QKeySequence::NoMatch;
createNewSequences(e, d->newEntries, ignoredModifiers); createNewSequences(e, d->newEntries, ignoredModifiers);
@ -410,15 +411,15 @@ QKeySequence::SequenceMatch QShortcutMap::find(QKeyEvent *e, int ignoredModifier
QKeySequence::SequenceMatch result = QKeySequence::NoMatch; QKeySequence::SequenceMatch result = QKeySequence::NoMatch;
for (int i = d->newEntries.size()-1; i >= 0 ; --i) { for (int i = d->newEntries.size()-1; i >= 0 ; --i) {
QShortcutEntry entry(d->newEntries.at(i)); // needed for searching QShortcutEntry entry(d->newEntries.at(i)); // needed for searching
qCDebug(lcShortcutMap) << "Looking for shortcuts matching" << entry.keyseq; qCDebug(lcShortcutMap) << "Looking for shortcuts matching" << entry.keySequence;
QKeySequence::SequenceMatch bestMatchForEntry = QKeySequence::NoMatch; QKeySequence::SequenceMatch bestMatchForEntry = QKeySequence::NoMatch;
const auto itEnd = d->sequences.constEnd(); const auto itEnd = d->shortcuts.constEnd();
auto it = std::lower_bound(d->sequences.constBegin(), itEnd, entry); auto it = std::lower_bound(d->shortcuts.constBegin(), itEnd, entry);
for (; it != itEnd; ++it) { for (; it != itEnd; ++it) {
QKeySequence::SequenceMatch match = matches(entry.keyseq, (*it).keyseq); QKeySequence::SequenceMatch match = matches(entry.keySequence, (*it).keySequence);
qCDebug(lcShortcutMap) << " -" << match << "for shortcut" << it->keyseq; qCDebug(lcShortcutMap) << " -" << match << "for shortcut" << it->keySequence;
// If we got a valid match, there might still be more keys to check against, // If we got a valid match, there might still be more keys to check against,
// but if we get no match, we know that there are no more possible matches. // but if we get no match, we know that there are no more possible matches.
@ -595,7 +596,7 @@ void QShortcutMap::dispatchEvent(QKeyEvent *e)
if (!d->identicals.size()) if (!d->identicals.size())
return; return;
const QKeySequence &curKey = d->identicals.at(0)->keyseq; const QKeySequence &curKey = d->identicals.at(0)->keySequence;
if (d->prevSequence != curKey) { if (d->prevSequence != curKey) {
d->ambigCount = 0; d->ambigCount = 0;
d->prevSequence = curKey; d->prevSequence = curKey;
@ -626,15 +627,15 @@ void QShortcutMap::dispatchEvent(QKeyEvent *e)
if (ambiguousShortcuts.size() > 1) { if (ambiguousShortcuts.size() > 1) {
qCDebug(lcShortcutMap) << "The following shortcuts are about to be activated ambiguously:"; qCDebug(lcShortcutMap) << "The following shortcuts are about to be activated ambiguously:";
for (const QShortcutEntry *entry : std::as_const(ambiguousShortcuts)) for (const QShortcutEntry *entry : std::as_const(ambiguousShortcuts))
qCDebug(lcShortcutMap).nospace() << "- " << entry->keyseq << " (belonging to " << entry->owner << ")"; qCDebug(lcShortcutMap).nospace() << "- " << entry->keySequence << " (belonging to " << entry->owner << ")";
} }
qCDebug(lcShortcutMap).nospace() qCDebug(lcShortcutMap).nospace()
<< "QShortcutMap::dispatchEvent(): Sending QShortcutEvent(\"" << "QShortcutMap::dispatchEvent(): Sending QShortcutEvent(\""
<< next->keyseq.toString() << "\", " << next->id << ", " << next->keySequence.toString() << "\", " << next->id << ", "
<< static_cast<bool>(enabledShortcuts>1) << ") to object(" << next->owner << ')'; << static_cast<bool>(enabledShortcuts>1) << ") to object(" << next->owner << ')';
} }
QShortcutEvent se(next->keyseq, next->id, enabledShortcuts>1); QShortcutEvent se(next->keySequence, next->id, enabledShortcuts > 1);
QCoreApplication::sendEvent(const_cast<QObject *>(next->owner), &se); QCoreApplication::sendEvent(const_cast<QObject *>(next->owner), &se);
} }
@ -642,7 +643,7 @@ QList<QKeySequence> QShortcutMap::keySequences(bool getAll) const
{ {
Q_D(const QShortcutMap); Q_D(const QShortcutMap);
QList<QKeySequence> keys; QList<QKeySequence> keys;
for (auto sequence : d->sequences) { for (auto sequence : d->shortcuts) {
bool addSequence = false; bool addSequence = false;
if (sequence.enabled) { if (sequence.enabled) {
if (getAll || sequence.context == Qt::ApplicationShortcut || if (getAll || sequence.context == Qt::ApplicationShortcut ||
@ -671,7 +672,7 @@ QList<QKeySequence> QShortcutMap::keySequences(bool getAll) const
} }
} }
if (addSequence) if (addSequence)
keys << sequence.keyseq; keys << sequence.keySequence;
} }
} }
return keys; return keys;
@ -686,8 +687,8 @@ QList<QKeySequence> QShortcutMap::keySequences(bool getAll) const
void QShortcutMap::dumpMap() const void QShortcutMap::dumpMap() const
{ {
Q_D(const QShortcutMap); Q_D(const QShortcutMap);
for (int i = 0; i < d->sequences.size(); ++i) for (int i = 0; i < d->shortcuts.size(); ++i)
qDebug().nospace() << &(d->sequences.at(i)); qDebug().nospace() << &(d->shortcuts.at(i));
} }
#endif #endif