This fixes races in thread-local and global tagging in notmuch-search (e.g., "+", "-", "a", "*", etc.). Previously, these would modify tags of new messages that arrived after the search. Now they only operate on the messages that were in the threads when the search was performed. This prevents surprises like archiving messages that arrived in a thread after the search results were shown.
This eliminates `notmuch-search-find-thread-id-region(-search)' because these functions strongly encouraged racy usage. This fixes the two broken tests added by the previous patch. --- devel/TODO | 5 ----- emacs/notmuch.el | 40 +++++++++++++++++++++++++++------------- test/emacs | 4 +--- 3 files changed, 28 insertions(+), 21 deletions(-) diff --git a/devel/TODO b/devel/TODO index f212483..1cf4089 100644 --- a/devel/TODO +++ b/devel/TODO @@ -14,11 +14,6 @@ sender's name containing ';' which causes emacs to drop a search result.) This may require removing the outer array from the current "notmuch search --format=json" results. -Fix '*' to work by simply calling '+' or '-' on a region consisting of -the entire buffer, (this would avoid one race condition---while still -leaving other race conditions---but could also potentially make '*' a -very expensive operation). - Add a global keybinding table for notmuch, and then view-specific tables that add to it. diff --git a/emacs/notmuch.el b/emacs/notmuch.el index 2c7678d..1c64745 100644 --- a/emacs/notmuch.el +++ b/emacs/notmuch.el @@ -461,14 +461,25 @@ If BARE is set then do not prefix with \"thread:\"" (let ((thread (plist-get (notmuch-search-get-result) :thread))) (when thread (concat (unless bare "thread:") thread)))) -(defun notmuch-search-find-thread-id-region (beg end) - "Return a list of threads for the current region" - (mapcar (lambda (thread) (concat "thread:" thread)) - (notmuch-search-properties-in-region :thread beg end))) - -(defun notmuch-search-find-thread-id-region-search (beg end) - "Return a search string for threads for the current region" - (mapconcat 'identity (notmuch-search-find-thread-id-region beg end) " or ")) +(defun notmuch-search-find-stable-query () + "Return the stable queries for the current thread. + +This returns a list (MATCHED-QUERY UNMATCHED-QUERY) for the +matched and unmatched messages in the current thread." + (plist-get (notmuch-search-get-result) :query)) + +(defun notmuch-search-find-stable-query-region (beg end &optional only-matched) + "Return the stable query for the current region. + +If ONLY-MATCHED is non-nil, include only matched messages. If it +is nil, include both matched and unmatched messages." + (let ((query-list nil) (all (not only-matched))) + (dolist (queries (notmuch-search-properties-in-region :query beg end)) + (when (first queries) + (push (first queries) query-list)) + (when (and all (second queries)) + (push (second queries) query-list))) + (concat "(" (mapconcat 'identity query-list ") or (") ")"))) (defun notmuch-search-find-authors () "Return the authors for the current thread" @@ -525,9 +536,12 @@ If BARE is set then do not prefix with \"thread:\"" (setq output (append output (notmuch-search-get-tags pos))))) output)) -(defun notmuch-search-tag-region (beg end &optional tag-changes) - "Change tags for threads in the given region." - (let ((search-string (notmuch-search-find-thread-id-region-search beg end))) +(defun notmuch-search-tag-region (beg end &optional tag-changes only-matched) + "Change tags for threads in the given region. + +If ONLY-MATCHED is non-nil, only tag matched messages." + (let ((search-string (notmuch-search-find-stable-query-region + beg end only-matched))) (setq tag-changes (notmuch-tag search-string tag-changes)) (notmuch-search-foreach-result beg end (lambda (pos) @@ -796,7 +810,7 @@ non-authors is found, assume that all of the authors match." See `notmuch-tag' for information on the format of TAG-CHANGES." (interactive) - (apply 'notmuch-tag notmuch-search-query-string tag-changes)) + (notmuch-search-tag-region (point-min) (point-max) tag-changes t)) (defun notmuch-search-buffer-title (query) "Returns the title for a buffer with notmuch search results." @@ -899,7 +913,7 @@ the configured default sort order." (save-excursion (let ((proc (notmuch-start-notmuch "notmuch-search" buffer #'notmuch-search-process-sentinel - "search" "--format=sexp" "--format-version=1" + "search" "--format=sexp" "--format-version=2" (if oldest-first "--sort=oldest-first" "--sort=newest-first") diff --git a/test/emacs b/test/emacs index e0dcd3c..88eb1e5 100755 --- a/test/emacs +++ b/test/emacs @@ -889,7 +889,7 @@ $PWD/notmuch_fail exited with status 1 (see *Notmuch errors* for more details) --- [XXX] $PWD/notmuch_fail exited with status 1 -command: $PWD/notmuch_fail search --format\=sexp --format-version\=1 --sort\=newest-first tag\:inbox +command: $PWD/notmuch_fail search --format\=sexp --format-version\=2 --sort\=newest-first tag\:inbox exit status: 1" test_begin_subtest "Search handles subprocess warnings" @@ -923,7 +923,6 @@ This is a warning This is another warning" test_begin_subtest "Search thread tag operations are race-free" -test_subtest_known_broken add_message '[subject]="Search race test"' gen_msg_id_1=$gen_msg_id generate_message '[in-reply-to]="<'$gen_msg_id_1'>"' \ @@ -937,7 +936,6 @@ output=$(notmuch search --output=messages 'tag:search-thread-race-tag') test_expect_equal "$output" "id:$gen_msg_id_1" test_begin_subtest "Search global tag operations are race-free" -test_subtest_known_broken generate_message '[in-reply-to]="<'$gen_msg_id_1'>"' \ '[references]="<'$gen_msg_id_1'>"' \ '[subject]="Re: Search race test"' -- 1.8.4.rc3