diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-10 08:53:13 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-10 08:53:13 +0200 |
| commit | 91311854173c06b5007302341fcf1840585d9d37 (patch) | |
| tree | 0dfc0d8c8fc2d4fe5622870bcfd91a5d20d8f8d7 | |
| parent | b59b9ec616fb5c965cfd32385879162989589bb6 (diff) | |
| download | qtmaildir-91311854173c06b5007302341fcf1840585d9d37.tar.gz qtmaildir-91311854173c06b5007302341fcf1840585d9d37.zip | |
feat(ui): let the user choose newest or oldest first
Two entries, straight to notmuch. This adds a feature rather than replacing one:
the column header was decorative and nothing implemented click-to-sort, so
removing the header with the grid lost nothing.
Stored in uistate.conf, never in the hand-edited config, and range-guarded on
read: a stale file can hold anything, which is the lesson item 58 recorded.
SortOrder needed qRegisterMetaType despite carrying Q_ENUM. Q_ENUM gives the
enum a meta-object entry, not a metatype registered under the name invokeMethod
resolves, so the queued runQuery would have dropped its sort argument at runtime
and every query would have silently run newest-first. Nothing in the suite
exercises a real worker thread, so this was asserted directly rather than left
to a warning nobody would see. It is registered beside the type rather than in
MainWindow's constructor: a first attempt put it there and passed only because
the test that catches it never constructs a MainWindow.
The account dropdown's entries now carry their account's colour as a swatch,
which is what makes the accent bar on a card mean anything: a colour down a
card's edge says nothing until something maps it to a name. Raw colour here
rather than the blended line colour, since a swatch is a filled patch like a
chip rather than a thin line. Its test builds its own two-account config: reading
the environment's made it SKIP wherever no accounts are configured, which is a
test that asserts nothing while reporting success.
| -rw-r--r-- | src/mainwindow.cpp | 42 | ||||
| -rw-r--r-- | src/mainwindow.h | 1 | ||||
| -rw-r--r-- | src/notmuchworker.cpp | 16 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 90 | ||||
| -rw-r--r-- | tests/test_notmuchworker.cpp | 22 |
5 files changed, 169 insertions, 2 deletions
diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 57a6988..5881fae 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -163,6 +163,12 @@ void MainWindow::restoreUiState() // by CardDelegate, so there are no widths to restore; a blob saved by an // older version is simply ignored (item 53's Upgrading note). + // Range-guarded on read: a stale or hand-edited file can hold anything, + // and setCurrentIndex() on a value with no row silently selects nothing. + const int sort = + state.value(QStringLiteral("threadlist/sortOrder"), 0).toInt(); + m_sortOrder->setCurrentIndex(sort == 1 ? 1 : 0); + // The config value is the starting point for a profile that has never // zoomed; once the user does, the state file is what they last had. // clampZoom() rejects the garbage a hand-edited file can hold. @@ -178,6 +184,8 @@ void MainWindow::saveUiState() const state.setValue(QStringLiteral("window/geometry"), saveGeometry()); state.setValue(QStringLiteral("window/state"), saveState()); state.setValue(QStringLiteral("window/splitter"), m_splitter->saveState()); + state.setValue(QStringLiteral("threadlist/sortOrder"), + m_sortOrder->currentIndex()); state.setValue(QStringLiteral("message/zoom"), m_messageView->zoomFactor()); } @@ -419,9 +427,34 @@ void MainWindow::buildUi() // Query row. auto *queryRow = new QHBoxLayout; m_accountBox = new QComboBox(central); + m_accountBox->setObjectName(QStringLiteral("accountBox")); m_accountBox->addItem(tr("All accounts"), QString()); - for (const Account &account : m_config.accounts()) + for (const Account &account : m_config.accounts()) { m_accountBox->addItem(account.key, account.key); + // The RAW account colour here, not CardDelegate's blended line colour: + // a swatch is a filled patch like a chip, not a thin line, so it wants + // the colour the account was actually given. Qt renders a + // DecorationRole colour as a swatch itself, with no delegate. + // + // This is what makes the accent bar on a card mean anything: a colour + // down a card's edge says nothing until something maps it to a name. + m_accountBox->setItemData( + m_accountBox->count() - 1, + m_tagColors.colourFor(TagColors::tagForAccountKey(account.key)), + Qt::DecorationRole); + } + + // Sort order. Two entries, straight to notmuch: this ADDS a feature rather + // than replacing one, since the old column header was decorative and + // nothing implemented click-to-sort. + m_sortOrder = new QComboBox(central); + m_sortOrder->setObjectName(QStringLiteral("sortOrder")); + // Order matters: the index is what uistate.conf stores. + m_sortOrder->addItem(tr("Newest first")); + m_sortOrder->addItem(tr("Oldest first")); + m_sortOrder->setToolTip(tr("The order threads are listed in")); + connect(m_sortOrder, &QComboBox::currentIndexChanged, + this, &MainWindow::runCurrentQuery); m_queryEdit = new QLineEdit(central); m_queryEdit->setPlaceholderText(tr("notmuch query, e.g. tag:inbox")); @@ -510,6 +543,7 @@ void MainWindow::buildUi() // would squeeze the field, but three is the real-world case today. Item 23 // already specifies buttons-plus-menu and is where that belongs. queryRow->addWidget(m_accountBox); + queryRow->addWidget(m_sortOrder); queryRow->addWidget(m_queryEdit, 1); for (const SavedQuery &saved : m_config.savedQueries()) { auto *button = new QPushButton(saved.name, central); @@ -1465,9 +1499,13 @@ void MainWindow::runCurrentQuery() m_queryComplete = false; updateViewWideActions(); + const auto sort = m_sortOrder->currentIndex() == 1 + ? NotmuchWorker::OldestFirst + : NotmuchWorker::NewestFirst; QMetaObject::invokeMethod(m_worker, "runQuery", Qt::QueuedConnection, Q_ARG(QString, query), - Q_ARG(quint64, m_generation)); + Q_ARG(quint64, m_generation), + Q_ARG(NotmuchWorker::SortOrder, sort)); } void MainWindow::onThreadsReady(const QVector<ThreadSummary> &threads, diff --git a/src/mainwindow.h b/src/mainwindow.h index 6b9e557..ccac5ad 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -468,6 +468,7 @@ private: /// placeholder's own text wraps every couple of words. static constexpr int kMinMessagePaneWidth = 300; QComboBox *m_accountBox = nullptr; + QComboBox *m_sortOrder = nullptr; QLabel *m_statusLabel = nullptr; /// Expires a transient status message. See showTransientStatus(). diff --git a/src/notmuchworker.cpp b/src/notmuchworker.cpp index a6b0a29..d34c032 100644 --- a/src/notmuchworker.cpp +++ b/src/notmuchworker.cpp @@ -119,9 +119,25 @@ void walkReplies(notmuch_messages_t *messages, int depth, } // namespace +/// Registers SortOrder for queued calls, once, before main() runs. +/// +/// Q_ENUM alone is NOT enough for a queued Q_ARG: it gives the enum a +/// meta-object entry, not a metatype registered under the name invokeMethod +/// resolves, so MainWindow's queued runQuery would drop its sort argument at +/// runtime with a warning and every query would silently run newest-first. +/// +/// Here rather than in MainWindow's constructor, because the registration +/// belongs to the type rather than to one consumer: a caller that never +/// constructs a MainWindow (a test, or a future headless mode) needs it too, +/// and that is exactly how the first attempt at this passed by accident and +/// failed under test. +static const int kSortOrderMetaType = + qRegisterMetaType<NotmuchWorker::SortOrder>("NotmuchWorker::SortOrder"); + NotmuchWorker::NotmuchWorker(const QString ¬muchConfigPath, QObject *parent) : QObject(parent), m_configPath(notmuchConfigPath) { + Q_UNUSED(kSortOrderMetaType); } NotmuchWorker::~NotmuchWorker() diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index b3511ec..cdb08ca 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -51,6 +51,7 @@ #include <QImage> #include <QPainter> +#include <QComboBox> #include <QScrollBar> #include "tagchip.h" #include "threadlistmodel.h" @@ -107,6 +108,8 @@ private slots: void nextThreadLeavesTheLastReply(); void altDownSkipsReplies(); void bothThreadStepBindingsReachTheAction(); + void sortChoiceSurvivesRestart(); + void accountEntriesCarryTheirColour(); void replyRowsKeepTheirTextUnderTheThreadLine(); void clickingTheExpanderTogglesTheThread(); void selectingAMessageRowTargetsThatMessageNotItsThread(); @@ -773,6 +776,93 @@ void TestMainWindow::bothThreadStepBindingsReachTheAction() } } +void TestMainWindow::sortChoiceSurvivesRestart() +{ + QStandardPaths::setTestModeEnabled(true); + QFile::remove(MainWindow::uiStatePath()); + + { + const Config config; + MainWindow window(config); + auto *sort = window.findChild<QComboBox *>(QStringLiteral("sortOrder")); + QVERIFY(sort); + QCOMPARE(sort->count(), 2); + QCOMPARE(sort->currentIndex(), 0); // Newest first by default. + sort->setCurrentIndex(1); + window.close(); + } + + const Config config; + MainWindow second(config); + auto *sort = second.findChild<QComboBox *>(QStringLiteral("sortOrder")); + QVERIFY(sort); + QCOMPARE(sort->currentIndex(), 1); + + // A stale or hand-edited file can hold anything, which is the lesson item + // 58 recorded: an out-of-range value must fall back rather than select a + // row that does not exist. + { + QSettings state(MainWindow::uiStatePath(), QSettings::IniFormat); + state.setValue(QStringLiteral("threadlist/sortOrder"), 47); + } + MainWindow third(config); + auto *thirdSort = third.findChild<QComboBox *>(QStringLiteral("sortOrder")); + QCOMPARE(thirdSort->currentIndex(), 0); + + QFile::remove(MainWindow::uiStatePath()); + QStandardPaths::setTestModeEnabled(false); +} + +void TestMainWindow::accountEntriesCarryTheirColour() +{ + // Its own config, not the environment's. Reading the real one made this + // SKIP wherever no accounts are configured, which is a test that asserts + // nothing while reporting success. + QTemporaryDir dir; + const QString path = dir.filePath(QStringLiteral("qtmaildir.conf")); + { + QSettings s(path, QSettings::IniFormat); + s.beginGroup(QStringLiteral("account.work")); + s.setValue(QStringLiteral("maildir"), QStringLiteral("work")); + s.setValue(QStringLiteral("color"), QStringLiteral("#3d7fd1")); + s.endGroup(); + s.beginGroup(QStringLiteral("account.personal")); + s.setValue(QStringLiteral("maildir"), QStringLiteral("personal")); + s.endGroup(); + } + + Config config; + config.load(path); + QCOMPARE(config.accounts().size(), 2); + + MainWindow window(config); + auto *box = window.findChild<QComboBox *>(QStringLiteral("accountBox")); + QVERIFY(box); + QCOMPARE(box->count(), 3); + + // "All accounts" is not an account and carries no swatch. + QVERIFY(!box->itemData(0, Qt::DecorationRole).isValid()); + + // Every real account does, including the one with no color= key: + // colourFor() never fails, deriving a stable colour from the tag name, so + // adding an account and forgetting to colour it degrades to something + // usable rather than to nothing. + QSet<QRgb> seen; + for (int i = 1; i < box->count(); ++i) { + const QVariant swatch = box->itemData(i, Qt::DecorationRole); + QVERIFY2(swatch.isValid(), + qPrintable(QStringLiteral("account %1 carries no swatch") + .arg(box->itemText(i)))); + const QColor colour = swatch.value<QColor>(); + QVERIFY(colour.isValid()); + seen.insert(colour.rgb()); + } + + // Guard: two accounts sharing one colour would make the swatches useless + // as a key to the accent bars, and would let a broken lookup pass. + QCOMPARE(seen.size(), 2); +} + void TestMainWindow::cardsNeverScrollSideways() { const Config config; diff --git a/tests/test_notmuchworker.cpp b/tests/test_notmuchworker.cpp index 3342013..88dcf0b 100644 --- a/tests/test_notmuchworker.cpp +++ b/tests/test_notmuchworker.cpp @@ -40,6 +40,7 @@ private slots: void unreadableConfigEmitsError(); void queryPassesGenerationThrough(); void oldestFirstReversesTheOrder(); + void theSortOrderCrossesAQueuedCall(); void loadThreadReturnsMessagesOldestFirst(); void loadThreadMarksMatchedMessages(); @@ -346,6 +347,27 @@ void TestNotmuchWorker::oldestFirstReversesTheOrder() QCOMPARE(oldest.last().threadId, newest.first().threadId); } +void TestNotmuchWorker::theSortOrderCrossesAQueuedCall() +{ + // MainWindow reaches the worker with invokeMethod(..., QueuedConnection) + // across a thread boundary, and a Q_ARG whose type the meta-object system + // does not know FAILS AT RUNTIME with a warning, not at compile time. So + // the enum's registration is asserted here rather than assumed from Q_ENUM. + QVERIFY2(QMetaType::fromName("NotmuchWorker::SortOrder").isValid(), + "SortOrder is not a registered metatype, so the queued runQuery " + "call will drop its sort argument at runtime"); + + NotmuchWorker worker(m_fixture.configPath()); + QSignalSpy ready(&worker, &NotmuchWorker::threadsReady); + + // The real call shape, invoked by NAME exactly as MainWindow does. + QVERIFY(QMetaObject::invokeMethod( + &worker, "runQuery", Qt::DirectConnection, + Q_ARG(QString, QStringLiteral("*")), Q_ARG(quint64, 1), + Q_ARG(NotmuchWorker::SortOrder, NotmuchWorker::OldestFirst))); + QCOMPARE(ready.size(), 1); +} + void TestNotmuchWorker::loadThreadReturnsMessagesOldestFirst() { const QString threadId = threadIdOf(QStringLiteral("Release notes")); |
