diff options
| -rw-r--r-- | src/mainwindow.cpp | 19 | ||||
| -rw-r--r-- | src/threadlistmodel.cpp | 35 | ||||
| -rw-r--r-- | src/threadlistmodel.h | 10 | ||||
| -rw-r--r-- | src/types.h | 7 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 44 | ||||
| -rw-r--r-- | tests/test_threadlistmodel.cpp | 42 |
6 files changed, 155 insertions, 2 deletions
diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 2a79988..1385f01 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -100,9 +100,18 @@ void MainWindow::restoreUiState() m_splitter->restoreState(splitter); } + // A header blob saved against a different set of columns must be + // discarded, not restored. QHeaderView::restoreState() returns TRUE for a + // blob with fewer sections than the model and applies the old widths to + // the wrong columns: adding the attachment column in front shifted every + // saved width one place right, silently mangling the layout with no error + // to detect it by (verified on Qt 6.11). The column count is stored + // alongside and the blob is only used when it still matches. const QByteArray header = state.value(QStringLiteral("threadlist/header")) .toByteArray(); - if (!header.isEmpty()) { + const int savedColumns = + state.value(QStringLiteral("threadlist/columns")).toInt(); + if (!header.isEmpty() && savedColumns == ThreadListModel::ColumnCount) { m_threadView->horizontalHeader()->restoreState(header); } @@ -123,6 +132,9 @@ void MainWindow::saveUiState() const state.setValue(QStringLiteral("window/splitter"), m_splitter->saveState()); state.setValue(QStringLiteral("threadlist/header"), m_threadView->horizontalHeader()->saveState()); + // Guards the blob above: see restoreUiState(). + state.setValue(QStringLiteral("threadlist/columns"), + int(ThreadListModel::ColumnCount)); state.setValue(QStringLiteral("message/zoom"), m_messageView->zoomFactor()); } @@ -275,6 +287,11 @@ void MainWindow::buildUi() // Starting widths only; a drag overrides them, and they are what the // saved-widths item will persist. + // Without this the attachment column cannot be narrow at all: the default + // minimum section size is 58px on this platform, and setColumnWidth() + // clamps to it silently rather than reporting the smaller value back. + m_threadView->horizontalHeader()->setMinimumSectionSize(24); + m_threadView->setColumnWidth(ThreadListModel::AttachmentColumn, 28); m_threadView->setColumnWidth(ThreadListModel::DateColumn, 130); m_threadView->setColumnWidth(ThreadListModel::AuthorsColumn, 180); m_threadView->setColumnWidth(ThreadListModel::SubjectColumn, 520); diff --git a/src/threadlistmodel.cpp b/src/threadlistmodel.cpp index 2f2882e..2289e6c 100644 --- a/src/threadlistmodel.cpp +++ b/src/threadlistmodel.cpp @@ -20,6 +20,27 @@ #include <QBrush> #include <QFont> +#include <QFontDatabase> +#include <QFontMetrics> + +QString ThreadListModel::attachmentGlyph() +{ + // U+1F4CE PAPERCLIP, with a fallback for a system whose default font + // cannot draw it: an unrenderable codepoint shows as a tofu box, which + // reads as "something is broken" rather than "this has an attachment". + // Computed once; the font does not change under a running application. + static const QString glyph = [] { + const char32_t paperclip = 0x1F4CE; + const QString preferred = QString::fromUcs4(&paperclip, 1); + const QFontMetrics metrics{QFontDatabase::systemFont( + QFontDatabase::GeneralFont)}; + // "*" as the fallback: ASCII, present in every practical font, and + // unambiguous in a column that shows nothing else. + return metrics.inFontUcs4(paperclip) ? preferred + : QStringLiteral("*"); + }(); + return glyph; +} QColor ThreadListModel::deletedColour() { @@ -85,8 +106,19 @@ QVariant ThreadListModel::data(const QModelIndex &index, int role) const return {}; } + if (role == Qt::ToolTipRole && index.column() == AttachmentColumn) + return thread.hasAttachment() ? tr("Has an attachment") : QVariant(); + + if (role == Qt::TextAlignmentRole && index.column() == AttachmentColumn) + return QVariant::fromValue(Qt::AlignCenter); + if (role == Qt::DisplayRole) { switch (index.column()) { + case AttachmentColumn: + // A glyph rather than an icon resource: no new asset to ship, and + // it inherits the row's font, so it strikes through with a doomed + // thread like every other cell. + return thread.hasAttachment() ? attachmentGlyph() : QString(); case DateColumn: return thread.date.toString(QStringLiteral("yyyy-MM-dd hh:mm")); case AuthorsColumn: @@ -141,6 +173,9 @@ QVariant ThreadListModel::headerData(int section, Qt::Orientation orientation, return {}; switch (section) { + // No label: any text would set a minimum width far wider than the icon, + // which defeats the point of a narrow column. + case AttachmentColumn: return QString(); case DateColumn: return QStringLiteral("Date"); case AuthorsColumn: return QStringLiteral("From"); case SubjectColumn: return QStringLiteral("Subject"); diff --git a/src/threadlistmodel.h b/src/threadlistmodel.h index 7ed8fef..ab9378e 100644 --- a/src/threadlistmodel.h +++ b/src/threadlistmodel.h @@ -36,7 +36,11 @@ public: /// under the message pane, and the account tag renders as a chip in front /// of the subject. enum Column { - DateColumn = 0, + /// A paperclip when the thread has an attachment, so it is visible + /// without opening the thread. Icon only and deliberately narrow; + /// it carries no text. + AttachmentColumn = 0, + DateColumn, AuthorsColumn, SubjectColumn, ColumnCount, @@ -64,6 +68,10 @@ public: /// Muted rather than saturated: a bulk delete paints every selected row, /// and a wall of pure red is harder to read than the list it replaces. /// Exposed so a test names the same colour the model uses. + /// The character shown in AttachmentColumn for a thread that has one. + /// A paperclip when the system font can draw it, "*" otherwise. + static QString attachmentGlyph(); + static QColor deletedColour(); static QColor spamColour(); diff --git a/src/types.h b/src/types.h index e25c3a9..59d1d06 100644 --- a/src/types.h +++ b/src/types.h @@ -38,6 +38,13 @@ struct ThreadSummary bool isDeleted() const { return tags.contains(QStringLiteral("deleted")); } bool isSpam() const { return tags.contains(QStringLiteral("spam")); } + /// notmuch applies "attachment" itself while indexing, so this needs no + /// MIME parsing and no extra worker query: the tag is already in tags. + bool hasAttachment() const + { + return tags.contains(QStringLiteral("attachment")); + } + /// True while the thread is tagged for removal. notmuch deletes nothing /// itself: the tag marks the thread for whatever the user's sync script /// does next, so the row has to show it is on its way out. diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index b4d9e4a..ab904f9 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -25,10 +25,13 @@ #include <QStandardPaths> #include <QTemporaryDir> +#include <QTableView> + #include "config.h" #include "keymap.h" #include "mainwindow.h" #include "messageview.h" +#include "threadlistmodel.h" /// MainWindow is mostly wiring, and the parts that need a real database are /// still verified manually. What is checked here is the action registry: the @@ -47,6 +50,7 @@ private slots: void uiStateIsNotWrittenIntoTheUserConfig(); void uiStateSurvivesARestart(); void missingUiStateLeavesTheDefaults(); + void headerStateFromADifferentColumnLayoutIsDiscarded(); }; void TestMainWindow::everyKnownActionIsRegistered() @@ -222,6 +226,46 @@ void TestMainWindow::missingUiStateLeavesTheDefaults() QStandardPaths::setTestModeEnabled(false); } +void TestMainWindow::headerStateFromADifferentColumnLayoutIsDiscarded() +{ + // The upgrade hazard: a 0.3.0 state file holds a three-column header blob, + // and 0.4.0 added the attachment column in front. QHeaderView:: + // restoreState() returns TRUE for a blob with fewer sections than the + // model and applies the old widths shifted one column right, mangling the + // layout with no error to detect it by (verified on Qt 6.11). The stored + // column count is what makes that detectable. + QStandardPaths::setTestModeEnabled(true); + QFile::remove(MainWindow::uiStatePath()); + + { + const Config config; + MainWindow window(config); + window.close(); + } + + // Forge a state file from an older layout: same blob, wrong column count. + { + QSettings state(MainWindow::uiStatePath(), QSettings::IniFormat); + state.setValue(QStringLiteral("threadlist/columns"), + int(ThreadListModel::ColumnCount) - 1); + state.setValue(QStringLiteral("threadlist/header"), + QByteArray("not a header this model could have saved")); + } + + // Constructing must not apply it, and must not crash on the garbage blob. + const Config config; + MainWindow reopened(config); + + auto *view = reopened.findChild<QTableView *>(); + QVERIFY(view); + QCOMPARE(view->columnWidth(ThreadListModel::AttachmentColumn), 28); + QCOMPARE(view->columnWidth(ThreadListModel::DateColumn), 130); + QCOMPARE(view->columnWidth(ThreadListModel::SubjectColumn), 520); + + QFile::remove(MainWindow::uiStatePath()); + 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. diff --git a/tests/test_threadlistmodel.cpp b/tests/test_threadlistmodel.cpp index 5a72c64..a9cdb73 100644 --- a/tests/test_threadlistmodel.cpp +++ b/tests/test_threadlistmodel.cpp @@ -38,6 +38,7 @@ private slots: void unreadStylingSurvivesAnAccountChip(); void accountChipUsesTheConfiguredColour(); void deletedThreadsAreRedAndStruckThrough(); + void attachmentColumnIsFirstAndMarksOnlyTaggedThreads(); void spamThreadsAreOrangeAndStruckThrough(); void doomedStylingCoversEveryColumn(); void ordinaryThreadsCarryNoRowColour(); @@ -472,5 +473,46 @@ void TestThreadListModel::modelPassesQtTester() model.clear(); } +void TestThreadListModel::attachmentColumnIsFirstAndMarksOnlyTaggedThreads() +{ + // Leftmost, and narrow: the point is to see an attachment without opening + // the thread, which only works if the column is never scrolled away. + QCOMPARE(ThreadListModel::AttachmentColumn, 0); + + ThreadSummary plain = makeThread(QStringLiteral("t1"), + QStringLiteral("no attachment")); + ThreadSummary withFile = makeThread(QStringLiteral("t2"), + QStringLiteral("has one")); + // notmuch applies this tag itself while indexing, so no MIME parsing and + // no extra worker query are involved. + withFile.tags.append(QStringLiteral("attachment")); + + ThreadListModel model; + model.appendBatch({ plain, withFile }); + + const QModelIndex plainCell = + model.index(0, ThreadListModel::AttachmentColumn); + const QModelIndex fileCell = + model.index(1, ThreadListModel::AttachmentColumn); + + QVERIFY(model.data(plainCell, Qt::DisplayRole).toString().isEmpty()); + QCOMPARE(model.data(fileCell, Qt::DisplayRole).toString(), + ThreadListModel::attachmentGlyph()); + + // The glyph must be something a font can draw. An unrenderable codepoint + // shows as a tofu box, which reads as breakage rather than as a marker. + QVERIFY(!ThreadListModel::attachmentGlyph().isEmpty()); + + // Only the marked thread gets a tooltip, or an empty cell would claim to + // have an attachment on hover. + QVERIFY(model.data(plainCell, Qt::ToolTipRole).toString().isEmpty()); + QVERIFY(!model.data(fileCell, Qt::ToolTipRole).toString().isEmpty()); + + // The header carries no text: a label would set a minimum width far wider + // than the icon and defeat the narrow column. + QVERIFY(model.headerData(ThreadListModel::AttachmentColumn, Qt::Horizontal, + Qt::DisplayRole).toString().isEmpty()); +} + QTEST_MAIN(TestThreadListModel) #include "test_threadlistmodel.moc" |