From 98c1c615d147bfee3f17d4f420e1512af5a79436 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 3 Aug 2026 16:00:38 +0200 Subject: feat: persist window, splitter and column widths Resizing the window, the splitter or a thread-list column was undone by the next launch. State now round-trips through a separate settings file at ~/.local/state/qtmaildir/uistate.conf, written on close and read at startup. The state file is deliberately not the user's config: a base64 geometry blob does not belong in a hand-edited file, and rewriting that file on exit would drop its comments and key order, which QSettings does not preserve. Two details that are easy to get wrong: QStandardPaths::StateLocation appends both the organization and the application name, and both are "qtmaildir" here, so it resolves to ~/.local/state/qtmaildir/qtmaildir. The path is built from GenericStateLocation instead, matching Config::defaultPath(). Restore runs after buildMenus() rather than at the end of buildUi(): QMainWindow::restoreState() matches toolbars by object name and silently drops the position of one that does not exist yet. Every restore is conditional on a non-empty blob, so a missing or rejected state file leaves the built-in defaults instead of producing a zero-size window. Co-Authored-By: Claude Opus 5 --- .../plans/2026-08-03-post-0.1.0-usability.md | 20 +++++- src/mainwindow.cpp | 76 ++++++++++++++++++++-- src/mainwindow.h | 17 +++++ tests/test_mainwindow.cpp | 61 +++++++++++++++++ 4 files changed, 168 insertions(+), 6 deletions(-) diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md index 7a6ecc2..debf79a 100644 --- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md +++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md @@ -34,7 +34,7 @@ taking that too literally. | # | Item | Cluster | Size | Status | |---|------|---------|------|--------| -| 1 | Splitter/column widths do not survive restart | persistence | S | open | +| 1 | Splitter/column widths do not survive restart | persistence | S | **done** | | 2 | No way to see full message details (From/To/Cc/Subject) | information | M | open | | 3 | Too few clickable affordances, shortcuts are the only route | discoverability | M | **done** | | 4 | Message-pane font size does not survive restart | persistence | S | open | @@ -89,6 +89,24 @@ as well. Establish it once, in whichever lands first. confirm both held. Then delete the state file and confirm the app still starts with the 1200x800 default rather than a zero-size window. +### Outcome (done) + +Built as described. `MainWindow::uiStatePath()` establishes the state file the +plan calls for, so items 4 and 10 inherit it. Two things worth recording: + +- **`QStandardPaths::StateLocation` is the wrong enum here.** It appends both + the organization and the application name, and this app sets both to + `qtmaildir`, so it yields `~/.local/state/qtmaildir/qtmaildir/`. The path is + built from `GenericStateLocation` plus an explicit `/qtmaildir`, the same + shape as `Config::defaultPath()`. A test pins the component count. +- **`restoreUiState()` runs after `buildMenus()`, not at the end of + `buildUi()`** as the plan proposed. `QMainWindow::restoreState()` matches + toolbars by object name, so a toolbar that does not exist yet has its + position silently dropped. + +Every restore is guarded on a non-empty blob, so absent state leaves the +`buildUi()` defaults rather than producing a zero-size window. + ## 2. No way to see full message details **Observed:** From, To, Cc, Subject and the rest are not visible for the diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 48b475c..081984d 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -19,9 +19,12 @@ #include "mainwindow.h" #include +#include #include #include #include +#include +#include #include #include #include @@ -33,6 +36,7 @@ #include #include #include +#include #include #include #include @@ -61,6 +65,65 @@ QString MainWindow::cidPrefixForIndex(int index) return QStringLiteral("m%1").arg(index); } +QString MainWindow::uiStatePath() +{ + // GenericStateLocation, not StateLocation: the latter appends both the + // organization and the application name, and both are "qtmaildir" here, + // so it yields ~/.local/state/qtmaildir/qtmaildir. Built the same way + // Config::defaultPath() builds its own. + const QString base = + QStandardPaths::writableLocation(QStandardPaths::GenericStateLocation); + return base + QStringLiteral("/qtmaildir/uistate.conf"); +} + +void MainWindow::restoreUiState() +{ + QSettings state(uiStatePath(), QSettings::IniFormat); + + // Every restore is conditional: an absent or rejected blob must leave the + // buildUi() defaults alone rather than produce a zero-size window. + const QByteArray geometry = state.value(QStringLiteral("window/geometry")) + .toByteArray(); + if (!geometry.isEmpty()) { + restoreGeometry(geometry); + } + + const QByteArray windowState = state.value(QStringLiteral("window/state")) + .toByteArray(); + if (!windowState.isEmpty()) { + restoreState(windowState); + } + + const QByteArray splitter = state.value(QStringLiteral("window/splitter")) + .toByteArray(); + if (!splitter.isEmpty()) { + m_splitter->restoreState(splitter); + } + + const QByteArray header = state.value(QStringLiteral("threadlist/header")) + .toByteArray(); + if (!header.isEmpty()) { + m_threadView->horizontalHeader()->restoreState(header); + } +} + +void MainWindow::saveUiState() const +{ + QDir().mkpath(QFileInfo(uiStatePath()).absolutePath()); + QSettings state(uiStatePath(), QSettings::IniFormat); + state.setValue(QStringLiteral("window/geometry"), saveGeometry()); + state.setValue(QStringLiteral("window/state"), saveState()); + state.setValue(QStringLiteral("window/splitter"), m_splitter->saveState()); + state.setValue(QStringLiteral("threadlist/header"), + m_threadView->horizontalHeader()->saveState()); +} + +void MainWindow::closeEvent(QCloseEvent *event) +{ + saveUiState(); + QMainWindow::closeEvent(event); +} + MainWindow::MainWindow(const Config &config, QWidget *parent) : QMainWindow(parent), m_config(config) { @@ -87,6 +150,9 @@ MainWindow::MainWindow(const Config &config, QWidget *parent) buildUi(); registerActions(); buildMenus(); + // After buildMenus(): QMainWindow::restoreState() matches toolbars by + // object name, so they must already exist or their position is dropped. + restoreUiState(); wireWorker(); showWarnings(); @@ -210,11 +276,11 @@ void MainWindow::buildUi() connect(m_messageView, &MessageView::statusMessage, this, [this](const QString &text) { m_statusLabel->setText(text); }); - auto *splitter = new QSplitter(Qt::Horizontal, central); - splitter->addWidget(m_threadView); - splitter->addWidget(m_messageView); - splitter->setStretchFactor(1, 2); - layout->addWidget(splitter, 1); + m_splitter = new QSplitter(Qt::Horizontal, central); + m_splitter->addWidget(m_threadView); + m_splitter->addWidget(m_messageView); + m_splitter->setStretchFactor(1, 2); + layout->addWidget(m_splitter, 1); layout->addWidget(m_syncLog); diff --git a/src/mainwindow.h b/src/mainwindow.h index 4445894..f4186ff 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -38,6 +38,7 @@ class QLabel; class QPushButton; class QComboBox; class QPlainTextEdit; +class QSplitter; class ThreadListModel; class MessageView; @@ -63,6 +64,15 @@ public: /// cid: references from resolving to another's. static QString cidPrefixForIndex(int index); + /// Path of the machine-written UI state file. Deliberately not + /// Config::defaultPath(): the config is hand-edited and must never gain a + /// base64 geometry blob, nor be rewritten on exit (QSettings does not + /// preserve comments or key order). + static QString uiStatePath(); + +protected: + void closeEvent(QCloseEvent *event) override; + private slots: void runCurrentQuery(); void onThreadsReady(const QVector &threads, quint64 generation); @@ -74,6 +84,12 @@ private slots: private: void buildUi(); + + /// Restores window geometry, splitter and thread-list header widths. + /// A missing or rejected blob leaves the buildUi() defaults in place. + void restoreUiState(); + void saveUiState() const; + void registerActions(); void buildMenus(); void wireWorker(); @@ -116,6 +132,7 @@ private: QLineEdit *m_queryEdit = nullptr; QTableView *m_threadView = nullptr; + QSplitter *m_splitter = nullptr; QComboBox *m_accountBox = nullptr; QPushButton *m_syncButton = nullptr; QLabel *m_statusLabel = nullptr; diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 6bfa925..628472b 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -20,7 +20,9 @@ #include #include +#include #include +#include #include #include "config.h" @@ -41,6 +43,9 @@ private slots: void configuredBindingReachesTheAction(); void cidPrefixesAreBangFree(); void cidPrefixesAreDistinctPerMessage(); + void uiStateIsNotWrittenIntoTheUserConfig(); + void uiStateSurvivesARestart(); + void missingUiStateLeavesTheDefaults(); }; void TestMainWindow::everyKnownActionIsRegistered() @@ -158,6 +163,62 @@ void TestMainWindow::cidPrefixesAreDistinctPerMessage() } } +void TestMainWindow::uiStateIsNotWrittenIntoTheUserConfig() +{ + // The config file is hand-edited and must never gain a base64 geometry + // blob, nor be rewritten on exit: QSettings preserves neither comments nor + // key order, so writing it would quietly destroy the user's formatting. + QVERIFY(MainWindow::uiStatePath() != Config::defaultPath()); + + // One qtmaildir component, not two. QStandardPaths::StateLocation appends + // both the organization and the application name, and here both are + // "qtmaildir", so using it nests the directory inside itself. + QCOMPARE(MainWindow::uiStatePath().count(QStringLiteral("/qtmaildir/")), 1); + QVERIFY(MainWindow::uiStatePath().endsWith( + QStringLiteral("/qtmaildir/uistate.conf"))); +} + +void TestMainWindow::uiStateSurvivesARestart() +{ + // Test mode redirects QStandardPaths at the process level, so the state + // file lands in a scratch directory rather than the real ~/.local/state. + QStandardPaths::setTestModeEnabled(true); + QFile::remove(MainWindow::uiStatePath()); + + const QSize resized(940, 620); + { + const Config config; + MainWindow window(config); + window.resize(resized); + window.close(); // closeEvent() is what persists the state + } + + QVERIFY2(QFile::exists(MainWindow::uiStatePath()), + qPrintable(QStringLiteral("no state file at %1") + .arg(MainWindow::uiStatePath()))); + + const Config config; + MainWindow reopened(config); + QCOMPARE(reopened.size(), resized); + + QFile::remove(MainWindow::uiStatePath()); + QStandardPaths::setTestModeEnabled(false); +} + +void TestMainWindow::missingUiStateLeavesTheDefaults() +{ + // A restore that silently succeeded on an empty blob would give a + // zero-size window on first launch. Absent state must be a no-op. + QStandardPaths::setTestModeEnabled(true); + QFile::remove(MainWindow::uiStatePath()); + + const Config config; + MainWindow window(config); + QCOMPARE(window.size(), QSize(1200, 800)); + + QStandardPaths::setTestModeEnabled(false); +} + // Constructing a MainWindow needs a QApplication and a platform plugin. The // test has no display under ctest, so it runs offscreen unless the caller // asked for something else. -- cgit v1.2.3