diff options
| -rw-r--r-- | src/keymap.cpp | 18 | ||||
| -rw-r--r-- | tests/test_keymap.cpp | 47 |
2 files changed, 65 insertions, 0 deletions
diff --git a/src/keymap.cpp b/src/keymap.cpp index 6e43f1b..205d2f5 100644 --- a/src/keymap.cpp +++ b/src/keymap.cpp @@ -48,6 +48,14 @@ void KeyMap::loadOverrides(QSettings &settings) { const QStringList known = knownActions(); + // Sequences bound so far *within this override pass*. Defaults already + // sit in m_bindings before this runs, so a plain m_bindings.contains() + // check would misfire on every legitimate override of a default (e.g. + // "j=archive" overriding the default 'j' binding). Only a collision + // between two entries in this same pass (e.g. two INI keys that + // normalize to the same QKeySequence, such as "y" and "Y") is a bug. + QHash<QKeySequence, QString> seenThisPass; + settings.beginGroup(QStringLiteral("keys")); const QStringList keys = settings.childKeys(); for (const QString &key : keys) { @@ -71,6 +79,16 @@ void KeyMap::loadOverrides(QSettings &settings) continue; } + const auto previous = seenThisPass.constFind(sequence); + if (previous != seenThisPass.constEnd()) { + m_warnings.append( + QStringLiteral("Key sequence '%1' bound to both '%2' and '%3' " + "in [keys]; keeping '%2'") + .arg(key, previous.value(), action)); + continue; + } + seenThisPass.insert(sequence, action); + m_bindings.insert(sequence, action); } settings.endGroup(); diff --git a/tests/test_keymap.cpp b/tests/test_keymap.cpp index 10863f2..229fba6 100644 --- a/tests/test_keymap.cpp +++ b/tests/test_keymap.cpp @@ -13,6 +13,7 @@ private slots: void chordSequenceParses(); void unknownActionIsReported(); void invalidSequenceIsReported(); + void collidingOverridesAreReported(); }; void TestKeyMap::defaultsAreLoaded() @@ -127,5 +128,51 @@ void TestKeyMap::invalidSequenceIsReported() QCOMPARE(map.warnings().size(), 1); } +void TestKeyMap::collidingOverridesAreReported() +{ + // "y" and "Y" both normalize to the same QKeySequence ("Y"), so binding + // both in [keys] is a genuine collision that must not silently drop one. + { + QTemporaryDir dir; + const QString path = dir.filePath(QStringLiteral("t.conf")); + { + QSettings s(path, QSettings::IniFormat); + s.beginGroup(QStringLiteral("keys")); + s.setValue(QStringLiteral("y"), QStringLiteral("archive")); + s.setValue(QStringLiteral("Y"), QStringLiteral("delete")); + s.endGroup(); + } + + KeyMap map; + QSettings s(path, QSettings::IniFormat); + map.loadOverrides(s); + + QCOMPARE(map.warnings().size(), 1); + QVERIFY(map.warnings().first().contains(QStringLiteral("archive"))); + QVERIFY(map.warnings().first().contains(QStringLiteral("delete"))); + } + + // Overriding a default is not a collision: loadDefaults() puts 'j' in + // the map first, then the override pass rebinds the same 'j'. That must + // stay silent (regression check for iniOverridesDefault's scenario). + { + QTemporaryDir dir; + const QString path = dir.filePath(QStringLiteral("t.conf")); + { + QSettings s(path, QSettings::IniFormat); + s.beginGroup(QStringLiteral("keys")); + s.setValue(QStringLiteral("j"), QStringLiteral("archive")); + s.endGroup(); + } + + KeyMap map; + map.loadDefaults(); + QSettings s(path, QSettings::IniFormat); + map.loadOverrides(s); + + QCOMPARE(map.warnings().size(), 0); + } +} + QTEST_MAIN(TestKeyMap) #include "test_keymap.moc" |