Re: [notmuch] [PATCH] New function notmuch-search-operate-all: operate on all messages in the current query.
On Mon, 23 Nov 2009 14:07:20 +0100, Jed Brown wrote: > > Second, since we're in the search view which shows threads, we should > > really be operating on threads. But this tag command won't work like the > > '+' and '-' commands in this buffer. Those commands will add/remove a > > tag to/from every message in the thread[*]. The new '*' command, however > > will only be changing tags on messages that match the query. > > I'm not convinced that we want to operate on the entire thread. > Applying the tag only to the matched messages is enough to find the > thread, and it may really be desirable to only have it applied to > certain messages in the thread. For example, I might have constructed > an elaborate query to find five messages, including a couple that are > burried in 100-message threads in which case I would definitely not want > to tag entire threads. That's a legitimate point. But my point is that whatever behavior we choose here, I want the commands that operate on a single thread (+, -, a) to operate exactly the same as the command that operates on all threads (*). Having these behave subtly different, (as in the current patch) is going to lead to confusion on the part of the user. So, tagging only matching messages could make a lot of sense. If so, let's make the operations on single threads work the same. The race-condition issues apply to both operations, so I won't make the current patch block on resolving those. -Carl ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[notmuch] [PATCH] New function notmuch-search-operate-all: operate on all messages in the current query.
On Mon, 23 Nov 2009 14:07:20 +0100, Jed Brown wrote: > > Second, since we're in the search view which shows threads, we should > > really be operating on threads. But this tag command won't work like the > > '+' and '-' commands in this buffer. Those commands will add/remove a > > tag to/from every message in the thread[*]. The new '*' command, however > > will only be changing tags on messages that match the query. > > I'm not convinced that we want to operate on the entire thread. > Applying the tag only to the matched messages is enough to find the > thread, and it may really be desirable to only have it applied to > certain messages in the thread. For example, I might have constructed > an elaborate query to find five messages, including a couple that are > burried in 100-message threads in which case I would definitely not want > to tag entire threads. That's a legitimate point. But my point is that whatever behavior we choose here, I want the commands that operate on a single thread (+, -, a) to operate exactly the same as the command that operates on all threads (*). Having these behave subtly different, (as in the current patch) is going to lead to confusion on the part of the user. So, tagging only matching messages could make a lot of sense. If so, let's make the operations on single threads work the same. The race-condition issues apply to both operations, so I won't make the current patch block on resolving those. -Carl
[notmuch] [PATCH] New function notmuch-search-operate-all: operate on all messages in the current query.
On Mon, 23 Nov 2009 05:14:25 +0100, Carl Worth wrote: > Second, I like that you just used the search string again, (as opposed > to just walking through the buffer looking at thread IDs). That seems > elegant. It was *easy*. > First, this creates a race condition in that the user will rightly > expect to only be adding/removing tags from the visible results. But if > new mail has been incorporated between creating the current view and > running '*' then threads could be tagged that were never seen and that > could be problematic. Agreed and I had also thought of the case you described. Note that Gmail does not solve the race condition. When in summary mode: * Marking a thread as read applies to all messages in the thread. The thread contents are being updated concurrently so you may mark threads you have already seen. * Same story with archiving (aka. remove inbox). * Starring a thread applies to the last message in the thread, this could be a newly arrived message that is not currently displayed. I think that handling a concurrent changes to the match results is a somewhat hard problem. You have to store the message ids of everything in the current query if you want to avoid the race. > Second, since we're in the search view which shows threads, we should > really be operating on threads. But this tag command won't work like the > '+' and '-' commands in this buffer. Those commands will add/remove a > tag to/from every message in the thread[*]. The new '*' command, however > will only be changing tags on messages that match the query. I'm not convinced that we want to operate on the entire thread. Applying the tag only to the matched messages is enough to find the thread, and it may really be desirable to only have it applied to certain messages in the thread. For example, I might have constructed an elaborate query to find five messages, including a couple that are burried in 100-message threads in which case I would definitely not want to tag entire threads. > So I think we should fix both of these issues by looping over each line > of the buffer and calling (notmuch-search-add-tag) or > (notmuch-search-remove-tag). Presumably you don't mean this literally because that would be horribly slow. What you want is to gather up all the message ids in all the threads with at least one message matching the current query (and currently displayed). I think this is only possible if notmuch-search holds message IDs for everything in the matched threads. If we are happy to just tag the matched messages instead of the entire thread, we would only store the message ids for the matched messages. > Oh, here's one: We could add something like "notmuch tag --expecting= > 1/23 " that would not do the tag unless the search string > matched 1 message out of 23 total messages in the thread. Then we could > give a warning to the user. I like this, but it still presents a minor race condition (suppose another client swaps the unread tag on two messages in the same thread). The only way to guarantee that you are operating on the displayed messages is to store their message ids. > That works for the single-thread case, but the warning would be harder > for the user to deal with in the '*' case. Or maybe not---the emacs > function could just stop on the first line with the error and then the > user would see what's going on. If you can implement notmuch tag --expecting with similar efficiency to the present, then I would vote for notifying the user and asking for confirmation if the number of matches has changed. This would be significantly safer than what Gmail does which ought to be sufficient for now given the age of notmuch. Jed
[notmuch] [PATCH] New function notmuch-search-operate-all: operate on all messages in the current query.
On Sun, 22 Nov 2009 21:12:16 +0100, Jed Brown wrote: > It is often convenient to change tags on several messages at once. This > function applies any number of tag whitespace-delimited tag > modifications to all messages matching the current query. > > I have bound this to `*'. Very nice, Jed! I've been wanting this feature for a while, and I think you just improved on my idea in at least two ways. First I was imagining that the '*' would be a prefix command, but I like it just prompting for the '+' or '-' as part of the argument. That's no more typing and allows for doing multiple tags at once. Second, I like that you just used the search string again, (as opposed to just walking through the buffer looking at thread IDs). That seems elegant. On second thought, however, using the search string is problematic for at least two reasons: First, this creates a race condition in that the user will rightly expect to only be adding/removing tags from the visible results. But if new mail has been incorporated between creating the current view and running '*' then threads could be tagged that were never seen and that could be problematic. Second, since we're in the search view which shows threads, we should really be operating on threads. But this tag command won't work like the '+' and '-' commands in this buffer. Those commands will add/remove a tag to/from every message in the thread[*]. The new '*' command, however will only be changing tags on messages that match the query. So I think we should fix both of these issues by looping over each line of the buffer and calling (notmuch-search-add-tag) or (notmuch-search-remove-tag). What do you think? -Carl [*] These existing '+' and '-' operations (as well as 'a') that act on the entire thread also have a race condition. They are potentially modifying more messages than the original search matched. This is often harmless, (you aren't even *seeing* the messages so how can you complain if more get modified than were there when you did the search.). But it can actually be fatal: Imagine I sent a message to the list, and then in the search view I see that message appear and it has [1/1] indicating it's the only message in the thread. I might archive this "knowing" I've read the message already, but this could actually archive a reply as well that arrived between when I did the search and when I archived. So that's a bug that we really should fix. [And noted that archiving an entire thread from the notmuch-show-mode does not have this bug since it acts only on the explicit messages that are presented.] One option to fix this would be for "notmuch search" to list all the message IDs that matched in place of the thread ID (for notmuch-search-mode to hide away like it does with the thread ID currently). But that seems like it might get problematic with some hugely long threads. Anyway, I'm interested in ideas for what we could do here. Oh, here's one: We could add something like "notmuch tag --expecting= 1/23 " that would not do the tag unless the search string matched 1 message out of 23 total messages in the thread. Then we could give a warning to the user. That works for the single-thread case, but the warning would be harder for the user to deal with in the '*' case. Or maybe not---the emacs function could just stop on the first line with the error and then the user would see what's going on. And we could take a prefix argument to ignore such errors. Any other thoughts on this?
[notmuch] [PATCH] New function notmuch-search-operate-all: operate on all messages in the current query.
It is often convenient to change tags on several messages at once. This function applies any number of tag whitespace-delimited tag modifications to all messages matching the current query. I have bound this to `*'. Signed-off-by: Jed Brown --- notmuch.el | 24 1 files changed, 24 insertions(+), 0 deletions(-) diff --git a/notmuch.el b/notmuch.el index a547415..b848e9a 100644 --- a/notmuch.el +++ b/notmuch.el @@ -764,6 +764,7 @@ thread from that buffer can be show when done with this one)." (define-key map (kbd "RET") 'notmuch-search-show-thread) (define-key map "+" 'notmuch-search-add-tag) (define-key map "-" 'notmuch-search-remove-tag) +(define-key map "*" 'notmuch-search-operate-all) (define-key map "<" 'beginning-of-buffer) (define-key map ">" 'notmuch-search-goto-last-thread) (define-key map "=" 'notmuch-search-refresh-view) @@ -940,6 +941,29 @@ This function advances point to the next line when finished." (notmuch-search-remove-tag "inbox") (forward-line)) +(defun notmuch-search-operate-all (action) + "Operate on all messages matching the current query. Any +number of whitespace separated actions can be given. Each action +must have one of the two forms + + +tagname Add the tag `tagname' + -tagname Remove the tag `tagname' + +Each character of the tag name may consist of alphanumeric +characters as well as `_.+-'. +" + (interactive "sOperation (+add -drop): notmuch tag ") + (let ((action-split (split-string action " +"))) +;; Perform some validation +(let ((words action-split)) + (when (null words) (error "No operation given")) + (while words + (unless (string-match-p "^[\+\-][_\+\-\\w]+$" (car words)) + (error "Action must be of the form `+thistag -that_tag'")) + (setq words (cdr words +(apply 'notmuch-call-notmuch-process "tag" + (append action-split (list notmuch-search-query-string) nil + (defun notmuch-search (query &optional oldest-first) "Run \"notmuch search\" with the given query string and display results." (interactive "sNotmuch search: ") -- 1.6.5.3
