Skip to content

Commit fc29fb2

Browse files
committed
Fixed code quality issues
1 parent a1f8e48 commit fc29fb2

File tree

5 files changed

+121
-6
lines changed

5 files changed

+121
-6
lines changed

scripts/build-wasm.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ set -e
55
# IMPORTANT: Change this path to match your emsdk installation location
66
source "$HOME/dev/emsdk/emsdk_env.sh"
77

8-
# Paths - automatically detect script location
9-
MMAPPER_SRC="$(cd "$(dirname "$0")" && pwd)"
8+
# Paths - automatically detect project root (parent of scripts directory)
9+
MMAPPER_SRC="$(cd "$(dirname "$0")/.." && pwd)"
1010
QT_WASM="$MMAPPER_SRC/6.5.3/wasm_multithread"
1111
QT_HOST="$MMAPPER_SRC/6.5.3/macos"
1212

src/configuration/HotkeyManager.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,7 @@ QString HotkeyManager::normalizeKeyString(const QString &keyString)
253253
QStringList parts = keyString.split('+', Qt::SkipEmptyParts);
254254

255255
if (parts.isEmpty()) {
256+
qWarning() << "HotkeyManager: empty or invalid key string:" << keyString;
256257
return QString();
257258
}
258259

@@ -279,13 +280,16 @@ QString HotkeyManager::normalizeKeyString(const QString &keyString)
279280
hasAlt = true;
280281
} else if (upperPart == "META" || upperPart == "CMD" || upperPart == "COMMAND") {
281282
hasMeta = true;
283+
} else {
284+
qWarning() << "HotkeyManager: unrecognized modifier:" << part << "in:" << keyString;
282285
}
283286
}
284287

285288
// Validate the base key
286289
QString upperBaseKey = baseKey.toUpper();
287290
if (!isValidBaseKey(upperBaseKey)) {
288-
return QString(); // Invalid key name
291+
qWarning() << "HotkeyManager: invalid base key:" << baseKey << "in:" << keyString;
292+
return QString();
289293
}
290294

291295
// Add modifiers in canonical order

src/preferences/clientpage.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -128,9 +128,8 @@ ClientPage::ClientPage(QWidget *parent)
128128
});
129129

130130
connect(ui->commandSeparatorLineEdit, &QLineEdit::textChanged, this, [](const QString &text) {
131-
if (!text.isEmpty()) {
132-
setConfig().integratedClient.commandSeparator = text;
133-
}
131+
// Keep config in sync with the UI, including when the separator is cleared
132+
setConfig().integratedClient.commandSeparator = text;
134133
});
135134

136135
ui->commandSeparatorLineEdit->setValidator(new CustomSeparatorValidator(this));

tests/TestHotkeyManager.cpp

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55

66
#include "../src/configuration/HotkeyManager.h"
77

8+
#include <QSettings>
9+
#include <QStringList>
810
#include <QtTest/QtTest>
911

1012
TestHotkeyManager::TestHotkeyManager() = default;
@@ -372,4 +374,111 @@ void TestHotkeyManager::invalidKeyValidationTest()
372374
QVERIFY(manager.hasHotkey("HYPHEN"));
373375
}
374376

377+
void TestHotkeyManager::duplicateKeyBehaviorTest()
378+
{
379+
HotkeyManager manager;
380+
381+
// Test that duplicate keys in imported content use the last definition
382+
QString contentWithDuplicates = "_hotkey F1 first\n"
383+
"_hotkey F2 middle\n"
384+
"_hotkey F1 second\n";
385+
386+
manager.importFromCliFormat(contentWithDuplicates);
387+
388+
// getCommand should return the last definition
389+
QCOMPARE(manager.getCommand("F1"), QString("second"));
390+
QCOMPARE(manager.getCommand("F2"), QString("middle"));
391+
392+
// Test that setHotkey replaces existing entry
393+
manager.importFromCliFormat("_hotkey F1 original\n");
394+
QCOMPARE(manager.getCommand("F1"), QString("original"));
395+
QCOMPARE(manager.getAllHotkeys().size(), 1);
396+
397+
manager.setHotkey("F1", "replaced");
398+
QCOMPARE(manager.getCommand("F1"), QString("replaced"));
399+
QCOMPARE(manager.getAllHotkeys().size(), 1); // Still 1, not 2
400+
}
401+
402+
void TestHotkeyManager::commentPreservationTest()
403+
{
404+
HotkeyManager manager;
405+
406+
// Test that comments and formatting survive import/export round trip
407+
const QString cliFormat = "# Leading comment\n"
408+
"\n"
409+
"# Section header\n"
410+
"_hotkey F1 open\n"
411+
"\n"
412+
"# Another comment\n"
413+
"_hotkey F2 close\n";
414+
415+
manager.importFromCliFormat(cliFormat);
416+
const QString exported = manager.exportToCliFormat();
417+
418+
// Verify comments are preserved in export
419+
QVERIFY(exported.contains("# Leading comment"));
420+
QVERIFY(exported.contains("# Section header"));
421+
QVERIFY(exported.contains("# Another comment"));
422+
423+
// Verify hotkeys are still correct
424+
QVERIFY(exported.contains("_hotkey F1 open"));
425+
QVERIFY(exported.contains("_hotkey F2 close"));
426+
427+
// Verify order is preserved (comments before their hotkeys)
428+
int posLeading = exported.indexOf("# Leading comment");
429+
int posSection = exported.indexOf("# Section header");
430+
int posF1 = exported.indexOf("_hotkey F1");
431+
int posAnother = exported.indexOf("# Another comment");
432+
int posF2 = exported.indexOf("_hotkey F2");
433+
434+
QVERIFY(posLeading < posSection);
435+
QVERIFY(posSection < posF1);
436+
QVERIFY(posF1 < posAnother);
437+
QVERIFY(posAnother < posF2);
438+
}
439+
440+
void TestHotkeyManager::settingsPersistenceTest()
441+
{
442+
// Use a unique organization/app name to avoid conflicts with real settings
443+
const QString testOrg = "MMapperTest";
444+
const QString testApp = "HotkeyManagerTest";
445+
446+
// Clean up any existing test settings
447+
QSettings cleanupSettings(testOrg, testApp);
448+
cleanupSettings.clear();
449+
cleanupSettings.sync();
450+
451+
{
452+
// Create a manager and set some hotkeys
453+
HotkeyManager manager;
454+
manager.importFromCliFormat("# Test config\n"
455+
"_hotkey F1 look\n"
456+
"_hotkey CTRL+F2 attack\n");
457+
458+
QCOMPARE(manager.getCommand("F1"), QString("look"));
459+
QCOMPARE(manager.getCommand("CTRL+F2"), QString("attack"));
460+
461+
// Save to settings
462+
manager.saveToSettings();
463+
}
464+
465+
{
466+
// Create a new manager and load from settings
467+
HotkeyManager manager;
468+
manager.loadFromSettings();
469+
470+
// Verify hotkeys were persisted
471+
QCOMPARE(manager.getCommand("F1"), QString("look"));
472+
QCOMPARE(manager.getCommand("CTRL+F2"), QString("attack"));
473+
474+
// Verify comment was preserved
475+
QString exported = manager.exportToCliFormat();
476+
QVERIFY(exported.contains("# Test config"));
477+
}
478+
479+
// Clean up test settings
480+
cleanupSettings.clear();
481+
cleanupSettings.sync();
482+
}
483+
375484
QTEST_MAIN(TestHotkeyManager)

tests/TestHotkeyManager.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,4 +24,7 @@ private Q_SLOTS:
2424
void removeHotkeyTest();
2525
void hasHotkeyTest();
2626
void invalidKeyValidationTest();
27+
void duplicateKeyBehaviorTest();
28+
void commentPreservationTest();
29+
void settingsPersistenceTest();
2730
};

0 commit comments

Comments
 (0)