feat(ui): step by index, and give thread stepping a second binding

next_thread and prev_thread now walk with indexBelow/indexAbove, skipping
message rows, so they keep meaning thread-to-thread whatever is expanded.
Stepping message-to-message needs no code: QTreeView's own Up/Down walk VISIBLE
rows and already enter an expanded thread, and being the view's key handling
rather than a shortcut they stay inert when the message pane, a menu or an entry
bar has focus.

Item 60 turns out to have been fixed already, in 5487d58 on this branch, by
threadRowOf() walking up to the containing thread before doing the arithmetic.
The backlog entry was written against master, where that helper does not exist,
so it described a defect this branch had resolved a commit earlier. Verified by
writing both failing tests first and watching them pass: from the last reply of
an expanded thread, and from a thread root with its replies showing. They are
kept, because the property they assert is the one this change must not lose.
What the rewrite buys is that nothing is keyed on a row number any more, which
is the rule a deeper tree would break next.

Alt+Up/Down added alongside Ctrl+J/K. That required KeyMap::sequencesFor and a
move from setShortcut to setShortcuts, because the singular setter keeps only
the last binding and the second one was silently unreachable. Alt because
Shift+arrows is the built-in extend-selection that multi-row tagging depends on,
and because a bare arrow cannot be a window shortcut without breaking every text
field in the window, as Return already demonstrated. sequencesFor puts
sequenceFor's own choice first so the menus advertise an unchanged binding, and
sorts the tail, since QHash order is unspecified.

1 files changed, 28 insertions, 17 deletions

diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index 30148d4..57a6988 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -629,9 +629,11 @@ QAction *MainWindow::addAction(const QString &name, const QString &text,
 
     // The binding comes from KeyMap, so a [keys] override reaches the menus
     // and the shortcut reference as well as the keyboard.
-    const QKeySequence sequence = m_keyMap.sequenceFor(name);
-    if (!sequence.isEmpty())
-        action->setShortcut(sequence);
+    // Plural: an action can carry more than one binding, and setShortcut()
+    // keeps only the last one given. next_thread has both Ctrl+J and Alt+Down.
+    const QList<QKeySequence> sequences = m_keyMap.sequencesFor(name);
+    if (!sequences.isEmpty())
+        action->setShortcuts(sequences);
 
     // Shortcuts must work while focus is in the thread list or the message
     // view, not only on the window itself.
@@ -655,23 +657,32 @@ void MainWindow::registerActions()
     });
     addAction(QStringLiteral("next_thread"), tr("&Next thread"),
               tr("Select the next thread"), [this]() {
-        // The THREAD after this one, which is not "the next row" once replies
-        // are expanded: from a thread row the next row may be its own first
-        // reply, and from a reply row the row number counts siblings, not
-        // threads. Both are resolved by walking up to the containing thread
-        // first.
-        const QModelIndex current = m_threadView->currentIndex();
-        const QModelIndex thread = threadRowOf(current);
-        const int row = thread.isValid() ? thread.row() + 1 : 0;
-        if (row < m_model->rowCount())
-            selectThreadRow(row);
+        // Walked by INDEX, never by row number. A tree numbers rows per
+        // parent, so current.row() + 1 names a SIBLING: from the last reply of
+        // an expanded thread it asks for a row that does not exist, and from a
+        // thread row it counts top-level threads only by accident (item 60).
+        //
+        // The skip loop is what keeps this meaning thread-to-thread while the
+        // view's own Up/Down still steps message-to-message.
+        QModelIndex index = m_threadView->indexBelow(
+            m_threadView->currentIndex());
+        while (index.isValid()
+               && index.data(ThreadListModel::IsMessageRole).toBool()) {
+            index = m_threadView->indexBelow(index);
+        }
+        if (index.isValid())
+            selectRowAt(index);
     });
     addAction(QStringLiteral("prev_thread"), tr("&Previous thread"),
               tr("Select the previous thread"), [this]() {
-        const QModelIndex current = m_threadView->currentIndex();
-        const QModelIndex thread = threadRowOf(current);
-        if (thread.isValid() && thread.row() > 0)
-            selectThreadRow(thread.row() - 1);
+        QModelIndex index = m_threadView->indexAbove(
+            m_threadView->currentIndex());
+        while (index.isValid()
+               && index.data(ThreadListModel::IsMessageRole).toBool()) {
+            index = m_threadView->indexAbove(index);
+        }
+        if (index.isValid())
+            selectRowAt(index);
     });
     addAction(QStringLiteral("open_thread"), tr("&Open thread"),
               tr("Focus the thread list"), [this]() {