[PATCH v2 0/8] Improve tag change completion

[PATCH v2 0/8] Improve tag change completion

[Austin Clements]

This is v2 of id:1382471457-26056-1-git-send-email-amdragon@mit.edu.
It improves some documentation strings, fixes one bug, and elimintes
some redundant code.  The diff from v1 is below.

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index ef77839..f66d669 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1800,7 +1800,6 @@ See `notmuch-tag' for information on the format of TAG-CHANGES."
    (list (let (tags)
 	   (notmuch-show-mapc
 	    (lambda () (setq tags (append (notmuch-show-get-tags) tags))))
-	   (delete-dups tags)
 	   (notmuch-read-tag-changes tags "Tag thread"))))
   (notmuch-tag (notmuch-show-get-messages-ids-search) tag-changes)
   (notmuch-show-mapc
diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
index feee17c..7b21006 100644
--- a/emacs/notmuch-tag.el
+++ b/emacs/notmuch-tag.el
@@ -189,6 +189,9 @@ the messages that were tagged"
 `notmuch-read-tag-changes' function.")
 
 (defun notmuch-tag-completions (&rest search-terms)
+  "Return a list of tags for messages matching SEARCH-TERMS.
+
+Returns all tags if no search terms are given."
   (if (null search-terms)
       (setq search-terms (list "*")))
   (split-string
@@ -199,7 +202,7 @@ the messages that were tagged"
    "\n+" t))
 
 (defun notmuch-select-tag-with-completion (prompt &rest search-terms)
-  (let ((tag-list (notmuch-tag-completions search-terms)))
+  (let ((tag-list (apply #'notmuch-tag-completions search-terms)))
     (completing-read prompt tag-list nil nil nil 'notmuch-select-tag-history)))
 
 (defun notmuch-read-tag-changes (current-tags &optional prompt initial-input)
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 5492e1b..53e9826 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -584,7 +584,10 @@ Returns (TAG-CHANGES REGION-BEGIN REGION-END)."
   "Change tags for the currently selected thread or region.
 
 See `notmuch-tag' for information on the format of TAG-CHANGES.
-If BEG or END are nil, applies to the thread at point."
+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."
   (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)))

[PATCH v2 1/8] emacs: Fix misuse of `notmuch-tag'

[Austin Clements]

The calling convention for `notmuch-tag' changed in commit 97aa3c06 to
take a list of tag changes instead of a &rest argument, but the call
from `notmuch-search-tag-all' still passed a &rest argument.  This
happened to work for interactive calls because tag-changes would be
nil, so the `apply' call would pass only the query string to
`notmuch-tag' and simply omit the &optional tag-changes argument.
---
 emacs/notmuch.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index c47c6b5..e0511c8 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -837,7 +837,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-tag notmuch-search-query-string tag-changes))
 
 (defun notmuch-search-buffer-title (query)
   "Returns the title for a buffer with notmuch search results."
-- 
1.8.4.rc3

[PATCH v2 2/8] emacs: Take prompt and current tags in `notmuch-read-tag-changes'

[Austin Clements]

This modifies the interface of `notmuch-read-tag-changes' to take an
optional prompt string as well as a list of existing tags instead of a
query.  This list of tags is used to populate the tag removal
completions and lets the caller compute these in a more
efficient/consistent manner than performing a potentially large or
complex query.  This patch also updates the sole current caller of
`notmuch-read-tag-changes'.
---
 emacs/notmuch-tag.el | 29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
index 064cfa8..3ad53b2 100644
--- a/emacs/notmuch-tag.el
+++ b/emacs/notmuch-tag.el
@@ -188,7 +188,10 @@ the messages that were tagged"
   "Variable to store minibuffer history for
 `notmuch-read-tag-changes' function.")
 
-(defun notmuch-tag-completions (&optional search-terms)
+(defun notmuch-tag-completions (&rest search-terms)
+  "Return a list of tags for messages matching SEARCH-TERMS.
+
+Returns all tags if no search terms are given."
   (if (null search-terms)
       (setq search-terms (list "*")))
   (split-string
@@ -199,17 +202,24 @@ the messages that were tagged"
    "\n+" t))
 
 (defun notmuch-select-tag-with-completion (prompt &rest search-terms)
-  (let ((tag-list (notmuch-tag-completions search-terms)))
+  (let ((tag-list (apply #'notmuch-tag-completions search-terms)))
     (completing-read prompt tag-list nil nil nil 'notmuch-select-tag-history)))
 
-(defun notmuch-read-tag-changes (&optional initial-input &rest search-terms)
+(defun notmuch-read-tag-changes (current-tags &optional prompt initial-input)
+  "Prompt for tag changes in the minibuffer.
+
+CURRENT-TAGS is a list of tags that are present on the message or
+messages to be changed.  These are offered as tag removal
+completions.  CURRENT-TAGS may contain duplicates.  PROMPT, if
+non-nil, is the query string to present in the minibuffer.  It
+defaults to \"Tags\".  INITIAL-INPUT, if non-nil, will be the
+initial input in the minibuffer."
+
   (let* ((all-tag-list (notmuch-tag-completions))
 	 (add-tag-list (mapcar (apply-partially 'concat "+") all-tag-list))
-	 (remove-tag-list (mapcar (apply-partially 'concat "-")
-				  (if (null search-terms)
-				      all-tag-list
-				    (notmuch-tag-completions search-terms))))
+	 (remove-tag-list (mapcar (apply-partially 'concat "-") current-tags))
 	 (tag-list (append add-tag-list remove-tag-list))
+	 (prompt (concat (or prompt "Tags") " (+add -drop): "))
 	 (crm-separator " ")
 	 ;; By default, space is bound to "complete word" function.
 	 ;; Re-bind it to insert a space instead.  Note that <tab>
@@ -219,7 +229,7 @@ the messages that were tagged"
 	    (set-keymap-parent map crm-local-completion-map)
 	    (define-key map " " 'self-insert-command)
 	    map)))
-    (delete "" (completing-read-multiple "Tags (+add -drop): "
+    (delete "" (completing-read-multiple prompt
 		tag-list nil nil initial-input
 		'notmuch-read-tag-changes-history))))
 
@@ -260,7 +270,8 @@ notmuch-after-tag-hook will be run."
   ;; Perform some validation
   (if (string-or-null-p tag-changes)
       (if (or (string= tag-changes "-") (string= tag-changes "+") (null tag-changes))
-	  (setq tag-changes (notmuch-read-tag-changes tag-changes query))
+	  (setq tag-changes (notmuch-read-tag-changes
+			     (notmuch-tag-completions query) nil tag-changes))
 	(setq tag-changes (list tag-changes))))
   (mapc (lambda (tag-change)
 	  (unless (string-match-p "^[-+]\\S-+$" tag-change)
-- 
1.8.4.rc3

[PATCH v2 3/8] emacs: Use interactive specifications for tag changes in show

[Austin Clements]

This modifies all tagging operations in show to call
`notmuch-read-tag-changes' in their interactive specification to input
tag changes, rather than depending on lower-level functions to prompt
for tag changes regardless of their calling context.

Besides being more Elispy and providing a more consistent programmatic
API, this enables callers to provide two call site-specific pieces of
information: an appropriate prompt, and the set of visible tags.  The
prompt lets us differentiate * from +/-.  Providing visible tags
enables a more consistent user experience than retrieving the
(potentially different) tags from the database, and avoids a
round-trip to the CLI and database.
---
 emacs/notmuch-show.el | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 7325792..f66d669 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1780,23 +1780,28 @@ TAG-CHANGES is a list of tag operations for `notmuch-tag'."
       (notmuch-tag (notmuch-show-get-message-id) tag-changes)
       (notmuch-show-set-tags new-tags))))
 
-(defun notmuch-show-tag (&optional tag-changes)
+(defun notmuch-show-tag (tag-changes)
   "Change tags for the current message.
 
 See `notmuch-tag' for information on the format of TAG-CHANGES."
-  (interactive)
-  (let* ((tag-changes (notmuch-tag (notmuch-show-get-message-id) tag-changes))
-	 (current-tags (notmuch-show-get-tags))
+  (interactive (list (notmuch-read-tag-changes (notmuch-show-get-tags)
+					       "Tag message")))
+  (notmuch-tag (notmuch-show-get-message-id) tag-changes)
+  (let* ((current-tags (notmuch-show-get-tags))
 	 (new-tags (notmuch-update-tags current-tags tag-changes)))
     (unless (equal current-tags new-tags)
       (notmuch-show-set-tags new-tags))))
 
-(defun notmuch-show-tag-all (&optional tag-changes)
+(defun notmuch-show-tag-all (tag-changes)
   "Change tags for all messages in the current show buffer.
 
 See `notmuch-tag' for information on the format of TAG-CHANGES."
-  (interactive)
-  (setq tag-changes (notmuch-tag (notmuch-show-get-messages-ids-search) tag-changes))
+  (interactive
+   (list (let (tags)
+	   (notmuch-show-mapc
+	    (lambda () (setq tags (append (notmuch-show-get-tags) tags))))
+	   (notmuch-read-tag-changes tags "Tag thread"))))
+  (notmuch-tag (notmuch-show-get-messages-ids-search) tag-changes)
   (notmuch-show-mapc
    (lambda ()
      (let* ((current-tags (notmuch-show-get-tags))
@@ -1804,19 +1809,21 @@ See `notmuch-tag' for information on the format of TAG-CHANGES."
        (unless (equal current-tags new-tags)
 	 (notmuch-show-set-tags new-tags))))))
 
-(defun notmuch-show-add-tag ()
+(defun notmuch-show-add-tag (tag-changes)
   "Change tags for the current message (defaulting to add).
 
 Same as `notmuch-show-tag' but sets initial input to '+'."
-  (interactive)
-  (notmuch-show-tag "+"))
+  (interactive
+   (list (notmuch-read-tag-changes (notmuch-show-get-tags) "Tag message" "+")))
+  (notmuch-show-tag tag-changes))
 
-(defun notmuch-show-remove-tag ()
+(defun notmuch-show-remove-tag (tag-changes)
   "Change tags for the current message (defaulting to remove).
 
 Same as `notmuch-show-tag' but sets initial input to '-'."
-  (interactive)
-  (notmuch-show-tag "-"))
+  (interactive
+   (list (notmuch-read-tag-changes (notmuch-show-get-tags) "Tag message" "-")))
+  (notmuch-show-tag tag-changes))
 
 (defun notmuch-show-toggle-visibility-headers ()
   "Toggle the visibility of the current message headers."
-- 
1.8.4.rc3

[PATCH v2 4/8] emacs: Use interactive specifications for tag changes in search

[Austin Clements]

This is similar to the previous commit, but applies to search.

Search is somewhat more complicated because its tagging operations can
also apply to a region.  Hence, this lifts interactive prompting into
a helper function.  This also takes advantage of the new ability to
provide a prompt to distinguish tagging a single thread from tagging a
region of threads.
---
 emacs/notmuch.el | 73 ++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 47 insertions(+), 26 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index e0511c8..53e9826 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -560,43 +560,62 @@ will be signaled."
 	(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."
+(defun notmuch-search-interactive-region ()
+  "Return the bounds of the current interactive region.
+
+This returns (BEG END), where BEG and END are the bounds of the
+region if the region is active, or both `point' otherwise."
+  (if (region-active-p)
+      (list (region-beginning) (region-end))
+    (list (point) (point))))
+
+(defun notmuch-search-interactive-tag-changes (&optional initial-input)
+  "Prompt for tag changes for the current thread or region.
+
+Returns (TAG-CHANGES REGION-BEGIN REGION-END)."
+  (let* ((region (notmuch-search-interactive-region))
+	 (beg (first region)) (end (second region))
+	 (prompt (if (= beg end) "Tag thread" "Tag region")))
+    (cons (notmuch-read-tag-changes
+	   (notmuch-search-get-tags-region beg end) prompt initial-input)
+	  region)))
+
+(defun notmuch-search-tag (tag-changes &optional beg end)
+  "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."
+  (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)))
-    (setq tag-changes (notmuch-tag search-string tag-changes))
+    (notmuch-tag search-string tag-changes)
     (notmuch-search-foreach-result beg end
       (lambda (pos)
 	(notmuch-search-set-tags
 	 (notmuch-update-tags (notmuch-search-get-tags pos) tag-changes)
 	 pos)))))
 
-(defun notmuch-search-tag (&optional tag-changes)
-  "Change tags for the currently selected thread or region.
-
-See `notmuch-tag' for information on the format of TAG-CHANGES."
-  (interactive)
-  (let* ((beg (if (region-active-p) (region-beginning) (point)))
-	 (end (if (region-active-p) (region-end) (point))))
-    (notmuch-search-tag-region beg end tag-changes)))
-
-(defun notmuch-search-add-tag ()
-  "Change tags for the current thread (defaulting to add).
+(defun notmuch-search-add-tag (tag-changes &optional beg end)
+  "Change tags for the current thread or region (defaulting to add).
 
 Same as `notmuch-search-tag' but sets initial input to '+'."
-  (interactive)
-  (notmuch-search-tag "+"))
+  (interactive (notmuch-search-interactive-tag-changes "+"))
+  (notmuch-search-tag tag-changes beg end))
 
-(defun notmuch-search-remove-tag ()
-  "Change tags for the current thread (defaulting to remove).
+(defun notmuch-search-remove-tag (tag-changes &optional beg end)
+  "Change tags for the current thread or region (defaulting to remove).
 
 Same as `notmuch-search-tag' but sets initial input to '-'."
-  (interactive)
-  (notmuch-search-tag "-"))
+  (interactive (notmuch-search-interactive-tag-changes "-"))
+  (notmuch-search-tag tag-changes beg end))
 
 (put 'notmuch-search-archive-thread 'notmuch-prefix-doc
      "Un-archive the currently selected thread.")
-(defun notmuch-search-archive-thread (&optional unarchive)
-  "Archive the currently selected thread.
+(defun notmuch-search-archive-thread (&optional unarchive beg end)
+  "Archive the currently selected thread or region.
 
 Archive each message in the currently selected thread by applying
 the tag changes in `notmuch-archive-tags' to each (remove the
@@ -605,10 +624,10 @@ messages will be \"unarchived\" (i.e. the tag changes in
 `notmuch-archive-tags' will be reversed).
 
 This function advances the next thread when finished."
-  (interactive "P")
+  (interactive (cons current-prefix-arg (notmuch-search-interactive-region)))
   (when notmuch-archive-tags
     (notmuch-search-tag
-     (notmuch-tag-change-list notmuch-archive-tags unarchive)))
+     (notmuch-tag-change-list notmuch-archive-tags unarchive) beg end))
   (notmuch-search-next-thread))
 
 (defun notmuch-search-update-result (result &optional pos)
@@ -832,11 +851,13 @@ non-authors is found, assume that all of the authors match."
 	(notmuch-sexp-parse-partial-list 'notmuch-search-show-result
 					 results-buf)))))
 
-(defun notmuch-search-tag-all (&optional tag-changes)
+(defun notmuch-search-tag-all (tag-changes)
   "Add/remove tags from all messages in current search buffer.
 
 See `notmuch-tag' for information on the format of TAG-CHANGES."
-  (interactive)
+  (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))
 
 (defun notmuch-search-buffer-title (query)
-- 
1.8.4.rc3

[PATCH v2 5/8] pick: Fix incorrect use of `notmuch-pick-tag'

[Austin Clements]

`notmuch-pick-tag' takes a list of tag changes, but
`notmuch-pick-archive-message' passes it a &rest argument.  This
happens to work if `notmuch-archive-tags' contains a single tag (which
it usually does), but will break if it does not.
---
 contrib/notmuch-pick/notmuch-pick.el | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/contrib/notmuch-pick/notmuch-pick.el b/contrib/notmuch-pick/notmuch-pick.el
index 0aa651e..8f504ed 100644
--- a/contrib/notmuch-pick/notmuch-pick.el
+++ b/contrib/notmuch-pick/notmuch-pick.el
@@ -542,8 +542,7 @@ message will be \"unarchived\", i.e. the tag changes in
 `notmuch-archive-tags' will be reversed."
   (interactive "P")
   (when notmuch-archive-tags
-    (apply 'notmuch-pick-tag
-	   (notmuch-tag-change-list notmuch-archive-tags unarchive))))
+    (notmuch-pick-tag (notmuch-tag-change-list notmuch-archive-tags unarchive))))
 
 (defun notmuch-pick-archive-message-then-next (&optional unarchive)
   "Archive the current message and move to next matching message."
-- 
1.8.4.rc3

[PATCH v2 6/8] pick: Use list form of tag-changes in test

[Austin Clements]

Currently we support both string and list forms of tag-changes for
historical reasons.  This is about to change, so fix pick's tests that
use the legacy string form of tag-changes.
---
 contrib/notmuch-pick/test/emacs-pick | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/contrib/notmuch-pick/test/emacs-pick b/contrib/notmuch-pick/test/emacs-pick
index a802d0e..40b0738 100755
--- a/contrib/notmuch-pick/test/emacs-pick
+++ b/contrib/notmuch-pick/test/emacs-pick
@@ -44,7 +44,7 @@ test_emacs '(add-to-list (quote load-path) "'$PICK_DIR'")
 	    (notmuch-pick "tag:inbox")
 	    (notmuch-test-wait)
 	    (forward-line)
-	    (notmuch-pick-tag "+test_tag")
+	    (notmuch-pick-tag (list "+test_tag"))
 	    (test-output)
 	    (delete-other-windows)'
 test_expect_equal_file OUTPUT $EXPECTED/notmuch-pick-tag-inbox-tagged
@@ -59,7 +59,7 @@ test_emacs '(add-to-list (quote load-path) "'$PICK_DIR'")
 	    (notmuch-pick "tag:inbox")
 	    (notmuch-test-wait)
 	    (forward-line)
-	    (notmuch-pick-tag "-test_tag")
+	    (notmuch-pick-tag (list "-test_tag"))
 	    (test-output)
 	    (delete-other-windows)'
 test_expect_equal_file OUTPUT $EXPECTED/notmuch-pick-tag-inbox
@@ -75,7 +75,7 @@ test_emacs '(add-to-list (quote load-path) "'$PICK_DIR'")
 	    (notmuch-test-wait)
 	    ;; move to a sizable thread
 	    (forward-line 26)
-	    (notmuch-pick-tag-thread "+test_thread_tag")
+	    (notmuch-pick-tag-thread (list "+test_thread_tag"))
 	    (test-output)
 	    (delete-other-windows)'
 test_expect_equal_file OUTPUT $EXPECTED/notmuch-pick-tag-inbox-thread-tagged
@@ -98,7 +98,7 @@ test_emacs '(add-to-list (quote load-path) "'$PICK_DIR'")
 	    (notmuch-test-wait)
 	    ;; move to the same sizable thread as above
 	    (forward-line 26)
-	    (notmuch-pick-tag-thread "-test_thread_tag")
+	    (notmuch-pick-tag-thread (list "-test_thread_tag"))
 	    (test-output)
 	    (delete-other-windows)'
 test_expect_equal_file OUTPUT $EXPECTED/notmuch-pick-tag-inbox
-- 
1.8.4.rc3

[PATCH v2 7/8] pick: Use interactive specifications for tag changes

[Austin Clements]

---
 contrib/notmuch-pick/notmuch-pick.el | 36 +++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/contrib/notmuch-pick/notmuch-pick.el b/contrib/notmuch-pick/notmuch-pick.el
index 8f504ed..13682ea 100644
--- a/contrib/notmuch-pick/notmuch-pick.el
+++ b/contrib/notmuch-pick/notmuch-pick.el
@@ -37,7 +37,7 @@
 (declare-function notmuch-show-strip-re "notmuch-show" (subject))
 (declare-function notmuch-show-spaces-n "notmuch-show" (n))
 (declare-function notmuch-read-query "notmuch" (prompt))
-(declare-function notmuch-read-tag-changes "notmuch" (&optional initial-input &rest search-terms))
+(declare-function notmuch-read-tag-changes "notmuch" (current-tags &optional prompt initial-input))
 (declare-function notmuch-update-tags "notmuch" (current-tags tag-changes))
 (declare-function notmuch-hello-trim "notmuch-hello" (search))
 (declare-function notmuch-search-find-thread-id "notmuch" ())
@@ -372,21 +372,24 @@ Does NOT change the database."
       (notmuch-pick-set-tags new-tags)
       (notmuch-pick-refresh-result))))
 
-(defun notmuch-pick-tag (&optional tag-changes)
+(defun notmuch-pick-tag (tag-changes)
   "Change tags for the current message"
-  (interactive)
-  (setq tag-changes (notmuch-tag (notmuch-pick-get-message-id) tag-changes))
+  (interactive
+   (list (notmuch-read-tag-changes (notmuch-pick-get-tags) "Tag message")))
+  (notmuch-tag (notmuch-pick-get-message-id) tag-changes)
   (notmuch-pick-tag-update-display tag-changes))
 
-(defun notmuch-pick-add-tag ()
+(defun notmuch-pick-add-tag (tag-changes)
   "Same as `notmuch-pick-tag' but sets initial input to '+'."
-  (interactive)
-  (notmuch-pick-tag "+"))
+  (interactive
+   (list (notmuch-read-tag-changes (notmuch-pick-get-tags) "Tag message" "+")))
+  (notmuch-pick-tag tag-changes))
 
-(defun notmuch-pick-remove-tag ()
+(defun notmuch-pick-remove-tag (tag-changes)
   "Same as `notmuch-pick-tag' but sets initial input to '-'."
-  (interactive)
-  (notmuch-pick-tag "-"))
+  (interactive
+   (list (notmuch-read-tag-changes (notmuch-pick-get-tags) "Tag message" "-")))
+  (notmuch-pick-tag tag-changes))
 
 ;; The next two functions close the message window before searching or
 ;; picking but they do so after the user has entered the query (in
@@ -626,13 +629,16 @@ message will be \"unarchived\", i.e. the tag changes in
 	     (notmuch-pick-thread-mapcar 'notmuch-pick-get-message-id)
 	     " or "))
 
-(defun notmuch-pick-tag-thread (&optional tag-changes)
+(defun notmuch-pick-tag-thread (tag-changes)
   "Tag all messages in the current thread"
-  (interactive)
+  (interactive
+   (let ((tags (apply #'append (notmuch-pick-thread-mapcar
+				(lambda () (notmuch-pick-get-tags))))))
+     (list (notmuch-read-tag-changes tags "Tag thread"))))
   (when (notmuch-pick-get-message-properties)
-    (let ((tag-changes (notmuch-tag (notmuch-pick-get-messages-ids-thread-search) tag-changes)))
-      (notmuch-pick-thread-mapcar
-       (lambda () (notmuch-pick-tag-update-display tag-changes))))))
+    (notmuch-tag (notmuch-pick-get-messages-ids-thread-search) tag-changes)
+    (notmuch-pick-thread-mapcar
+     (lambda () (notmuch-pick-tag-update-display tag-changes)))))
 
 (defun notmuch-pick-archive-thread (&optional unarchive)
   "Archive each message in thread.
-- 
1.8.4.rc3

[PATCH v2 8/8] emacs: Remove interactive behavior of `notmuch-tag'

[Austin Clements]

We no longer use this, since we've lifted all interactive behavior to
the appropriate interactive entry points.  Because of this,
`notmuch-tag' also no longer needs to return the tag changes list,
since the caller always passes it in.
---
 emacs/notmuch-tag.el | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
index 3ad53b2..7b21006 100644
--- a/emacs/notmuch-tag.el
+++ b/emacs/notmuch-tag.el
@@ -252,27 +252,18 @@ from TAGS if present."
 	   (error "Changed tag must be of the form `+this_tag' or `-that_tag'")))))
     (sort result-tags 'string<)))
 
-(defun notmuch-tag (query &optional tag-changes)
+(defun notmuch-tag (query tag-changes)
   "Add/remove tags in TAG-CHANGES to messages matching QUERY.
 
 QUERY should be a string containing the search-terms.
-TAG-CHANGES can take multiple forms.  If TAG-CHANGES is a list of
-strings of the form \"+tag\" or \"-tag\" then those are the tag
-changes applied.  If TAG-CHANGES is a string then it is
-interpreted as a single tag change.  If TAG-CHANGES is the string
-\"-\" or \"+\", or null, then the user is prompted to enter the
-tag changes.
+TAG-CHANGES is a list of strings of the form \"+tag\" or
+\"-tag\" to add or remove tags, respectively.
 
 Note: Other code should always use this function alter tags of
 messages instead of running (notmuch-call-notmuch-process \"tag\" ..)
 directly, so that hooks specified in notmuch-before-tag-hook and
 notmuch-after-tag-hook will be run."
   ;; Perform some validation
-  (if (string-or-null-p tag-changes)
-      (if (or (string= tag-changes "-") (string= tag-changes "+") (null tag-changes))
-	  (setq tag-changes (notmuch-read-tag-changes
-			     (notmuch-tag-completions query) nil tag-changes))
-	(setq tag-changes (list tag-changes))))
   (mapc (lambda (tag-change)
 	  (unless (string-match-p "^[-+]\\S-+$" tag-change)
 	    (error "Tag must be of the form `+this_tag' or `-that_tag'")))
@@ -281,9 +272,7 @@ notmuch-after-tag-hook will be run."
     (run-hooks 'notmuch-before-tag-hook)
     (apply 'notmuch-call-notmuch-process "tag"
 	   (append tag-changes (list "--" query)))
-    (run-hooks 'notmuch-after-tag-hook))
-  ;; in all cases we return tag-changes as a list
-  tag-changes)
+    (run-hooks 'notmuch-after-tag-hook)))
 
 (defun notmuch-tag-change-list (tags &optional reverse)
   "Convert TAGS into a list of tag changes.
-- 
1.8.4.rc3

Re: [PATCH v2 0/8] Improve tag change completion

[Mark Walters]

In might of id:20131023154404.GE20337@mit.edu I am now happy with this
series. LGTM +1 

Best wishes

Mark


On Wed, 23 Oct 2013, Austin Clements <amdragon@MIT.EDU> wrote:
> This is v2 of id:1382471457-26056-1-git-send-email-amdragon@mit.edu.
> It improves some documentation strings, fixes one bug, and elimintes
> some redundant code.  The diff from v1 is below.
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index ef77839..f66d669 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -1800,7 +1800,6 @@ See `notmuch-tag' for information on the format of TAG-CHANGES."
>     (list (let (tags)
>  	   (notmuch-show-mapc
>  	    (lambda () (setq tags (append (notmuch-show-get-tags) tags))))
> -	   (delete-dups tags)
>  	   (notmuch-read-tag-changes tags "Tag thread"))))
>    (notmuch-tag (notmuch-show-get-messages-ids-search) tag-changes)
>    (notmuch-show-mapc
> diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
> index feee17c..7b21006 100644
> --- a/emacs/notmuch-tag.el
> +++ b/emacs/notmuch-tag.el
> @@ -189,6 +189,9 @@ the messages that were tagged"
>  `notmuch-read-tag-changes' function.")
>  
>  (defun notmuch-tag-completions (&rest search-terms)
> +  "Return a list of tags for messages matching SEARCH-TERMS.
> +
> +Returns all tags if no search terms are given."
>    (if (null search-terms)
>        (setq search-terms (list "*")))
>    (split-string
> @@ -199,7 +202,7 @@ the messages that were tagged"
>     "\n+" t))
>  
>  (defun notmuch-select-tag-with-completion (prompt &rest search-terms)
> -  (let ((tag-list (notmuch-tag-completions search-terms)))
> +  (let ((tag-list (apply #'notmuch-tag-completions search-terms)))
>      (completing-read prompt tag-list nil nil nil 'notmuch-select-tag-history)))
>  
>  (defun notmuch-read-tag-changes (current-tags &optional prompt initial-input)
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index 5492e1b..53e9826 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -584,7 +584,10 @@ Returns (TAG-CHANGES REGION-BEGIN REGION-END)."
>    "Change tags for the currently selected thread or region.
>  
>  See `notmuch-tag' for information on the format of TAG-CHANGES.
> -If BEG or END are nil, applies to the thread at point."
> +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."
>    (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)))

Re: [PATCH v2 0/8] Improve tag change completion

[Tomi Ollila]

On Wed, Oct 23 2013, Mark Walters <markwalters1009@gmail.com> wrote:

> In might of id:20131023154404.GE20337@mit.edu I am now happy with this
> series. LGTM +1 

Looks Good To Me, too - and worked in my test tagging operations as 
announced +1

Tomi

>
> Best wishes
>
> Mark
>
>
> On Wed, 23 Oct 2013, Austin Clements <amdragon@MIT.EDU> wrote:
>> This is v2 of id:1382471457-26056-1-git-send-email-amdragon@mit.edu.
>> It improves some documentation strings, fixes one bug, and elimintes
>> some redundant code.  The diff from v1 is below.
>>
>> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
>> index ef77839..f66d669 100644
>> --- a/emacs/notmuch-show.el
>> +++ b/emacs/notmuch-show.el
>> @@ -1800,7 +1800,6 @@ See `notmuch-tag' for information on the format of TAG-CHANGES."
>>     (list (let (tags)
>>  	   (notmuch-show-mapc
>>  	    (lambda () (setq tags (append (notmuch-show-get-tags) tags))))
>> -	   (delete-dups tags)
>>  	   (notmuch-read-tag-changes tags "Tag thread"))))
>>    (notmuch-tag (notmuch-show-get-messages-ids-search) tag-changes)
>>    (notmuch-show-mapc
>> diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
>> index feee17c..7b21006 100644
>> --- a/emacs/notmuch-tag.el
>> +++ b/emacs/notmuch-tag.el
>> @@ -189,6 +189,9 @@ the messages that were tagged"
>>  `notmuch-read-tag-changes' function.")
>>  
>>  (defun notmuch-tag-completions (&rest search-terms)
>> +  "Return a list of tags for messages matching SEARCH-TERMS.
>> +
>> +Returns all tags if no search terms are given."
>>    (if (null search-terms)
>>        (setq search-terms (list "*")))
>>    (split-string
>> @@ -199,7 +202,7 @@ the messages that were tagged"
>>     "\n+" t))
>>  
>>  (defun notmuch-select-tag-with-completion (prompt &rest search-terms)
>> -  (let ((tag-list (notmuch-tag-completions search-terms)))
>> +  (let ((tag-list (apply #'notmuch-tag-completions search-terms)))
>>      (completing-read prompt tag-list nil nil nil 'notmuch-select-tag-history)))
>>  
>>  (defun notmuch-read-tag-changes (current-tags &optional prompt initial-input)
>> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
>> index 5492e1b..53e9826 100644
>> --- a/emacs/notmuch.el
>> +++ b/emacs/notmuch.el
>> @@ -584,7 +584,10 @@ Returns (TAG-CHANGES REGION-BEGIN REGION-END)."
>>    "Change tags for the currently selected thread or region.
>>  
>>  See `notmuch-tag' for information on the format of TAG-CHANGES.
>> -If BEG or END are nil, applies to the thread at point."
>> +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."
>>    (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)))
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2 0/8] Improve tag change completion

[David Bremner]

Austin Clements <amdragon@MIT.EDU> writes:

> This is v2 of id:1382471457-26056-1-git-send-email-amdragon@mit.edu.
> It improves some documentation strings, fixes one bug, and elimintes
> some redundant code.  The diff from v1 is below.

pushed,

d