diff options
| -rw-r--r-- | src/avatar.cpp | 69 | ||||
| -rw-r--r-- | src/carddelegate.cpp | 26 | ||||
| -rw-r--r-- | src/carddelegate.h | 12 | ||||
| -rw-r--r-- | src/notmuchworker.cpp | 91 | ||||
| -rw-r--r-- | src/threadlistmodel.cpp | 7 | ||||
| -rw-r--r-- | src/types.h | 12 | ||||
| -rw-r--r-- | tests/test_avatar.cpp | 115 | ||||
| -rw-r--r-- | tests/test_carddelegate.cpp | 23 | ||||
| -rw-r--r-- | tests/test_notmuchworker.cpp | 30 | ||||
| -rw-r--r-- | tests/test_threadlistmodel.cpp | 31 |
10 files changed, 358 insertions, 58 deletions
diff --git a/src/avatar.cpp b/src/avatar.cpp index 7aaa6eb..9b292a2 100644 --- a/src/avatar.cpp +++ b/src/avatar.cpp @@ -26,6 +26,42 @@ namespace { +/// The bare display name from whatever the card's first line holds. +/// +/// That string is the RAW header for a reply row (`Name <addr>`) and a +/// comma-joined author summary for a thread row, so a naive space split gives +/// `T<` for one and one letter from each of two different people for the +/// other. Take the first entry, drop the angle-addr and any quoting, and +/// report nothing when what remains is itself an address: the caller's address +/// branch reads that far better than the local part would. +QString displayNameOf(const QString &raw) +{ + QString name = raw.trimmed(); + + // `Name <addr>`: everything before the bracket is the name. When there is + // nothing before it, the address itself is not a name. + const int bracket = name.indexOf(QLatin1Char('<')); + if (bracket >= 0) + name = name.left(bracket).trimmed(); + + // A comma joins either two authors or a `"Rossi, Mario"` quoted name. The + // quotes tell them apart, so strip them only after splitting. + if (!name.startsWith(QLatin1Char('"'))) { + const int comma = name.indexOf(QLatin1Char(',')); + if (comma >= 0) + name = name.left(comma).trimmed(); + } + if (name.size() >= 2 && name.startsWith(QLatin1Char('"')) + && name.endsWith(QLatin1Char('"'))) { + name = name.mid(1, name.size() - 2).trimmed(); + } + + // A bare address left standing is not a name. + if (name.contains(QLatin1Char('@'))) + return QString(); + return name; +} + QString twoFrom(const QString &text) { const QString trimmed = text.trimmed(); @@ -43,8 +79,19 @@ namespace Avatar { QString initialsFor(const QString &displayName, const QString &address, const QString &accountLabel) { - const QStringList words = displayName.split(QLatin1Char(' '), - Qt::SkipEmptyParts); + // Words, and a separator is not one. `INE - Expert IT Training` split on + // spaces alone gave `I-`, because the dash counted as the second word. + QStringList words; + const QStringList parts = displayNameOf(displayName) + .split(QLatin1Char(' '), Qt::SkipEmptyParts); + for (const QString &part : parts) { + // Trim leading punctuation so `(Team)` still contributes its `T`. + int at = 0; + while (at < part.size() && !part.at(at).isLetterOrNumber()) + ++at; + if (at < part.size()) + words.append(part.mid(at)); + } if (words.size() >= 2) { return (words.at(0).left(1) + words.at(1).left(1)).toUpper(); } @@ -83,7 +130,10 @@ Fill fillFor(const QString &displayName, bool isBusinessSender) // heuristic, or a listed sender could never be pinned. if (isBusinessSender) return Fill::TwoTone; - return displayName.trimmed().isEmpty() ? Fill::TwoTone : Fill::Identicon; + // The same normalisation initialsFor() uses: a raw `<addr>` header or a + // bare address is not a display name, so it must not read as a person. + return displayNameOf(displayName).isEmpty() ? Fill::TwoTone + : Fill::Identicon; } QColor colourFor(const QString &address) @@ -150,9 +200,18 @@ QPixmap pixmapFor(const QString &seed, const QString &initials, Fill fill, // behind the letters stays large and flat, which is the whole reason // this fill exists beside the identicon. const int angle = static_cast<quint8>(digest.at(1)) * 360 / 256; - QLineF axis = QLineF::fromPolar(side, angle); + // The axis must span the DIAMETER through the centre, not a radius + // from it. fromPolar() starts at the origin, so translating by half + // the side put p1 at the centre and the 0.5 colour stop on the + // squircle's edge: one hue filled almost the whole face and the fill + // read as flat. The diagonal, so the split still crosses the shape at + // any angle. + const qreal reach = side * 0.71; + QLineF axis = QLineF::fromPolar(reach, angle); axis.translate(side / 2.0, side / 2.0); - QLinearGradient gradient(axis.p2(), axis.p1()); + QLineF back = QLineF::fromPolar(reach, angle + 180.0); + back.translate(side / 2.0, side / 2.0); + QLinearGradient gradient(back.p2(), axis.p2()); gradient.setColorAt(0.0, base); gradient.setColorAt(0.499, base); gradient.setColorAt(0.5, base.darker(135)); diff --git a/src/carddelegate.cpp b/src/carddelegate.cpp index 89c3ed0..2dd29a2 100644 --- a/src/carddelegate.cpp +++ b/src/carddelegate.cpp @@ -107,12 +107,20 @@ QRect CardDelegate::fadeRectFor(const QRect &card, const QRect &innermostSpine) { // The EXCLUSIVE right edge, then a rect built from it: QRect::right() is // inclusive, which is the trap CardLayout already documents. - const int end = card.left() + int(card.width() * kFadeFraction); - const int start = innermostSpine.isEmpty() ? card.left() - : innermostSpine.left(); - if (end <= start) + // Right to left: the wash is opaque at the card's RIGHT edge, where a + // hard stop is the card's own boundary, and fades out before reaching the + // accent bar, which already states the account. Drawing it the other way + // put the hard stop mid-card and read as a slab. + // + // A reply's left limit is its own spine rather than the card's edge, so + // the wash steps right with the nesting and stays shorter. + const int left = innermostSpine.isEmpty() ? card.left() + : innermostSpine.left(); + const int start = card.right() + 1 - int(card.width() * kFadeFraction); + if (card.right() + 1 <= qMax(start, left)) return QRect(); - return QRect(start, card.top(), end - start, card.height()); + return QRect(qMax(start, left), card.top(), + card.right() + 1 - qMax(start, left), card.height()); } QColor CardDelegate::accentLineColour(const QColor &accountColour) @@ -219,7 +227,13 @@ void CardDelegate::paint(QPainter *painter, const QStyleOptionViewItem &option, // A reply's wash is weaker than its root's, so an expanded thread // reads as one block with the root leading it. from.setAlphaF(card.accentRect.isEmpty() ? 0.14 : 0.30); - QLinearGradient gradient(fade.topLeft(), fade.topRight()); + // Right to left: opaque at the fade's far end, transparent where the + // accent bar already carries the colour. Drawing it the other way put + // the strongest wash under the bar, which is the one place the account + // is already stated. + QLinearGradient gradient(fade.topRight(), fade.topLeft()); + // Opaque at the right edge, gone at the left, so the only hard stop + // is the card's own boundary. gradient.setColorAt(0.0, from); from.setAlphaF(0.0); gradient.setColorAt(1.0, from); diff --git a/src/carddelegate.h b/src/carddelegate.h index 18c735f..cfbd23e 100644 --- a/src/carddelegate.h +++ b/src/carddelegate.h @@ -81,16 +81,18 @@ public: /// Where the account's fade runs, given the card and the row's innermost /// spine (an empty rect for a thread root, which has none). /// - /// A root's fade starts at the card's left edge; a reply's starts at its - /// own spine, which IS its coloured left border, so the wash steps right - /// with the nesting. Both end at 60% of the card's width, so a deeper - /// reply's wash is shorter as well as further right. + /// Runs RIGHT to left: opaque at the card's right edge and gone 60% of the + /// width in, so the only hard stop is the card's own boundary and the + /// accent bar is left to state the account on its own. A reply is clamped + /// at its own spine, which IS its coloured left border, so its wash is + /// shorter as well as further right. /// /// Static and rect-in, rect-out so the geometry is assertable without a /// painter, for the same reason CardLayout is. static QRect fadeRectFor(const QRect &card, const QRect &innermostSpine); - /// How far across the card the account's colour reaches. + /// How far back across the card, from its right edge, the account's + /// colour reaches. static constexpr qreal kFadeFraction = 0.60; /// A tag chip's colour, drained for the SIBLING tier (item 111). diff --git a/src/notmuchworker.cpp b/src/notmuchworker.cpp index 3de79e1..e78d8c8 100644 --- a/src/notmuchworker.cpp +++ b/src/notmuchworker.cpp @@ -81,6 +81,51 @@ QStringList tagsOf(notmuch_thread_t *thread) return result; } +/// The first mailbox address in a raw address header, display names dropped. +/// +/// Parsed with GMime rather than split: the header is untrusted and a display +/// name may legally contain an `@`, so "Ian <a@b>" split on `@` yields +/// nonsense. +QString firstMailboxOf(const QString &rawHeader) +{ + // GMime must be initialised once per process before any parse, or the + // first internet_address_list_parse() call dereferences an uninitialised + // type registry and SEGVs. Function-local static, exactly as the other + // gmime-using units do, so this file does not lean on libnotmuch having + // initialised it as a side effect (measured 2026-08-26: it currently + // does, but that is not documented anywhere). + static const bool initialised = [] { + g_mime_init(); + return true; + }(); + Q_UNUSED(initialised); + + const QByteArray utf8 = rawHeader.trimmed().toUtf8(); + if (utf8.isEmpty()) + return QString(); + + InternetAddressList *list = + internet_address_list_parse(nullptr, utf8.constData()); + if (!list) + return QString(); + + QString address; + const int count = internet_address_list_length(list); + for (int i = 0; i < count; ++i) { + InternetAddress *entry = internet_address_list_get_address(list, i); + if (!entry || !INTERNET_ADDRESS_IS_MAILBOX(entry)) + continue; + const char *addr = + internet_address_mailbox_get_addr(INTERNET_ADDRESS_MAILBOX(entry)); + if (addr && *addr) { + address = QString::fromUtf8(addr); + break; + } + } + g_object_unref(list); + return address; +} + /// Who a thread's messages were addressed to, summarised for one line. /// /// EXPENSIVE, and only called when a query asks. "To" is not served from @@ -94,7 +139,7 @@ QStringList tagsOf(notmuch_thread_t *thread) /// frees again, and the walk finishes before the caller drops the thread. This /// is the same rule walkReplies follows, and getting it wrong is a double-free /// rather than a leak. -QString recipientsOf(notmuch_thread_t *thread) +QString recipientsOf(notmuch_thread_t *thread, QString *firstAddress) { // The first message with a usable To wins. A thread is one conversation, // and the alternative, folding every message's recipients together, is the @@ -114,8 +159,14 @@ QString recipientsOf(notmuch_thread_t *thread) continue; const QString summary = recipientSummary(QString::fromUtf8(to)); - if (!summary.isEmpty()) + if (!summary.isEmpty()) { + // The same header, for the avatar's hash (item 169). Parsed rather + // than split for the reason senderAddressOf() documents: a display + // name may legally contain an `@`. + if (firstAddress) + *firstAddress = firstMailboxOf(QString::fromUtf8(to)); return summary; + } } return QString(); } @@ -129,41 +180,10 @@ QString recipientsOf(notmuch_thread_t *thread) /// may legally contain an `@`, and "Ian <a@b>" split on `@` yields nonsense. QString senderAddressOf(notmuch_message_t *message) { - // GMime must be initialised once per process before any parse, or the - // first internet_address_list_parse() call dereferences an uninitialised - // type registry and SEGVs. Function-local static, exactly as the other - // gmime-using units do, so this file does not lean on libnotmuch having - // initialised it as a side effect (measured 2026-08-26: it currently - // does, but that is not documented anywhere). - static const bool initialised = [] { - g_mime_init(); - return true; - }(); - Q_UNUSED(initialised); - const char *from = notmuch_message_get_header(message, "From"); if (!from || !*from) return QString(); - - InternetAddressList *list = internet_address_list_parse(nullptr, from); - if (!list) - return QString(); - - QString address; - const int count = internet_address_list_length(list); - for (int i = 0; i < count; ++i) { - InternetAddress *entry = internet_address_list_get_address(list, i); - if (!entry || !INTERNET_ADDRESS_IS_MAILBOX(entry)) - continue; - const char *addr = - internet_address_mailbox_get_addr(INTERNET_ADDRESS_MAILBOX(entry)); - if (addr && *addr) { - address = QString::fromUtf8(addr); - break; - } - } - g_object_unref(list); - return address; + return firstMailboxOf(QString::fromUtf8(from)); } /// Collects the message ids a query matches. Returns false if the query could @@ -455,7 +475,8 @@ void NotmuchWorker::runQuery(const QString &query, quint64 generation, summary.matchedCount = notmuch_thread_get_matched_messages(thread.get()); summary.tags = tagsOf(thread.get()); if (withRecipients) - summary.recipients = recipientsOf(thread.get()); + summary.recipients = + recipientsOf(thread.get(), &summary.firstMessageRecipient); // The message the row's card stands for. Raw pointers on purpose: // messages reached through a thread are owned by the THREAD and freed diff --git a/src/threadlistmodel.cpp b/src/threadlistmodel.cpp index a6fada3..f085b79 100644 --- a/src/threadlistmodel.cpp +++ b/src/threadlistmodel.cpp @@ -689,6 +689,13 @@ QVariant ThreadListModel::data(const QModelIndex &index, int role) const // The bare address of the message this card stands for, carried from // the query. This is what the avatar's hash and the business-senders // lookup consume; the display name below is what the card draws. + // + // The recipient first in a flat view, matching SendersRole below and + // for the same reason: the sender there is the user on every row, so + // every card would hash to one pattern. Falls back to the sender when + // the fold found no usable To. + if (!thread.firstMessageRecipient.isEmpty()) + return thread.firstMessageRecipient; return thread.firstMessageSender; case SenderNameRole: // The same string the card's first line shows: recipients in a flat diff --git a/src/types.h b/src/types.h index a1689ab..a0f772b 100644 --- a/src/types.h +++ b/src/types.h @@ -113,6 +113,18 @@ struct ThreadSummary /// row and carries nothing. QString recipients; + /// The FIRST recipient's bare address, for the avatar in a flat view. + /// + /// Filled by the same walk as `recipients` and under the same flag, so it + /// costs nothing extra: the To header is already parsed there. + /// + /// A Sent or Drafts card's `firstMessageSender` is the user on every row, + /// so every card would carry one pattern and only the initials would vary. + /// The avatar answers "who is this row about", and there that is the + /// recipient. `firstMessageSender` stays the fallback for a row with no + /// usable To. + QString firstMessageRecipient; + bool isUnread() const { return tags.contains(QStringLiteral("unread")); } bool isFlagged() const { return tags.contains(QStringLiteral("flagged")); } /// True when this message was forwarded. The Maildir "P" (passed) flag, diff --git a/tests/test_avatar.cpp b/tests/test_avatar.cpp index 72e5941..cea88b3 100644 --- a/tests/test_avatar.cpp +++ b/tests/test_avatar.cpp @@ -30,10 +30,14 @@ private slots: void bareAddressTakesLocalAndDomain(); void nothingUsableFallsBackToTheAccountLabel(); void initialsAreAlwaysTwoLetters(); + void aRawFromHeaderIsNotSplitOnItsBracket(); + void aCommaJoinedAuthorListTakesTheFirstAuthor(); + void aSeparatorIsNotAWord(); void aDisplayNameMeansAPerson(); void theListOverridesADisplayName(); void aColourIsStablePerAddress(); void aPixmapIsStableAndDiffersPerSeed(); + void bothTwoToneHuesReachTheFace(); }; void TestAvatar::twoWordNameTakesOneLetterFromEach() @@ -95,6 +99,61 @@ void TestAvatar::initialsAreAlwaysTwoLetters() } } +void TestAvatar::aRawFromHeaderIsNotSplitOnItsBracket() +{ + // A reply row's first line is the RAW header, so a naive space split gave + // the name's first letter and a literal `<`. + QCOMPARE(Avatar::initialsFor( + QStringLiteral("tsujan <notifications@github.com>"), + QStringLiteral("notifications@github.com"), + QStringLiteral("Work")), + QStringLiteral("TS")); + // A bare address in the name's place is not a name: the address branch + // answers, rather than the local part's first two letters. + QCOMPARE(Avatar::initialsFor(QStringLiteral("info@moomhotel.com"), + QStringLiteral("info@moomhotel.com"), + QStringLiteral("Work")), + QStringLiteral("IM")); + // And the fill agrees: neither of those is a display name. + QCOMPARE(Avatar::fillFor(QStringLiteral("info@moomhotel.com"), false), + Avatar::Fill::TwoTone); +} + +void TestAvatar::aCommaJoinedAuthorListTakesTheFirstAuthor() +{ + // notmuch's author summary joins participants with a comma, so one letter + // from each gave initials belonging to two different people. + QCOMPARE(Avatar::initialsFor(QStringLiteral("Standreas, tsujan"), + QStringLiteral("notifications@github.com"), + QStringLiteral("Work")), + QStringLiteral("ST")); + // A QUOTED name may legally contain a comma and must survive whole. + QCOMPARE(Avatar::initialsFor(QStringLiteral("\"Rossi, Mario\""), + QStringLiteral("m@example.org"), + QStringLiteral("Work")), + QStringLiteral("RM")); +} + +void TestAvatar::aSeparatorIsNotAWord() +{ + // `INE - Expert IT Training` took the dash as its second word and drew + // `I-`. A word has to carry a letter or a digit. + QCOMPARE(Avatar::initialsFor(QStringLiteral("INE - Expert IT Training"), + QStringLiteral("news@example.org"), + QStringLiteral("Work")), + QStringLiteral("IE")); + // Leading punctuation is trimmed rather than disqualifying the word. + QCOMPARE(Avatar::initialsFor(QStringLiteral("(Acme) Support"), + QStringLiteral("s@example.org"), + QStringLiteral("Work")), + QStringLiteral("AS")); + // Punctuation ONLY is no name at all: the address answers. + QCOMPARE(Avatar::initialsFor(QStringLiteral("- ---"), + QStringLiteral("news@example.org"), + QStringLiteral("Work")), + QStringLiteral("NE")); +} + void TestAvatar::aDisplayNameMeansAPerson() { // The case the user asked for by name: a corporate address that presents @@ -149,5 +208,61 @@ void TestAvatar::aPixmapIsStableAndDiffersPerSeed() QVERIFY(first.toImage() != twoTone.toImage()); } +void TestAvatar::bothTwoToneHuesReachTheFace() +{ + // The split has to cross the squircle, not graze its edge. Building the + // gradient axis as a RADIUS from the centre put the 0.5 stop on the + // boundary, so one hue filled almost the whole face and the fill read as + // flat: reported against noreply@cofidis.it. A colour count is what + // distinguishes the two, since both versions paint every pixel. + // + // Several seeds, because one unlucky angle proves nothing either way. + const QStringList seeds { QStringLiteral("noreply@cofidis.it"), + QStringLiteral("a@example.org"), + QStringLiteral("b@example.org"), + QStringLiteral("c@example.org") }; + for (const QString &seed : seeds) { + const QImage face = + Avatar::pixmapFor(seed, QStringLiteral("XX"), + Avatar::Fill::TwoTone, 64, QFont()).toImage(); + + // The two hues, as painted. Sampled by counting pixels of each rather + // than probing a corner: which corner gets which hue depends on the + // hashed angle. + const QColor base = Avatar::colourFor(seed); + const QColor dark = base.darker(135); + int light = 0, shade = 0; + for (int y = 0; y < face.height(); ++y) { + for (int x = 0; x < face.width(); ++x) { + const QColor pixel = face.pixelColor(x, y); + if (pixel.alpha() < 255) + continue; // The squircle's antialiased corners. + // Nearest of the two, not an exact match: the gradient + // interpolates in premultiplied space and the pixel format + // rounds, so an exact compare finds NEITHER hue and the probe + // reports 0 against 0 whatever the code does. + const int toBase = qAbs(pixel.red() - base.red()) + + qAbs(pixel.green() - base.green()) + + qAbs(pixel.blue() - base.blue()); + const int toDark = qAbs(pixel.red() - dark.red()) + + qAbs(pixel.green() - dark.green()) + + qAbs(pixel.blue() - dark.blue()); + if (toBase < toDark) + ++light; + else + ++shade; + } + } + + // A fifth of the face each: enough that neither is a sliver, loose + // enough that the hashed angle is free to put the split anywhere. + const int fifth = face.width() * face.height() / 5; + QVERIFY2(light > fifth && shade > fifth, + qPrintable(QStringLiteral("%1: one hue took the face, %2 " + "light against %3 dark") + .arg(seed).arg(light).arg(shade))); + } +} + QTEST_MAIN(TestAvatar) #include "test_avatar.moc" diff --git a/tests/test_carddelegate.cpp b/tests/test_carddelegate.cpp index 8b53309..bb21ae1 100644 --- a/tests/test_carddelegate.cpp +++ b/tests/test_carddelegate.cpp @@ -267,7 +267,9 @@ void TestCardDelegate::theFadeEndsAtSixtyPercentOfTheCard() { const QRect card(0, 0, 500, 60); const QRect root = CardDelegate::fadeRectFor(card, QRect()); - QCOMPARE(root.left(), card.left()); + // Anchored at the card's RIGHT edge: the hard stop belongs where the card + // ends, not 60% across it, which read as a slab. + QCOMPARE(root.right(), card.right()); QCOMPARE(root.width(), 300); } @@ -276,14 +278,21 @@ void TestCardDelegate::aReplyFadeStartsAtItsOwnSpine() const QRect card(0, 0, 500, 60); // The innermost spine of a nested reply, which is its own coloured border. const QRect spine(80, 0, 2, 60); - const QRect reply = CardDelegate::fadeRectFor(card, spine); - - // It hangs off the spine, not off the card's edge. - QCOMPARE(reply.left(), spine.left()); - // And still ends at 60% of the CARD, so a deeper reply's wash is shorter - // as well as further right. + // A spine deep enough to cut into the wash, which starts at 40% here. + const QRect deep(300, 0, 2, 60); + const QRect reply = CardDelegate::fadeRectFor(card, deep); + + // Clamped at the spine, so the wash never runs under a reply's own border. + QCOMPARE(reply.left(), deep.left()); + // Still anchored at the card's right edge, so a deeper reply's wash is + // shorter rather than displaced. QCOMPARE(reply.right(), CardDelegate::fadeRectFor(card, QRect()).right()); QVERIFY(reply.width() < CardDelegate::fadeRectFor(card, QRect()).width()); + + // A shallow spine sits left of where the wash begins and changes nothing. + const QRect shallow(80, 0, 2, 60); + QCOMPARE(CardDelegate::fadeRectFor(card, shallow), + CardDelegate::fadeRectFor(card, QRect())); } void TestCardDelegate::theDelegateAsksForAScaledSquircle() diff --git a/tests/test_notmuchworker.cpp b/tests/test_notmuchworker.cpp index 4fa8629..9a0896d 100644 --- a/tests/test_notmuchworker.cpp +++ b/tests/test_notmuchworker.cpp @@ -76,6 +76,7 @@ private slots: void recipientsAreAbsentUnlessAskedFor(); void recipientsAreFoldedWhenAskedFor(); void recipientsCrossAQueuedCall(); + void theFirstRecipientsAddressCrossesForTheAvatar(); void requestCountsAnswersOneCountPerQuery(); void requestCountsKeepsPositionOnAnInvalidQuery(); @@ -1078,6 +1079,35 @@ void TestNotmuchWorker::recipientsAreFoldedWhenAskedFor() "two plus one: %1").arg(summary))); } +void TestNotmuchWorker::theFirstRecipientsAddressCrossesForTheAvatar() +{ + // Item 169's flat-view avatar. `recipients` is a DISPLAY summary and + // carries no address at all when every recipient has a name, so the hash + // needs the bare one; it rides the same fold, so it costs nothing extra. + const QVector<ThreadSummary> one = + runQuery(QStringLiteral("subject:Preventivo"), + NotmuchWorker::NewestFirst, true); + QCOMPARE(one.size(), 1); + QCOMPARE(one.at(0).firstMessageRecipient, + QStringLiteral("mario@example.org")); + + // A quoted display name containing a comma must not defeat the parse, for + // the same reason it must not defeat the summary. + const QVector<ThreadSummary> many = + runQuery(QStringLiteral("subject:Riunione"), + NotmuchWorker::NewestFirst, true); + QCOMPARE(many.size(), 1); + QCOMPARE(many.at(0).firstMessageRecipient, + QStringLiteral("mario@example.org")); + + // And it stays empty when the query never asked, exactly as `recipients` + // does: it is behind the same performance contract. + const QVector<ThreadSummary> unasked = + runQuery(QStringLiteral("subject:Preventivo")); + QCOMPARE(unasked.size(), 1); + QVERIFY(unasked.at(0).firstMessageRecipient.isEmpty()); +} + void TestNotmuchWorker::recipientsCrossAQueuedCall() { // The trap CLAUDE.md records for SortOrder, in the shape it takes for this diff --git a/tests/test_threadlistmodel.cpp b/tests/test_threadlistmodel.cpp index 5c10b6c..9ca35e3 100644 --- a/tests/test_threadlistmodel.cpp +++ b/tests/test_threadlistmodel.cpp @@ -104,6 +104,7 @@ private slots: void recipientsReplaceTheSenderWhenPresent(); void aRowCarriesItsSenderAndAccountAddress(); void aMessageRowCarriesItsOwnSenderAndAddress(); + void aFlatViewsAvatarFollowsTheRecipient(); }; static ThreadSummary makeThread(const QString &id, const QString &subject) @@ -560,6 +561,36 @@ void TestThreadListModel::aMessageRowCarriesItsOwnSenderAndAddress() QStringLiteral("Bob <bob@example.org>")); } +void TestThreadListModel::aFlatViewsAvatarFollowsTheRecipient() +{ + // In a Sent or Drafts view firstMessageSender is the USER on every row, so + // hashing it gives one pattern for the whole list. The recipient is what + // the row is about, and SendersRole already follows the same rule. + ThreadListModel model; + ThreadSummary summary; + summary.threadId = QStringLiteral("t1"); + summary.subject = QStringLiteral("Subject"); + summary.authors = QStringLiteral("Me"); + summary.firstMessageId = QStringLiteral("m1"); + summary.firstMessageSender = QStringLiteral("me@example.org"); + summary.recipients = QStringLiteral("John Doe"); + summary.firstMessageRecipient = QStringLiteral("john@example.org"); + model.appendBatch({ summary }); + + QCOMPARE(model.index(0, 0).data(ThreadListModel::SenderAddressRole) + .toString(), + QStringLiteral("john@example.org")); + + // No usable To: the sender is the fallback rather than a blank seed. + ThreadListModel bare; + summary.recipients.clear(); + summary.firstMessageRecipient.clear(); + bare.appendBatch({ summary }); + QCOMPARE(bare.index(0, 0).data(ThreadListModel::SenderAddressRole) + .toString(), + QStringLiteral("me@example.org")); +} + void TestThreadListModel::theReplyCountExcludesTheRootMessage() { ThreadListModel model; |