From f0381d6095b1fce9b0d223208cb92552208b9e4c Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Sat, 8 Aug 2026 10:22:33 +0200 Subject: feat(model): expose a thread's replies as child rows setThreadMessages drops the depth-0 message: it is the thread's first message and the root row already stands for it. Keeping it would show a thread of seven as one root and seven children, contradicting the reply count the row advertises. Calling again replaces rather than appends, so a thread reloaded after a sync does not list its replies twice. A message row reports its own sender and subject, not the thread's. That is the mistake worth guarding: the thread's author summary usually contains the first sender too, so reading it renders something plausible for the root's own reply and wrong for every other one. Mutation-checked, and the wrong version returns 'Alice' where 'Bob' belongs. Child rows carry no tag pills. The strip is a row-wide band of the thread's tags; one under each reply would stripe the list and repeat identical tags down the expansion. --- src/threadlistmodel.cpp | 129 ++++++++++++++++++++++++++++++++++++++++++++++-- src/threadlistmodel.h | 29 +++++++++++ 2 files changed, 153 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/threadlistmodel.cpp b/src/threadlistmodel.cpp index dabc447..3bc02d6 100644 --- a/src/threadlistmodel.cpp +++ b/src/threadlistmodel.cpp @@ -169,12 +169,60 @@ QVariant ThreadListModel::data(const QModelIndex &index, int role) const return {}; } - // Message rows are handled in Task 4; until then only thread rows exist and - // a child index cannot be produced. The bound is checked against the thread - // list only after establishing this IS a thread row, since a child row's + // A message row. Handled before the bounds check below, since a child row's // number indexes its siblings, not m_threads. - if (index.parent().isValid()) - return {}; + if (isMessageRow(index)) { + const MessageNode node = messageAt(index); + if (node.messageId.isEmpty()) + return {}; + + switch (role) { + case IsMessageRole: + return true; + case MessageIdRole: + return node.messageId; + case MessageDepthRole: + return node.depth; + case ThreadIdRole: + // A message row still belongs to a thread, and a caller that only + // needs the containing thread must not have to walk up itself. + return node.threadId; + case TagsRole: + case PillTagsRole: + // No strip under a child row: the strip is a ROW-wide band carrying + // the thread's tags, and one under every reply would stripe the + // list and repeat the same tags down the whole expansion. + return QStringList(); + case PillColoursRole: + return QVariantList(); + case AccountLabelRole: + return QString(); + case Qt::DisplayRole: + switch (index.column()) { + case AuthorsColumn: + // The REPLY's sender, not the thread's author summary. Reading + // the thread's fields here would look almost right, since the + // first sender usually appears in both. + return node.from; + case SubjectColumn: + return node.subject; + case DateColumn: + return node.date.toString(QStringLiteral("yyyy-MM-dd hh:mm")); + case AttachmentColumn: + return node.hasAttachment() ? attachmentGlyph() : QString(); + case FlagColumn: + return node.isFlagged() ? flagGlyph() : QString(); + default: + return {}; + } + case Qt::ForegroundRole: + // Same rule as a thread row: read recedes, unread stays at the + // palette's own colour. + return node.isUnread() ? QVariant() : QVariant(readColour()); + default: + return {}; + } + } if (index.row() >= m_threads.size()) return {}; @@ -184,6 +232,19 @@ QVariant ThreadListModel::data(const QModelIndex &index, int role) const if (role == ThreadIdRole) return thread.threadId; + // Answered rather than left to fall through as an invalid QVariant. An + // invalid one converts to false and an empty string anyway, so the + // behaviour is the same, but a role the model never mentions is a latent + // bug the next reader has to prove is harmless. + if (role == IsMessageRole) + return false; + + if (role == MessageIdRole) + return QString(); + + if (role == MessageDepthRole) + return 0; + if (role == TagsRole) return thread.tags; @@ -376,6 +437,64 @@ void ThreadListModel::clear() endResetModel(); } +void ThreadListModel::setThreadMessages(const QString &threadId, + const QVector &nodes) +{ + for (int row = 0; row < m_threads.size(); ++row) { + if (m_threads.at(row).summary.threadId != threadId) + continue; + + const QModelIndex parent = index(row, 0, QModelIndex()); + + // Replace, not append. A thread reloaded after a sync would otherwise + // list every reply twice. + if (!m_threads.at(row).children.isEmpty()) { + beginRemoveRows(parent, 0, m_threads.at(row).children.size() - 1); + m_threads[row].children.clear(); + endRemoveRows(); + } + + QVector children; + children.reserve(nodes.size()); + for (const MessageNode &node : nodes) { + if (node.depth > 0) + children.append(node); + } + + if (!children.isEmpty()) { + beginInsertRows(parent, 0, children.size() - 1); + m_threads[row].children = children; + endInsertRows(); + } + + // Set even when there are no replies: that is the difference between a + // single-message thread and one whose replies were never fetched. + m_threads[row].loaded = true; + return; + } +} + +bool ThreadListModel::isMessageRow(const QModelIndex &index) const +{ + return index.isValid() && index.parent().isValid(); +} + +MessageNode ThreadListModel::messageAt(const QModelIndex &index) const +{ + if (!isMessageRow(index)) + return {}; + + const int threadRow = index.parent().row(); + if (threadRow < 0 || threadRow >= m_threads.size()) + return {}; + + const QVector &children = m_threads.at(threadRow).children; + if (index.row() < 0 || index.row() >= children.size()) + return {}; + + return children.at(index.row()); +} + ThreadSummary ThreadListModel::threadAt(int row) const { if (row < 0 || row >= m_threads.size()) diff --git a/src/threadlistmodel.h b/src/threadlistmodel.h index 1aa0271..a7ce5d3 100644 --- a/src/threadlistmodel.h +++ b/src/threadlistmodel.h @@ -84,6 +84,18 @@ public: /// model because it owns the TagColors instance; a delegate reading /// config itself would be a second source of truth. PillColoursRole, + + /// True when the row is a MESSAGE row rather than a thread root. + /// Drives both the action scope and whether the view paints a tag + /// strip under the row. + IsMessageRole, + + /// The message id behind a message row. Empty on a thread root. + MessageIdRole, + + /// The message's reply depth, for the view's indentation. 1 for a + /// direct reply, since depth 0 is the root row itself. + MessageDepthRole, }; /// Row fill for a thread tagged `deleted`, and for one tagged `spam`. @@ -131,6 +143,23 @@ public: ThreadSummary threadAt(int row) const; + /// Fills in a thread's message rows once the worker has walked its tree. + /// + /// The depth-0 message is dropped: it is the thread's first message and the + /// ROOT row already stands for it. Keeping it would show a thread of seven + /// as one root and seven children, contradicting the reply count the row + /// advertises. Calling again replaces the rows rather than appending, so a + /// thread reloaded after a sync does not list its replies twice. + void setThreadMessages(const QString &threadId, + const QVector &nodes); + + /// True when the index is a message row rather than a thread root. + bool isMessageRow(const QModelIndex &index) const; + + /// The message row's node, or a default-constructed one for any index that + /// is not a message row. + MessageNode messageAt(const QModelIndex &index) const; + /// The account keys behind a thread's account tags, for item 49's /// per-account sync. /// -- cgit v1.2.3