improvements to some emacs search tagging functions

improvements to some emacs search tagging functions

[Jameson Graef Rollins]

This series fixes a couple of minor annoyances/bugs with tagging in
search-mode:

* The old search-tag-thread function would not tag regions, and the
  main search-tag function could not just take whole tagging
  operations as input.  This series fixes that so that all tagging
  operations, by thread or by region, are now handled with the main
  search-tag function.  Tags can be specified as a sting or a list, or
  the user can be prompted (with tab completion) if no tag is
  specifically specified.

* Tag tab completion was often incomplete or would throw errors.  Tag
  tab completion is now more full-featured.

The last patch, which fix tag tab completion for excluded tags, needs
Mark Walters new exclude fixes [0] to work.

The series removes one public-facing function
(notmuch-search-tag-thread), and renames another
(notmuch-search-archive-thread --> notmuch-search-archive).  But I
think it's worth it since it makes for a much cleaner interface.

jamie.

[0] id:"1333234442-28616-1-git-send-email-markwalters1009@gmail.com"

[PATCH 1/6] emacs: update tag-completion function

[Jameson Graef Rollins]

"search-tags" is deprecated, so we move to the more modern "search
--output=tags".
---
 emacs/notmuch.el |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index f0afa07..38ae005 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -89,7 +89,7 @@ For example:
    (with-output-to-string
      (with-current-buffer standard-output
        (apply 'call-process notmuch-command nil t
-	      nil "search-tags" search-terms)))
+	      nil "search" "--output=tags" search-terms)))
    "\n+" t))
 
 (defun notmuch-select-tag-with-completion (prompt &rest search-terms)
-- 
1.7.9.1

[PATCH 2/6] emacs: have tag-completion return all tags for nil input

[Jameson Graef Rollins]

Previously the function would fail if the initial input was nil.  Now
it will return a list of all tags, which obviously makes much more
sense.
---
 emacs/notmuch.el |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 38ae005..0ab8fc2 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -85,6 +85,8 @@ For example:
 `notmuch-read-tag-changes' function.")
 
 (defun notmuch-tag-completions (&optional search-terms)
+  (if (eq search-terms nil)
+      (setq search-terms (list "*")))
   (split-string
    (with-output-to-string
      (with-current-buffer standard-output
-- 
1.7.9.1

[PATCH 3/6] emacs: modify notmuch-search-tag to not prompt if tags provided as argument

[Jameson Graef Rollins]

If the argument is a full string or a list, the function will assume
this is a tag string or list and will not prompt the user.  If the
argument is nil or the exact strings "-" or "+" then the user will be
prompted.

The function doc is updated accordingly.
---
 emacs/notmuch.el |   16 ++++++++++++----
 1 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 0ab8fc2..3b78584 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -646,13 +646,21 @@ added or removed for all threads in the region from BEG to END."
 	   (notmuch-update-tags (notmuch-search-get-tags) tag-changes))
 	  (forward-line))))))
 
-(defun notmuch-search-tag (&optional initial-input)
-  "Change tags for the currently selected thread or region."
+(defun notmuch-search-tag (&optional tags)
+  "Change tags for the currently selected thread or region.
+
+If TAGS is a string or list it will be interpreted as tags to
+apply to the selected messages.  If TAGS is nil or either of the
+strings `-' or `+' the user will be prompted to enter tags (with
+tab completion)."
   (interactive)
   (let* ((beg (if (region-active-p) (region-beginning) (point)))
 	 (end (if (region-active-p) (region-end) (point)))
-	 (search-string (notmuch-search-find-thread-id-region-search beg end))
-	 (tags (notmuch-read-tag-changes initial-input search-string)))
+	 (search-string (notmuch-search-find-thread-id-region-search beg end)))
+    (if (string-or-null-p tags)
+	(if (or (string= tags "-") (string= tags "+") (eq tags nil))
+	    (setq tags (notmuch-read-tag-changes tags search-string))
+	  (setq tags (list tags))))
     (apply 'notmuch-search-tag-region beg end tags)))
 
 (defun notmuch-search-add-tag ()
-- 
1.7.9.1

[PATCH 4/6] emacs: remove redundant notmuch-search-tag-thread function

[Jameson Graef Rollins]

Now that the notmuch-search-tag functions provides the exact same
functionality, this function is now entirely redundant.  Keeping it
around would just be bloat and confusion.  notmuch-search-tag should
now be the canonical function for search-mode tag changes.

The archive-thread function (which was the only function to use
tag-thread) is updating accordingly.
---
 emacs/notmuch.el |    8 +-------
 1 files changed, 1 insertions(+), 7 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 3b78584..e1bff42 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -624,12 +624,6 @@ the messages that were tagged"
 	(forward-line 1))
       output)))
 
-(defun notmuch-search-tag-thread (&rest tag-changes)
-  "Change tags for the currently selected thread.
-
-See `notmuch-search-tag-region' for details."
-  (apply 'notmuch-search-tag-region (point) (point) tag-changes))
-
 (defun notmuch-search-tag-region (beg end &rest tag-changes)
   "Change tags for threads in the given region.
 
@@ -678,7 +672,7 @@ tab completion)."
 
 This function advances the next thread when finished."
   (interactive)
-  (notmuch-search-tag-thread "-inbox")
+  (notmuch-search-tag "-inbox")
   (notmuch-search-next-thread))
 
 (defvar notmuch-search-process-filter-data nil
-- 
1.7.9.1

[PATCH 5/6] emacs: change name of search-archive-thread function to just search-archive

[Jameson Graef Rollins]

This changes the name of this function to be more consistent with the
current thread tagging functions.  The documentation is also updated
and improved.
---
 emacs/notmuch.el |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index e1bff42..19423ea 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -289,7 +289,7 @@ For a mouse binding, return nil."
     (define-key map "f" 'notmuch-search-filter)
     (define-key map [mouse-1] 'notmuch-search-show-thread)
     (define-key map "*" 'notmuch-search-tag-all)
-    (define-key map "a" 'notmuch-search-archive-thread)
+    (define-key map "a" 'notmuch-search-archive)
     (define-key map "-" 'notmuch-search-remove-tag)
     (define-key map "+" 'notmuch-search-add-tag)
     (define-key map (kbd "RET") 'notmuch-search-show-thread)
@@ -667,10 +667,10 @@ tab completion)."
   (interactive)
   (notmuch-search-tag "-"))
 
-(defun notmuch-search-archive-thread ()
-  "Archive the currently selected thread (remove its \"inbox\" tag).
+(defun notmuch-search-archive ()
+  "Archive the currently selected thread or region (remove \"inbox\" tags).
 
-This function advances the next thread when finished."
+The cursor is advanced to the next thread after tagging."
   (interactive)
   (notmuch-search-tag "-inbox")
   (notmuch-search-next-thread))
-- 
1.7.9.1

[PATCH 6/6] emacs: include tags from excluded messages in tag tab completion

[Jameson Graef Rollins]

The new message exclude functionality will hide tags that only exist
on excluded messages.  However, one might very well want to manually
modify excluded tags.  This makes sure tags from excluded messages are
always available in tab completion.
---
 emacs/notmuch.el |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 19423ea..7cba496 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -91,7 +91,7 @@ For example:
    (with-output-to-string
      (with-current-buffer standard-output
        (apply 'call-process notmuch-command nil t
-	      nil "search" "--output=tags" search-terms)))
+	      nil "search" "--output=tags" "--exclude=false" search-terms)))
    "\n+" t))
 
 (defun notmuch-select-tag-with-completion (prompt &rest search-terms)
-- 
1.7.9.1

Re: [PATCH 2/6] emacs: have tag-completion return all tags for nil input

[Dmitry Kurochkin]

Jameson Graef Rollins <jrollins@finestructure.net> writes:

> Previously the function would fail if the initial input was nil.  Now
> it will return a list of all tags, which obviously makes much more
> sense.
> ---
>  emacs/notmuch.el |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index 38ae005..0ab8fc2 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -85,6 +85,8 @@ For example:
>  `notmuch-read-tag-changes' function.")
>  
>  (defun notmuch-tag-completions (&optional search-terms)
> +  (if (eq search-terms nil)

Please use `null'.

Regards,
  Dmitry

> +      (setq search-terms (list "*")))
>    (split-string
>     (with-output-to-string
>       (with-current-buffer standard-output
> -- 
> 1.7.9.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: improvements to some emacs search tagging functions

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 125 bytes --]

I just realized all the same improvements need to be made for show-mode
as well.  I'll try to get to those tomorrow.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH 3/6] emacs: modify notmuch-search-tag to not prompt if tags provided as argument

[Dmitry Kurochkin]

Jameson Graef Rollins <jrollins@finestructure.net> writes:

> If the argument is a full string or a list, the function will assume
> this is a tag string or list and will not prompt the user.  If the
> argument is nil or the exact strings "-" or "+" then the user will be
> prompted.
>
> The function doc is updated accordingly.
> ---
>  emacs/notmuch.el |   16 ++++++++++++----
>  1 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index 0ab8fc2..3b78584 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -646,13 +646,21 @@ added or removed for all threads in the region from BEG to END."
>  	   (notmuch-update-tags (notmuch-search-get-tags) tag-changes))
>  	  (forward-line))))))
>  
> -(defun notmuch-search-tag (&optional initial-input)
> -  "Change tags for the currently selected thread or region."
> +(defun notmuch-search-tag (&optional tags)
> +  "Change tags for the currently selected thread or region.
> +
> +If TAGS is a string or list it will be interpreted as tags to
> +apply to the selected messages.  If TAGS is nil or either of the
> +strings `-' or `+' the user will be prompted to enter tags (with
> +tab completion)."

The TAGS argument name may be confusing.  Other tagging functions tend
to use TAG-CHANGES for these.  Can you please change the argument name
here for consistency?

>    (interactive)
>    (let* ((beg (if (region-active-p) (region-beginning) (point)))
>  	 (end (if (region-active-p) (region-end) (point)))
> -	 (search-string (notmuch-search-find-thread-id-region-search beg end))
> -	 (tags (notmuch-read-tag-changes initial-input search-string)))
> +	 (search-string (notmuch-search-find-thread-id-region-search beg end)))
> +    (if (string-or-null-p tags)
> +	(if (or (string= tags "-") (string= tags "+") (eq tags nil))

Should we add a check for an empty string?

Please use `null'.

> +	    (setq tags (notmuch-read-tag-changes tags search-string))
> +	  (setq tags (list tags))))

Should we change `notmuch-tag' to accept strings instead of handling it
here?

Regards,
  Dmitry

>      (apply 'notmuch-search-tag-region beg end tags)))
>  
>  (defun notmuch-search-add-tag ()
> -- 
> 1.7.9.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

emacs tagging cleanup

[Jameson Graef Rollins]

This is a rework of the series [0].  It addresses some of the comments
from Dmitry, and extends the scope to clean up the tagging interface
in both search and show mode.

[0] id:"1333354853-25729-1-git-send-email-jrollins@finestructure.net"

The goal here is to present a cleaner tagging interface to the user.
We want things to be simple, not confusing or redundant.  To that end
I have extended the notmuch-tag function to handle prompting if
needed.  The main user-facing functions are now:

  notmuch-search-tag      tag thread or region
  notmuch-search-tag-all  tag all thread in search buffer
  notmuch-show-tag        tag message
  notmuch-show-tag-all    tag all message in show buffer

I think this provides a much cleaner interface that is more useful to
users.  In particular, this improves the ability to tag regions in
search mode.

I have eliminated two user-facing functions (notmuch-search-tag-thread
and notmuch-show-tag-message) since they are now redundant.  This
might cause a slight hiccup for those using those functions, but I
think it's better in the long run.

Unfortunately something in the last patch (last two patches, I guess)
is now causing a two of our more esoteric tests to fail.  After
spending half the day trying to figure out why I'm at a loss.  And I
don't want all this work to be wasted so I'm submitting it anyway.
Hopefully someone (maybe the creator of those tests?) can figure out
what's going on.

jamie.

[PATCH 1/8] emacs: create notmuch-tag.el, and move appropriate functions from notmuch.el

[Jameson Graef Rollins]

Tagging functions are used in notmuch.el, notmuch-show.el, and
notmuch-message.el.  There are enough common functions for tagging
that it makes sense to put them all in their own library.

No code is modified, just moved around.
---
 emacs/Makefile.local     |    1 +
 emacs/notmuch-message.el |    1 +
 emacs/notmuch-show.el    |    3 +-
 emacs/notmuch-tag.el     |  133 ++++++++++++++++++++++++++++++++++++++++++++++
 emacs/notmuch.el         |  107 +------------------------------------
 5 files changed, 137 insertions(+), 108 deletions(-)
 create mode 100644 emacs/notmuch-tag.el

diff --git a/emacs/Makefile.local b/emacs/Makefile.local
index 4fee0e8..fb82247 100644
--- a/emacs/Makefile.local
+++ b/emacs/Makefile.local
@@ -13,6 +13,7 @@ emacs_sources := \
 	$(dir)/notmuch-maildir-fcc.el \
 	$(dir)/notmuch-message.el \
 	$(dir)/notmuch-crypto.el \
+	$(dir)/notmuch-tag.el \
 	$(dir)/coolj.el \
 	$(dir)/notmuch-print.el
 
diff --git a/emacs/notmuch-message.el b/emacs/notmuch-message.el
index 3010281..5964caa 100644
--- a/emacs/notmuch-message.el
+++ b/emacs/notmuch-message.el
@@ -20,6 +20,7 @@
 ;; Authors: Jesse Rosenthal <jrosenthal@jhu.edu>
 
 (require 'message)
+(require 'notmuch-tag)
 (require 'notmuch-mua)
 
 (defcustom notmuch-message-replied-tags '("replied")
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 30b26d1..a4c313d 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -30,6 +30,7 @@
 (require 'goto-addr)
 
 (require 'notmuch-lib)
+(require 'notmuch-tag)
 (require 'notmuch-query)
 (require 'notmuch-wash)
 (require 'notmuch-mua)
@@ -38,10 +39,8 @@
 
 (declare-function notmuch-call-notmuch-process "notmuch" (&rest args))
 (declare-function notmuch-fontify-headers "notmuch" nil)
-(declare-function notmuch-read-tag-changes "notmuch" (&optional initial-input &rest search-terms))
 (declare-function notmuch-search-next-thread "notmuch" nil)
 (declare-function notmuch-search-show-thread "notmuch" nil)
-(declare-function notmuch-update-tags "notmuch" (current-tags tag-changes))
 
 (defcustom notmuch-message-headers '("Subject" "To" "Cc" "Date")
   "Headers that should be shown in a message, in this order.
diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
new file mode 100644
index 0000000..81b4b00
--- /dev/null
+++ b/emacs/notmuch-tag.el
@@ -0,0 +1,133 @@
+;; notmuch-tag.el --- tag messages within emacs
+;;
+;; Copyright © Carl Worth
+;;
+;; This file is part of Notmuch.
+;;
+;; Notmuch is free software: you can redistribute it and/or modify it
+;; under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+;;
+;; Notmuch is distributed in the hope that it will be useful, but
+;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+;; General Public License for more details.
+;;
+;; You should have received a copy of the GNU General Public License
+;; along with Notmuch.  If not, see <http://www.gnu.org/licenses/>.
+;;
+;; Authors: Carl Worth <cworth@cworth.org>
+
+(eval-when-compile (require 'cl))
+(require 'crm)
+(require 'notmuch-lib)
+
+(defcustom notmuch-before-tag-hook nil
+  "Hooks that are run before tags of a message are modified.
+
+'tags' will contain the tags that are about to be added or removed as
+a list of strings of the form \"+TAG\" or \"-TAG\".
+'query' will be a string containing the search query that determines
+the messages that are about to be tagged"
+
+  :type 'hook
+  :options '(notmuch-hl-line-mode)
+  :group 'notmuch-hooks)
+
+(defcustom notmuch-after-tag-hook nil
+  "Hooks that are run after tags of a message are modified.
+
+'tags' will contain the tags that were added or removed as
+a list of strings of the form \"+TAG\" or \"-TAG\".
+'query' will be a string containing the search query that determines
+the messages that were tagged"
+  :type 'hook
+  :options '(notmuch-hl-line-mode)
+  :group 'notmuch-hooks)
+
+(defvar notmuch-select-tag-history nil
+  "Variable to store minibuffer history for
+`notmuch-select-tag-with-completion' function.")
+
+(defvar notmuch-read-tag-changes-history nil
+  "Variable to store minibuffer history for
+`notmuch-read-tag-changes' function.")
+
+(defun notmuch-tag-completions (&optional search-terms)
+  (split-string
+   (with-output-to-string
+     (with-current-buffer standard-output
+       (apply 'call-process notmuch-command nil t
+	      nil "search-tags" search-terms)))
+   "\n+" t))
+
+(defun notmuch-select-tag-with-completion (prompt &rest search-terms)
+  (let ((tag-list (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)
+  (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))))
+	 (tag-list (append add-tag-list remove-tag-list))
+	 (crm-separator " ")
+	 ;; By default, space is bound to "complete word" function.
+	 ;; Re-bind it to insert a space instead.  Note that <tab>
+	 ;; still does the completion.
+	 (crm-local-completion-map
+	  (let ((map (make-sparse-keymap)))
+	    (set-keymap-parent map crm-local-completion-map)
+	    (define-key map " " 'self-insert-command)
+	    map)))
+    (delete "" (completing-read-multiple "Tags (+add -drop): "
+		tag-list nil nil initial-input
+		'notmuch-read-tag-changes-history))))
+
+(defun notmuch-update-tags (tags tag-changes)
+  "Return a copy of TAGS with additions and removals from TAG-CHANGES.
+
+TAG-CHANGES must be a list of tags names, each prefixed with
+either a \"+\" to indicate the tag should be added to TAGS if not
+present or a \"-\" to indicate that the tag should be removed
+from TAGS if present."
+  (let ((result-tags (copy-sequence tags)))
+    (dolist (tag-change tag-changes)
+      (let ((op (string-to-char tag-change))
+	    (tag (unless (string= tag-change "") (substring tag-change 1))))
+	(case op
+	  (?+ (unless (member tag result-tags)
+		(push tag result-tags)))
+	  (?- (setq result-tags (delete tag result-tags)))
+	  (otherwise
+	   (error "Changed tag must be of the form `+this_tag' or `-that_tag'")))))
+    (sort result-tags 'string<)))
+
+(defun notmuch-tag (query &rest tag-changes)
+  "Add/remove tags in TAG-CHANGES to messages matching QUERY.
+
+TAG-CHANGES should be a list of strings of the form \"+tag\" or
+\"-tag\" and QUERY should be a string containing the
+search-query.
+
+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
+  (mapc (lambda (tag-change)
+	  (unless (string-match-p "^[-+]\\S-+$" tag-change)
+	    (error "Tag must be of the form `+this_tag' or `-that_tag'")))
+	tag-changes)
+  (unless (null tag-changes)
+    (run-hooks 'notmuch-before-tag-hook)
+    (apply 'notmuch-call-notmuch-process "tag"
+	   (append tag-changes (list "--" query)))
+    (run-hooks 'notmuch-after-tag-hook)))
+
+;;
+
+(provide 'notmuch-tag)
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index f0afa07..9aec96d 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -48,11 +48,11 @@
 ;; required, but is available from http://notmuchmail.org).
 
 (eval-when-compile (require 'cl))
-(require 'crm)
 (require 'mm-view)
 (require 'message)
 
 (require 'notmuch-lib)
+(require 'notmuch-tag)
 (require 'notmuch-show)
 (require 'notmuch-mua)
 (require 'notmuch-hello)
@@ -76,66 +76,6 @@ For example:
 (defvar notmuch-query-history nil
   "Variable to store minibuffer history for notmuch queries")
 
-(defvar notmuch-select-tag-history nil
-  "Variable to store minibuffer history for
-`notmuch-select-tag-with-completion' function.")
-
-(defvar notmuch-read-tag-changes-history nil
-  "Variable to store minibuffer history for
-`notmuch-read-tag-changes' function.")
-
-(defun notmuch-tag-completions (&optional search-terms)
-  (split-string
-   (with-output-to-string
-     (with-current-buffer standard-output
-       (apply 'call-process notmuch-command nil t
-	      nil "search-tags" search-terms)))
-   "\n+" t))
-
-(defun notmuch-select-tag-with-completion (prompt &rest search-terms)
-  (let ((tag-list (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)
-  (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))))
-	 (tag-list (append add-tag-list remove-tag-list))
-	 (crm-separator " ")
-	 ;; By default, space is bound to "complete word" function.
-	 ;; Re-bind it to insert a space instead.  Note that <tab>
-	 ;; still does the completion.
-	 (crm-local-completion-map
-	  (let ((map (make-sparse-keymap)))
-	    (set-keymap-parent map crm-local-completion-map)
-	    (define-key map " " 'self-insert-command)
-	    map)))
-    (delete "" (completing-read-multiple "Tags (+add -drop): "
-		tag-list nil nil initial-input
-		'notmuch-read-tag-changes-history))))
-
-(defun notmuch-update-tags (tags tag-changes)
-  "Return a copy of TAGS with additions and removals from TAG-CHANGES.
-
-TAG-CHANGES must be a list of tags names, each prefixed with
-either a \"+\" to indicate the tag should be added to TAGS if not
-present or a \"-\" to indicate that the tag should be removed
-from TAGS if present."
-  (let ((result-tags (copy-sequence tags)))
-    (dolist (tag-change tag-changes)
-      (let ((op (string-to-char tag-change))
-	    (tag (unless (string= tag-change "") (substring tag-change 1))))
-	(case op
-	  (?+ (unless (member tag result-tags)
-		(push tag result-tags)))
-	  (?- (setq result-tags (delete tag result-tags)))
-	  (otherwise
-	   (error "Changed tag must be of the form `+this_tag' or `-that_tag'")))))
-    (sort result-tags 'string<)))
-
 (defun notmuch-foreach-mime-part (function mm-handle)
   (cond ((stringp (car mm-handle))
          (dolist (part (cdr mm-handle))
@@ -543,51 +483,6 @@ and will also appear in a buffer named \"*Notmuch errors*\"."
 	    (error (buffer-substring beg end))
 	    ))))))
 
-(defun notmuch-tag (query &rest tag-changes)
-  "Add/remove tags in TAG-CHANGES to messages matching QUERY.
-
-TAG-CHANGES should be a list of strings of the form \"+tag\" or
-\"-tag\" and QUERY should be a string containing the
-search-query.
-
-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
-  (mapc (lambda (tag-change)
-	  (unless (string-match-p "^[-+]\\S-+$" tag-change)
-	    (error "Tag must be of the form `+this_tag' or `-that_tag'")))
-	tag-changes)
-  (unless (null tag-changes)
-    (run-hooks 'notmuch-before-tag-hook)
-    (apply 'notmuch-call-notmuch-process "tag"
-	   (append tag-changes (list "--" query)))
-    (run-hooks 'notmuch-after-tag-hook)))
-
-(defcustom notmuch-before-tag-hook nil
-  "Hooks that are run before tags of a message are modified.
-
-'tags' will contain the tags that are about to be added or removed as
-a list of strings of the form \"+TAG\" or \"-TAG\".
-'query' will be a string containing the search query that determines
-the messages that are about to be tagged"
-
-  :type 'hook
-  :options '(notmuch-hl-line-mode)
-  :group 'notmuch-hooks)
-
-(defcustom notmuch-after-tag-hook nil
-  "Hooks that are run after tags of a message are modified.
-
-'tags' will contain the tags that were added or removed as
-a list of strings of the form \"+TAG\" or \"-TAG\".
-'query' will be a string containing the search query that determines
-the messages that were tagged"
-  :type 'hook
-  :options '(notmuch-hl-line-mode)
-  :group 'notmuch-hooks)
-
 (defun notmuch-search-set-tags (tags)
   (save-excursion
     (end-of-line)
-- 
1.7.9.1

[PATCH 2/8] emacs: update tag-completion function

[Jameson Graef Rollins]

"search-tags" is deprecated, so we move to the more modern and
supported "search --output=tags".
---
 emacs/notmuch-tag.el |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
index 81b4b00..5240d13 100644
--- a/emacs/notmuch-tag.el
+++ b/emacs/notmuch-tag.el
@@ -59,7 +59,7 @@ the messages that were tagged"
    (with-output-to-string
      (with-current-buffer standard-output
        (apply 'call-process notmuch-command nil t
-	      nil "search-tags" search-terms)))
+	      nil "search" "--output=tags" search-terms)))
    "\n+" t))
 
 (defun notmuch-select-tag-with-completion (prompt &rest search-terms)
-- 
1.7.9.1

[PATCH 3/8] emacs: have tag-completion return all tags for nil input

[Jameson Graef Rollins]

Previously the function would fail if the initial input was nil.  Now
it will return a list of all tags, which obviously makes much more
sense.
---
 emacs/notmuch-tag.el |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
index 5240d13..b1848b4 100644
--- a/emacs/notmuch-tag.el
+++ b/emacs/notmuch-tag.el
@@ -55,6 +55,8 @@ the messages that were tagged"
 `notmuch-read-tag-changes' function.")
 
 (defun notmuch-tag-completions (&optional search-terms)
+  (if (null search-terms)
+      (setq search-terms (list "*")))
   (split-string
    (with-output-to-string
      (with-current-buffer standard-output
-- 
1.7.9.1

[PATCH 4/8] emacs: allow notmuch-tag to accept string inputs and prompt for tags

[Jameson Graef Rollins]

notmuch-tag is extended to accept various formats of the tag changes.
In particular, user prompting for tag changes is now incorporated
here, so it is common for modes.
---
 emacs/notmuch-tag.el |   20 +++++++++++++++-----
 1 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
index b1848b4..ed59c2d 100644
--- a/emacs/notmuch-tag.el
+++ b/emacs/notmuch-tag.el
@@ -108,18 +108,26 @@ 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 &rest tag-changes)
+(defun notmuch-tag (query &optional tag-changes)
   "Add/remove tags in TAG-CHANGES to messages matching QUERY.
 
-TAG-CHANGES should be a list of strings of the form \"+tag\" or
-\"-tag\" and QUERY should be a string containing the
-search-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.
 
 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 tag-changes query))
+	(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'")))
@@ -128,7 +136,9 @@ 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)))
+    (run-hooks 'notmuch-after-tag-hook))
+  ;; return the list of actual changed tags
+  tag-changes)
 
 ;;
 
-- 
1.7.9.1

[PATCH 5/8] emacs: modify search tag functions to use new notmuch-tag interface

[Jameson Graef Rollins]

The main change here is to modify argument parsing so as to not force
tag-changes to be a list, and to let notmuch-tag handle prompting the
user when required.  doc strings are also updated and cleaned up.
---
 emacs/notmuch.el |   36 +++++++++++++-----------------------
 1 files changed, 13 insertions(+), 23 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 9aec96d..a03a526 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -523,13 +523,10 @@ and will also appear in a buffer named \"*Notmuch errors*\"."
 See `notmuch-search-tag-region' for details."
   (apply 'notmuch-search-tag-region (point) (point) tag-changes))
 
-(defun notmuch-search-tag-region (beg end &rest tag-changes)
-  "Change tags for threads in the given region.
-
-TAGS is a list of tag operations for `notmuch-tag'.  The tags are
-added or removed for all threads in the region from BEG to END."
+(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)))
-    (apply 'notmuch-tag search-string tag-changes)
+    (setq tag-changes (funcall 'notmuch-tag search-string tag-changes))
     (save-excursion
       (let ((last-line (line-number-at-pos end))
 	    (max-line (- (line-number-at-pos (point-max)) 2)))
@@ -539,14 +536,14 @@ added or removed for all threads in the region from BEG to END."
 	   (notmuch-update-tags (notmuch-search-get-tags) tag-changes))
 	  (forward-line))))))
 
-(defun notmuch-search-tag (&optional initial-input)
-  "Change tags for the currently selected thread or region."
+(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)))
-	 (search-string (notmuch-search-find-thread-id-region-search beg end))
-	 (tags (notmuch-read-tag-changes initial-input search-string)))
-    (apply 'notmuch-search-tag-region beg end tags)))
+	 (end (if (region-active-p) (region-end) (point))))
+    (funcall 'notmuch-search-tag-region beg end tag-changes)))
 
 (defun notmuch-search-add-tag ()
   "Same as `notmuch-search-tag' but sets initial input to '+'."
@@ -790,18 +787,11 @@ non-authors is found, assume that all of the authors match."
 	      (goto-char found-target)))
       (delete-process proc))))
 
-(defun notmuch-search-tag-all (&rest tag-changes)
-  "Add/remove tags from all matching messages.
+(defun notmuch-search-tag-all (&optional tag-changes)
+  "Add/remove tags from all messages in current search buffer.
 
-This command adds or removes tags from all messages matching the
-current search terms. When called interactively, this command
-will prompt for tags to be added or removed. Tags prefixed with
-'+' will be added and tags prefixed with '-' will be removed.
-
-Each character of the tag name may consist of alphanumeric
-characters as well as `_.+-'.
-"
-  (interactive (notmuch-read-tag-changes))
+See `notmuch-tag' for information on the format of TAG-CHANGES."
+  (interactive)
   (apply 'notmuch-tag notmuch-search-query-string tag-changes))
 
 (defun notmuch-search-buffer-title (query)
-- 
1.7.9.1

[PATCH 6/8] emacs: eliminate search-tag-thread in favor of just search-tag

[Jameson Graef Rollins]

notmuch-search-tag-thread is now completely redundant with
notmuch-search-tag so we eliminate it to simplify the interface.
---
 emacs/notmuch.el |    8 +-------
 1 files changed, 1 insertions(+), 7 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index a03a526..d79a448 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -517,12 +517,6 @@ and will also appear in a buffer named \"*Notmuch errors*\"."
 	(forward-line 1))
       output)))
 
-(defun notmuch-search-tag-thread (&rest tag-changes)
-  "Change tags for the currently selected thread.
-
-See `notmuch-search-tag-region' for details."
-  (apply 'notmuch-search-tag-region (point) (point) tag-changes))
-
 (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)))
@@ -560,7 +554,7 @@ See `notmuch-tag' for information on the format of TAG-CHANGES."
 
 This function advances the next thread when finished."
   (interactive)
-  (notmuch-search-tag-thread "-inbox")
+  (notmuch-search-tag "-inbox")
   (notmuch-search-next-thread))
 
 (defvar notmuch-search-process-filter-data nil
-- 
1.7.9.1

[PATCH 7/8] emacs: modify show tag functions to use new notmuch-tag interface

[Jameson Graef Rollins]

The main change here is to modify argument parsing so as to not force
tag-changes to be a list, and to let notmuch-tag handle prompting the
user when required.  doc strings are also updated and cleaned up.
---
 emacs/notmuch-show.el |   26 +++++++++++++++-----------
 1 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index a4c313d..69bca02 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1641,22 +1641,26 @@ TAG-CHANGES is a list of tag operations for `notmuch-tag'."
   (let* ((current-tags (notmuch-show-get-tags))
 	 (new-tags (notmuch-update-tags current-tags tag-changes)))
     (unless (equal current-tags new-tags)
-      (apply 'notmuch-tag (notmuch-show-get-message-id) tag-changes)
+      (funcall 'notmuch-tag (notmuch-show-get-message-id) tag-changes)
       (notmuch-show-set-tags new-tags))))
 
-(defun notmuch-show-tag (&optional initial-input)
-  "Change tags for the current message, read input from the minibuffer."
+(defun notmuch-show-tag (&optional tag-changes)
+  "Change tags for the current message.
+
+See `notmuch-tag' for information on the format of TAG-CHANGES."
   (interactive)
-  (let ((tag-changes (notmuch-read-tag-changes
-		      initial-input (notmuch-show-get-message-id))))
-    (apply 'notmuch-show-tag-message tag-changes)))
+  (setq tag-changes (funcall '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 (&rest tag-changes)
-  "Change tags for all messages in the current buffer.
+(defun notmuch-show-tag-all (&optional tag-changes)
+  "Change tags for all messages in the current show buffer.
 
-TAG-CHANGES is a list of tag operations for `notmuch-tag'."
-  (interactive (notmuch-read-tag-changes nil notmuch-show-thread-id))
-  (apply 'notmuch-tag (notmuch-show-get-messages-ids-search) tag-changes)
+See `notmuch-tag' for information on the format of TAG-CHANGES."
+  (interactive)
+  (setq tag-changes (funcall 'notmuch-tag (notmuch-show-get-messages-ids-search) tag-changes))
   (notmuch-show-mapc
    (lambda ()
      (let* ((current-tags (notmuch-show-get-tags))
-- 
1.7.9.1

[PATCH 8/8] emacs: eliminate show-tag-message in favor of just show-tag

[Jameson Graef Rollins]

notmuch-show-tag-message is now completely redundant with
notmuch-show-tag so we eliminate it to simplify the interface.
---
 emacs/notmuch-show.el |   14 ++------------
 1 files changed, 2 insertions(+), 12 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 69bca02..f174910 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1396,7 +1396,7 @@ current thread."
 
 (defun notmuch-show-mark-read ()
   "Mark the current message as read."
-  (notmuch-show-tag-message "-unread"))
+  (notmuch-show-tag "-unread"))
 
 ;; Functions for getting attributes of several messages in the current
 ;; thread.
@@ -1634,16 +1634,6 @@ than only the current message."
 	    (message (format "Command '%s' exited abnormally with code %d"
 			     shell-command exit-code))))))))
 
-(defun notmuch-show-tag-message (&rest tag-changes)
-  "Change tags for the current message.
-
-TAG-CHANGES is a list of tag operations for `notmuch-tag'."
-  (let* ((current-tags (notmuch-show-get-tags))
-	 (new-tags (notmuch-update-tags current-tags tag-changes)))
-    (unless (equal current-tags new-tags)
-      (funcall 'notmuch-tag (notmuch-show-get-message-id) tag-changes)
-      (notmuch-show-set-tags new-tags))))
-
 (defun notmuch-show-tag (&optional tag-changes)
   "Change tags for the current message.
 
@@ -1768,7 +1758,7 @@ If a prefix argument is given, the message will be
 removed)."
   (interactive "P")
   (let ((op (if unarchive "+" "-")))
-    (notmuch-show-tag-message (concat op "inbox"))))
+    (notmuch-show-tag (concat op "inbox"))))
 
 (defun notmuch-show-archive-message-then-next-or-exit ()
   "Archive the current message, then show the next open message in the current thread.
-- 
1.7.9.1

Re: [PATCH 7/8] emacs: modify show tag functions to use new notmuch-tag interface

[Mark Walters]

On Sun, 08 Apr 2012, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> The main change here is to modify argument parsing so as to not force
> tag-changes to be a list, and to let notmuch-tag handle prompting the
> user when required.  doc strings are also updated and cleaned up.
> ---
>  emacs/notmuch-show.el |   26 +++++++++++++++-----------
>  1 files changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index a4c313d..69bca02 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -1641,22 +1641,26 @@ TAG-CHANGES is a list of tag operations for `notmuch-tag'."
>    (let* ((current-tags (notmuch-show-get-tags))
>  	 (new-tags (notmuch-update-tags current-tags tag-changes)))
>      (unless (equal current-tags new-tags)
> -      (apply 'notmuch-tag (notmuch-show-get-message-id) tag-changes)
> +      (funcall 'notmuch-tag (notmuch-show-get-message-id) tag-changes)
>        (notmuch-show-set-tags new-tags))))
>  
> -(defun notmuch-show-tag (&optional initial-input)
> -  "Change tags for the current message, read input from the minibuffer."
> +(defun notmuch-show-tag (&optional tag-changes)
> +  "Change tags for the current message.
> +
> +See `notmuch-tag' for information on the format of TAG-CHANGES."
>    (interactive)
> -  (let ((tag-changes (notmuch-read-tag-changes
> -		      initial-input (notmuch-show-get-message-id))))
> -    (apply 'notmuch-show-tag-message tag-changes)))
> +  (setq tag-changes (funcall '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))))

Hi. If I am following this correctly the setq line funcalls notmuch tag
regardless of whether there will be a tag change.
 
whereas the code from patch 8/8
> 	 -(defun notmuch-show-tag-message (&rest tag-changes)
> 	 -  "Change tags for the current message.
> 	 -
> 	 -TAG-CHANGES is a list of tag operations for `notmuch-tag'."
> 	 -  (let* ((current-tags (notmuch-show-get-tags))
> 	 -	 (new-tags (notmuch-update-tags current-tags tag-changes)))
> 	 -    (unless (equal current-tags new-tags)
> 	 -      (funcall 'notmuch-tag (notmuch-show-get-message-id) tag-changes)
> 	 -      (notmuch-show-set-tags new-tags))))
> 	 -

seems to only do the funcall when the tags change.

I think this is what is making the two tests fail: they count the number
of invocations of notmuch and in case there is one invocation of notmuch
show and one of notmuch tag -unread message-id, where before it was just
the single notmuch show.

Best wishes

Mark

Re: [PATCH 7/8] emacs: modify show tag functions to use new notmuch-tag interface

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 3107 bytes --]

On Sat, Apr 07 2012, Mark Walters <markwalters1009@gmail.com> wrote:
> I think this is what is making the two tests fail: they count the number
> of invocations of notmuch and in case there is one invocation of notmuch
> show and one of notmuch tag -unread message-id, where before it was just
> the single notmuch show.

Good call, Mark.  After a bit of testing it looks like that is what's
going on.  I was confused, since I had thought that the call to
notmuch-show should have involved two notmuch calls originally as well,
one for retrieving the message and the other removing the unread tag.
However, it appears the messages in those tests don't have unread tags
after all.  Not sure why, but that explains it.

So I guess the upshot is that moving all the common prompting and tag
validation stuff into notmuch-tag means that in certain cases there will
be extra notmuch calls, even if no tags are changed.  Is that a problem?

What I can do, though, is add extra validation to notmuch-tag to not
actually call notmuch tag, or any of the pre- and post- tagging hooks,
if no tags are changing.  This will still require one call to notmuch to
retrieve the current set of tags for the query, but at least it wont tag
or call the hooks if nothing is changing.  That seems reasonable to me,
but please let me know if you think it's not.

I've pasted below a new version of notmuch-tag that addresses these
issues.  Let me know what you think, and I'll resubmit the series.

jamie.


(defun notmuch-tag (query &optional 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.

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 tag-changes query))
	(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'")))
	tag-changes)
  (let* ((current-tags (notmuch-tag-completions (list query)))
	 (new-tags (notmuch-update-tags current-tags tag-changes)))
    (if (equal current-tags new-tags)
	;; if no tags are changing, return nil
	nil
      (run-hooks 'notmuch-before-tag-hook)
      (apply 'notmuch-call-notmuch-process "tag"
	     (append tag-changes (list "--" query)))
      (run-hooks 'notmuch-after-tag-hook)
      ;; otherwise, return the list of actual changed tags
      tag-changes)))

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH 2/8] emacs: update tag-completion function

[Mark Walters]

On Sun, 08 Apr 2012, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> "search-tags" is deprecated, so we move to the more modern and
> supported "search --output=tags".
> ---
>  emacs/notmuch-tag.el |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
> index 81b4b00..5240d13 100644
> --- a/emacs/notmuch-tag.el
> +++ b/emacs/notmuch-tag.el
> @@ -59,7 +59,7 @@ the messages that were tagged"
>     (with-output-to-string
>       (with-current-buffer standard-output
>         (apply 'call-process notmuch-command nil t
> -	      nil "search-tags" search-terms)))
> +	      nil "search" "--output=tags" search-terms)))

Perhaps make this 
  + nil "search" "--output=tags" "--exclude=false" search-terms)))

to fix the existing bug that if the message is excluded the completion
can get confused. Or I can submit that separately if you prefer.

Best wishes

Mark

>     "\n+" t))
>  
>  (defun notmuch-select-tag-with-completion (prompt &rest search-terms)
> -- 
> 1.7.9.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 2/8] emacs: update tag-completion function

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 618 bytes --]

On Mon, Apr 09 2012, Mark Walters <markwalters1009@gmail.com> wrote:
>> -	      nil "search-tags" search-terms)))
>> +	      nil "search" "--output=tags" search-terms)))
>
> Perhaps make this 
>   + nil "search" "--output=tags" "--exclude=false" search-terms)))
>
> to fix the existing bug that if the message is excluded the completion
> can get confused. Or I can submit that separately if you prefer.

Hey, Mark.  Yes, that was included as a separate patch in v1 of the
series, but now that your exclude stuff has been pushed this should be
included as well.  I'll send a patch for this now.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH 7/8] emacs: modify show tag functions to use new notmuch-tag interface

[Mark Walters]

On Mon, 09 Apr 2012, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> On Sat, Apr 07 2012, Mark Walters <markwalters1009@gmail.com> wrote:
>> I think this is what is making the two tests fail: they count the number
>> of invocations of notmuch and in case there is one invocation of notmuch
>> show and one of notmuch tag -unread message-id, where before it was just
>> the single notmuch show.
>
> Good call, Mark.  After a bit of testing it looks like that is what's
> going on.  I was confused, since I had thought that the call to
> notmuch-show should have involved two notmuch calls originally as well,
> one for retrieving the message and the other removing the unread tag.
> However, it appears the messages in those tests don't have unread tags
> after all.  Not sure why, but that explains it.
>
> So I guess the upshot is that moving all the common prompting and tag
> validation stuff into notmuch-tag means that in certain cases there will
> be extra notmuch calls, even if no tags are changed.  Is that a problem?
>
> What I can do, though, is add extra validation to notmuch-tag to not
> actually call notmuch tag, or any of the pre- and post- tagging hooks,
> if no tags are changing.  This will still require one call to notmuch to
> retrieve the current set of tags for the query, but at least it wont tag
> or call the hooks if nothing is changing.  That seems reasonable to me,
> but please let me know if you think it's not.
>
> I've pasted below a new version of notmuch-tag that addresses these
> issues.  Let me know what you think, and I'll resubmit the series.
>
> jamie.
>
>
> (defun notmuch-tag (query &optional 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.
>
> 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 tag-changes query))
> 	(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'")))
> 	tag-changes)
>   (let* ((current-tags (notmuch-tag-completions (list query)))
> 	 (new-tags (notmuch-update-tags current-tags tag-changes)))
>     (if (equal current-tags new-tags)
> 	;; if no tags are changing, return nil
> 	nil
>       (run-hooks 'notmuch-before-tag-hook)
>       (apply 'notmuch-call-notmuch-process "tag"
> 	     (append tag-changes (list "--" query)))
>       (run-hooks 'notmuch-after-tag-hook)
>       ;; otherwise, return the list of actual changed tags
>       tag-changes)))

Does this actually do the right thing if tagging more than one message?
It looks to me like it would go wrong if you tried +inbox to a thread
where some messages already have tag inbox (but I could be confused)?

Also, I was going to say that I was not sure there was much point in
optimising in the emacs code when the cli does anyway, but there is a
question with xapian locking: with the orginally posted patch you can't
use n or p in show view while the database is locked (eg a background
notmuch new) as you get "A Xapian exception occurred opening database:
Unable to get write lock on ..."

Possibly, you could pass a current-tags variable to notmuch tag (and it
would not add anything in that list or delete anything not in the
list). But the 2 code paths might be viewed as being too different to be
worth unifying. Or possibly have a "tag-single-message" command? 

Best wishes

Mark

Re: [PATCH 5/8] emacs: modify search tag functions to use new notmuch-tag interface

[Mark Walters]

On Sun, 08 Apr 2012, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> The main change here is to modify argument parsing so as to not force
> tag-changes to be a list, and to let notmuch-tag handle prompting the
> user when required.  doc strings are also updated and cleaned up.
> ---
>  emacs/notmuch.el |   36 +++++++++++++-----------------------
>  1 files changed, 13 insertions(+), 23 deletions(-)
>
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index 9aec96d..a03a526 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -523,13 +523,10 @@ and will also appear in a buffer named \"*Notmuch errors*\"."
>  See `notmuch-search-tag-region' for details."
>    (apply 'notmuch-search-tag-region (point) (point) tag-changes))
>  
> -(defun notmuch-search-tag-region (beg end &rest tag-changes)
> -  "Change tags for threads in the given region.
> -
> -TAGS is a list of tag operations for `notmuch-tag'.  The tags are
> -added or removed for all threads in the region from BEG to END."
> +(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)))
> -    (apply 'notmuch-tag search-string tag-changes)
> +    (setq tag-changes (funcall 'notmuch-tag search-string tag-changes))
>      (save-excursion
>        (let ((last-line (line-number-at-pos end))
>  	    (max-line (- (line-number-at-pos (point-max)) 2)))
> @@ -539,14 +536,14 @@ added or removed for all threads in the region from BEG to END."
>  	   (notmuch-update-tags (notmuch-search-get-tags) tag-changes))
>  	  (forward-line))))))
>  
> -(defun notmuch-search-tag (&optional initial-input)
> -  "Change tags for the currently selected thread or region."
> +(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)))
> -	 (search-string (notmuch-search-find-thread-id-region-search beg end))
> -	 (tags (notmuch-read-tag-changes initial-input search-string)))
> -    (apply 'notmuch-search-tag-region beg end tags)))
> +	 (end (if (region-active-p) (region-end) (point))))
> +    (funcall 'notmuch-search-tag-region beg end tag-changes)))
>  
>  (defun notmuch-search-add-tag ()
>    "Same as `notmuch-search-tag' but sets initial input to '+'."
> @@ -790,18 +787,11 @@ non-authors is found, assume that all of the authors match."
>  	      (goto-char found-target)))
>        (delete-process proc))))
>  
> -(defun notmuch-search-tag-all (&rest tag-changes)
> -  "Add/remove tags from all matching messages.
> +(defun notmuch-search-tag-all (&optional tag-changes)
> +  "Add/remove tags from all messages in current search buffer.
>  
> -This command adds or removes tags from all messages matching the
> -current search terms. When called interactively, this command
> -will prompt for tags to be added or removed. Tags prefixed with
> -'+' will be added and tags prefixed with '-' will be removed.
> -
> -Each character of the tag name may consist of alphanumeric
> -characters as well as `_.+-'.
> -"
> -  (interactive (notmuch-read-tag-changes))
> +See `notmuch-tag' for information on the format of TAG-CHANGES."
> +  (interactive)
>    (apply 'notmuch-tag notmuch-search-query-string tag-changes))

My only comment here is that I wonder about the tag-all command. I can
see that this is a performance win over applying tag region to the whole
buffer but is it another place where we have a race with incoming mail
(ie we tag everything that now matches rather than everything in the
buffer).

A second advantage with the tag-region form is that it automatically
does excludes. Currently notmuch-tag does not do excludes as it looked
very hairy to implement correctly: thus the tag-all command could spend
a lot of time tagging excluded messages.

OTOH this seems to be a "feature" of the existing code so a "fix" is not
required for this series.

(Note I would not consider this a full review as my lisp is not good
enough; eg I am unsure  why the change from apply to funcall was
needed).

Best wishes

Mark

>  
>  (defun notmuch-search-buffer-title (query)
> -- 
> 1.7.9.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 7/8] emacs: modify show tag functions to use new notmuch-tag interface

[Mark Walters]

On Sun, 08 Apr 2012, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> The main change here is to modify argument parsing so as to not force
> tag-changes to be a list, and to let notmuch-tag handle prompting the
> user when required.  doc strings are also updated and cleaned up.

One other comment on this patch is that it would be nice to have a tag
region in show view as well. Of course that is a feature request rather
than a requirement for this patch series!

Best wishes

Mark

> ---
>  emacs/notmuch-show.el |   26 +++++++++++++++-----------
>  1 files changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index a4c313d..69bca02 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -1641,22 +1641,26 @@ TAG-CHANGES is a list of tag operations for `notmuch-tag'."
>    (let* ((current-tags (notmuch-show-get-tags))
>  	 (new-tags (notmuch-update-tags current-tags tag-changes)))
>      (unless (equal current-tags new-tags)
> -      (apply 'notmuch-tag (notmuch-show-get-message-id) tag-changes)
> +      (funcall 'notmuch-tag (notmuch-show-get-message-id) tag-changes)
>        (notmuch-show-set-tags new-tags))))
>  
> -(defun notmuch-show-tag (&optional initial-input)
> -  "Change tags for the current message, read input from the minibuffer."
> +(defun notmuch-show-tag (&optional tag-changes)
> +  "Change tags for the current message.
> +
> +See `notmuch-tag' for information on the format of TAG-CHANGES."
>    (interactive)
> -  (let ((tag-changes (notmuch-read-tag-changes
> -		      initial-input (notmuch-show-get-message-id))))
> -    (apply 'notmuch-show-tag-message tag-changes)))
> +  (setq tag-changes (funcall '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 (&rest tag-changes)
> -  "Change tags for all messages in the current buffer.
> +(defun notmuch-show-tag-all (&optional tag-changes)
> +  "Change tags for all messages in the current show buffer.
>  
> -TAG-CHANGES is a list of tag operations for `notmuch-tag'."
> -  (interactive (notmuch-read-tag-changes nil notmuch-show-thread-id))
> -  (apply 'notmuch-tag (notmuch-show-get-messages-ids-search) tag-changes)
> +See `notmuch-tag' for information on the format of TAG-CHANGES."
> +  (interactive)
> +  (setq tag-changes (funcall 'notmuch-tag (notmuch-show-get-messages-ids-search) tag-changes))
>    (notmuch-show-mapc
>     (lambda ()
>       (let* ((current-tags (notmuch-show-get-tags))
> -- 
> 1.7.9.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

emacs tagging cleanup, v3

[Jameson Graef Rollins]

This is a further cleaned up and simplified version of [0].  The scope
is slightly reduced, but we still end up with a simpler user
interface, with the primary tagging functions being:

notmuch-search-tag
notmuch-show-tag

In this series only one user facing tagging function,
notmuch-search-tag-thread, is removed, since it is completely
redundant with the now more flexible notmuch-search-tag.

jamie.

[0] id:"1333845338-22960-1-git-send-email-jrollins@finestructure.net"

[PATCH 1/5] emacs: create notmuch-tag.el, and move appropriate functions from notmuch.el

[Jameson Graef Rollins]

Tagging functions are used in notmuch.el, notmuch-show.el, and
notmuch-message.el.  There are enough common functions for tagging
that it makes sense to put them all in their own library.

No code is modified, just moved around.
---
 emacs/Makefile.local     |    1 +
 emacs/notmuch-message.el |    1 +
 emacs/notmuch-show.el    |    3 +-
 emacs/notmuch-tag.el     |  135 ++++++++++++++++++++++++++++++++++++++++++++++
 emacs/notmuch.el         |  109 +------------------------------------
 5 files changed, 139 insertions(+), 110 deletions(-)
 create mode 100644 emacs/notmuch-tag.el

diff --git a/emacs/Makefile.local b/emacs/Makefile.local
index 4fee0e8..fb82247 100644
--- a/emacs/Makefile.local
+++ b/emacs/Makefile.local
@@ -13,6 +13,7 @@ emacs_sources := \
 	$(dir)/notmuch-maildir-fcc.el \
 	$(dir)/notmuch-message.el \
 	$(dir)/notmuch-crypto.el \
+	$(dir)/notmuch-tag.el \
 	$(dir)/coolj.el \
 	$(dir)/notmuch-print.el
 
diff --git a/emacs/notmuch-message.el b/emacs/notmuch-message.el
index 3010281..5964caa 100644
--- a/emacs/notmuch-message.el
+++ b/emacs/notmuch-message.el
@@ -20,6 +20,7 @@
 ;; Authors: Jesse Rosenthal <jrosenthal@jhu.edu>
 
 (require 'message)
+(require 'notmuch-tag)
 (require 'notmuch-mua)
 
 (defcustom notmuch-message-replied-tags '("replied")
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 30b26d1..a4c313d 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -30,6 +30,7 @@
 (require 'goto-addr)
 
 (require 'notmuch-lib)
+(require 'notmuch-tag)
 (require 'notmuch-query)
 (require 'notmuch-wash)
 (require 'notmuch-mua)
@@ -38,10 +39,8 @@
 
 (declare-function notmuch-call-notmuch-process "notmuch" (&rest args))
 (declare-function notmuch-fontify-headers "notmuch" nil)
-(declare-function notmuch-read-tag-changes "notmuch" (&optional initial-input &rest search-terms))
 (declare-function notmuch-search-next-thread "notmuch" nil)
 (declare-function notmuch-search-show-thread "notmuch" nil)
-(declare-function notmuch-update-tags "notmuch" (current-tags tag-changes))
 
 (defcustom notmuch-message-headers '("Subject" "To" "Cc" "Date")
   "Headers that should be shown in a message, in this order.
diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
new file mode 100644
index 0000000..c25cff8
--- /dev/null
+++ b/emacs/notmuch-tag.el
@@ -0,0 +1,135 @@
+;; notmuch-tag.el --- tag messages within emacs
+;;
+;; Copyright © Carl Worth
+;;
+;; This file is part of Notmuch.
+;;
+;; Notmuch is free software: you can redistribute it and/or modify it
+;; under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+;;
+;; Notmuch is distributed in the hope that it will be useful, but
+;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+;; General Public License for more details.
+;;
+;; You should have received a copy of the GNU General Public License
+;; along with Notmuch.  If not, see <http://www.gnu.org/licenses/>.
+;;
+;; Authors: Carl Worth <cworth@cworth.org>
+
+(eval-when-compile (require 'cl))
+(require 'crm)
+(require 'notmuch-lib)
+
+(defcustom notmuch-before-tag-hook nil
+  "Hooks that are run before tags of a message are modified.
+
+'tags' will contain the tags that are about to be added or removed as
+a list of strings of the form \"+TAG\" or \"-TAG\".
+'query' will be a string containing the search query that determines
+the messages that are about to be tagged"
+
+  :type 'hook
+  :options '(notmuch-hl-line-mode)
+  :group 'notmuch-hooks)
+
+(defcustom notmuch-after-tag-hook nil
+  "Hooks that are run after tags of a message are modified.
+
+'tags' will contain the tags that were added or removed as
+a list of strings of the form \"+TAG\" or \"-TAG\".
+'query' will be a string containing the search query that determines
+the messages that were tagged"
+  :type 'hook
+  :options '(notmuch-hl-line-mode)
+  :group 'notmuch-hooks)
+
+(defvar notmuch-select-tag-history nil
+  "Variable to store minibuffer history for
+`notmuch-select-tag-with-completion' function.")
+
+(defvar notmuch-read-tag-changes-history nil
+  "Variable to store minibuffer history for
+`notmuch-read-tag-changes' function.")
+
+(defun notmuch-tag-completions (&optional search-terms)
+  (if (null search-terms)
+      (setq search-terms (list "*")))
+  (split-string
+   (with-output-to-string
+     (with-current-buffer standard-output
+       (apply 'call-process notmuch-command nil t
+	      nil "search" "--output=tags" "--exclude=false" search-terms)))
+   "\n+" t))
+
+(defun notmuch-select-tag-with-completion (prompt &rest search-terms)
+  (let ((tag-list (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)
+  (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))))
+	 (tag-list (append add-tag-list remove-tag-list))
+	 (crm-separator " ")
+	 ;; By default, space is bound to "complete word" function.
+	 ;; Re-bind it to insert a space instead.  Note that <tab>
+	 ;; still does the completion.
+	 (crm-local-completion-map
+	  (let ((map (make-sparse-keymap)))
+	    (set-keymap-parent map crm-local-completion-map)
+	    (define-key map " " 'self-insert-command)
+	    map)))
+    (delete "" (completing-read-multiple "Tags (+add -drop): "
+		tag-list nil nil initial-input
+		'notmuch-read-tag-changes-history))))
+
+(defun notmuch-update-tags (tags tag-changes)
+  "Return a copy of TAGS with additions and removals from TAG-CHANGES.
+
+TAG-CHANGES must be a list of tags names, each prefixed with
+either a \"+\" to indicate the tag should be added to TAGS if not
+present or a \"-\" to indicate that the tag should be removed
+from TAGS if present."
+  (let ((result-tags (copy-sequence tags)))
+    (dolist (tag-change tag-changes)
+      (let ((op (string-to-char tag-change))
+	    (tag (unless (string= tag-change "") (substring tag-change 1))))
+	(case op
+	  (?+ (unless (member tag result-tags)
+		(push tag result-tags)))
+	  (?- (setq result-tags (delete tag result-tags)))
+	  (otherwise
+	   (error "Changed tag must be of the form `+this_tag' or `-that_tag'")))))
+    (sort result-tags 'string<)))
+
+(defun notmuch-tag (query &rest tag-changes)
+  "Add/remove tags in TAG-CHANGES to messages matching QUERY.
+
+TAG-CHANGES should be a list of strings of the form \"+tag\" or
+\"-tag\" and QUERY should be a string containing the
+search-query.
+
+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
+  (mapc (lambda (tag-change)
+	  (unless (string-match-p "^[-+]\\S-+$" tag-change)
+	    (error "Tag must be of the form `+this_tag' or `-that_tag'")))
+	tag-changes)
+  (unless (null tag-changes)
+    (run-hooks 'notmuch-before-tag-hook)
+    (apply 'notmuch-call-notmuch-process "tag"
+	   (append tag-changes (list "--" query)))
+    (run-hooks 'notmuch-after-tag-hook)))
+
+;;
+
+(provide 'notmuch-tag)
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index ba833e6..9aec96d 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -48,11 +48,11 @@
 ;; required, but is available from http://notmuchmail.org).
 
 (eval-when-compile (require 'cl))
-(require 'crm)
 (require 'mm-view)
 (require 'message)
 
 (require 'notmuch-lib)
+(require 'notmuch-tag)
 (require 'notmuch-show)
 (require 'notmuch-mua)
 (require 'notmuch-hello)
@@ -76,68 +76,6 @@ For example:
 (defvar notmuch-query-history nil
   "Variable to store minibuffer history for notmuch queries")
 
-(defvar notmuch-select-tag-history nil
-  "Variable to store minibuffer history for
-`notmuch-select-tag-with-completion' function.")
-
-(defvar notmuch-read-tag-changes-history nil
-  "Variable to store minibuffer history for
-`notmuch-read-tag-changes' function.")
-
-(defun notmuch-tag-completions (&optional search-terms)
-  (if (null search-terms)
-      (setq search-terms (list "*")))
-  (split-string
-   (with-output-to-string
-     (with-current-buffer standard-output
-       (apply 'call-process notmuch-command nil t
-	      nil "search" "--output=tags" "--exclude=false" search-terms)))
-   "\n+" t))
-
-(defun notmuch-select-tag-with-completion (prompt &rest search-terms)
-  (let ((tag-list (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)
-  (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))))
-	 (tag-list (append add-tag-list remove-tag-list))
-	 (crm-separator " ")
-	 ;; By default, space is bound to "complete word" function.
-	 ;; Re-bind it to insert a space instead.  Note that <tab>
-	 ;; still does the completion.
-	 (crm-local-completion-map
-	  (let ((map (make-sparse-keymap)))
-	    (set-keymap-parent map crm-local-completion-map)
-	    (define-key map " " 'self-insert-command)
-	    map)))
-    (delete "" (completing-read-multiple "Tags (+add -drop): "
-		tag-list nil nil initial-input
-		'notmuch-read-tag-changes-history))))
-
-(defun notmuch-update-tags (tags tag-changes)
-  "Return a copy of TAGS with additions and removals from TAG-CHANGES.
-
-TAG-CHANGES must be a list of tags names, each prefixed with
-either a \"+\" to indicate the tag should be added to TAGS if not
-present or a \"-\" to indicate that the tag should be removed
-from TAGS if present."
-  (let ((result-tags (copy-sequence tags)))
-    (dolist (tag-change tag-changes)
-      (let ((op (string-to-char tag-change))
-	    (tag (unless (string= tag-change "") (substring tag-change 1))))
-	(case op
-	  (?+ (unless (member tag result-tags)
-		(push tag result-tags)))
-	  (?- (setq result-tags (delete tag result-tags)))
-	  (otherwise
-	   (error "Changed tag must be of the form `+this_tag' or `-that_tag'")))))
-    (sort result-tags 'string<)))
-
 (defun notmuch-foreach-mime-part (function mm-handle)
   (cond ((stringp (car mm-handle))
          (dolist (part (cdr mm-handle))
@@ -545,51 +483,6 @@ and will also appear in a buffer named \"*Notmuch errors*\"."
 	    (error (buffer-substring beg end))
 	    ))))))
 
-(defun notmuch-tag (query &rest tag-changes)
-  "Add/remove tags in TAG-CHANGES to messages matching QUERY.
-
-TAG-CHANGES should be a list of strings of the form \"+tag\" or
-\"-tag\" and QUERY should be a string containing the
-search-query.
-
-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
-  (mapc (lambda (tag-change)
-	  (unless (string-match-p "^[-+]\\S-+$" tag-change)
-	    (error "Tag must be of the form `+this_tag' or `-that_tag'")))
-	tag-changes)
-  (unless (null tag-changes)
-    (run-hooks 'notmuch-before-tag-hook)
-    (apply 'notmuch-call-notmuch-process "tag"
-	   (append tag-changes (list "--" query)))
-    (run-hooks 'notmuch-after-tag-hook)))
-
-(defcustom notmuch-before-tag-hook nil
-  "Hooks that are run before tags of a message are modified.
-
-'tags' will contain the tags that are about to be added or removed as
-a list of strings of the form \"+TAG\" or \"-TAG\".
-'query' will be a string containing the search query that determines
-the messages that are about to be tagged"
-
-  :type 'hook
-  :options '(notmuch-hl-line-mode)
-  :group 'notmuch-hooks)
-
-(defcustom notmuch-after-tag-hook nil
-  "Hooks that are run after tags of a message are modified.
-
-'tags' will contain the tags that were added or removed as
-a list of strings of the form \"+TAG\" or \"-TAG\".
-'query' will be a string containing the search query that determines
-the messages that were tagged"
-  :type 'hook
-  :options '(notmuch-hl-line-mode)
-  :group 'notmuch-hooks)
-
 (defun notmuch-search-set-tags (tags)
   (save-excursion
     (end-of-line)
-- 
1.7.9.5

[PATCH 2/5] emacs: allow notmuch-tag to accept string inputs and prompt for tags

[Jameson Graef Rollins]

notmuch-tag is extended to accept various formats of the tag changes.
In particular, user prompting for tag changes is now incorporated
here, so it is common for modes.

The tag binary and the notmuch-{before,after}-tag-hooks are only
called if tag changes is non-nil.

The actual tag-changes applied are returned by the function.
---
 emacs/notmuch-tag.el |   20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
index c25cff8..dd7f9d7 100644
--- a/emacs/notmuch-tag.el
+++ b/emacs/notmuch-tag.el
@@ -108,18 +108,26 @@ 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 &rest tag-changes)
+(defun notmuch-tag (query &optional tag-changes)
   "Add/remove tags in TAG-CHANGES to messages matching QUERY.
 
-TAG-CHANGES should be a list of strings of the form \"+tag\" or
-\"-tag\" and QUERY should be a string containing the
-search-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.
 
 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 tag-changes query))
+	(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'")))
@@ -128,7 +136,9 @@ 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)))
+    (run-hooks 'notmuch-after-tag-hook))
+  ;; return the list of actual changed tags
+  tag-changes)
 
 ;;
 
-- 
1.7.9.5

[PATCH 3/5] emacs: modify search tag functions to use new notmuch-tag interface

[Jameson Graef Rollins]

The main change here is to modify argument parsing so as to not force
tag-changes to be a list, and to let notmuch-tag handle prompting the
user when required.  doc strings are also updated and cleaned up.
---
 emacs/notmuch.el |   36 +++++++++++++-----------------------
 1 file changed, 13 insertions(+), 23 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 9aec96d..a03a526 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -523,13 +523,10 @@ and will also appear in a buffer named \"*Notmuch errors*\"."
 See `notmuch-search-tag-region' for details."
   (apply 'notmuch-search-tag-region (point) (point) tag-changes))
 
-(defun notmuch-search-tag-region (beg end &rest tag-changes)
-  "Change tags for threads in the given region.
-
-TAGS is a list of tag operations for `notmuch-tag'.  The tags are
-added or removed for all threads in the region from BEG to END."
+(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)))
-    (apply 'notmuch-tag search-string tag-changes)
+    (setq tag-changes (funcall 'notmuch-tag search-string tag-changes))
     (save-excursion
       (let ((last-line (line-number-at-pos end))
 	    (max-line (- (line-number-at-pos (point-max)) 2)))
@@ -539,14 +536,14 @@ added or removed for all threads in the region from BEG to END."
 	   (notmuch-update-tags (notmuch-search-get-tags) tag-changes))
 	  (forward-line))))))
 
-(defun notmuch-search-tag (&optional initial-input)
-  "Change tags for the currently selected thread or region."
+(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)))
-	 (search-string (notmuch-search-find-thread-id-region-search beg end))
-	 (tags (notmuch-read-tag-changes initial-input search-string)))
-    (apply 'notmuch-search-tag-region beg end tags)))
+	 (end (if (region-active-p) (region-end) (point))))
+    (funcall 'notmuch-search-tag-region beg end tag-changes)))
 
 (defun notmuch-search-add-tag ()
   "Same as `notmuch-search-tag' but sets initial input to '+'."
@@ -790,18 +787,11 @@ non-authors is found, assume that all of the authors match."
 	      (goto-char found-target)))
       (delete-process proc))))
 
-(defun notmuch-search-tag-all (&rest tag-changes)
-  "Add/remove tags from all matching messages.
+(defun notmuch-search-tag-all (&optional tag-changes)
+  "Add/remove tags from all messages in current search buffer.
 
-This command adds or removes tags from all messages matching the
-current search terms. When called interactively, this command
-will prompt for tags to be added or removed. Tags prefixed with
-'+' will be added and tags prefixed with '-' will be removed.
-
-Each character of the tag name may consist of alphanumeric
-characters as well as `_.+-'.
-"
-  (interactive (notmuch-read-tag-changes))
+See `notmuch-tag' for information on the format of TAG-CHANGES."
+  (interactive)
   (apply 'notmuch-tag notmuch-search-query-string tag-changes))
 
 (defun notmuch-search-buffer-title (query)
-- 
1.7.9.5

[PATCH 4/5] emacs: modify show tag functions to use new notmuch-tag interface

[Jameson Graef Rollins]

The main change here is to modify argument parsing so as to not force
tag-changes to be a list, and to let notmuch-tag handle prompting the
user when required.  doc strings are also updated and cleaned up.
---
 emacs/notmuch-show.el |   26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index a4c313d..69bca02 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1641,22 +1641,26 @@ TAG-CHANGES is a list of tag operations for `notmuch-tag'."
   (let* ((current-tags (notmuch-show-get-tags))
 	 (new-tags (notmuch-update-tags current-tags tag-changes)))
     (unless (equal current-tags new-tags)
-      (apply 'notmuch-tag (notmuch-show-get-message-id) tag-changes)
+      (funcall 'notmuch-tag (notmuch-show-get-message-id) tag-changes)
       (notmuch-show-set-tags new-tags))))
 
-(defun notmuch-show-tag (&optional initial-input)
-  "Change tags for the current message, read input from the minibuffer."
+(defun notmuch-show-tag (&optional tag-changes)
+  "Change tags for the current message.
+
+See `notmuch-tag' for information on the format of TAG-CHANGES."
   (interactive)
-  (let ((tag-changes (notmuch-read-tag-changes
-		      initial-input (notmuch-show-get-message-id))))
-    (apply 'notmuch-show-tag-message tag-changes)))
+  (setq tag-changes (funcall '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 (&rest tag-changes)
-  "Change tags for all messages in the current buffer.
+(defun notmuch-show-tag-all (&optional tag-changes)
+  "Change tags for all messages in the current show buffer.
 
-TAG-CHANGES is a list of tag operations for `notmuch-tag'."
-  (interactive (notmuch-read-tag-changes nil notmuch-show-thread-id))
-  (apply 'notmuch-tag (notmuch-show-get-messages-ids-search) tag-changes)
+See `notmuch-tag' for information on the format of TAG-CHANGES."
+  (interactive)
+  (setq tag-changes (funcall 'notmuch-tag (notmuch-show-get-messages-ids-search) tag-changes))
   (notmuch-show-mapc
    (lambda ()
      (let* ((current-tags (notmuch-show-get-tags))
-- 
1.7.9.5

[PATCH 5/5] emacs: eliminate search-tag-thread in favor of just search-tag

[Jameson Graef Rollins]

notmuch-search-tag-thread is now completely redundant with
notmuch-search-tag so we eliminate it to simplify the interface.
---
 emacs/notmuch.el |    8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index a03a526..cfc3b29 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -517,12 +517,6 @@ and will also appear in a buffer named \"*Notmuch errors*\"."
 	(forward-line 1))
       output)))
 
-(defun notmuch-search-tag-thread (&rest tag-changes)
-  "Change tags for the currently selected thread.
-
-See `notmuch-search-tag-region' for details."
-  (apply 'notmuch-search-tag-region (point) (point) tag-changes))
-
 (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)))
@@ -560,7 +554,7 @@ See `notmuch-tag' for information on the format of TAG-CHANGES."
 
 This function advances the next thread when finished."
   (interactive)
-  (notmuch-search-tag-thread "-inbox")
+  (notmuch-search-tag '("-inbox"))
   (notmuch-search-next-thread))
 
 (defvar notmuch-search-process-filter-data nil
-- 
1.7.9.5

Re: [PATCH 5/8] emacs: modify search tag functions to use new notmuch-tag interface

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 1641 bytes --]

On Mon, Apr 09 2012, Mark Walters <markwalters1009@gmail.com> wrote:
> My only comment here is that I wonder about the tag-all command. I can
> see that this is a performance win over applying tag region to the whole
> buffer but is it another place where we have a race with incoming mail
> (ie we tag everything that now matches rather than everything in the
> buffer).

Yes, I think this is a problem.  notmuch-{search,show}-tag-all are both
subject to race conditions.  I suggest we remove notmuch-search-tag-all
in favor of the region tagging capabilities of notmuch-show-tag.  I will
submit a patch for that.  notmuch-show-tag-all is trickier, since there
no tag-by-region capability at the moment.  That will have to be added
before this function can be replaced.

> A second advantage with the tag-region form is that it automatically
> does excludes. Currently notmuch-tag does not do excludes as it looked
> very hairy to implement correctly: thus the tag-all command could spend
> a lot of time tagging excluded messages.

Actually, I don't think this is true, at least for search mode.  In
search mode threads are tagged by their thread id, therefore any
excluded messages in the thread will receive the same tagging
operations.

The current operation is at least, in my opinion, self consistent.  If
we're going to tag by thread then all messages in the thread should be
tagged, whether or not they're excluded.  I do think, however, that it
might be nice to add a tagging function that only tags the matching
messages in the thread.  In that case excluded messages would not be
tagged.  That's for future work, though.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH 2/5] emacs: allow notmuch-tag to accept string inputs and prompt for tags

[Mark Walters]

On Sat, 14 Apr 2012, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> notmuch-tag is extended to accept various formats of the tag changes.
> In particular, user prompting for tag changes is now incorporated
> here, so it is common for modes.
>
> The tag binary and the notmuch-{before,after}-tag-hooks are only
> called if tag changes is non-nil.
>
> The actual tag-changes applied are returned by the function.
> ---
>  emacs/notmuch-tag.el |   20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
> index c25cff8..dd7f9d7 100644
> --- a/emacs/notmuch-tag.el
> +++ b/emacs/notmuch-tag.el
> @@ -108,18 +108,26 @@ 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 &rest tag-changes)
> +(defun notmuch-tag (query &optional tag-changes)
>    "Add/remove tags in TAG-CHANGES to messages matching QUERY.
>  
> -TAG-CHANGES should be a list of strings of the form \"+tag\" or
> -\"-tag\" and QUERY should be a string containing the
> -search-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.
>  
>  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 tag-changes query))
> +	(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'")))
> @@ -128,7 +136,9 @@ 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)))
> +    (run-hooks 'notmuch-after-tag-hook))

Hi

The series looks good to me with one minor point:

> +  ;; return the list of actual changed tags
> +  tag-changes)

I found the comment confusing: I read it as the function looked at the
tags the message had before and after and returned the
difference. Perhaps something like "in all cases we return tag-changes
as a list" (and a similar comment for the commit message).

Best wishes

Mark

[PATCH v2 2/5] emacs: allow notmuch-tag to accept string inputs and prompt for tags

[Jameson Graef Rollins]

notmuch-tag is extended to accept various formats of the tag changes.
In particular, user prompting for tag changes is now incorporated
here, so it is common for modes.

The tag binary and the notmuch-{before,after}-tag-hooks are only
called if tag changes is non-nil.

In all cases tag-changes is returned as a list.
---
This addresses Mark Walters concerns in regards to a code comment.

 emacs/notmuch-tag.el |   20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
index c25cff8..0c0fc87 100644
--- a/emacs/notmuch-tag.el
+++ b/emacs/notmuch-tag.el
@@ -108,18 +108,26 @@ 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 &rest tag-changes)
+(defun notmuch-tag (query &optional tag-changes)
   "Add/remove tags in TAG-CHANGES to messages matching QUERY.
 
-TAG-CHANGES should be a list of strings of the form \"+tag\" or
-\"-tag\" and QUERY should be a string containing the
-search-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.
 
 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 tag-changes query))
+	(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'")))
@@ -128,7 +136,9 @@ 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)))
+    (run-hooks 'notmuch-after-tag-hook))
+  ;; in all cases we return tag-changes as a list
+  tag-changes)
 
 ;;
 
-- 
1.7.9.5

Re: emacs tagging cleanup, v3

[Tomi Ollila]

On Sat, Apr 14 2012, Jameson Graef Rollins <jrollins@finestructure.net> wrote:

> This is a further cleaned up and simplified version of [0].  The scope
> is slightly reduced, but we still end up with a simpler user
> interface, with the primary tagging functions being:
>
> notmuch-search-tag
> notmuch-show-tag
>
> In this series only one user facing tagging function,
> notmuch-search-tag-thread, is removed, since it is completely
> redundant with the now more flexible notmuch-search-tag.

This series, consisting:

id:"1334429574-12918-2-git-send-email-jrollins@finestructure.net"
id:"1334436547-10260-1-git-send-email-jrollins@finestructure.net"
id:"1334429574-12918-4-git-send-email-jrollins@finestructure.net"
id:"1334429574-12918-5-git-send-email-jrollins@finestructure.net"
id:"1334429574-12918-6-git-send-email-jrollins@finestructure.net"

looks good to me.

> jamie.

Tomi

> [0] id:"1333845338-22960-1-git-send-email-jrollins@finestructure.net"