diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-03 16:00:38 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-03 16:00:38 +0200 |
| commit | e6ea00dce8c74bf0d48e769734958a741ca46ef9 (patch) | |
| tree | 2b56659912cfa1f284faefa2e04d841e9a51852e | |
| parent | ec390fb46e1a36d8406dde487228bbb0f348a20a (diff) | |
| download | qtmaildir-e6ea00dce8c74bf0d48e769734958a741ca46ef9.tar.gz qtmaildir-e6ea00dce8c74bf0d48e769734958a741ca46ef9.zip | |
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 <noreply@anthropic.com>
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 20 | ||||
| -rw-r--r-- | src/mainwindow.cpp | 76 | ||||
| -rw-r--r-- | src/mainwindow.h | 17 | ||||
| -rw-r--r-- | 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 915c8cb..fc4ddbd 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 <QAction> +#include <QCloseEvent> #include <QComboBox> #include <QDialog> #include <QDialogButtonBox> +#include <QDir> +#include <QFileInfo> #include <QHBoxLayout> #include <QHeaderView> #include <QLabel> @@ -33,6 +36,7 @@ #include <QPushButton> #include <QSettings> #include <QSplitter> +#include <QStandardPaths> #include <QStatusBar> #include <QTableView> #include <QToolBar> @@ -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<ThreadSummary> &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 <QAction> #include <QDir> +#include <QFile> #include <QSettings> +#include <QStandardPaths> #include <QTemporaryDir> #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. |
