Add move next / previous thread command
Add a move next thread and move previous thread command allowing the user to move between threads without going back to the search result. The default binding for these commands are J/K.
Very quickly as I haven't tested this:
The code looks neat and clean to me, you'd need to fix the failing tests but it seems that this is an issue of re-generating it's sources: PYTHON="python3" make -C docs cleanall (python if not on debian)..
The bigger issue is if such a command should be included at all. I've deliberately not included it in the past because
- I wanted to decouple thread from search buffers as much as possible
- thread buffers do have one "parent search buffer" but that doesn't have to be the case in future, for instance one could imagine opening a thread directly from CLI calls just as the notmuch CLI can. At the moment though, thread buffer do have a reference to "their" search, I can't recall why that was.
- search buffers are snapshots at the time they are opened and therefore may be outdated later. Some people consider this a feature (because they do not dynamically change), others really want continuously updated search buffers, which would make a feature like this one (semantically) unstable.
I updated the PR. This might not be the most elegant solution to the design but it should solve a few cases:
-
ThreadBufferdoes not need aSearchBufferto be initialized. - The
_search_buffervariable is now a weak reference which means if the thread buffer will not hold reference to the search buffer after it's destroyed. This is not the most elegant code as we cannot get a weak reference to None. But at least it should work. This is inspired by what notmuch implementation in Emacs is doing. Although, in Emacs, hitting the next thread shortcut results in the buffer being destroyed. I am not sure this is the best behavior and it might be an implementation issue. - I rebuilt the documentation and the tests are now passing.
- I think the snapshot issue with search buffers is not just an issue of this PR but a larger issue. I might be a good idea to have an auto-refresh option if some users want that. But in that case I understand that this CL makes things trickier considering the auto-refresh might only be triggered once the search buffer is active. I see two solutions to that: background threads or
I updated the PR. There is slight issue when 2 threads are opened from the same search buffer. If you, then the focused thread is shared between the 2, so you have the following behavior:
- Open tab 1 on thread 1
- Open tab 2 on thread 1
- Go to next thread on tab 2: tab 2 now displays thread 2
- Go to next thread on tab 1: tab 1 now displays thread 3
I checked and notmuch-emacs has the same issue. This can be solved by having each thread keeping track of which item they are focusing. But I don't think this is such an issue anyway so I kept it as it is.
oh yes of course that must happen! i haven thought of this 😄. The fix you propose should be easy really, as the list walker has getter for the current position and afaik, the next/prev position getter take those positions as optional parameter..