Skip to content

Commit

Permalink
sort: avoid assertion under <attach-message>
Browse files Browse the repository at this point in the history
Fixes: neomutt#2996

Commit b698a6a added an assert that tctx->tree was set and dropped an
'if', because I could not at the time find any scenario where we would
try sorting threads but not find any.  But in the meantime, I found
(at least) one: when using <attach-message> and selecting the same
folder as already being visited, and when no new mail has arrived in
the meantime, then ALL messages already have e->thread set, and there
is no need to sort subthreads.  But the comment behind the assert was
correct: in that code path, we forgot to restore the value of $sort
after temporarily munging it to the value of $sort_aux; and this
latent problem can be observed even in the 20210205 release (I did not
research to find when it was first introduced).

Later, commit aaa2ece changed things so that we no longer munge
$sort in the first place.  So now all we need to do is remove the
bogus assertion, while still restoring the 'if' to avoid unnecessary
work if there are no new threads that need subthread sorting.

Note that the issue of potentially leaving $sort munged to $sort_aux
if ctx->tree remains unset after searching for new threads is present
even in upstream mutt.  But upstream never saw the problem with
<attach-message> because it always opened a NEW mailbox context,
passing init=true.  It is only in neomutt (and possibly only with
notmuch folders, since I didn't manage to reproduce with maildir),
where we try harder to reuse an existing mailbox and pass init=false
while tctx->tree is still NULL.
  • Loading branch information
ebblake authored and flatcap committed Jul 2, 2021
1 parent e822117 commit 91c190a
Showing 1 changed file with 9 additions and 7 deletions.
16 changes: 9 additions & 7 deletions mutt_thread.c
Original file line number Diff line number Diff line change
Expand Up @@ -1096,15 +1096,17 @@ void mutt_sort_threads(struct ThreadsContext *tctx, bool init)

/* if $sort_aux or similar changed after the mailbox is sorted, then
* all the subthreads need to be resorted */
assert(tctx->tree);
mutt_sort_subthreads(tctx, init || OptSortSubthreads);
OptSortSubthreads = false;
if (tctx->tree)
{
mutt_sort_subthreads(tctx, init || OptSortSubthreads);
OptSortSubthreads = false;

/* Put the list into an array. */
linearize_tree(tctx);
/* Put the list into an array. */
linearize_tree(tctx);

/* Draw the thread tree. */
mutt_draw_tree(tctx);
/* Draw the thread tree. */
mutt_draw_tree(tctx);
}
}

/**
Expand Down

0 comments on commit 91c190a

Please sign in to comment.