emacs: Fix search tagging races

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

@@ -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.
 	

emacs/notmuch.el

@@ -400,14 +400,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)
+(defun notmuch-search-find-stable-query ()
-  "Return a list of threads for the current region"
+  "Return the stable queries for the current thread.
-  (mapcar (lambda (thread) (concat "thread:" thread))
-	  (notmuch-search-properties-in-region :thread beg end)))
 
-(defun notmuch-search-find-thread-id-region-search (beg end)
+This returns a list (MATCHED-QUERY UNMATCHED-QUERY) for the
-  "Return a search string for threads for the current region"
+matched and unmatched messages in the current thread."
-  (mapconcat 'identity (notmuch-search-find-thread-id-region beg end) " or "))
+  (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"
@@ -499,17 +510,20 @@ Returns (TAG-CHANGES REGION-BEGIN REGION-END)."
 	   (notmuch-search-get-tags-region beg end) prompt initial-input)
 	  region)))
 
-(defun notmuch-search-tag (tag-changes &optional beg end)
+(defun notmuch-search-tag (tag-changes &optional beg end only-matched)
   "Change tags for the currently selected thread or region.
 
 See `notmuch-tag' for information on the format of TAG-CHANGES.
 When called interactively, this uses the region if the region is
 active.  When called directly, BEG and END provide the region.
 If these are nil or not provided, this applies to the thread at
-point."
+point.
+
+If ONLY-MATCHED is non-nil, only tag matched messages."
   (interactive (notmuch-search-interactive-tag-changes))
   (unless (and beg end) (setq beg (point) end (point)))
-  (let ((search-string (notmuch-search-find-thread-id-region-search beg end)))
+  (let ((search-string (notmuch-search-find-stable-query-region
+			beg end only-matched)))
     (notmuch-tag search-string tag-changes)
     (notmuch-search-foreach-result beg end
       (lambda (pos)
@@ -779,7 +793,7 @@ See `notmuch-tag' for information on the format of TAG-CHANGES."
   (interactive
    (list (notmuch-read-tag-changes
 	  (notmuch-search-get-tags-region (point-min) (point-max)) "Tag all")))
-  (notmuch-tag notmuch-search-query-string tag-changes))
+  (notmuch-search-tag tag-changes (point-min) (point-max) t))
 
 (defun notmuch-search-buffer-title (query)
   "Returns the title for a buffer with notmuch search results."
@@ -883,7 +897,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")

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"'