aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-09-29 20:06:24 +0200
committerDanilo M. <danix@danix.xyz>2026-09-29 20:06:24 +0200
commit236ac86ca09a24300e724c2372f3576bf3f6ff84 (patch)
tree9ba6eefb4ddd7d5e37e1cb167faf334ae2290cd0
parentfca0a2226e52b4e01ba199950a4eb308040c3e11 (diff)
downloadqtmaildir-236ac86ca09a24300e724c2372f3576bf3f6ff84.tar.gz
qtmaildir-236ac86ca09a24300e724c2372f3576bf3f6ff84.zip
fix: open a conversation's first message itself for --messagecli-selectors
--message naming the first message of a multi-message conversation selected the conversation row, so the pane showed the dashboard rather than the message. The recovery matched the conversation row on its own message id, which is the root's. The rule is that --message always targets the message, shown inside its expanded thread, and that --thread is the way to ask for the conversation. Since item 177 setThreadMessages keeps a conversation's first message as child 0, so the root already has a row of its own. applyPendingRecovery() now lets the thread row answer for a named message only when that row is not a conversation, which leaves a thread of one opening on its message as before, and finds the root among the children like any reply. The provisional pass still selects the thread while the tree loads and is refined to the root's row when the children arrive. The same recovery serves the stale notice and a double-click. A stale notice raised while reading the root's row now reopens that row, which is the same request. A double-click on a conversation row asks for the conversation, so it no longer names the root and still lands on the dashboard. recoveryOnTheFirstMessageSelectsTheThreadRow asserted the old behaviour on the pre-177 premise that the root has no child row; it is retargeted as recoveryOnTheFirstMessageSelectsItsOwnRow. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
-rw-r--r--CHANGELOG.md7
-rw-r--r--README.md7
-rw-r--r--src/mainwindow.cpp49
-rw-r--r--tests/test_mainwindow.cpp209
4 files changed, 243 insertions, 29 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 830c09e..6701417 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -16,8 +16,11 @@ point at which they are stable.
- `--account`, `--thread` and `--message` on the command line, so another
program can open qtmaildir at a particular account's view, conversation or
message. The three combine, and Qt's own options are accepted beside them.
- `--account` alone opens the startup view in that account; `--thread` and
- `--message` look in every account unless `--account` narrows them, and
+ `--account` alone opens the startup view in that account. `--thread` shows
+ the conversation's overview and `--message` always shows the message
+ itself inside its expanded conversation, the first message included.
+ `--thread` and `--message` look in every account unless `--account`
+ narrows them, and
`--message` accepts a Message-ID with or without its angle brackets. A
selector that matches nothing is named in the status bar and the window
keeps the view it had.
diff --git a/README.md b/README.md
index 8443b57..4ceb7d7 100644
--- a/README.md
+++ b/README.md
@@ -55,9 +55,10 @@ qtmaildir [options]
```
`--account` on its own opens the startup view (`startup_query`) in that
-account. `--thread` and `--message` open the whole conversation with that
-message selected, and look for it in every account, switching the account
-selector to All accounts. The three selectors combine: `--account work
+account. `--thread` opens the whole conversation on its overview. `--message`
+opens the same conversation expanded, with that message selected and shown,
+including when it is the conversation's first message. Both look in every
+account, switching the account selector to All accounts. The three selectors combine: `--account work
--message '<abc@example.org>'` looks for that message in the work account only,
and a message that lives elsewhere is a miss. `--message` takes a Message-ID
with or without its angle brackets. `--thread` takes a notmuch thread id (hex
diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index 19bf3c2..434f5d0 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -5382,8 +5382,9 @@ void MainWindow::applySelectors(const LaunchSelectors &requested)
// the thread when its row arrives and selects it. Item 91's
// double-click already reuses it; this is the third caller.
//
- // The empty message id is meaningful to it: land on the ROOT row,
- // which is the thread's first message.
+ // The empty message id is meaningful to it: land on the thread's own
+ // row, the dashboard for a conversation and the message for a thread
+ // of one.
recoverStaleThread(selectors.threadId, QString());
// After the query, which clears it: this is that query's own miss.
m_launchMiss = tr("No thread matched '%1'.").arg(selectors.threadId);
@@ -5492,10 +5493,16 @@ void MainWindow::onRowDoubleClicked(const QModelIndex &index)
messageId = node.messageId;
} else {
threadId = m_model->data(index, ThreadListModel::ThreadIdRole).toString();
- // The thread's first message, so the pane opens on it rather than on
- // nothing. Empty is fine and means the same thing to the recovery: land
- // on the root, which IS that message.
- messageId = m_model->data(index, ThreadListModel::MessageIdRole).toString();
+ // A CONVERSATION row asks for the conversation, so no message is
+ // named and the recovery lands on the row itself, on the dashboard. A
+ // named message is always a message row to the recovery, which would
+ // open the root's own row instead.
+ //
+ // A thread of one is its message: named, so the pane opens on it.
+ // Empty is fine there too and means the same thing to the recovery.
+ if (!m_model->isConversationRow(index))
+ messageId =
+ m_model->data(index, ThreadListModel::MessageIdRole).toString();
}
if (threadId.isEmpty())
@@ -5600,18 +5607,31 @@ void MainWindow::applyPendingRecovery()
// happen before any attempt to find one.
m_threadView->expand(thread);
- // The thread's first message IS the root card rather than a child row:
- // setThreadMessages drops depth 0 because the root stands for it, so
- // looking for it among the children finds nothing and the selection
- // would silently land nowhere.
+ // A named message is ALWAYS a message row, never the conversation.
+ // The user's rule for --message: "we target ALWAYS the message. If
+ // that message is part of a thread we show it inside the thread, but
+ // still the message. If I want the thread there's --thread." The
+ // stale notice and a double-click on a message row mean the same.
+ //
+ // So the thread row answers for a message only when it is NOT a
+ // conversation: a thread of one is its message and has no child to
+ // select. A conversation carries its first message as child 0 since
+ // item 177 (setThreadMessages keeps it), so the root is found among
+ // the children below like any reply. Matching the conversation row on
+ // its own message id, as this did, landed on the dashboard.
+ //
+ // An empty id is the request for the thread itself: --thread, a
+ // double-click on a conversation row, a stale dashboard.
//
// selectRowAt(), not setCurrentIndex(): a current index without a
// selection is what QTreeView sets by itself on focus, and
// onThreadSelected() deliberately ignores that, so pointing at the row
// renders nothing and leaves the pane blank.
if (m_recoverMessageId.isEmpty()
- || m_model->data(thread, ThreadListModel::MessageIdRole).toString()
- == m_recoverMessageId) {
+ || (!m_model->isConversationRow(thread)
+ && m_model->data(thread, ThreadListModel::MessageIdRole)
+ .toString()
+ == m_recoverMessageId)) {
selectRowAt(thread);
m_recoverThreadId.clear();
m_recoverMessageId.clear();
@@ -5627,9 +5647,8 @@ void MainWindow::applyPendingRecovery()
// (`src/threadlistmodel.cpp`), so the root check above cannot match yet
// and returning here would leave the user looking at a collapsed thread
// and a blank pane until the replies happen to arrive. Selecting the
- // thread renders its first message immediately, which is the right
- // answer outright when that is what they were reading, and is refined
- // to the correct reply on the next pass when it is not.
+ // thread puts something on screen immediately, and it is refined to
+ // the named message's own row, the root's included, on the next pass.
//
// The target is deliberately NOT cleared: this pass is provisional.
if (m_model->rowCount(thread) == 0) {
diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp
index a22c153..0aaf8ff 100644
--- a/tests/test_mainwindow.cpp
+++ b/tests/test_mainwindow.cpp
@@ -327,6 +327,8 @@ private slots:
void aThreadSelectorOpensThatThread();
void aMessageSelectorOpensItsThread();
void aMessageSelectorRendersItsReplyWhateverTheKeyboardLastHeld();
+ void aMessageSelectorForARootShowsTheMessageNotTheDashboard();
+ void doubleClickingAConversationStillLandsOnItsDashboard();
void aThreadSelectorThatIsNotHexIsRefused();
void anEmptySelectorSetChangesNothing();
void aBracketedMessageSelectorOpensThatMessage();
@@ -387,7 +389,7 @@ private slots:
void theStaleNoticeCarriesTheMessageBeingRead();
void recoveringAStaleThreadQueriesTheWholeThread();
void recoveryReselectsTheMessageThatWasBeingRead();
- void recoveryOnTheFirstMessageSelectsTheThreadRow();
+ void recoveryOnTheFirstMessageSelectsItsOwnRow();
void doubleClickingAThreadOpensThatThreadAlone();
void doubleClickingAReplyOpensItsThreadNotTheReplyAlone();
void doubleClickingDoesNotLeaveTheMarkReadTimerArmed();
@@ -3476,12 +3478,15 @@ void TestMainWindow::recoveryReselectsTheMessageThatWasBeingRead()
QStringLiteral("m2@example.org"));
}
-void TestMainWindow::recoveryOnTheFirstMessageSelectsTheThreadRow()
+void TestMainWindow::recoveryOnTheFirstMessageSelectsItsOwnRow()
{
- // The trap in the model: setThreadMessages DROPS the depth-0 message,
- // because the root card is that message. So a reader recovering from the
- // thread's first message must land on the ROOT row; looking for it among
- // the children finds nothing and would leave the selection nowhere.
+ // A named message is always a MESSAGE row, the conversation's first
+ // included. This test asserted the opposite until the --message rule:
+ // "we target ALWAYS the message". Its premise had gone with item 177,
+ // which made setThreadMessages keep the depth-0 message as a
+ // conversation's child 0, so the root has a row of its own and landing
+ // on the conversation row showed the dashboard instead of the message.
+ // A thread of one still lands on its only row, which is its message.
const Config config;
MainWindow window(config);
@@ -3525,9 +3530,12 @@ void TestMainWindow::recoveryOnTheFirstMessageSelectsTheThreadRow()
const QModelIndex current = view->currentIndex();
QVERIFY2(current.isValid(), "recovery selected nothing");
- QVERIFY2(!model->isMessageRow(current),
- "the thread's first message is the ROOT row, not a child");
- QCOMPARE(model->threadAt(current.row()).threadId, QStringLiteral("T1"));
+ QVERIFY2(model->isMessageRow(current),
+ "the first message landed on the conversation row, which shows "
+ "the dashboard rather than the message");
+ QCOMPARE(model->messageAt(current).messageId,
+ QStringLiteral("m0@example.org"));
+ QCOMPARE(model->threadFor(current).threadId, QStringLiteral("T1"));
}
void TestMainWindow::doubleClickingAThreadOpensThatThreadAlone()
@@ -18114,6 +18122,189 @@ void TestMainWindow::aMessageSelectorRendersItsReplyWhateverTheKeyboardLastHeld(
15000);
}
+void TestMainWindow::aMessageSelectorForARootShowsTheMessageNotTheDashboard()
+{
+ // --message naming a conversation's FIRST message. The user's rule:
+ // "When running with --message we target ALWAYS the message. If that
+ // message is part of a thread we show it inside the thread, but still the
+ // message. If I want the thread there's --thread."
+ //
+ // The conversation row's own message id IS the root's, so a recovery that
+ // matched it there selected the conversation and the pane showed the
+ // dashboard. Under item 177 the root has a child row of its own inside an
+ // expanded conversation, and that row is what must end up current.
+ WorkerBackedWindow backed;
+ // Dates verified with `date -d <yyyy-mm-dd> +%A`: Qt::RFC2822Date
+ // validates the weekday against the date.
+ QVERIFY(backed.fixture().addMessage(
+ QStringLiteral("inbox"), QStringLiteral("b1@example.org"),
+ QStringLiteral("Bravo opens"), QStringLiteral("c@example.org"),
+ QStringLiteral("Sun, 16 Aug 2026 10:00:00 +0200"),
+ QStringLiteral("Bravo one.")));
+ QVERIFY(backed.fixture().addMessage(
+ QStringLiteral("inbox"), QStringLiteral("b2@example.org"),
+ QStringLiteral("Bravo later reply"), QStringLiteral("d@example.org"),
+ QStringLiteral("Mon, 17 Aug 2026 10:00:00 +0200"),
+ QStringLiteral("Bravo two."), true,
+ QStringLiteral("b1@example.org")));
+ QVERIFY(backed.fixture().addMessage(
+ QStringLiteral("inbox"), QStringLiteral("c1@example.org"),
+ QStringLiteral("Charlie alone"), QStringLiteral("e@example.org"),
+ QStringLiteral("Tue, 18 Aug 2026 10:00:00 +0200"),
+ QStringLiteral("Charlie one.")));
+ QVERIFY2(backed.build(), qPrintable(backed.error()));
+
+ NotmuchWorker probe(backed.config().notmuchConfig());
+ const QString bravo =
+ probe.threadIdForTesting(QStringLiteral("id:b1@example.org"));
+ const QString other =
+ probe.threadIdForTesting(QStringLiteral("id:c1@example.org"));
+ QVERIFY(!bravo.isEmpty());
+ QVERIFY(!other.isEmpty());
+
+ MainWindow window(backed.config());
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<ThreadListView *>();
+ QVERIFY(view);
+ auto *pane = window.findChild<MessageView *>();
+ QVERIFY(pane);
+ auto *header = pane->findChild<QLabel *>(QStringLiteral("messageHeader"));
+ QVERIFY(header);
+ auto *queryEdit = window.findChild<QLineEdit *>(QStringLiteral("queryEdit"));
+ QVERIFY(queryEdit);
+
+ // A different state first, asserted, so the checks after the selector
+ // mean something: another thread listed beside it, and another message on
+ // display.
+ queryEdit->setText(QStringLiteral("tag:inbox"));
+ emit queryEdit->returnPressed();
+ QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 2, 15000);
+ QModelIndex otherRow;
+ for (int row = 0; row < model->rowCount(QModelIndex()); ++row) {
+ if (model->threadAt(row).threadId == other)
+ otherRow = model->index(row, 0, QModelIndex());
+ }
+ QVERIFY(otherRow.isValid());
+ view->selectionModel()->select(
+ otherRow, QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows);
+ view->setCurrentIndex(otherRow);
+ QTRY_VERIFY_WITH_TIMEOUT(
+ !pane->showingPlaceholder() && !pane->showingDashboard()
+ && header->text().contains(QStringLiteral("Charlie alone")),
+ 15000);
+
+ LaunchSelectors selectors;
+ selectors.messageId = QStringLiteral("b1@example.org");
+ window.applySelectors(selectors);
+
+ // The conversation alone is listed, and expanded.
+ QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1
+ && model->threadAt(0).threadId == bravo,
+ 15000);
+ const QModelIndex conversation = model->index(0, 0, QModelIndex());
+ QTRY_VERIFY_WITH_TIMEOUT(view->isExpanded(conversation), 15000);
+
+ // The ROOT's own message row current and selected, never the
+ // conversation row that stands for the whole thread.
+ QTRY_VERIFY_WITH_TIMEOUT(
+ view->currentIndex().isValid()
+ && model->isMessageRow(view->currentIndex())
+ && model->messageAt(view->currentIndex()).messageId
+ == QStringLiteral("b1@example.org"),
+ 15000);
+ QVERIFY(view->selectionModel()->isSelected(view->currentIndex()));
+ QCOMPARE(view->currentIndex().parent(), conversation);
+
+ // What the user sees: that message, not the dashboard and not the
+ // placeholder.
+ QTRY_VERIFY_WITH_TIMEOUT(
+ !pane->showingDashboard() && !pane->showingPlaceholder()
+ && header->text().contains(QStringLiteral("Bravo opens")),
+ 15000);
+}
+
+void TestMainWindow::doubleClickingAConversationStillLandsOnItsDashboard()
+{
+ // The other side of --message's rule. A double-click on a CONVERSATION
+ // row asks for the conversation, and the recovery now treats any named
+ // message as a message row, so naming the root here would open the
+ // root's own row. The gesture names none, and lands on the dashboard.
+ WorkerBackedWindow backed;
+ // Dates verified with `date -d <yyyy-mm-dd> +%A`.
+ QVERIFY(backed.fixture().addMessage(
+ QStringLiteral("inbox"), QStringLiteral("b1@example.org"),
+ QStringLiteral("Bravo opens"), QStringLiteral("c@example.org"),
+ QStringLiteral("Sun, 16 Aug 2026 10:00:00 +0200"),
+ QStringLiteral("Bravo one.")));
+ QVERIFY(backed.fixture().addMessage(
+ QStringLiteral("inbox"), QStringLiteral("b2@example.org"),
+ QStringLiteral("Bravo later reply"), QStringLiteral("d@example.org"),
+ QStringLiteral("Mon, 17 Aug 2026 10:00:00 +0200"),
+ QStringLiteral("Bravo two."), true,
+ QStringLiteral("b1@example.org")));
+ QVERIFY(backed.fixture().addMessage(
+ QStringLiteral("inbox"), QStringLiteral("c1@example.org"),
+ QStringLiteral("Charlie alone"), QStringLiteral("e@example.org"),
+ QStringLiteral("Tue, 18 Aug 2026 10:00:00 +0200"),
+ QStringLiteral("Charlie one.")));
+ QVERIFY2(backed.build(), qPrintable(backed.error()));
+
+ NotmuchWorker probe(backed.config().notmuchConfig());
+ const QString bravo =
+ probe.threadIdForTesting(QStringLiteral("id:b1@example.org"));
+ const QString other =
+ probe.threadIdForTesting(QStringLiteral("id:c1@example.org"));
+ QVERIFY(!bravo.isEmpty());
+ QVERIFY(!other.isEmpty());
+
+ MainWindow window(backed.config());
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<ThreadListView *>();
+ QVERIFY(view);
+ auto *pane = window.findChild<MessageView *>();
+ QVERIFY(pane);
+ auto *header = pane->findChild<QLabel *>(QStringLiteral("messageHeader"));
+ QVERIFY(header);
+ auto *queryEdit = window.findChild<QLineEdit *>(QStringLiteral("queryEdit"));
+ QVERIFY(queryEdit);
+
+ // A message on display first, so landing on the dashboard is a change.
+ queryEdit->setText(QStringLiteral("tag:inbox"));
+ emit queryEdit->returnPressed();
+ QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 2, 15000);
+ QModelIndex otherRow;
+ QModelIndex bravoRow;
+ for (int row = 0; row < model->rowCount(QModelIndex()); ++row) {
+ if (model->threadAt(row).threadId == other)
+ otherRow = model->index(row, 0, QModelIndex());
+ if (model->threadAt(row).threadId == bravo)
+ bravoRow = model->index(row, 0, QModelIndex());
+ }
+ QVERIFY(otherRow.isValid());
+ QVERIFY(bravoRow.isValid());
+ view->selectionModel()->select(
+ otherRow, QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows);
+ view->setCurrentIndex(otherRow);
+ QTRY_VERIFY_WITH_TIMEOUT(
+ !pane->showingPlaceholder() && !pane->showingDashboard()
+ && header->text().contains(QStringLiteral("Charlie alone")),
+ 15000);
+
+ emit view->doubleClicked(bravoRow);
+
+ QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1
+ && model->threadAt(0).threadId == bravo,
+ 15000);
+ const QModelIndex conversation = model->index(0, 0, QModelIndex());
+ QTRY_VERIFY_WITH_TIMEOUT(view->isExpanded(conversation)
+ && !window.hasPendingRecoveryForTesting(),
+ 15000);
+ QCOMPARE(view->currentIndex(), conversation);
+ QTRY_VERIFY_WITH_TIMEOUT(pane->showingDashboard(), 15000);
+}
+
void TestMainWindow::aThreadSelectorThatIsNotHexIsRefused()
{
// The thread id now comes from another program's command line or the