diff options
| -rw-r--r-- | CHANGELOG.md | 8 | ||||
| -rw-r--r-- | README.md | 7 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 26 | ||||
| -rw-r--r-- | src/carddelegate.cpp | 10 | ||||
| -rw-r--r-- | src/cardlayout.cpp | 31 | ||||
| -rw-r--r-- | src/cardlayout.h | 18 | ||||
| -rw-r--r-- | src/config.cpp | 25 | ||||
| -rw-r--r-- | src/config.h | 11 | ||||
| -rw-r--r-- | src/mainwindow.cpp | 1 | ||||
| -rw-r--r-- | src/threadlistmodel.cpp | 4 | ||||
| -rw-r--r-- | src/threadlistmodel.h | 12 | ||||
| -rw-r--r-- | tests/test_cardlayout.cpp | 46 | ||||
| -rw-r--r-- | tests/test_config.cpp | 50 |
13 files changed, 232 insertions, 17 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 13e2c39..55aa11f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,14 @@ point at which they are stable. ## [Unreleased] +### Added + +- `[general] date_format`, an optional pattern for the date on a thread card. + Absent or empty keeps the system locale's short format, which is unchanged + and remains the default. A pattern containing no date or time field is + refused with a message rather than printing the same fixed text on every + card. + ### Changed - The Sync button carries the refresh icon instead of a mailbox one. With the @@ -129,6 +129,13 @@ identity. ; and quits when it finishes; "never" quits silently. A sync that fails never ; closes the window, so a failure cannot discard the changes quietly. ; sync_on_exit = ask +; Optional. How the date on a thread card is written, as a QDateTime pattern +; (yyyy year, MM month, dd day, hh:mm time; anything in single quotes is kept +; literally). Absent or empty means your system locale's short format, which is +; what every other application on your desktop shows, and is the default. A +; pattern that contains no date or time field at all is refused with a message, +; since it would print the same fixed text on every card. +; date_format = yyyy-MM-dd hh:mm [completion] ; Optional. Extra content types offered after mimetype:, APPENDED to the 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 ff6efc4..844217e 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 @@ -118,7 +118,7 @@ taking that too literally. | 59 | Archive and Mark all read shipped with the same icon | presentation | XS | **done** | | 60 | Next thread dead-ends on the last reply of an expanded thread | defect | XS | **done**; already fixed by 5487d58, see below | | 61 | `test_mainwindow` fails intermittently, about 1 run in 20 | testing | S | open; predates the card list, reproduced on f72dba9 | -| 62 | No config option for the date format on a card | presentation | XS | open | +| 62 | No config option for the date format on a card | presentation | XS | **done** 2026-08-11 | | 63 | No way to see sent mail, and no filter for it | workflow | S | open | | 64 | The Sync button carries a mailbox icon, not a refresh one | presentation | XS | **done** 2026-08-11 | | 65 | No full code review and optimization pass | correctness | ? | open, unspecified | @@ -4092,6 +4092,30 @@ and `CardDelegate` passes it down. **Size: XS.** One key, one parameter, one width calculation. +### Outcome (done) + +Built as specced: `[general] date_format`, empty by default, passed down as a +parameter rather than read inside `CardLayout`. Three things worth recording. + +- **The format reaches the LAYOUT, not only the painter.** It sits on + `CardLayout::Input`, because `compute()` reserves the date's width from + `widestDateSample()`. A pattern that reached only the `drawText` call would be + elided into a rect sized for the system format, which is the same clipping + the bold-font fault produced. The test asserts both halves and was confirmed + by mutation: making the width ignore the format fails it. +- **`widestDateSample()`'s static cache had to go.** It memoised one sample, so + whichever format arrived first would have sized every later one. It is now a + plain call, at the cost of one `QLocale` lookup per row, which is what + formatting the date itself already costs. +- **Validating a pattern is harder than it looks, and the first test fixture + was wrong.** `toString()` treats nearly every letter as a field, so `banana` + formats as `bpmnpmnpm` (`a` is AM/PM, `n` the minute) and `hello` as `22ello`. + Those are nonsense but they vary with the instant, so a "does this contain a + field" check cannot reject them and should not pretend to. What `Config` + rejects is the case that actually harms: a pattern whose output is CONSTANT, + found by formatting two different instants and comparing. `xyz` is such a + pattern and is what the test uses. + ## 63. No way to see sent mail, and no filter for it **Observed (user, from the notes):** "Sent mail filter". diff --git a/src/carddelegate.cpp b/src/carddelegate.cpp index bce9eec..3039853 100644 --- a/src/carddelegate.cpp +++ b/src/carddelegate.cpp @@ -36,6 +36,7 @@ CardLayout::Input inputFor(const QModelIndex &index) in.isMessage = index.data(ThreadListModel::IsMessageRole).toBool(); in.depth = index.data(ThreadListModel::MessageDepthRole).toInt(); in.replyCount = index.data(ThreadListModel::ReplyCountRole).toInt(); + in.dateFormat = index.data(ThreadListModel::DateFormatRole).toString(); return in; } @@ -172,8 +173,13 @@ void CardDelegate::paint(QPainter *painter, const QStyleOptionViewItem &option, Qt::ElideRight, card.senderRect.width())); const QDateTime date = index.data(ThreadListModel::DateRole).toDateTime(); - painter->drawText(card.dateRect, Qt::AlignVCenter | Qt::AlignRight, - CardLayout::formatDate(date)); + // The same format the layout reserved width from. Reading the role again + // rather than a second config lookup, so the drawn string and the rect it + // is drawn into cannot come from different patterns. + painter->drawText( + card.dateRect, Qt::AlignVCenter | Qt::AlignRight, + CardLayout::formatDate( + date, index.data(ThreadListModel::DateFormatRole).toString())); // Line 2: the flag mark, the subject, the attachment mark. QString subject = index.data(ThreadListModel::SubjectRole).toString(); diff --git a/src/cardlayout.cpp b/src/cardlayout.cpp index f542df0..d60bff9 100644 --- a/src/cardlayout.cpp +++ b/src/cardlayout.cpp @@ -21,12 +21,16 @@ #include <QFontMetrics> #include <QLocale> -QString CardLayout::formatDate(const QDateTime &date) +QString CardLayout::formatDate(const QDateTime &date, const QString &format) { - // The system locale's own short format, not a hardcoded pattern: an - // Italian desktop writes 10/08/2025, not 2025-08-10, and a mail client - // that disagrees with every other application on screen is simply wrong. - return QLocale::system().toString(date, QLocale::ShortFormat); + // The system locale's own short format by default, not a hardcoded + // pattern: an Italian desktop writes 10/08/2025, not 2025-08-10, and a mail + // client that disagrees with every other application on screen is simply + // wrong. [general] date_format overrides it for a user who wants one + // specific shape regardless of the locale. + if (format.isEmpty()) + return QLocale::system().toString(date, QLocale::ShortFormat); + return QLocale::system().toString(date, format); } QString CardLayout::expanderLabel(int replyCount, bool expanded) @@ -45,17 +49,19 @@ QString CardLayout::expanderLabel(int replyCount, bool expanded) return QStringLiteral("%1 %2 %3").arg(glyph).arg(replyCount).arg(word); } -QString CardLayout::widestDateSample() +QString CardLayout::widestDateSample(const QString &format) { // A real date run through the same formatter, with the wide digits and a // two-digit day and month, so the reserved width matches what is drawn // whatever the locale's pattern turns out to be. Guessing a pattern here // would reintroduce the clipping this exists to prevent. - static const QString sample = [] { - const QDateTime wide(QDate(2028, 12, 28), QTime(22, 58)); - return formatDate(wide); - }(); - return sample; + // + // Not cached in a static any more: the sample depends on the format, and a + // single static computed for whichever format arrived first would reserve + // the system format's width for a custom pattern. The formatter is one + // QLocale call per row, which is the same cost the date itself already pays. + const QDateTime wide(QDate(2028, 12, 28), QTime(22, 58)); + return formatDate(wide, format); } QFont CardLayout::smallFont(const QFont &cardFont) @@ -145,7 +151,8 @@ CardLayout CardLayout::compute(const Input &input, const QRect &rect, QFont dateFont = font; dateFont.setBold(true); const int dateWidth = - QFontMetrics(dateFont).horizontalAdvance(widestDateSample()); + QFontMetrics(dateFont).horizontalAdvance( + widestDateSample(input.dateFormat)); out.dateRect = QRect(right - dateWidth, lineOneTop, dateWidth, metrics.height()); out.senderRect = QRect(out.contentLeft, lineOneTop, diff --git a/src/cardlayout.h b/src/cardlayout.h index ef6a563..1a2fd18 100644 --- a/src/cardlayout.h +++ b/src/cardlayout.h @@ -46,6 +46,16 @@ struct CardLayout bool isMessage = false; int depth = 0; ///< 0 for a thread root, 1 for a direct reply. int replyCount = 0; ///< 0 means no expander. + + /// A QDateTime::toString() pattern from [general] date_format, or empty + /// for the system's short format. + /// + /// It lives on the INPUT rather than being read where the date is + /// drawn, because the width reserved for the date is computed from the + /// same format inside compute(). A pattern reaching the painter but not + /// the geometry is exactly how a longer date gets elided into a rect + /// sized for a shorter one. + QString dateFormat; }; /// Width of the account accent bar down a thread card's left edge. @@ -129,10 +139,14 @@ struct CardLayout /// drawn into it come from one place: a locale whose short format is /// longer than the reserved rect would clip, which is exactly the fault /// bold text produced. - static QString formatDate(const QDateTime &date); + /// `format` is a QDateTime::toString() pattern, or empty for the system's + /// short format. Config validates it, so an unusable pattern never gets + /// this far. + static QString formatDate(const QDateTime &date, + const QString &format = QString()); /// The widest string formatDate() can return, for reserving space. - static QString widestDateSample(); + static QString widestDateSample(const QString &format = QString()); /// The expander's label: the reply count with its glyph, as drawn. /// diff --git a/src/config.cpp b/src/config.cpp index 4f5831c..9e223f8 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -23,7 +23,9 @@ // so this reports the same numbers rather than keeping a second copy. #include "messageview.h" +#include <QDateTime> #include <QFileInfo> +#include <QLocale> #include <QSettings> #include <QStandardPaths> @@ -146,6 +148,29 @@ void Config::load(const QString &path) } } + // Absent or empty means the system locale's short format, which is what + // every other application on the desktop shows. Only a non-empty pattern is + // validated, and a rejected one falls back to that same default. + const QString dateFormat = + settings.value(QStringLiteral("date_format")).toString().trimmed(); + if (!dateFormat.isEmpty()) { + // QDateTime::toString() with a pattern carrying no date or time field + // returns the pattern verbatim rather than failing, so "banana" would + // print "banana" on every card. Formatting two DIFFERENT instants and + // comparing is what catches that: a pattern with any real field gives + // two different strings, one with none gives the same string twice. + const QDateTime a(QDate(2028, 12, 28), QTime(22, 58)); + const QDateTime b(QDate(2019, 1, 3), QTime(4, 5)); + const QLocale locale = QLocale::system(); + if (locale.toString(a, dateFormat) == locale.toString(b, dateFormat)) { + addProblem(QStringLiteral("Date format '%1' contains no date or " + "time field; using the system format.") + .arg(dateFormat)); + } else { + m_dateFormat = dateFormat; + } + } + // Absent is silent, the default being 2000. Present but unparseable warns, // for the same reason message_zoom does: the user asked for something and // is not getting it. diff --git a/src/config.h b/src/config.h index 3dd9011..bcbfb34 100644 --- a/src/config.h +++ b/src/config.h @@ -114,6 +114,16 @@ public: /// Optional alternate notmuch config file. Empty means "let notmuch decide". QString notmuchConfig() const { return m_notmuchConfig; } + /// A QDateTime::toString() pattern for the date on a card, or empty for the + /// system locale's short format. + /// + /// Empty is both the default and what an unusable pattern falls back to, so + /// a caller never has to distinguish "unset" from "rejected": either way + /// the locale decides. Validated at load, because toString() with a pattern + /// carrying no date field returns the pattern verbatim, which would print + /// the same fixed string on every card rather than failing visibly. + QString dateFormat() const { return m_dateFormat; } + /// The saved query to open at startup, by name. Falls back to "Unread" /// when unset, and to the first saved query when no query by that name /// exists: [queries] is read through childKeys(), which sorts @@ -185,6 +195,7 @@ private: QString m_syncLog; int m_toolbarIconSize = 24; QString m_notmuchConfig; + QString m_dateFormat; qreal m_messageZoom = 1.0; bool m_completionOnFocus = false; int m_markReadDelayMs = 2000; diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index d9eb989..374f228 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -558,6 +558,7 @@ void MainWindow::buildUi() // Thread list and message pane. m_model = new ThreadListModel(this); m_model->setTagColors(&m_tagColors); + m_model->setDateFormat(m_config.dateFormat()); // ThreadListView, not a plain QTableView: it paints the row-wide tag // strip under each row's cells, which no delegate can do because a // delegate is confined to one column's rectangle. diff --git a/src/threadlistmodel.cpp b/src/threadlistmodel.cpp index 21a6378..33356f0 100644 --- a/src/threadlistmodel.cpp +++ b/src/threadlistmodel.cpp @@ -323,6 +323,8 @@ QVariant ThreadListModel::data(const QModelIndex &index, int role) const // A reply never offers an expander: nesting past the first level is // drawn from depth, not from further parent-child structure. return 0; + case DateFormatRole: + return m_dateFormat; case Qt::BackgroundRole: // Tinted, so an expanded thread reads as one block rather than as // more table rows. Applied per cell here; ThreadListView fills the @@ -480,6 +482,8 @@ QVariant ThreadListModel::data(const QModelIndex &index, int role) const case ReplyCountRole: // totalCount includes the root message, which is the card itself. return qMax(0, thread.totalCount - 1); + case DateFormatRole: + return m_dateFormat; default: break; } diff --git a/src/threadlistmodel.h b/src/threadlistmodel.h index 01dd9d0..777841e 100644 --- a/src/threadlistmodel.h +++ b/src/threadlistmodel.h @@ -116,6 +116,14 @@ public: HasAttachmentRole, ///< bool IsFlaggedRole, ///< bool ReplyCountRole, ///< int; 0 when a thread has no replies. + + /// The [general] date_format pattern, or empty for the system's short + /// format. Same row value for every row. + /// + /// Supplied by the model for the same reason as PillColoursRole: it is + /// the one thing here that holds config, and a delegate reading config + /// itself would be a second source of truth. + DateFormatRole, }; /// The mark drawn on a card's second line when the message has an @@ -159,6 +167,9 @@ public: /// Without one, chips fall back to a colour generated from the tag name. void setTagColors(const TagColors *colours) { m_tagColors = colours; } + /// The pattern DateFormatRole answers with. Empty means the system format. + void setDateFormat(const QString &format) { m_dateFormat = format; } + QModelIndex index(int row, int column, const QModelIndex &parent = {}) const override; QModelIndex parent(const QModelIndex &child) const override; @@ -272,4 +283,5 @@ private: QVector<ThreadNode> m_threads; const TagColors *m_tagColors = nullptr; + QString m_dateFormat; }; diff --git a/tests/test_cardlayout.cpp b/tests/test_cardlayout.cpp index fdc18bf..bce6a0f 100644 --- a/tests/test_cardlayout.cpp +++ b/tests/test_cardlayout.cpp @@ -40,6 +40,7 @@ private slots: void replyCardCarriesNoAccentBar(); void theDateFitsWhenTheCardIsBold(); void theDateFollowsTheSystemLocale(); + void aConfiguredDateFormatIsUsedAndReservedFor(); }; namespace { @@ -343,6 +344,51 @@ void TestCardLayout::theDateFollowsTheSystemLocale() "formatting of a date"); } +void TestCardLayout::aConfiguredDateFormatIsUsedAndReservedFor() +{ + const QDateTime when(QDate(2025, 8, 10), QTime(6, 26)); + + // A pattern deliberately much longer than any locale's short format, so + // the width claim below cannot pass by accident on a locale whose own + // dates happen to be wide enough already. + const QString format = QStringLiteral("dddd d MMMM yyyy 'at' hh:mm:ss"); + + QCOMPARE(CardLayout::formatDate(when, format), + QLocale::system().toString(when, format)); + QVERIFY2(CardLayout::formatDate(when, format) + != CardLayout::formatDate(when), + "a configured format produced the same string as the system one, " + "so the parameter is being ignored"); + + // An empty format is what an absent or rejected config key gives, and it + // must mean the system format rather than an empty date. + QCOMPARE(CardLayout::formatDate(when, QString()), + CardLayout::formatDate(when)); + + // The load-bearing half: the reserved width has to follow the SAME format, + // or a long pattern is elided into a rect sized for a short one. This is + // the fault that a static, format-independent widest-date sample produces. + QFont font; + const int h = CardLayout::heightFor(font); + CardLayout::Input in = threadInput(); + in.dateFormat = format; + const CardLayout card = CardLayout::compute(in, QRect(0, 0, 900, h), font); + QFont bold = font; + bold.setBold(true); + QVERIFY2(card.dateRect.width() + >= QFontMetrics(bold).horizontalAdvance( + CardLayout::formatDate(when, format)), + "the reserved date width is narrower than the configured format's " + "own output"); + + // And it is genuinely wider than the default's, which proves the width + // moved with the format rather than a generous constant covering both. + const CardLayout plain = + CardLayout::compute(threadInput(), QRect(0, 0, 900, h), font); + QVERIFY2(card.dateRect.width() > plain.dateRect.width(), + "a longer date format reserved no more width than the default"); +} + void TestCardLayout::theDateFitsWhenTheCardIsBold() { // An UNREAD card draws BOLD, and bold is wider. The layout is computed from diff --git a/tests/test_config.cpp b/tests/test_config.cpp index 7e0d6fb..b9321e0 100644 --- a/tests/test_config.cpp +++ b/tests/test_config.cpp @@ -52,6 +52,9 @@ private slots: void markReadDelayDefaultsToTwoSeconds(); void markReadDelayIsActuallyRead(); void markReadDelayAcceptsZeroAndNegative(); + void dateFormatDefaultsToEmpty(); + void dateFormatIsActuallyRead(); + void dateFormatWithoutAFieldIsRejectedAndReported(); void markReadDelayRejectsGarbage(); void syncOnExitDefaultsToAsk(); void syncOnExitReadsAllThreeValues(); @@ -570,6 +573,53 @@ void TestConfig::markReadDelayAcceptsZeroAndNegative() QVERIFY(never.problems().isEmpty()); } +void TestConfig::dateFormatDefaultsToEmpty() +{ + // Empty is what tells CardLayout to use the system's short format, which is + // the shipped behaviour and must survive this key existing. + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral("[general]\n"))); + QVERIFY(config.dateFormat().isEmpty()); + QVERIFY(config.problems().isEmpty()); +} + +void TestConfig::dateFormatIsActuallyRead() +{ + // A pattern that is not the default, which is what proves the key is read + // at all: a "general/date_format" lookup matches nothing and would still + // pass a test that only checked the empty default. + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral("[general]\n" + "date_format=yyyy-MM-dd\n"))); + QCOMPARE(config.dateFormat(), QStringLiteral("yyyy-MM-dd")); + QVERIFY(config.problems().isEmpty()); +} + +void TestConfig::dateFormatWithoutAFieldIsRejectedAndReported() +{ + // The specific trap: QDateTime::toString() with a pattern carrying no date + // or time field returns something fixed rather than failing, so this would + // print the same string on every card and look like a rendering fault + // rather than a config one. + // + // "xyz" and not a friendlier-looking word, because almost every letter is + // a field character: "banana" formats as "bpmnpmnpm" (a is AM/PM, n is the + // minute) and "hello" as "22ello" (h is the hour). Those are nonsense but + // they do vary with the instant, so they are not what this rejects and the + // check would fail against them. What it catches is a pattern whose output + // is CONSTANT, which is the case that silently shows one date forever. + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral("[general]\n" + "date_format=xyz\n"))); + QVERIFY2(config.dateFormat().isEmpty(), + "a pattern with no date field was accepted"); + QCOMPARE(config.problems().size(), 1); + QVERIFY(config.problems().first().contains(QStringLiteral("xyz"))); +} + void TestConfig::markReadDelayRejectsGarbage() { // Absent is silent, but present-and-unparseable means the user asked for |