Fix toggletags - Githubissues

pazz / alot

Terminal-based Mail User Agent

GNU General Public License v3.0

683 stars 163 forks source link

Fix toggletags #1564

Closed mjg closed 3 years ago

mjg commented 3 years ago

There still seems to be quite a fall-out from the conversion of alot to nm2. This PR so far only solves one related to the toggletags command and the question whether toggle is per message or per thread (best described in the commit messages).

I have other weird effects, such as notmuch search giving a thread with 16 messages and alot search giving me only 1 message for that thread, and so does the thread buffer. I have not tracked this down yet.

This PR might serve to collect more fixes or be merged directly if deemed correct. Am I the only one experiencing these issues? I don't see any related bug reports.

pazz commented 3 years ago

It is entirely possible that this has been overlooked when moving to notmuch2. I also agree that the semantics of "toggletags" on a whole thread in search mode should be based on the union of tags on all containted messages, and affect all messages so that they agree on the toggled tag after the operation.

Happy to push this. Could you fix the last three codeclimate comments?

mjg commented 3 years ago

It is entirely possible that this has been overlooked when moving to notmuch2. I also agree that the semantics of "toggletags" on a whole thread in search mode should be based on the union of tags on all containted messages, and affect all messages so that they agree on the toggled tag after the operation.

Especially since it used to be like that.

I had started to doubt myself, or rather: my local patches that I'm running with, and removed all of them to make sure it's not them. But still I'm wondering there were no reports at all - maybe not too many people are using an alot version with nm2 yet?

Happy to push this. Could you fix the last three codeclimate comments?

Of course, I just wanted to be sure this is going in the right direction.

mjg commented 3 years ago

About codeclimate:

I reduce the nesting in the writer queue code by 1 level, codeclimate must have complained before.
The insistance on 79 chars no matter what makes me write uglier code. There needs to be some fuzziness for that limt.

pazz commented 3 years ago

Shouldn't provide collect_tags() rather fo into the bindings? We can add it here for now as long as they don't have that implemented. Generally, I would prefer to keep the PRs small so please don't add too much more here. I honestly don't remember if on notmuch v1, alot's tagging behaviour was message based, but I'm fine with the proposed behaviour.