From aecb22de0c6644380ea3b1e909c864fd24501dc7 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Sat, 15 Aug 2026 11:39:11 +0200 Subject: fix(startup): let startup_query name a built-in filter, and run it Two defects, both reachable only after item 93. startupSavedQuery() searched the saved queries alone, so a startup_query of "Inbox" matched nothing once Inbox shipped as a built-in filter and the duplicated entry was removed from queries.json. It then fell back to m_savedQueries.first(), which is an arbitrary choice that used to look reasonable while every install carried an Inbox entry: with the duplicates gone it opened on a leftover search for one sender, and an empty queries.json opened on nothing at all. The search now covers the saved queries first, so the user's own entry wins a name collision, then the built-in filters; the fallback is the Unread filter, which is always present. The default startup name has always been "Unread" and now resolves for the first time: before this it named nothing unless the user happened to have such an entry. The constructor also read startup.query directly, and a generated entry stores no query at all, so even a matching filter opened an empty bar. It resolves through Config::resolvedQuery() now, unscoped, since the account dropdown starts on "All accounts". Icons per the user's choices: a star for Important rather than the flag action's own icon, since on the query row an icon reads as a category rather than as an instruction, and mail-folder-sent rather than mail-sent. Three tests changed rather than adapted, because their premises were the defect. Two asserted the first-saved-query fallback. aCronSyncDoesNotRefreshBeforeAnyQueryHasRun assumed a fresh window had run no query, which is no longer true; it is now aCronSyncRefreshesTheLastRunQueryNotTheQueryBar and asserts the property that actually matters on a cron timer, through a new lastRunQueryForTesting() seam, since a legitimate refresh bumps the generation and the counter cannot tell the two apart. --- src/config.cpp | 27 ++++++++++++++++++++------- src/mainwindow.cpp | 27 +++++++++++++++++++++------ src/mainwindow.h | 8 ++++++++ 3 files changed, 49 insertions(+), 13 deletions(-) (limited to 'src') diff --git a/src/config.cpp b/src/config.cpp index 0b7fe86..8ca7910 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -784,18 +784,31 @@ QString Config::resolvedQuery(const SavedQuery &query, SavedQuery Config::startupSavedQuery() const { - if (m_savedQueries.isEmpty()) - return {}; - + // The user's own queries first, so a saved query wins a name collision with + // a built-in filter: they named theirs deliberately, where the filter's + // name is one this application chose for them. for (const SavedQuery &query : m_savedQueries) { if (query.name.compare(m_startupQuery, Qt::CaseInsensitive) == 0) return query; } - // Named a query that does not exist. Not worth a warning: the default is - // a name the user never wrote, so an install with no [queries] Unread - // entry would warn on every launch about a key it never set. - return m_savedQueries.first(); + // Then the built-in filters. Without this, a startup_query of "Inbox" + // matched nothing once item 93 shipped Inbox as a filter and the duplicated + // saved query was removed, and the app started on whatever query happened + // to be first in the file. + for (const SavedQuery &filter : builtinFilters()) { + if (filter.name.compare(m_startupQuery, Qt::CaseInsensitive) == 0) + return filter; + } + + // Named nothing that exists. Falling back to m_savedQueries.first() is what + // this used to do and it is worse than it looks: after the duplicated + // entries were removed it could be any leftover query, so a startup view + // became a search for one sender, and an empty queries.json started nothing + // at all. A filter is always present, so the fallback can be one. + // + // Still not worth a warning: the default is a name the user never wrote. + return builtinFilter(QStringLiteral("unread")); } Account Config::account(const QString &key) const diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 4343479..3ebcbc4 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -404,9 +404,18 @@ MainWindow::MainWindow(const Config &config, QWidget *parent) // Not savedQueries().first(): [queries] is read through childKeys(), which // sorts alphabetically, so "first" means whatever happens to sort first // rather than anything the user chose. Config resolves the name. + // resolvedQuery(), not startup.query: a generated entry stores no query at + // all, since its text is composed from the accounts at run time. Reading + // the field directly meant a startup_query naming a built-in filter opened + // an empty bar and ran nothing. + // + // No account scope here. The dropdown starts on "All accounts", which is + // the empty key, so this is the unscoped form either way; passing the + // selection would be reading a widget the user has not touched yet. const SavedQuery startup = m_config.startupSavedQuery(); - if (!startup.query.isEmpty()) { - m_queryEdit->setText(startup.query); + const QString startupQuery = m_config.resolvedQuery(startup, QString()); + if (!startupQuery.isEmpty()) { + m_queryEdit->setText(startupQuery); runCurrentQuery(); } } @@ -1746,13 +1755,19 @@ void MainWindow::buildSavedQueryRow(QWidget *parent, QVBoxLayout *layout) // is chrome. A name the running theme lacks degrades to text on its // own, which is why nothing here checks whether it resolved. // - // mail-mark-important matches the `flag` action's own icon, since both - // reach the same tag: the filter finds what the action marks. + // A STAR for Important, not mail-mark-important, which the `flag` + // action uses. Item 57 recorded the user asking for a star when the + // action was renamed, and on the query row the icon is read as a + // category rather than as "do this to the selection", so the two can + // differ. Chosen by the user on sight, 2026-08-15. + // + // mail-folder-sent, not mail-sent: the former is the folder shape every + // theme ships, the latter is the envelope-in-flight some do not. static const QHash filterIcons = { { QStringLiteral("unread"), QStringLiteral("mail-mark-unread") }, { QStringLiteral("inbox"), QStringLiteral("mail-inbox") }, - { QStringLiteral("flagged"), QStringLiteral("mail-mark-important") }, - { QStringLiteral("sent"), QStringLiteral("mail-sent") }, + { QStringLiteral("flagged"), QStringLiteral("starred") }, + { QStringLiteral("sent"), QStringLiteral("mail-folder-sent") }, }; button->setIcon( QIcon::fromTheme(filterIcons.value(filter.generated))); diff --git a/src/mainwindow.h b/src/mainwindow.h index bb1ba75..03f21bb 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -186,6 +186,14 @@ public: /// stale, so a test standing in for the worker has to know the current one. quint64 currentGenerationForTesting() const { return m_generation; } + /// The query the visible list was actually built from. + /// + /// A test seam: a refresh re-runs THIS, never the text in the query bar, + /// and the difference is only observable through the value itself. The + /// generation counter cannot stand in for it, since a legitimate refresh + /// bumps the generation too. + QString lastRunQueryForTesting() const { return m_lastQuery; } + /// Puts the window into the state refreshCurrentQuery() leaves it in, and /// returns the generation the refresh's replies must carry. /// -- cgit v1.2.3