fix: only interrupt startup for real configuration problems

Found while walking the task 13 checklist against real mail. Item 1
("startup shows no configuration warnings with a valid config") failed:
with a perfectly valid config that simply had no [sync] command, every
launch opened a blocking modal that had to be dismissed before the window
could be used.

Config now separates the two cases. A problem is something configured but
wrong (a sync command that does not exist, an account with no maildir);
those still open a dialog, as does every KeyMap warning, since each one
means a binding the user wrote is being ignored. A notice is an optional
feature simply not being configured; it reports to the status bar only.
Nothing is broken in that case, and a modal on every launch teaches the
user to dismiss dialogs unread, which defeats the ones that matter.

problems() is a subset of warnings(), so callers wanting everything need
only the latter.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

4 files changed, 123 insertions, 8 deletions

diff --git a/src/config.cpp b/src/config.cpp
index b12130d..0253a74 100644
--- a/src/config.cpp
+++ b/src/config.cpp
@@ -19,6 +19,17 @@ QString Config::defaultPath()
     return base + QStringLiteral("/qtmaildir/qtmaildir.conf");
 }
 
+void Config::addProblem(const QString &message)
+{
+    m_warnings.append(message);
+    m_problems.append(message);
+}
+
+void Config::addNotice(const QString &message)
+{
+    m_warnings.append(message);
+}
+
 void Config::load(const QString &path)
 {
     QSettings settings(path, QSettings::IniFormat);
@@ -28,10 +39,12 @@ void Config::load(const QString &path)
 
     m_syncCommand = settings.value(QStringLiteral("sync/command")).toString();
     if (m_syncCommand.isEmpty()) {
-        m_warnings.append(QStringLiteral(
+        // Not a problem: sync is optional, and nothing the user asked for is
+        // being ignored. A modal here would fire on every launch.
+        addNotice(QStringLiteral(
             "No sync command configured ([sync] command); syncing is disabled."));
     } else if (!QFileInfo::exists(m_syncCommand.split(QLatin1Char(' ')).first())) {
-        m_warnings.append(
+        addProblem(
             QStringLiteral("Sync command '%1' does not exist; syncing is disabled.")
                 .arg(m_syncCommand));
         m_syncCommand.clear();
@@ -61,7 +74,7 @@ void Config::load(const QString &path)
         settings.endGroup();
 
         if (!account.isValid()) {
-            m_warnings.append(
+            addProblem(
                 QStringLiteral("Account '%1' has no maildir; ignoring it.")
                     .arg(account.key));
             continue;
diff --git a/src/config.h b/src/config.h
index f037db5..80f6101 100644
--- a/src/config.h
+++ b/src/config.h
@@ -50,13 +50,29 @@ public:
     /// Optional alternate notmuch config file. Empty means "let notmuch decide".
     QString notmuchConfig() const { return m_notmuchConfig; }
 
-    /// Non-fatal problems, shown once in a startup banner.
+    /// Every non-fatal problem, both kinds below. Shown in the status bar.
     QStringList warnings() const { return m_warnings; }
 
+    /// The subset worth interrupting startup for: something in the config is
+    /// wrong and the user's stated intent is not being honoured (a malformed
+    /// account, an unparseable key binding, a sync command that does not
+    /// exist). An optional setting simply being absent is NOT one of these:
+    /// nothing is broken, the feature is just off, and a modal on every launch
+    /// trains the user to dismiss dialogs without reading them.
+    QStringList problems() const { return m_problems; }
+
 private:
+    /// Records a problem: something configured but wrong. Also appears in
+    /// warnings(), so callers that want everything need only that one.
+    void addProblem(const QString &message);
+
+    /// Records a notice: nothing is wrong, a feature is simply not configured.
+    void addNotice(const QString &message);
+
     QList<Account> m_accounts;
     QList<SavedQuery> m_savedQueries;
     QString m_syncCommand;
     QString m_notmuchConfig;
     QStringList m_warnings;
+    QStringList m_problems;
 };
diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index ed16010..d6eccf9 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -274,15 +274,25 @@ void MainWindow::wireWorker()
 
 void MainWindow::showWarnings()
 {
-    QStringList warnings = m_config.warnings() + m_keyMap.warnings();
+    const QStringList warnings = m_config.warnings() + m_keyMap.warnings();
     if (warnings.isEmpty())
         return;
 
     // Non-fatal: the app runs degraded rather than refusing to start.
     m_statusLabel->setText(
-        tr("%1 configuration warning(s); see Help").arg(warnings.size()));
-    QMessageBox::warning(this, tr("Configuration warnings"),
-                         warnings.join(QLatin1Char('\n')));
+        tr("%n configuration warning(s)", "", warnings.size()));
+
+    // Interrupt startup only for things that are actually wrong. Every KeyMap
+    // warning qualifies (each one means a binding the user wrote is being
+    // ignored), but a Config notice such as "no sync command configured" does
+    // not: nothing is broken, the feature is simply off, and a modal on every
+    // launch teaches the user to dismiss dialogs unread.
+    const QStringList problems = m_config.problems() + m_keyMap.warnings();
+    if (problems.isEmpty())
+        return;
+
+    QMessageBox::warning(this, tr("Configuration problems"),
+                         problems.join(QLatin1Char('\n')));
 }
 
 void MainWindow::runCurrentQuery()
diff --git a/tests/test_config.cpp b/tests/test_config.cpp
index 0caa93c..50d7e67 100644
--- a/tests/test_config.cpp
+++ b/tests/test_config.cpp
@@ -12,6 +12,10 @@ private slots:
     void missingSyncCommandIsEmpty();
     void accountWithoutMaildirIsRejected();
     void scopedQueryWrapsCorrectly();
+    void absentSyncCommandIsNoticeNotProblem();
+    void brokenSyncCommandIsAProblem();
+    void malformedAccountIsAProblem();
+    void validConfigHasNoProblems();
 };
 
 static QString writeIni(const QTemporaryDir &dir, const QString &body)
@@ -128,5 +132,77 @@ void TestConfig::scopedQueryWrapsCorrectly()
              QStringLiteral("path:\"work-mail/**\""));
 }
 
+void TestConfig::absentSyncCommandIsNoticeNotProblem()
+{
+    // An optional feature simply not being configured must not interrupt
+    // startup: the modal would fire on every launch and train the user to
+    // dismiss dialogs without reading them.
+    QTemporaryDir dir;
+    const QString path = writeIni(dir, QStringLiteral(
+        "[account.work]\n"
+        "maildir=work-mail\n"));
+
+    Config config;
+    config.load(path);
+
+    QCOMPARE(config.warnings().size(), 1);
+    QVERIFY(config.warnings().first().contains(QStringLiteral("No sync command")));
+    QVERIFY(config.problems().isEmpty());
+}
+
+void TestConfig::brokenSyncCommandIsAProblem()
+{
+    // Configured but missing is different: the user asked for sync and is not
+    // getting it, so they need telling.
+    QTemporaryDir dir;
+    const QString path = writeIni(dir, QStringLiteral(
+        "[sync]\n"
+        "command=/nonexistent/qtmaildir-test/mailsync.sh\n"
+        "\n"
+        "[account.work]\n"
+        "maildir=work-mail\n"));
+
+    Config config;
+    config.load(path);
+
+    QCOMPARE(config.problems().size(), 1);
+    QVERIFY(config.problems().first().contains(QStringLiteral("does not exist")));
+    // Problems are a subset of warnings, so a caller wanting everything needs
+    // only warnings().
+    QVERIFY(config.warnings().contains(config.problems().first()));
+}
+
+void TestConfig::malformedAccountIsAProblem()
+{
+    QTemporaryDir dir;
+    const QString path = writeIni(dir, QStringLiteral(
+        "[account.broken]\n"
+        "name=No Maildir Here\n"));
+
+    Config config;
+    config.load(path);
+
+    QVERIFY(!config.problems().isEmpty());
+    QVERIFY(config.problems().first().contains(QStringLiteral("broken")));
+}
+
+void TestConfig::validConfigHasNoProblems()
+{
+    QTemporaryDir dir;
+    const QString path = writeIni(dir, QStringLiteral(
+        "[sync]\n"
+        "command=/bin/true\n"
+        "\n"
+        "[account.work]\n"
+        "maildir=work-mail\n"
+        "address=user@example.org\n"));
+
+    Config config;
+    config.load(path);
+
+    QVERIFY(config.problems().isEmpty());
+    QVERIFY(config.warnings().isEmpty());
+}
+
 QTEST_MAIN(TestConfig)
 #include "test_config.moc"