wangp / bower

A curses terminal client for the Notmuch email system
Other
119 stars 11 forks source link

Archive action #20

Closed stain closed 7 years ago

stain commented 7 years ago

A quick proposal for an "Archive" function.

If you push . on a message in index view. It should:

This is similar to "Toggle unread", but is useful in particular when using bower tag:inbox, as compatible with the archiving function of other notmuch clients.

I'm afraid I'm quite new to Mercury so this pull request is just a start - I didn't manage to do multiple TagDeltas at once, and for some reason the display is not updated unless you click = - I only managed

Thanks for an excellent mail client with such good threading - really archiving is the only feature I seem to be missing!

stain commented 7 years ago

I guess it could also make sense to have "Archive" key also inside thread/msg view, but then only do it on that message?

wangp commented 7 years ago

Hi,

To make multiple tag changes at once, you should just need to change the type of modify_tag_cursor_line (as below) then fix whatever the type checker points out.

:- pred modify_tag_cursor_line(pred(index_line, index_line, list(tag_delta)), screen, index_info, index_info, io, io).

In the archive predicate, you'll want to update the i_tags field instead of i_std_tags.

Happy hacking!

stain commented 7 years ago

Thanks for the list() hint, that worked! I am not sure how to update i_flags - I need to update both as it also marks the line unread?

I am not sure how to update my Line with the new TagSet - I don't think I should have changed the predicate to semidet?

wangp commented 7 years ago

Actually, you need to update all of i_tags, i_std_tags, and i_nonstd_tags_width, but set_index_line_tags already does what we need. Untested:

archive(Line0, Line, TagDeltas) :-
    TagSet0 = Line0 ^ i_tags,
    set.delete_list(TagSet0, [tag("inbox"), tag("unread")], TagSet),
    set_index_line_tags(TagSet, Line0, Line),
    TagDeltas = [tag_delta("-inbox"), tag_delta("-unread")].
stain commented 7 years ago

Thanks, that worked, except for argument ordering of set.delete_list/3 :)

I also added . for archiving individual messages in thread view which works using same set operations.

this got me thinking if . is the right key, given that there's no obvious "unarchive" (mark unread, add to inbox) button across keyboard layouts. Anyway.. PR ready for your consideration :)

wangp commented 7 years ago

Any reason for .?

a is taken for adding to the addressbook but we could consider moving that uncommon action to some other key. The thread view has I that returns to the index view and marks the entire thread as read; perhaps we need a similar action for archiving.

One possibility might be a for archive, A for archive entire thread, Ctrl-A for add to addressbook (though Ctrl-A is strongly associated with "select all" when there is a list of some sort). Not sure yet.

stain commented 7 years ago

I would have preferred a but didn't feel brave enough to change address book - but if you agree then that would make sense.

Perhaps @ is a good mnemonic for address book?

The way I ended up using the key now is as a part of a kind of prefilter workflow where I go through all new messages and either use arrow down (to keep thread in current state) or . to read+archive it - thus with . being close to arrow down on British English keyboard that felt natural.

(It should be safe to use archive just for "mark read and go down" purpose when not using tag:inbox as well)

The key archive would be good for being easy to remember, but also it is close to delete and flag - so that would be better.

Currently . in the index view archives the whole thread - so I would think a should do the same there (almost like "silencing" the thread in alot) - and A could put the whole thread back in the inbox (but perhaps don't change read/unread) - then A could be useful not just when doing a mistake, but also when finding other threads that needs to resurface.

I think within the message view, a should be for just the message, and then again A should match the index view and put that message back into inbox - an action for the whole thread would be cool - it would make sense then to go back to index view, so I would suggest Q for that, which would be similar to the current I which marks the whole thread as read.

(Or we could swap Q and I around.. given that I is first letter of Inbox?)

stain commented 7 years ago

Have a try now with the suggested key bindings! Seems to work quite well - not sure about Shift-Q as it's a bit awkward finger-wise..

Would you know why in thread view A does not refresh the line to show the inbox tag, while a works fine? It's not because unarchive/2 doesn't set the unread flag (I tried, and the n showed up again, but still without the re-added tag)

stain commented 7 years ago

The unarchive updates sometimes in threads - but not if it is a very tiny thread with no scrolling.

wangp commented 7 years ago

You need to use get_standard_tags in both thread_pager.archive and thread_pager.unarchive.

The advice I gave you for the index_view.m module was a bit wrong. It turns out (as my own cryptic comment says) that i_std_tags is the definitive source for its tags, so you cannot just take the i_tags set and recompute i_std_tags from it. (not sure) Once we figure out the desired behaviour, I will redo the implementation.


Giving the patch a quick spin, I think a should work as a toggle like N and F. When applied to a thread with the inbox tag the action is -inbox -unread. When applied to a thread without the inbox tag the action is +inbox. Is it actually necessary to remove the unread tag?

In the thread view, a would operate on a single message. That frees up A for the return-to-index-and-archive action.

It might be worth reserving ^A for an analogous feature to ^R in the thread view, though I rarely use ^R after adding I.

Which leaves @ for the addressbook.

wangp commented 7 years ago

I implemented the toggling behaviour on the 'archive' branch.

stain commented 7 years ago

Thanks!

I must admit I don't like the toggling behaviour, it means I can't use a to go through my index list without thinking, and would accidentally unarchive some of the threads just because they happen to not be in the inbox. (my default search is tag:inbox OR tag:flagged, searches like tag:inbox OR tag:unread might also make sense)

(I already accidentally unread things with the toggling n because that particular thread happen to already be read :)).

Since a marks both -unread and -inbox, what the user would expect to toggle on (unread, inbox or both) all depends on what search is open in bower and what kind of workflow they use.

For instance I think it should make sense to do a search for a specific tag (e.g. a mailing list) and then go downwards using a to mark threads as both read and no longer in the inbox - if they happen to be so (some lists may never reach my inbox).

I think if the thread is not unread and not in inbox, a should be a no-op.

So I would put a vote in to make both a (and possibly also d, n and f) to not be toggling, but rather modify the message always in the same way (and the caps do their opposite.)

The default should be to set the opposite of a regular new message, which would be something like unread inbox, without important or deleted. Thus I would go for:

This would then be easier to use with selected multiple threads with ' to avoid the "Selected threads differ in unread state." etc.

stain commented 7 years ago

Now I tried out the archive branch, and it works well, the update problem is also gone.

I now think toggling should be either all of the keys or none of them - with my favourite for not toggling. But if you don't want to change n, f and d to "fixed" behaviour, then better go with your toggling a :-)

A to go out of a thread view works well - however a toggling a inside a thread view works poorly as the thread usually has a mix of states.

wangp commented 7 years ago

I don't want to change too much as everything is working as I expect, obviously :) n is also required for repeating the string search.

however a toggling a inside a thread view works poorly as the thread usually has a mix of states.

Do you mean because you have unread messages, where some of them have inbox and some of them don't?

How about if a applies -inbox -unread on a message/thread that has either tag, and +inbox otherwise? You can try it on the archive branch.

stain commented 7 years ago

Right, so I might for instance want to do bower project5 to find emails in project5 (old and new), and then click a to archive my way down through the threads (even if some of them might already be both read and archived).

With the toggling behaviour I will end up accidentally flipping some of the threads back into the inbox and have to use the arrow keys as well.

However toggling behaviour is at least consistent with how n etc. work - and easy enough to flip back on a mistake (no need to reach for shift-key) - I am already getting used to it :)

I agree on your latest commit 61ce17baf861cddacdc35996f3750685a97f9113 - this works quite well in both thread and index mode. And I can still use the shift-A which is "always archive" when leaving thread mode.

Thanks for listening to my inputs and the introduction to the code!

Given your own archive branch I guess I can close this pull request and list this as a regular issue instead.