[PATCH] emacs: add completion to "tag all" operation ("*" binding)

[PATCH] emacs: add completion to "tag all" operation ("*" binding)

[Dmitry Kurochkin]

The patch adds <tab> completion to "tag all" operation bound to "*"
(`notmuch-search-operate-all' function).
---
 emacs/notmuch.el |   48 ++++++++++++++++++++++++++++++++++++------------
 1 files changed, 36 insertions(+), 12 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index e02966f..71b0221 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -48,6 +48,7 @@
 ;; required, but is available from http://notmuchmail.org).
 
 (eval-when-compile (require 'cl))
+(require 'crm)
 (require 'mm-view)
 (require 'message)
 
@@ -75,12 +76,33 @@ For example:
 (defvar notmuch-query-history nil
   "Variable to store minibuffer history for notmuch queries")
 
-(defun notmuch-select-tag-with-completion (prompt &rest search-terms)
+(defun notmuch-tag-completions (&optional search-terms prefixes)
   (let ((tag-list
 	 (with-output-to-string
 	   (with-current-buffer standard-output
 	     (apply 'call-process notmuch-command nil t nil "search-tags" search-terms)))))
-    (completing-read prompt (split-string tag-list "\n+" t) nil nil nil)))
+    (setq tag-list (split-string tag-list "\n+" t))
+    (if (null prefixes)
+	tag-list
+      (apply #'append
+	     (mapcar (lambda (tag)
+		       (mapcar (lambda (prefix)
+				 (concat prefix tag)) prefixes))
+		     tag-list)))))
+
+(defun notmuch-select-tag-with-completion (prompt &optional search-terms prefixes)
+  (let ((tag-list (notmuch-tag-completions search-terms prefixes)))
+    (completing-read prompt tag-list)))
+
+(defun notmuch-select-tags-with-completion (prompt &optional search-terms prefixes)
+  (let ((tag-list (notmuch-tag-completions search-terms prefixes))
+	(crm-separator " ")
+	(crm-local-completion-map (copy-keymap crm-local-completion-map)))
+    ;; By default, space is bound to "complete word" function.
+    ;; Re-bind it to insert a space instead.  Note that <tab> still
+    ;; does the completion.
+    (define-key crm-local-completion-map " " 'self-insert-command)
+    (completing-read-multiple prompt tag-list)))
 
 (defun notmuch-foreach-mime-part (function mm-handle)
   (cond ((stringp (car mm-handle))
@@ -860,16 +882,18 @@ will prompt for tags to be added or removed. Tags prefixed with
 Each character of the tag name may consist of alphanumeric
 characters as well as `_.+-'.
 "
-  (interactive "sOperation (+add -drop): notmuch tag ")
-  (let ((action-split (split-string action " +")))
-    ;; Perform some validation
-    (let ((words action-split))
-      (when (null words) (error "No operation given"))
-      (while words
-	(unless (string-match-p "^[-+][-+_.[:word:]]+$" (car words))
-	  (error "Action must be of the form `+thistag -that_tag'"))
-	(setq words (cdr words))))
-    (apply 'notmuch-tag notmuch-search-query-string action-split)))
+  (interactive (list (notmuch-select-tags-with-completion
+		      "Operation (+add -drop): notmuch tag " nil
+		      '("+" "-"))))
+  (setq action (delete "" action))
+  ;; Perform some validation
+  (let ((words action))
+    (when (null words) (error "No operation given"))
+    (while words
+      (unless (string-match-p "^[-+][-+_.[:word:]]+$" (car words))
+	(error "Action must be of the form `+thistag -that_tag'"))
+      (setq words (cdr words))))
+  (apply 'notmuch-tag notmuch-search-query-string action))
 
 (defun notmuch-search-buffer-title (query)
   "Returns the title for a buffer with notmuch search results."
-- 
1.7.8.3

[PATCH v2] emacs: add completion to "tag all" operation ("*" binding)

[Dmitry Kurochkin]

The patch adds <tab> completion to "tag all" operation bound to "*"
(`notmuch-search-operate-all' function).
---

Changes in v2:

* s/thistag/this_tag/ for consistency with "that_tag", since we touch
  the line anyway

Regards,
  Dmitry

 emacs/notmuch.el |   48 ++++++++++++++++++++++++++++++++++++------------
 1 files changed, 36 insertions(+), 12 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index e02966f..a57bf70 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -48,6 +48,7 @@
 ;; required, but is available from http://notmuchmail.org).
 
 (eval-when-compile (require 'cl))
+(require 'crm)
 (require 'mm-view)
 (require 'message)
 
@@ -75,12 +76,33 @@ For example:
 (defvar notmuch-query-history nil
   "Variable to store minibuffer history for notmuch queries")
 
-(defun notmuch-select-tag-with-completion (prompt &rest search-terms)
+(defun notmuch-tag-completions (&optional search-terms prefixes)
   (let ((tag-list
 	 (with-output-to-string
 	   (with-current-buffer standard-output
 	     (apply 'call-process notmuch-command nil t nil "search-tags" search-terms)))))
-    (completing-read prompt (split-string tag-list "\n+" t) nil nil nil)))
+    (setq tag-list (split-string tag-list "\n+" t))
+    (if (null prefixes)
+	tag-list
+      (apply #'append
+	     (mapcar (lambda (tag)
+		       (mapcar (lambda (prefix)
+				 (concat prefix tag)) prefixes))
+		     tag-list)))))
+
+(defun notmuch-select-tag-with-completion (prompt &optional search-terms prefixes)
+  (let ((tag-list (notmuch-tag-completions search-terms prefixes)))
+    (completing-read prompt tag-list)))
+
+(defun notmuch-select-tags-with-completion (prompt &optional search-terms prefixes)
+  (let ((tag-list (notmuch-tag-completions search-terms prefixes))
+	(crm-separator " ")
+	(crm-local-completion-map (copy-keymap crm-local-completion-map)))
+    ;; By default, space is bound to "complete word" function.
+    ;; Re-bind it to insert a space instead.  Note that <tab> still
+    ;; does the completion.
+    (define-key crm-local-completion-map " " 'self-insert-command)
+    (completing-read-multiple prompt tag-list)))
 
 (defun notmuch-foreach-mime-part (function mm-handle)
   (cond ((stringp (car mm-handle))
@@ -860,16 +882,18 @@ will prompt for tags to be added or removed. Tags prefixed with
 Each character of the tag name may consist of alphanumeric
 characters as well as `_.+-'.
 "
-  (interactive "sOperation (+add -drop): notmuch tag ")
-  (let ((action-split (split-string action " +")))
-    ;; Perform some validation
-    (let ((words action-split))
-      (when (null words) (error "No operation given"))
-      (while words
-	(unless (string-match-p "^[-+][-+_.[:word:]]+$" (car words))
-	  (error "Action must be of the form `+thistag -that_tag'"))
-	(setq words (cdr words))))
-    (apply 'notmuch-tag notmuch-search-query-string action-split)))
+  (interactive (list (notmuch-select-tags-with-completion
+		      "Operation (+add -drop): notmuch tag " nil
+		      '("+" "-"))))
+  (setq action (delete "" action))
+  ;; Perform some validation
+  (let ((words action))
+    (when (null words) (error "No operation given"))
+    (while words
+      (unless (string-match-p "^[-+][-+_.[:word:]]+$" (car words))
+	(error "Action must be of the form `+this_tag -that_tag'"))
+      (setq words (cdr words))))
+  (apply 'notmuch-tag notmuch-search-query-string action))
 
 (defun notmuch-search-buffer-title (query)
   "Returns the title for a buffer with notmuch search results."
-- 
1.7.8.3

Re: [PATCH] emacs: add completion to "tag all" operation ("*" binding)

[Austin Clements]

Neat.  As an aside, I would love to see a prompt like this for + and
-, allowing multiple tags to be modified at once (and starting out
with whichever of + or - got you in to the prompt).

Quoth Dmitry Kurochkin on Jan 26 at  5:12 am:
> The patch adds <tab> completion to "tag all" operation bound to "*"
> (`notmuch-search-operate-all' function).
> ---
>  emacs/notmuch.el |   48 ++++++++++++++++++++++++++++++++++++------------
>  1 files changed, 36 insertions(+), 12 deletions(-)
> 
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index e02966f..71b0221 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -48,6 +48,7 @@
>  ;; required, but is available from http://notmuchmail.org).
>  
>  (eval-when-compile (require 'cl))
> +(require 'crm)
>  (require 'mm-view)
>  (require 'message)
>  
> @@ -75,12 +76,33 @@ For example:
>  (defvar notmuch-query-history nil
>    "Variable to store minibuffer history for notmuch queries")
>  
> -(defun notmuch-select-tag-with-completion (prompt &rest search-terms)
> +(defun notmuch-tag-completions (&optional search-terms prefixes)
>    (let ((tag-list
>  	 (with-output-to-string
>  	   (with-current-buffer standard-output
>  	     (apply 'call-process notmuch-command nil t nil "search-tags" search-terms)))))
> -    (completing-read prompt (split-string tag-list "\n+" t) nil nil nil)))
> +    (setq tag-list (split-string tag-list "\n+" t))
> +    (if (null prefixes)
> +	tag-list
> +      (apply #'append
> +	     (mapcar (lambda (tag)
> +		       (mapcar (lambda (prefix)
> +				 (concat prefix tag)) prefixes))
> +		     tag-list)))))
> +
> +(defun notmuch-select-tag-with-completion (prompt &optional search-terms prefixes)

This changes the API of notmuch-select-tag-with-completion in a
non-backwards-compatible way.  I'm pretty sure this breaks
notmuch-search-remove-tag and notmuch-show-remove-tag, but I haven't
tested it.

> +  (let ((tag-list (notmuch-tag-completions search-terms prefixes)))
> +    (completing-read prompt tag-list)))
> +
> +(defun notmuch-select-tags-with-completion (prompt &optional search-terms prefixes)
> +  (let ((tag-list (notmuch-tag-completions search-terms prefixes))
> +	(crm-separator " ")
> +	(crm-local-completion-map (copy-keymap crm-local-completion-map)))

Alternatively, you could create a child keymap.
crm-local-completion-map is small enough that it probably doesn't
matter, so whatever makes the code clearer.

> +    ;; By default, space is bound to "complete word" function.
> +    ;; Re-bind it to insert a space instead.  Note that <tab> still
> +    ;; does the completion.
> +    (define-key crm-local-completion-map " " 'self-insert-command)
> +    (completing-read-multiple prompt tag-list)))
>  
>  (defun notmuch-foreach-mime-part (function mm-handle)
>    (cond ((stringp (car mm-handle))
> @@ -860,16 +882,18 @@ will prompt for tags to be added or removed. Tags prefixed with
>  Each character of the tag name may consist of alphanumeric
>  characters as well as `_.+-'.
>  "

Technically this changes the API of notmuch-search-operate-all, though
the new one is better.  Perhaps it should test for (stringp action)
and be backwards-compatible?

The argument should probably be called "actions" now and it may even
make sense to make it a &rest argument (though then you can't make it
backwards-compatible).

> -  (interactive "sOperation (+add -drop): notmuch tag ")
> -  (let ((action-split (split-string action " +")))
> -    ;; Perform some validation
> -    (let ((words action-split))
> -      (when (null words) (error "No operation given"))
> -      (while words
> -	(unless (string-match-p "^[-+][-+_.[:word:]]+$" (car words))
> -	  (error "Action must be of the form `+thistag -that_tag'"))
> -	(setq words (cdr words))))
> -    (apply 'notmuch-tag notmuch-search-query-string action-split)))
> +  (interactive (list (notmuch-select-tags-with-completion
> +		      "Operation (+add -drop): notmuch tag " nil
> +		      '("+" "-"))))
> +  (setq action (delete "" action))
> +  ;; Perform some validation
> +  (let ((words action))
> +    (when (null words) (error "No operation given"))
> +    (while words
> +      (unless (string-match-p "^[-+][-+_.[:word:]]+$" (car words))
> +	(error "Action must be of the form `+thistag -that_tag'"))
> +      (setq words (cdr words))))

This should really be a mapc or a dolist, but maybe that's for another
patch.

> +  (apply 'notmuch-tag notmuch-search-query-string action))
>  
>  (defun notmuch-search-buffer-title (query)
>    "Returns the title for a buffer with notmuch search results."

Re: [PATCH] emacs: add completion to "tag all" operation ("*" binding)

[Dmitry Kurochkin]

On Wed, 25 Jan 2012 20:37:27 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> Neat.  As an aside, I would love to see a prompt like this for + and
> -, allowing multiple tags to be modified at once (and starting out
> with whichever of + or - got you in to the prompt).
> 

Yeah, I was thinking about making "+" and "-" accepting multiple tags
using `notmuch-select-tags-with-completion'.  But that is for another
patch.

> Quoth Dmitry Kurochkin on Jan 26 at  5:12 am:
> > The patch adds <tab> completion to "tag all" operation bound to "*"
> > (`notmuch-search-operate-all' function).
> > ---
> >  emacs/notmuch.el |   48 ++++++++++++++++++++++++++++++++++++------------
> >  1 files changed, 36 insertions(+), 12 deletions(-)
> > 
> > diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> > index e02966f..71b0221 100644
> > --- a/emacs/notmuch.el
> > +++ b/emacs/notmuch.el
> > @@ -48,6 +48,7 @@
> >  ;; required, but is available from http://notmuchmail.org).
> >  
> >  (eval-when-compile (require 'cl))
> > +(require 'crm)
> >  (require 'mm-view)
> >  (require 'message)
> >  
> > @@ -75,12 +76,33 @@ For example:
> >  (defvar notmuch-query-history nil
> >    "Variable to store minibuffer history for notmuch queries")
> >  
> > -(defun notmuch-select-tag-with-completion (prompt &rest search-terms)
> > +(defun notmuch-tag-completions (&optional search-terms prefixes)
> >    (let ((tag-list
> >  	 (with-output-to-string
> >  	   (with-current-buffer standard-output
> >  	     (apply 'call-process notmuch-command nil t nil "search-tags" search-terms)))))
> > -    (completing-read prompt (split-string tag-list "\n+" t) nil nil nil)))
> > +    (setq tag-list (split-string tag-list "\n+" t))
> > +    (if (null prefixes)
> > +	tag-list
> > +      (apply #'append
> > +	     (mapcar (lambda (tag)
> > +		       (mapcar (lambda (prefix)
> > +				 (concat prefix tag)) prefixes))
> > +		     tag-list)))))
> > +
> > +(defun notmuch-select-tag-with-completion (prompt &optional search-terms prefixes)
> 
> This changes the API of notmuch-select-tag-with-completion in a
> non-backwards-compatible way.  I'm pretty sure this breaks
> notmuch-search-remove-tag and notmuch-show-remove-tag, but I haven't
> tested it.
> 

I tested only tag additions.  It works, and I assumed tag removal is no
different.  Will fix.

> > +  (let ((tag-list (notmuch-tag-completions search-terms prefixes)))
> > +    (completing-read prompt tag-list)))
> > +
> > +(defun notmuch-select-tags-with-completion (prompt &optional search-terms prefixes)
> > +  (let ((tag-list (notmuch-tag-completions search-terms prefixes))
> > +	(crm-separator " ")
> > +	(crm-local-completion-map (copy-keymap crm-local-completion-map)))
> 
> Alternatively, you could create a child keymap.
> crm-local-completion-map is small enough that it probably doesn't
> matter, so whatever makes the code clearer.
> 

Thanks, I will look into it.

> > +    ;; By default, space is bound to "complete word" function.
> > +    ;; Re-bind it to insert a space instead.  Note that <tab> still
> > +    ;; does the completion.
> > +    (define-key crm-local-completion-map " " 'self-insert-command)
> > +    (completing-read-multiple prompt tag-list)))
> >  
> >  (defun notmuch-foreach-mime-part (function mm-handle)
> >    (cond ((stringp (car mm-handle))
> > @@ -860,16 +882,18 @@ will prompt for tags to be added or removed. Tags prefixed with
> >  Each character of the tag name may consist of alphanumeric
> >  characters as well as `_.+-'.
> >  "
> 
> Technically this changes the API of notmuch-search-operate-all, though
> the new one is better.  Perhaps it should test for (stringp action)
> and be backwards-compatible?
> 

In this case, I do not think there are many users of the function.  So I
would prefer to keep the code clean.

> The argument should probably be called "actions" now and it may even
> make sense to make it a &rest argument (though then you can't make it
> backwards-compatible).
> 

Makes sense, will do.

> > -  (interactive "sOperation (+add -drop): notmuch tag ")
> > -  (let ((action-split (split-string action " +")))
> > -    ;; Perform some validation
> > -    (let ((words action-split))
> > -      (when (null words) (error "No operation given"))
> > -      (while words
> > -	(unless (string-match-p "^[-+][-+_.[:word:]]+$" (car words))
> > -	  (error "Action must be of the form `+thistag -that_tag'"))
> > -	(setq words (cdr words))))
> > -    (apply 'notmuch-tag notmuch-search-query-string action-split)))
> > +  (interactive (list (notmuch-select-tags-with-completion
> > +		      "Operation (+add -drop): notmuch tag " nil
> > +		      '("+" "-"))))
> > +  (setq action (delete "" action))
> > +  ;; Perform some validation
> > +  (let ((words action))
> > +    (when (null words) (error "No operation given"))
> > +    (while words
> > +      (unless (string-match-p "^[-+][-+_.[:word:]]+$" (car words))
> > +	(error "Action must be of the form `+thistag -that_tag'"))
> > +      (setq words (cdr words))))
> 
> This should really be a mapc or a dolist, but maybe that's for another
> patch.
> 

Yes, I changed "this_tag" spelling in v2, but would prefer to leave
non-trivial changes in this code for another patch.

Regards,
  Dmitry

> > +  (apply 'notmuch-tag notmuch-search-query-string action))
> >  
> >  (defun notmuch-search-buffer-title (query)
> >    "Returns the title for a buffer with notmuch search results."

Re: [PATCH] emacs: add completion to "tag all" operation ("*" binding)

[David Edmondson]

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

On Thu, 26 Jan 2012 05:47:07 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > > +  ;; Perform some validation
> > > +  (let ((words action))
> > > +    (when (null words) (error "No operation given"))
> > > +    (while words
> > > +      (unless (string-match-p "^[-+][-+_.[:word:]]+$" (car words))
> > > +	(error "Action must be of the form `+thistag -that_tag'"))
> > > +      (setq words (cdr words))))
> > 
> > This should really be a mapc or a dolist, but maybe that's for another
> > patch.
> 
> Yes, I changed "this_tag" spelling in v2, but would prefer to leave
> non-trivial changes in this code for another patch.

If you were going to submit another patch then fine, but the chances are
it will just get forgotten.

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

Re: [PATCH] emacs: add completion to "tag all" operation ("*" binding)

[Dmitry Kurochkin]

On Thu, 26 Jan 2012 08:46:00 +0000, David Edmondson <dme@dme.org> wrote:
> On Thu, 26 Jan 2012 05:47:07 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > > > +  ;; Perform some validation
> > > > +  (let ((words action))
> > > > +    (when (null words) (error "No operation given"))
> > > > +    (while words
> > > > +      (unless (string-match-p "^[-+][-+_.[:word:]]+$" (car words))
> > > > +	(error "Action must be of the form `+thistag -that_tag'"))
> > > > +      (setq words (cdr words))))
> > > 
> > > This should really be a mapc or a dolist, but maybe that's for another
> > > patch.
> > 
> > Yes, I changed "this_tag" spelling in v2, but would prefer to leave
> > non-trivial changes in this code for another patch.
> 
> If you were going to submit another patch then fine, but the chances are
> it will just get forgotten.

No worries, v5 has a cleanup patch added :)

Regards,
  Dmitry

[PATCH v3] emacs: add completion to "tag all" operation ("*" binding)

[Dmitry Kurochkin]

The patch adds <tab> completion to "tag all" operation bound to "*"
(`notmuch-search-operate-all' function).
---

Changes:

v3:

* fixed comments from Austin's review [1]

v2:

* s/thistag/this_tag/ for consistency with "that_tag", since we touch
  the line anyway

Regards,
  Dmitry

[1] id:"20120126013727.GB1176@mit.edu"

 emacs/notmuch-show.el |    4 +-
 emacs/notmuch.el      |   55 ++++++++++++++++++++++++++++++++++++------------
 2 files changed, 43 insertions(+), 16 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index e6a5b31..b23a981 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -38,7 +38,7 @@
 
 (declare-function notmuch-call-notmuch-process "notmuch" (&rest args))
 (declare-function notmuch-fontify-headers "notmuch" nil)
-(declare-function notmuch-select-tag-with-completion "notmuch" (prompt &rest search-terms))
+(declare-function notmuch-select-tag-with-completion "notmuch" (prompt &optional prefixes &rest search-terms))
 (declare-function notmuch-search-show-thread "notmuch" nil)
 
 (defcustom notmuch-message-headers '("Subject" "To" "Cc" "Date")
@@ -1474,7 +1474,7 @@ the result."
   "Remove a tag from the current message."
   (interactive
    (list (notmuch-select-tag-with-completion
-	  "Tag to remove: " (notmuch-show-get-message-id))))
+	  "Tag to remove: " nil (notmuch-show-get-message-id))))
 
   (let* ((current-tags (notmuch-show-get-tags))
 	 (new-tags (notmuch-show-del-tags-worker current-tags toremove)))
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index e02966f..ba8f8e1 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -48,6 +48,7 @@
 ;; required, but is available from http://notmuchmail.org).
 
 (eval-when-compile (require 'cl))
+(require 'crm)
 (require 'mm-view)
 (require 'message)
 
@@ -75,12 +76,36 @@ For example:
 (defvar notmuch-query-history nil
   "Variable to store minibuffer history for notmuch queries")
 
-(defun notmuch-select-tag-with-completion (prompt &rest search-terms)
+(defun notmuch-tag-completions (&optional prefixes search-terms)
   (let ((tag-list
 	 (with-output-to-string
 	   (with-current-buffer standard-output
 	     (apply 'call-process notmuch-command nil t nil "search-tags" search-terms)))))
-    (completing-read prompt (split-string tag-list "\n+" t) nil nil nil)))
+    (setq tag-list (split-string tag-list "\n+" t))
+    (if (null prefixes)
+	tag-list
+      (apply #'append
+	     (mapcar (lambda (tag)
+		       (mapcar (lambda (prefix)
+				 (concat prefix tag)) prefixes))
+		     tag-list)))))
+
+(defun notmuch-select-tag-with-completion (prompt &optional prefixes &rest search-terms)
+  (let ((tag-list (notmuch-tag-completions prefixes search-terms)))
+    (completing-read prompt tag-list)))
+
+(defun notmuch-select-tags-with-completion (prompt &optional prefixes &rest search-terms)
+  (let ((tag-list (notmuch-tag-completions prefixes search-terms))
+	(crm-separator " ")
+	(crm-local-completion-map
+	 (let ((map (make-sparse-keymap)))
+	   (set-keymap-parent map crm-local-completion-map)
+	   map)))
+    ;; By default, space is bound to "complete word" function.
+    ;; Re-bind it to insert a space instead.  Note that <tab> still
+    ;; does the completion.
+    (define-key crm-local-completion-map " " 'self-insert-command)
+    (completing-read-multiple prompt tag-list)))
 
 (defun notmuch-foreach-mime-part (function mm-handle)
   (cond ((stringp (car mm-handle))
@@ -606,7 +631,7 @@ The tag is removed from all messages in the currently selected
 thread or threads in the current region."
   (interactive
    (list (notmuch-select-tag-with-completion
-	  "Tag to remove: "
+	  "Tag to remove: " nil
 	  (if (region-active-p)
 	      (mapconcat 'identity
 			 (notmuch-search-find-thread-id-region (region-beginning) (region-end))
@@ -849,7 +874,7 @@ non-authors is found, assume that all of the authors match."
 	      (goto-char found-target)))
       (delete-process proc))))
 
-(defun notmuch-search-operate-all (action)
+(defun notmuch-search-operate-all (&rest actions)
   "Add/remove tags from all matching messages.
 
 This command adds or removes tags from all messages matching the
@@ -860,16 +885,18 @@ will prompt for tags to be added or removed. Tags prefixed with
 Each character of the tag name may consist of alphanumeric
 characters as well as `_.+-'.
 "
-  (interactive "sOperation (+add -drop): notmuch tag ")
-  (let ((action-split (split-string action " +")))
-    ;; Perform some validation
-    (let ((words action-split))
-      (when (null words) (error "No operation given"))
-      (while words
-	(unless (string-match-p "^[-+][-+_.[:word:]]+$" (car words))
-	  (error "Action must be of the form `+thistag -that_tag'"))
-	(setq words (cdr words))))
-    (apply 'notmuch-tag notmuch-search-query-string action-split)))
+  (interactive (notmuch-select-tags-with-completion
+		"Operations (+add -drop): notmuch tag "
+		'("+" "-")))
+  (setq actions (delete "" actions))
+  ;; Perform some validation
+  (let ((words actions))
+    (when (null words) (error "No operations given"))
+    (while words
+      (unless (string-match-p "^[-+][-+_.[:word:]]+$" (car words))
+	(error "Action must be of the form `+this_tag' or `-that_tag'"))
+      (setq words (cdr words))))
+  (apply 'notmuch-tag notmuch-search-query-string actions))
 
 (defun notmuch-search-buffer-title (query)
   "Returns the title for a buffer with notmuch search results."
-- 
1.7.8.3

[PATCH v4] emacs: add completion to "tag all" operation ("*" binding)

[Dmitry Kurochkin]

The patch adds <tab> completion to "tag all" operation bound to "*"
(`notmuch-search-operate-all' function).
---

On a second thought, `notmuch-select-tag-with-completion' should never
need `prefixes' argument at all.  So I reverted the API and related
changes.

Changes:

v4:

* do not change `notmuch-select-tag-with-completion' API, revert
  related changes

v3:

* fixed comments from Austin's review [1]

v2:

* s/thistag/this_tag/ for consistency with "that_tag", since we touch
  the line anyway

Regards,
  Dmitry

[1] id:"20120126013727.GB1176@mit.edu"

 emacs/notmuch.el |   53 ++++++++++++++++++++++++++++++++++++++++-------------
 1 files changed, 40 insertions(+), 13 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index e02966f..d2af630 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -48,6 +48,7 @@
 ;; required, but is available from http://notmuchmail.org).
 
 (eval-when-compile (require 'cl))
+(require 'crm)
 (require 'mm-view)
 (require 'message)
 
@@ -75,12 +76,36 @@ For example:
 (defvar notmuch-query-history nil
   "Variable to store minibuffer history for notmuch queries")
 
-(defun notmuch-select-tag-with-completion (prompt &rest search-terms)
+(defun notmuch-tag-completions (&optional prefixes search-terms)
   (let ((tag-list
 	 (with-output-to-string
 	   (with-current-buffer standard-output
 	     (apply 'call-process notmuch-command nil t nil "search-tags" search-terms)))))
-    (completing-read prompt (split-string tag-list "\n+" t) nil nil nil)))
+    (setq tag-list (split-string tag-list "\n+" t))
+    (if (null prefixes)
+	tag-list
+      (apply #'append
+	     (mapcar (lambda (tag)
+		       (mapcar (lambda (prefix)
+				 (concat prefix tag)) prefixes))
+		     tag-list)))))
+
+(defun notmuch-select-tag-with-completion (prompt &rest search-terms)
+  (let ((tag-list (notmuch-tag-completions nil search-terms)))
+    (completing-read prompt tag-list)))
+
+(defun notmuch-select-tags-with-completion (prompt &optional prefixes &rest search-terms)
+  (let ((tag-list (notmuch-tag-completions prefixes search-terms))
+	(crm-separator " ")
+	(crm-local-completion-map
+	 (let ((map (make-sparse-keymap)))
+	   (set-keymap-parent map crm-local-completion-map)
+	   map)))
+    ;; By default, space is bound to "complete word" function.
+    ;; Re-bind it to insert a space instead.  Note that <tab> still
+    ;; does the completion.
+    (define-key crm-local-completion-map " " 'self-insert-command)
+    (completing-read-multiple prompt tag-list)))
 
 (defun notmuch-foreach-mime-part (function mm-handle)
   (cond ((stringp (car mm-handle))
@@ -849,7 +874,7 @@ non-authors is found, assume that all of the authors match."
 	      (goto-char found-target)))
       (delete-process proc))))
 
-(defun notmuch-search-operate-all (action)
+(defun notmuch-search-operate-all (&rest actions)
   "Add/remove tags from all matching messages.
 
 This command adds or removes tags from all messages matching the
@@ -860,16 +885,18 @@ will prompt for tags to be added or removed. Tags prefixed with
 Each character of the tag name may consist of alphanumeric
 characters as well as `_.+-'.
 "
-  (interactive "sOperation (+add -drop): notmuch tag ")
-  (let ((action-split (split-string action " +")))
-    ;; Perform some validation
-    (let ((words action-split))
-      (when (null words) (error "No operation given"))
-      (while words
-	(unless (string-match-p "^[-+][-+_.[:word:]]+$" (car words))
-	  (error "Action must be of the form `+thistag -that_tag'"))
-	(setq words (cdr words))))
-    (apply 'notmuch-tag notmuch-search-query-string action-split)))
+  (interactive (notmuch-select-tags-with-completion
+		"Operations (+add -drop): notmuch tag "
+		'("+" "-")))
+  (setq actions (delete "" actions))
+  ;; Perform some validation
+  (let ((words actions))
+    (when (null words) (error "No operations given"))
+    (while words
+      (unless (string-match-p "^[-+][-+_.[:word:]]+$" (car words))
+	(error "Action must be of the form `+this_tag' or `-that_tag'"))
+      (setq words (cdr words))))
+  (apply 'notmuch-tag notmuch-search-query-string actions))
 
 (defun notmuch-search-buffer-title (query)
   "Returns the title for a buffer with notmuch search results."
-- 
1.7.8.3

Re: [PATCH v4] emacs: add completion to "tag all" operation ("*" binding)

[Austin Clements]

Quoth Dmitry Kurochkin on Jan 26 at  9:06 am:
> The patch adds <tab> completion to "tag all" operation bound to "*"
> (`notmuch-search-operate-all' function).
> ---
> 
> On a second thought, `notmuch-select-tag-with-completion' should never
> need `prefixes' argument at all.  So I reverted the API and related
> changes.
> 
> Changes:
> 
> v4:
> 
> * do not change `notmuch-select-tag-with-completion' API, revert
>   related changes
> 
> v3:
> 
> * fixed comments from Austin's review [1]
> 
> v2:
> 
> * s/thistag/this_tag/ for consistency with "that_tag", since we touch
>   the line anyway
> 
> Regards,
>   Dmitry
> 
> [1] id:"20120126013727.GB1176@mit.edu"
> 
>  emacs/notmuch.el |   53 ++++++++++++++++++++++++++++++++++++++++-------------
>  1 files changed, 40 insertions(+), 13 deletions(-)
> 
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index e02966f..d2af630 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -48,6 +48,7 @@
>  ;; required, but is available from http://notmuchmail.org).
>  
>  (eval-when-compile (require 'cl))
> +(require 'crm)
>  (require 'mm-view)
>  (require 'message)
>  
> @@ -75,12 +76,36 @@ For example:
>  (defvar notmuch-query-history nil
>    "Variable to store minibuffer history for notmuch queries")
>  
> -(defun notmuch-select-tag-with-completion (prompt &rest search-terms)
> +(defun notmuch-tag-completions (&optional prefixes search-terms)
>    (let ((tag-list
>  	 (with-output-to-string
>  	   (with-current-buffer standard-output
>  	     (apply 'call-process notmuch-command nil t nil "search-tags" search-terms)))))
> -    (completing-read prompt (split-string tag-list "\n+" t) nil nil nil)))
> +    (setq tag-list (split-string tag-list "\n+" t))

Since this setq is unconditional, you can do the split-string right in
the let binding, around the with-output-to-string.

> +    (if (null prefixes)
> +	tag-list
> +      (apply #'append
> +	     (mapcar (lambda (tag)
> +		       (mapcar (lambda (prefix)
> +				 (concat prefix tag)) prefixes))
> +		     tag-list)))))
> +
> +(defun notmuch-select-tag-with-completion (prompt &rest search-terms)
> +  (let ((tag-list (notmuch-tag-completions nil search-terms)))
> +    (completing-read prompt tag-list)))
> +
> +(defun notmuch-select-tags-with-completion (prompt &optional prefixes &rest search-terms)
> +  (let ((tag-list (notmuch-tag-completions prefixes search-terms))
> +	(crm-separator " ")
> +	(crm-local-completion-map
> +	 (let ((map (make-sparse-keymap)))
> +	   (set-keymap-parent map crm-local-completion-map)
> +	   map)))
> +    ;; By default, space is bound to "complete word" function.
> +    ;; Re-bind it to insert a space instead.  Note that <tab> still
> +    ;; does the completion.
> +    (define-key crm-local-completion-map " " 'self-insert-command)

You could do the define-key inside the (let ((map ..)) ..) so you get
back the fully formed keymap.  Your call.

> +    (completing-read-multiple prompt tag-list)))
>  
>  (defun notmuch-foreach-mime-part (function mm-handle)
>    (cond ((stringp (car mm-handle))
> @@ -849,7 +874,7 @@ non-authors is found, assume that all of the authors match."
>  	      (goto-char found-target)))
>        (delete-process proc))))
>  
> -(defun notmuch-search-operate-all (action)
> +(defun notmuch-search-operate-all (&rest actions)
>    "Add/remove tags from all matching messages.
>  
>  This command adds or removes tags from all messages matching the
> @@ -860,16 +885,18 @@ will prompt for tags to be added or removed. Tags prefixed with
>  Each character of the tag name may consist of alphanumeric
>  characters as well as `_.+-'.
>  "
> -  (interactive "sOperation (+add -drop): notmuch tag ")
> -  (let ((action-split (split-string action " +")))
> -    ;; Perform some validation
> -    (let ((words action-split))
> -      (when (null words) (error "No operation given"))
> -      (while words
> -	(unless (string-match-p "^[-+][-+_.[:word:]]+$" (car words))
> -	  (error "Action must be of the form `+thistag -that_tag'"))
> -	(setq words (cdr words))))
> -    (apply 'notmuch-tag notmuch-search-query-string action-split)))
> +  (interactive (notmuch-select-tags-with-completion
> +		"Operations (+add -drop): notmuch tag "
> +		'("+" "-")))
> +  (setq actions (delete "" actions))

Either this line isn't necessary or
notmuch-select-tags-with-completion can do something funny that it
should take care of internally.

> +  ;; Perform some validation
> +  (let ((words actions))
> +    (when (null words) (error "No operations given"))
> +    (while words
> +      (unless (string-match-p "^[-+][-+_.[:word:]]+$" (car words))
> +	(error "Action must be of the form `+this_tag' or `-that_tag'"))
> +      (setq words (cdr words))))
> +  (apply 'notmuch-tag notmuch-search-query-string actions))
>  
>  (defun notmuch-search-buffer-title (query)
>    "Returns the title for a buffer with notmuch search results."

Re: [PATCH v4] emacs: add completion to "tag all" operation ("*" binding)

[Dmitry Kurochkin]

On Thu, 26 Jan 2012 02:04:39 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> Quoth Dmitry Kurochkin on Jan 26 at  9:06 am:
> > The patch adds <tab> completion to "tag all" operation bound to "*"
> > (`notmuch-search-operate-all' function).
> > ---
> > 
> > On a second thought, `notmuch-select-tag-with-completion' should never
> > need `prefixes' argument at all.  So I reverted the API and related
> > changes.
> > 
> > Changes:
> > 
> > v4:
> > 
> > * do not change `notmuch-select-tag-with-completion' API, revert
> >   related changes
> > 
> > v3:
> > 
> > * fixed comments from Austin's review [1]
> > 
> > v2:
> > 
> > * s/thistag/this_tag/ for consistency with "that_tag", since we touch
> >   the line anyway
> > 
> > Regards,
> >   Dmitry
> > 
> > [1] id:"20120126013727.GB1176@mit.edu"
> > 
> >  emacs/notmuch.el |   53 ++++++++++++++++++++++++++++++++++++++++-------------
> >  1 files changed, 40 insertions(+), 13 deletions(-)
> > 
> > diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> > index e02966f..d2af630 100644
> > --- a/emacs/notmuch.el
> > +++ b/emacs/notmuch.el
> > @@ -48,6 +48,7 @@
> >  ;; required, but is available from http://notmuchmail.org).
> >  
> >  (eval-when-compile (require 'cl))
> > +(require 'crm)
> >  (require 'mm-view)
> >  (require 'message)
> >  
> > @@ -75,12 +76,36 @@ For example:
> >  (defvar notmuch-query-history nil
> >    "Variable to store minibuffer history for notmuch queries")
> >  
> > -(defun notmuch-select-tag-with-completion (prompt &rest search-terms)
> > +(defun notmuch-tag-completions (&optional prefixes search-terms)
> >    (let ((tag-list
> >  	 (with-output-to-string
> >  	   (with-current-buffer standard-output
> >  	     (apply 'call-process notmuch-command nil t nil "search-tags" search-terms)))))
> > -    (completing-read prompt (split-string tag-list "\n+" t) nil nil nil)))
> > +    (setq tag-list (split-string tag-list "\n+" t))
> 
> Since this setq is unconditional, you can do the split-string right in
> the let binding, around the with-output-to-string.
> 

I was thinking about it, but decided to use `setq', to make the diff
smaller and the let binding simpler.  Changed in v5.

> > +    (if (null prefixes)
> > +	tag-list
> > +      (apply #'append
> > +	     (mapcar (lambda (tag)
> > +		       (mapcar (lambda (prefix)
> > +				 (concat prefix tag)) prefixes))
> > +		     tag-list)))))
> > +
> > +(defun notmuch-select-tag-with-completion (prompt &rest search-terms)
> > +  (let ((tag-list (notmuch-tag-completions nil search-terms)))
> > +    (completing-read prompt tag-list)))
> > +
> > +(defun notmuch-select-tags-with-completion (prompt &optional prefixes &rest search-terms)
> > +  (let ((tag-list (notmuch-tag-completions prefixes search-terms))
> > +	(crm-separator " ")
> > +	(crm-local-completion-map
> > +	 (let ((map (make-sparse-keymap)))
> > +	   (set-keymap-parent map crm-local-completion-map)
> > +	   map)))
> > +    ;; By default, space is bound to "complete word" function.
> > +    ;; Re-bind it to insert a space instead.  Note that <tab> still
> > +    ;; does the completion.
> > +    (define-key crm-local-completion-map " " 'self-insert-command)
> 
> You could do the define-key inside the (let ((map ..)) ..) so you get
> back the fully formed keymap.  Your call.
> 

Good point.  Changed in v5.

> > +    (completing-read-multiple prompt tag-list)))
> >  
> >  (defun notmuch-foreach-mime-part (function mm-handle)
> >    (cond ((stringp (car mm-handle))
> > @@ -849,7 +874,7 @@ non-authors is found, assume that all of the authors match."
> >  	      (goto-char found-target)))
> >        (delete-process proc))))
> >  
> > -(defun notmuch-search-operate-all (action)
> > +(defun notmuch-search-operate-all (&rest actions)
> >    "Add/remove tags from all matching messages.
> >  
> >  This command adds or removes tags from all messages matching the
> > @@ -860,16 +885,18 @@ will prompt for tags to be added or removed. Tags prefixed with
> >  Each character of the tag name may consist of alphanumeric
> >  characters as well as `_.+-'.
> >  "
> > -  (interactive "sOperation (+add -drop): notmuch tag ")
> > -  (let ((action-split (split-string action " +")))
> > -    ;; Perform some validation
> > -    (let ((words action-split))
> > -      (when (null words) (error "No operation given"))
> > -      (while words
> > -	(unless (string-match-p "^[-+][-+_.[:word:]]+$" (car words))
> > -	  (error "Action must be of the form `+thistag -that_tag'"))
> > -	(setq words (cdr words))))
> > -    (apply 'notmuch-tag notmuch-search-query-string action-split)))
> > +  (interactive (notmuch-select-tags-with-completion
> > +		"Operations (+add -drop): notmuch tag "
> > +		'("+" "-")))
> > +  (setq actions (delete "" actions))
> 
> Either this line isn't necessary or
> notmuch-select-tags-with-completion can do something funny that it
> should take care of internally.
> 

It is necessary and it belongs to `notmuch-select-tags-with-completion',
thanks.  Changed in v5.

Regards,
  Dmitry

> > +  ;; Perform some validation
> > +  (let ((words actions))
> > +    (when (null words) (error "No operations given"))
> > +    (while words
> > +      (unless (string-match-p "^[-+][-+_.[:word:]]+$" (car words))
> > +	(error "Action must be of the form `+this_tag' or `-that_tag'"))
> > +      (setq words (cdr words))))
> > +  (apply 'notmuch-tag notmuch-search-query-string actions))
> >  
> >  (defun notmuch-search-buffer-title (query)
> >    "Returns the title for a buffer with notmuch search results."

[PATCH v5 0/2] emacs: add completion to "tag all" operation

[Dmitry Kurochkin]

Changes:

v5:

* fixed comments from Austin's review [2]

* add a second patch with `notmuch-search-operate-all' code cleanup
  suggested by Austin [1]

v4:

* do not change `notmuch-select-tag-with-completion' API, revert
  related changes

v3:

* fixed comments from Austin's review [1]

v2:

* s/thistag/this_tag/ for consistency with "that_tag", since we touch
  the line anyway

Regards,
  Dmitry

[1] id:"20120126013727.GB1176@mit.edu"
[2] id:"20120126070439.GC1940@mit.edu"

[PATCH v5 1/2] emacs: add completion to "tag all" operation ("*" binding)

[Dmitry Kurochkin]

The patch adds <tab> completion to "tag all" operation bound to "*"
(`notmuch-search-operate-all' function).
---
 emacs/notmuch.el |   60 +++++++++++++++++++++++++++++++++++++++--------------
 1 files changed, 44 insertions(+), 16 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index e02966f..291eca2 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -48,6 +48,7 @@
 ;; required, but is available from http://notmuchmail.org).
 
 (eval-when-compile (require 'cl))
+(require 'crm)
 (require 'mm-view)
 (require 'message)
 
@@ -75,12 +76,38 @@ For example:
 (defvar notmuch-query-history nil
   "Variable to store minibuffer history for notmuch queries")
 
-(defun notmuch-select-tag-with-completion (prompt &rest search-terms)
+(defun notmuch-tag-completions (&optional prefixes search-terms)
   (let ((tag-list
-	 (with-output-to-string
-	   (with-current-buffer standard-output
-	     (apply 'call-process notmuch-command nil t nil "search-tags" search-terms)))))
-    (completing-read prompt (split-string tag-list "\n+" t) nil nil nil)))
+	 (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)))
+    (if (null prefixes)
+	tag-list
+      (apply #'append
+	     (mapcar (lambda (tag)
+		       (mapcar (lambda (prefix)
+				 (concat prefix tag)) prefixes))
+		     tag-list)))))
+
+(defun notmuch-select-tag-with-completion (prompt &rest search-terms)
+  (let ((tag-list (notmuch-tag-completions nil search-terms)))
+    (completing-read prompt tag-list)))
+
+(defun notmuch-select-tags-with-completion (prompt &optional prefixes &rest search-terms)
+  (let ((tag-list (notmuch-tag-completions prefixes search-terms))
+	(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 prompt tag-list))))
 
 (defun notmuch-foreach-mime-part (function mm-handle)
   (cond ((stringp (car mm-handle))
@@ -849,7 +876,7 @@ non-authors is found, assume that all of the authors match."
 	      (goto-char found-target)))
       (delete-process proc))))
 
-(defun notmuch-search-operate-all (action)
+(defun notmuch-search-operate-all (&rest actions)
   "Add/remove tags from all matching messages.
 
 This command adds or removes tags from all messages matching the
@@ -860,16 +887,17 @@ will prompt for tags to be added or removed. Tags prefixed with
 Each character of the tag name may consist of alphanumeric
 characters as well as `_.+-'.
 "
-  (interactive "sOperation (+add -drop): notmuch tag ")
-  (let ((action-split (split-string action " +")))
-    ;; Perform some validation
-    (let ((words action-split))
-      (when (null words) (error "No operation given"))
-      (while words
-	(unless (string-match-p "^[-+][-+_.[:word:]]+$" (car words))
-	  (error "Action must be of the form `+thistag -that_tag'"))
-	(setq words (cdr words))))
-    (apply 'notmuch-tag notmuch-search-query-string action-split)))
+  (interactive (notmuch-select-tags-with-completion
+		"Operations (+add -drop): notmuch tag "
+		'("+" "-")))
+  ;; Perform some validation
+  (let ((words actions))
+    (when (null words) (error "No operations given"))
+    (while words
+      (unless (string-match-p "^[-+][-+_.[:word:]]+$" (car words))
+	(error "Action must be of the form `+this_tag' or `-that_tag'"))
+      (setq words (cdr words))))
+  (apply 'notmuch-tag notmuch-search-query-string actions))
 
 (defun notmuch-search-buffer-title (query)
   "Returns the title for a buffer with notmuch search results."
-- 
1.7.8.3

Re: [PATCH v5 1/2] emacs: add completion to "tag all" operation ("*" binding)

[David Bremner]

On Thu, 26 Jan 2012 21:34:48 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> The patch adds <tab> completion to "tag all" operation bound to "*"
> (`notmuch-search-operate-all' function).

pushed,

d

[PATCH v5 2/2] emacs: `notmuch-search-operate-all' code cleanup, no functional changes

[Dmitry Kurochkin]

---
 emacs/notmuch.el |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 291eca2..72f78ed 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -891,12 +891,11 @@ characters as well as `_.+-'.
 		"Operations (+add -drop): notmuch tag "
 		'("+" "-")))
   ;; Perform some validation
-  (let ((words actions))
-    (when (null words) (error "No operations given"))
-    (while words
-      (unless (string-match-p "^[-+][-+_.[:word:]]+$" (car words))
-	(error "Action must be of the form `+this_tag' or `-that_tag'"))
-      (setq words (cdr words))))
+  (when (null actions) (error "No operations given"))
+  (mapc (lambda (action)
+	  (unless (string-match-p "^[-+][-+_.[:word:]]+$" action)
+	    (error "Action must be of the form `+this_tag' or `-that_tag'")))
+	actions)
   (apply 'notmuch-tag notmuch-search-query-string actions))
 
 (defun notmuch-search-buffer-title (query)
-- 
1.7.8.3

Re: [PATCH v5 2/2] emacs: `notmuch-search-operate-all' code cleanup, no functional changes

[David Bremner]

On Thu, 26 Jan 2012 21:34:49 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> ---
>  emacs/notmuch.el |   11 +++++------
>  1 files changed, 5 insertions(+), 6 deletions(-)

Pushed, 

d.

Re: [PATCH v5 0/2] emacs: add completion to "tag all" operation

[David Edmondson]

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

The series look good to me.

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

Re: [PATCH v5 0/2] emacs: add completion to "tag all" operation

[Austin Clements]

Quoth Dmitry Kurochkin on Jan 26 at  9:34 pm:
> Changes:
> 
> v5:
> 
> * fixed comments from Austin's review [2]
> 
> * add a second patch with `notmuch-search-operate-all' code cleanup
>   suggested by Austin [1]

LGTM.

[PATCH v4] test: emacs: add test for `notmuch-search-operate-all'

[Pieter Praet]

`notmuch-search-operate-all' (bound to "*") adds and removes tags
to/from all messages which match the query used to populate the
current search buffer.

---

Rebased to current master.

Previous versions (chronologically):
- id:"1309450108-2793-1-git-send-email-pieter@praet.org"
- id:"1309762318-4530-5-git-send-email-pieter@praet.org"
- id:"1310313335-4159-5-git-send-email-pieter@praet.org"


 test/emacs |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/test/emacs b/test/emacs
index 8ca4c8a..e94ad94 100755
--- a/test/emacs
+++ b/test/emacs
@@ -124,6 +124,25 @@ test_emacs "(notmuch-show \"$os_x_darwin_thread\")
 output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
 test_expect_equal "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox unread)"
 
+test_begin_subtest "Add/remove tags to/from all matching messages."
+test_emacs '(notmuch-search "tag:inbox AND tags")
+	    (notmuch-test-wait)
+	    (notmuch-search-operate-all "+matching" "-inbox")
+	    (notmuch-search "tag:matching AND NOT tag:inbox")
+	    (notmuch-test-wait)
+	    (test-output)'
+cat <<EOF >EXPECTED
+  2009-11-18 [3/3]   Adrian Perez de Castro, Keith Packard, Carl Worth  [notmuch] Introducing myself (matching signed unread)
+  2009-11-18 [1/3]   Carl Worth, Israel Herraiz, Keith Packard       [notmuch] New to the list (inbox matching unread)
+  2009-11-18 [2/2]   Keith Packard, Carl Worth    [notmuch] [PATCH] Make notmuch-show 'X' (and 'x') commands remove inbox (and unread) tags (matching unread)
+  2009-11-18 [1/2]   Keith Packard, Alexander Botero-Lowry    [notmuch] [PATCH] Create a default notmuch-show-hook that highlights URLs and uses word-wrap (inbox matching unread)
+  2009-11-18 [1/1]   Jan Janak            [notmuch] [PATCH] notmuch new: Support for conversion of spool subdirectories into tags (matching unread)
+  2009-11-18 [1/1]   Stewart Smith        [notmuch] [PATCH] Fix linking with gcc to use g++ to link in C++ libs. (matching unread)
+  2009-11-17 [1/2]   Ingmar Vanhassel, Carl Worth  [notmuch] [PATCH] Typsos (inbox matching unread)
+End of search results.
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
 test_begin_subtest "Message with .. in Message-Id:"
 add_message [id]=123..456@example '[subject]="Message with .. in Message-Id"'
 test_emacs '(notmuch-search "id:\"123..456@example\"")
-- 
1.7.8.1

Re: [PATCH v4] test: emacs: add test for `notmuch-search-operate-all'

[Dmitry Kurochkin]

Hi Pieter.

On Mon, 30 Jan 2012 08:45:50 +0100, Pieter Praet <pieter@praet.org> wrote:
> `notmuch-search-operate-all' (bound to "*") adds and removes tags
> to/from all messages which match the query used to populate the
> current search buffer.
> 
> ---
> 
> Rebased to current master.
> 
> Previous versions (chronologically):
> - id:"1309450108-2793-1-git-send-email-pieter@praet.org"
> - id:"1309762318-4530-5-git-send-email-pieter@praet.org"
> - id:"1310313335-4159-5-git-send-email-pieter@praet.org"
> 

This looks like a useful patch series.  We definitely need more tests
for tagging operations in the Emacs UI.  Do you plan to revive it?

Note that not so long ago I posted a bunch of tagging-related patches
[1] that would conflict with this patch at least because of
`notmuch-search-operate-all' being renamed to `notmuch-search-tag-all'.

> 
>  test/emacs |   19 +++++++++++++++++++
>  1 files changed, 19 insertions(+), 0 deletions(-)
> 
> diff --git a/test/emacs b/test/emacs
> index 8ca4c8a..e94ad94 100755
> --- a/test/emacs
> +++ b/test/emacs
> @@ -124,6 +124,25 @@ test_emacs "(notmuch-show \"$os_x_darwin_thread\")
>  output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
>  test_expect_equal "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox unread)"
>  
> +test_begin_subtest "Add/remove tags to/from all matching messages."
> +test_emacs '(notmuch-search "tag:inbox AND tags")
> +	    (notmuch-test-wait)
> +	    (notmuch-search-operate-all "+matching" "-inbox")
> +	    (notmuch-search "tag:matching AND NOT tag:inbox")
> +	    (notmuch-test-wait)
> +	    (test-output)'
> +cat <<EOF >EXPECTED
> +  2009-11-18 [3/3]   Adrian Perez de Castro, Keith Packard, Carl Worth  [notmuch] Introducing myself (matching signed unread)
> +  2009-11-18 [1/3]   Carl Worth, Israel Herraiz, Keith Packard       [notmuch] New to the list (inbox matching unread)
> +  2009-11-18 [2/2]   Keith Packard, Carl Worth    [notmuch] [PATCH] Make notmuch-show 'X' (and 'x') commands remove inbox (and unread) tags (matching unread)
> +  2009-11-18 [1/2]   Keith Packard, Alexander Botero-Lowry    [notmuch] [PATCH] Create a default notmuch-show-hook that highlights URLs and uses word-wrap (inbox matching unread)
> +  2009-11-18 [1/1]   Jan Janak            [notmuch] [PATCH] notmuch new: Support for conversion of spool subdirectories into tags (matching unread)
> +  2009-11-18 [1/1]   Stewart Smith        [notmuch] [PATCH] Fix linking with gcc to use g++ to link in C++ libs. (matching unread)
> +  2009-11-17 [1/2]   Ingmar Vanhassel, Carl Worth  [notmuch] [PATCH] Typsos (inbox matching unread)
> +End of search results.
> +EOF
> +test_expect_equal_file OUTPUT EXPECTED

I am worried that this test would break because of changes in other
tests.  E.g. if a new test adds a new message which matches "tag:inbox
AND tags", this test would have to be updated.  I think we should avoid
this.  I see the following options here:

* Search for messages which are less likely to change, e.g. "from:carl".

* Rework the test to avoid using any fixed expected results, e.g.:

  - count all messages with tag:inbox

  - remove inbox tag, add some other distinct tag for all messages with
    tag:inbox

  - count all messages with tag:inbox again, check that it is 0

  - add the inbox tag back, remove the previously added tag, check the
    message count

I like the latter approach because it does not compare Emacs UI output
and hence would not break when that output changes.  What do you think?

Also, we should leave notmuch db in the same state as it was before the
test if possible.

Regards,
  Dmitry

[1] id:"1327901644-15799-1-git-send-email-dmitry.kurochkin@gmail.com"

> +
>  test_begin_subtest "Message with .. in Message-Id:"
>  add_message [id]=123..456@example '[subject]="Message with .. in Message-Id"'
>  test_emacs '(notmuch-search "id:\"123..456@example\"")
> -- 
> 1.7.8.1
> 

Re: [PATCH v4] test: emacs: add test for `notmuch-search-operate-all'

[Pieter Praet]

On Mon, 30 Jan 2012 12:13:48 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> Hi Pieter.
> 
> On Mon, 30 Jan 2012 08:45:50 +0100, Pieter Praet <pieter@praet.org> wrote:
> > `notmuch-search-operate-all' (bound to "*") adds and removes tags
> > to/from all messages which match the query used to populate the
> > current search buffer.
> > 
> > ---
> > 
> > Rebased to current master.
> > 
> > Previous versions (chronologically):
> > - id:"1309450108-2793-1-git-send-email-pieter@praet.org"
> > - id:"1309762318-4530-5-git-send-email-pieter@praet.org"
> > - id:"1310313335-4159-5-git-send-email-pieter@praet.org"
> > 
> 
> This looks like a useful patch series.  We definitely need more tests
> for tagging operations in the Emacs UI.  Do you plan to revive it?
> 

Absolutely.  In fact, it's still alive and kicking on a local branch :)


> Note that not so long ago I posted a bunch of tagging-related patches
> [1] that would conflict with this patch at least because of
> `notmuch-search-operate-all' being renamed to `notmuch-search-tag-all'.
> 

Nice series!  Will probably be pushed in the next few days,
so I'll hold off on resubmitting the tests until then.


> > 
> >  test/emacs |   19 +++++++++++++++++++
> >  1 files changed, 19 insertions(+), 0 deletions(-)
> > 
> > diff --git a/test/emacs b/test/emacs
> > index 8ca4c8a..e94ad94 100755
> > --- a/test/emacs
> > +++ b/test/emacs
> > @@ -124,6 +124,25 @@ test_emacs "(notmuch-show \"$os_x_darwin_thread\")
> >  output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
> >  test_expect_equal "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox unread)"
> >  
> > +test_begin_subtest "Add/remove tags to/from all matching messages."
> > +test_emacs '(notmuch-search "tag:inbox AND tags")
> > +	    (notmuch-test-wait)
> > +	    (notmuch-search-operate-all "+matching" "-inbox")
> > +	    (notmuch-search "tag:matching AND NOT tag:inbox")
> > +	    (notmuch-test-wait)
> > +	    (test-output)'
> > +cat <<EOF >EXPECTED
> > +  2009-11-18 [3/3]   Adrian Perez de Castro, Keith Packard, Carl Worth  [notmuch] Introducing myself (matching signed unread)
> > +  2009-11-18 [1/3]   Carl Worth, Israel Herraiz, Keith Packard       [notmuch] New to the list (inbox matching unread)
> > +  2009-11-18 [2/2]   Keith Packard, Carl Worth    [notmuch] [PATCH] Make notmuch-show 'X' (and 'x') commands remove inbox (and unread) tags (matching unread)
> > +  2009-11-18 [1/2]   Keith Packard, Alexander Botero-Lowry    [notmuch] [PATCH] Create a default notmuch-show-hook that highlights URLs and uses word-wrap (inbox matching unread)
> > +  2009-11-18 [1/1]   Jan Janak            [notmuch] [PATCH] notmuch new: Support for conversion of spool subdirectories into tags (matching unread)
> > +  2009-11-18 [1/1]   Stewart Smith        [notmuch] [PATCH] Fix linking with gcc to use g++ to link in C++ libs. (matching unread)
> > +  2009-11-17 [1/2]   Ingmar Vanhassel, Carl Worth  [notmuch] [PATCH] Typsos (inbox matching unread)
> > +End of search results.
> > +EOF
> > +test_expect_equal_file OUTPUT EXPECTED
> 
> I am worried that this test would break because of changes in other
> tests.  E.g. if a new test adds a new message which matches "tag:inbox
> AND tags", this test would have to be updated.  I think we should avoid
> this.  I see the following options here:
> 
> * Search for messages which are less likely to change, e.g. "from:carl".
> 
> * Rework the test to avoid using any fixed expected results, e.g.:
> 
>   - count all messages with tag:inbox
> 
>   - remove inbox tag, add some other distinct tag for all messages with
>     tag:inbox
> 
>   - count all messages with tag:inbox again, check that it is 0
> 
>   - add the inbox tag back, remove the previously added tag, check the
>     message count
> 
> I like the latter approach because it does not compare Emacs UI output
> and hence would not break when that output changes.  What do you think?
> 
> Also, we should leave notmuch db in the same state as it was before the
> test if possible.
> 

Good point(s)!

How about this:

  #+begin_src sh
    test_begin_subtest "Add/remove tags to/from all matching messages."
    expected=$(notmuch count from:cworth AND tag:inbox)
    test "${expected}" == "0" && expected="Need more matches!" # prevent false positives
    test_emacs "(notmuch-search \"from:cworth AND tag:inbox\")
            (notmuch-test-wait)
            (notmuch-search-operate-all \"+matching\" \"-inbox\")"
    output=$(notmuch count from:cworth AND tag:matching AND NOT tag:inbox)
    notmuch tag -matching +inbox -- from:cworth AND tag:matching AND NOT tag:inbox # restore db state!
    test_expect_equal "$output" "$expected"
  #+end_src


> Regards,
>   Dmitry
> 
> [1] id:"1327901644-15799-1-git-send-email-dmitry.kurochkin@gmail.com"
> 
> > +
> >  test_begin_subtest "Message with .. in Message-Id:"
> >  add_message [id]=123..456@example '[subject]="Message with .. in Message-Id"'
> >  test_emacs '(notmuch-search "id:\"123..456@example\"")
> > -- 
> > 1.7.8.1
> > 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch


Peace

-- 
Pieter

Re: [PATCH v4] test: emacs: add test for `notmuch-search-operate-all'

[Dmitry Kurochkin]

On Wed, 01 Feb 2012 14:44:48 +0100, Pieter Praet <pieter@praet.org> wrote:
> On Mon, 30 Jan 2012 12:13:48 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > Hi Pieter.
> > 
> > On Mon, 30 Jan 2012 08:45:50 +0100, Pieter Praet <pieter@praet.org> wrote:
> > > `notmuch-search-operate-all' (bound to "*") adds and removes tags
> > > to/from all messages which match the query used to populate the
> > > current search buffer.
> > > 
> > > ---
> > > 
> > > Rebased to current master.
> > > 
> > > Previous versions (chronologically):
> > > - id:"1309450108-2793-1-git-send-email-pieter@praet.org"
> > > - id:"1309762318-4530-5-git-send-email-pieter@praet.org"
> > > - id:"1310313335-4159-5-git-send-email-pieter@praet.org"
> > > 
> > 
> > This looks like a useful patch series.  We definitely need more tests
> > for tagging operations in the Emacs UI.  Do you plan to revive it?
> > 
> 
> Absolutely.  In fact, it's still alive and kicking on a local branch :)
> 
> 
> > Note that not so long ago I posted a bunch of tagging-related patches
> > [1] that would conflict with this patch at least because of
> > `notmuch-search-operate-all' being renamed to `notmuch-search-tag-all'.
> > 
> 
> Nice series!  Will probably be pushed in the next few days,
> so I'll hold off on resubmitting the tests until then.
> 
> 
> > > 
> > >  test/emacs |   19 +++++++++++++++++++
> > >  1 files changed, 19 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/test/emacs b/test/emacs
> > > index 8ca4c8a..e94ad94 100755
> > > --- a/test/emacs
> > > +++ b/test/emacs
> > > @@ -124,6 +124,25 @@ test_emacs "(notmuch-show \"$os_x_darwin_thread\")
> > >  output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
> > >  test_expect_equal "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox unread)"
> > >  
> > > +test_begin_subtest "Add/remove tags to/from all matching messages."
> > > +test_emacs '(notmuch-search "tag:inbox AND tags")
> > > +	    (notmuch-test-wait)
> > > +	    (notmuch-search-operate-all "+matching" "-inbox")
> > > +	    (notmuch-search "tag:matching AND NOT tag:inbox")
> > > +	    (notmuch-test-wait)
> > > +	    (test-output)'
> > > +cat <<EOF >EXPECTED
> > > +  2009-11-18 [3/3]   Adrian Perez de Castro, Keith Packard, Carl Worth  [notmuch] Introducing myself (matching signed unread)
> > > +  2009-11-18 [1/3]   Carl Worth, Israel Herraiz, Keith Packard       [notmuch] New to the list (inbox matching unread)
> > > +  2009-11-18 [2/2]   Keith Packard, Carl Worth    [notmuch] [PATCH] Make notmuch-show 'X' (and 'x') commands remove inbox (and unread) tags (matching unread)
> > > +  2009-11-18 [1/2]   Keith Packard, Alexander Botero-Lowry    [notmuch] [PATCH] Create a default notmuch-show-hook that highlights URLs and uses word-wrap (inbox matching unread)
> > > +  2009-11-18 [1/1]   Jan Janak            [notmuch] [PATCH] notmuch new: Support for conversion of spool subdirectories into tags (matching unread)
> > > +  2009-11-18 [1/1]   Stewart Smith        [notmuch] [PATCH] Fix linking with gcc to use g++ to link in C++ libs. (matching unread)
> > > +  2009-11-17 [1/2]   Ingmar Vanhassel, Carl Worth  [notmuch] [PATCH] Typsos (inbox matching unread)
> > > +End of search results.
> > > +EOF
> > > +test_expect_equal_file OUTPUT EXPECTED
> > 
> > I am worried that this test would break because of changes in other
> > tests.  E.g. if a new test adds a new message which matches "tag:inbox
> > AND tags", this test would have to be updated.  I think we should avoid
> > this.  I see the following options here:
> > 
> > * Search for messages which are less likely to change, e.g. "from:carl".
> > 
> > * Rework the test to avoid using any fixed expected results, e.g.:
> > 
> >   - count all messages with tag:inbox
> > 
> >   - remove inbox tag, add some other distinct tag for all messages with
> >     tag:inbox
> > 
> >   - count all messages with tag:inbox again, check that it is 0
> > 
> >   - add the inbox tag back, remove the previously added tag, check the
> >     message count
> > 
> > I like the latter approach because it does not compare Emacs UI output
> > and hence would not break when that output changes.  What do you think?
> > 
> > Also, we should leave notmuch db in the same state as it was before the
> > test if possible.
> > 
> 
> Good point(s)!
> 
> How about this:
> 
>   #+begin_src sh
>     test_begin_subtest "Add/remove tags to/from all matching messages."
>     expected=$(notmuch count from:cworth AND tag:inbox)
>     test "${expected}" == "0" && expected="Need more matches!" # prevent false positives
>     test_emacs "(notmuch-search \"from:cworth AND tag:inbox\")
>             (notmuch-test-wait)
>             (notmuch-search-operate-all \"+matching\" \"-inbox\")"
>     output=$(notmuch count from:cworth AND tag:matching AND NOT tag:inbox)
>     notmuch tag -matching +inbox -- from:cworth AND tag:matching AND NOT tag:inbox # restore db state!
>     test_expect_equal "$output" "$expected"
>   #+end_src
> 

I think "from:cworth" is not needed here.  In my previous email I meant
this as an alternative option to testing using counts.

Probably we should add more counts in the test?  E.g.:

* before tagging
  - count tag:matching, it must be zero

* after -inbox +matching
  - count tag:matching, it must be equal to $expected
  - count tag:inbox, it must be zero

* after +inbox -matching
  - count tag:matching, it must be zero
  - count tag:inbox, it must be equal to $expected

We can compare it in a single test_expect_equal call.  Actual and
expected results may be smth like:

  initial tag:matching count: n1
  tag:inbox count after tagging: n2
  tag:matching count after tagging: n3
  tag:inbox count after restoring: n4
  tag:matching count after restoring: n5

In case of failure, we should get a nice diff.

Regards,
  Dmitry

> 
> > Regards,
> >   Dmitry
> > 
> > [1] id:"1327901644-15799-1-git-send-email-dmitry.kurochkin@gmail.com"
> > 
> > > +
> > >  test_begin_subtest "Message with .. in Message-Id:"
> > >  add_message [id]=123..456@example '[subject]="Message with .. in Message-Id"'
> > >  test_emacs '(notmuch-search "id:\"123..456@example\"")
> > > -- 
> > > 1.7.8.1
> > > 
> > _______________________________________________
> > notmuch mailing list
> > notmuch@notmuchmail.org
> > http://notmuchmail.org/mailman/listinfo/notmuch
> 
> 
> Peace
> 
> -- 
> Pieter

Re: [PATCH v4] test: emacs: add test for `notmuch-search-operate-all'

[Pieter Praet]

On Wed, 01 Feb 2012 18:06:05 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> On Wed, 01 Feb 2012 14:44:48 +0100, Pieter Praet <pieter@praet.org> wrote:
> > On Mon, 30 Jan 2012 12:13:48 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > > Hi Pieter.
> > > 
> > > On Mon, 30 Jan 2012 08:45:50 +0100, Pieter Praet <pieter@praet.org> wrote:
> > > > `notmuch-search-operate-all' (bound to "*") adds and removes tags
> > > > to/from all messages which match the query used to populate the
> > > > current search buffer.
> > > > 
> > > > ---
> > > > 
> > > > Rebased to current master.
> > > > 
> > > > Previous versions (chronologically):
> > > > - id:"1309450108-2793-1-git-send-email-pieter@praet.org"
> > > > - id:"1309762318-4530-5-git-send-email-pieter@praet.org"
> > > > - id:"1310313335-4159-5-git-send-email-pieter@praet.org"
> > > > 
> > > 
> > > This looks like a useful patch series.  We definitely need more tests
> > > for tagging operations in the Emacs UI.  Do you plan to revive it?
> > > 
> > 
> > Absolutely.  In fact, it's still alive and kicking on a local branch :)
> > 
> > 
> > > Note that not so long ago I posted a bunch of tagging-related patches
> > > [1] that would conflict with this patch at least because of
> > > `notmuch-search-operate-all' being renamed to `notmuch-search-tag-all'.
> > > 
> > 
> > Nice series!  Will probably be pushed in the next few days,
> > so I'll hold off on resubmitting the tests until then.
> > 
> > 
> > > > 
> > > >  test/emacs |   19 +++++++++++++++++++
> > > >  1 files changed, 19 insertions(+), 0 deletions(-)
> > > > 
> > > > diff --git a/test/emacs b/test/emacs
> > > > index 8ca4c8a..e94ad94 100755
> > > > --- a/test/emacs
> > > > +++ b/test/emacs
> > > > @@ -124,6 +124,25 @@ test_emacs "(notmuch-show \"$os_x_darwin_thread\")
> > > >  output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
> > > >  test_expect_equal "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox unread)"
> > > >  
> > > > +test_begin_subtest "Add/remove tags to/from all matching messages."
> > > > +test_emacs '(notmuch-search "tag:inbox AND tags")
> > > > +	    (notmuch-test-wait)
> > > > +	    (notmuch-search-operate-all "+matching" "-inbox")
> > > > +	    (notmuch-search "tag:matching AND NOT tag:inbox")
> > > > +	    (notmuch-test-wait)
> > > > +	    (test-output)'
> > > > +cat <<EOF >EXPECTED
> > > > +  2009-11-18 [3/3]   Adrian Perez de Castro, Keith Packard, Carl Worth  [notmuch] Introducing myself (matching signed unread)
> > > > +  2009-11-18 [1/3]   Carl Worth, Israel Herraiz, Keith Packard       [notmuch] New to the list (inbox matching unread)
> > > > +  2009-11-18 [2/2]   Keith Packard, Carl Worth    [notmuch] [PATCH] Make notmuch-show 'X' (and 'x') commands remove inbox (and unread) tags (matching unread)
> > > > +  2009-11-18 [1/2]   Keith Packard, Alexander Botero-Lowry    [notmuch] [PATCH] Create a default notmuch-show-hook that highlights URLs and uses word-wrap (inbox matching unread)
> > > > +  2009-11-18 [1/1]   Jan Janak            [notmuch] [PATCH] notmuch new: Support for conversion of spool subdirectories into tags (matching unread)
> > > > +  2009-11-18 [1/1]   Stewart Smith        [notmuch] [PATCH] Fix linking with gcc to use g++ to link in C++ libs. (matching unread)
> > > > +  2009-11-17 [1/2]   Ingmar Vanhassel, Carl Worth  [notmuch] [PATCH] Typsos (inbox matching unread)
> > > > +End of search results.
> > > > +EOF
> > > > +test_expect_equal_file OUTPUT EXPECTED
> > > 
> > > I am worried that this test would break because of changes in other
> > > tests.  E.g. if a new test adds a new message which matches "tag:inbox
> > > AND tags", this test would have to be updated.  I think we should avoid
> > > this.  I see the following options here:
> > > 
> > > * Search for messages which are less likely to change, e.g. "from:carl".
> > > 
> > > * Rework the test to avoid using any fixed expected results, e.g.:
> > > 
> > >   - count all messages with tag:inbox
> > > 
> > >   - remove inbox tag, add some other distinct tag for all messages with
> > >     tag:inbox
> > > 
> > >   - count all messages with tag:inbox again, check that it is 0
> > > 
> > >   - add the inbox tag back, remove the previously added tag, check the
> > >     message count
> > > 
> > > I like the latter approach because it does not compare Emacs UI output
> > > and hence would not break when that output changes.  What do you think?
> > > 
> > > Also, we should leave notmuch db in the same state as it was before the
> > > test if possible.
> > > 
> > 
> > Good point(s)!
> > 
> > How about this:
> > 
> >   #+begin_src sh
> >     test_begin_subtest "Add/remove tags to/from all matching messages."
> >     expected=$(notmuch count from:cworth AND tag:inbox)
> >     test "${expected}" == "0" && expected="Need more matches!" # prevent false positives
> >     test_emacs "(notmuch-search \"from:cworth AND tag:inbox\")
> >             (notmuch-test-wait)
> >             (notmuch-search-operate-all \"+matching\" \"-inbox\")"
> >     output=$(notmuch count from:cworth AND tag:matching AND NOT tag:inbox)
> >     notmuch tag -matching +inbox -- from:cworth AND tag:matching AND NOT tag:inbox # restore db state!
> >     test_expect_equal "$output" "$expected"
> >   #+end_src
> > 
> 
> I think "from:cworth" is not needed here.  In my previous email I meant
> this as an alternative option to testing using counts.
> 

I know, but I decided to include it anyway, on the basis that it
increases the probability of the search buffer containing threads with
matched *as well as* unmatched messages, thus providing more insurance
wrt `notmuch-search-operate-all' doing The Right Thing (TM).


> Probably we should add more counts in the test?  E.g.:
> 
> * before tagging
>   - count tag:matching, it must be zero
> 
> * after -inbox +matching
>   - count tag:matching, it must be equal to $expected
>   - count tag:inbox, it must be zero
> 
> * after +inbox -matching
>   - count tag:matching, it must be zero
>   - count tag:inbox, it must be equal to $expected
> 
> We can compare it in a single test_expect_equal call.  Actual and
> expected results may be smth like:
> 
>   initial tag:matching count: n1
>   tag:inbox count after tagging: n2
>   tag:matching count after tagging: n3
>   tag:inbox count after restoring: n4
>   tag:matching count after restoring: n5
> 
> In case of failure, we should get a nice diff.
> 

OK, how about this?:

  #+begin_src sh
    test_begin_subtest "Add/remove tags to/from all matching messages."
    old="inbox" ; new="xobni" ; filter="AND from:cworth"
    o1=$(notmuch count tag:"${old}" "${filter}") ; n1=$(notmuch count tag:"${new}" "${filter}")
    test "${o1}" == "0" && o1="Need more matches!" # prevent false positives
    test_emacs "(notmuch-search \"tag:${old} ${filter}\")
            (notmuch-test-wait)
            (notmuch-search-operate-all \"+${new}\" \"-${old}\")"
    o2=$(notmuch count tag:"${old}" "${filter}") ; n2=$(notmuch count tag:"${new}" "${filter}")
    notmuch tag -"${new}" +"${old}" -- tag:"${new}" "${filter}" AND NOT tag:"${old}"} # restore db state!
    o3=$(notmuch count tag:"${old}" "${filter}") ; n3=$(notmuch count tag:"${new}" "${filter}")
    output="
    before:   ${old}=$o1 ${new}=$n1
    after:    ${old}=$o2 ${new}=$n2
    restored: ${old}=$o3 ${new}=$n3"
    expected="
    before:   ${old}=$o1 ${new}=0
    after:    ${old}=0 ${new}=$o1
    restored: ${old}=$o1 ${new}=0"
    test_expect_equal "$output" "$expected"
  #+end_src


> Regards,
>   Dmitry
> 
> > 
> > > Regards,
> > >   Dmitry
> > > 
> > > [1] id:"1327901644-15799-1-git-send-email-dmitry.kurochkin@gmail.com"
> > > 
> > > > +
> > > >  test_begin_subtest "Message with .. in Message-Id:"
> > > >  add_message [id]=123..456@example '[subject]="Message with .. in Message-Id"'
> > > >  test_emacs '(notmuch-search "id:\"123..456@example\"")
> > > > -- 
> > > > 1.7.8.1
> > > > 
> > > _______________________________________________
> > > notmuch mailing list
> > > notmuch@notmuchmail.org
> > > http://notmuchmail.org/mailman/listinfo/notmuch
> > 
> > 
> > Peace
> > 
> > -- 
> > Pieter


Peace

-- 
Pieter

Re: [PATCH v4] test: emacs: add test for `notmuch-search-operate-all'

[Dmitry Kurochkin]

Hi Pieter.

On Wed, 01 Feb 2012 21:32:32 +0100, Pieter Praet <pieter@praet.org> wrote:
> On Wed, 01 Feb 2012 18:06:05 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > On Wed, 01 Feb 2012 14:44:48 +0100, Pieter Praet <pieter@praet.org> wrote:
> > > On Mon, 30 Jan 2012 12:13:48 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > > > Hi Pieter.
> > > > 
> > > > On Mon, 30 Jan 2012 08:45:50 +0100, Pieter Praet <pieter@praet.org> wrote:
> > > > > `notmuch-search-operate-all' (bound to "*") adds and removes tags
> > > > > to/from all messages which match the query used to populate the
> > > > > current search buffer.
> > > > > 
> > > > > ---
> > > > > 
> > > > > Rebased to current master.
> > > > > 
> > > > > Previous versions (chronologically):
> > > > > - id:"1309450108-2793-1-git-send-email-pieter@praet.org"
> > > > > - id:"1309762318-4530-5-git-send-email-pieter@praet.org"
> > > > > - id:"1310313335-4159-5-git-send-email-pieter@praet.org"
> > > > > 
> > > > 
> > > > This looks like a useful patch series.  We definitely need more tests
> > > > for tagging operations in the Emacs UI.  Do you plan to revive it?
> > > > 
> > > 
> > > Absolutely.  In fact, it's still alive and kicking on a local branch :)
> > > 
> > > 
> > > > Note that not so long ago I posted a bunch of tagging-related patches
> > > > [1] that would conflict with this patch at least because of
> > > > `notmuch-search-operate-all' being renamed to `notmuch-search-tag-all'.
> > > > 
> > > 
> > > Nice series!  Will probably be pushed in the next few days,
> > > so I'll hold off on resubmitting the tests until then.
> > > 
> > > 
> > > > > 
> > > > >  test/emacs |   19 +++++++++++++++++++
> > > > >  1 files changed, 19 insertions(+), 0 deletions(-)
> > > > > 
> > > > > diff --git a/test/emacs b/test/emacs
> > > > > index 8ca4c8a..e94ad94 100755
> > > > > --- a/test/emacs
> > > > > +++ b/test/emacs
> > > > > @@ -124,6 +124,25 @@ test_emacs "(notmuch-show \"$os_x_darwin_thread\")
> > > > >  output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
> > > > >  test_expect_equal "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox unread)"
> > > > >  
> > > > > +test_begin_subtest "Add/remove tags to/from all matching messages."
> > > > > +test_emacs '(notmuch-search "tag:inbox AND tags")
> > > > > +	    (notmuch-test-wait)
> > > > > +	    (notmuch-search-operate-all "+matching" "-inbox")
> > > > > +	    (notmuch-search "tag:matching AND NOT tag:inbox")
> > > > > +	    (notmuch-test-wait)
> > > > > +	    (test-output)'
> > > > > +cat <<EOF >EXPECTED
> > > > > +  2009-11-18 [3/3]   Adrian Perez de Castro, Keith Packard, Carl Worth  [notmuch] Introducing myself (matching signed unread)
> > > > > +  2009-11-18 [1/3]   Carl Worth, Israel Herraiz, Keith Packard       [notmuch] New to the list (inbox matching unread)
> > > > > +  2009-11-18 [2/2]   Keith Packard, Carl Worth    [notmuch] [PATCH] Make notmuch-show 'X' (and 'x') commands remove inbox (and unread) tags (matching unread)
> > > > > +  2009-11-18 [1/2]   Keith Packard, Alexander Botero-Lowry    [notmuch] [PATCH] Create a default notmuch-show-hook that highlights URLs and uses word-wrap (inbox matching unread)
> > > > > +  2009-11-18 [1/1]   Jan Janak            [notmuch] [PATCH] notmuch new: Support for conversion of spool subdirectories into tags (matching unread)
> > > > > +  2009-11-18 [1/1]   Stewart Smith        [notmuch] [PATCH] Fix linking with gcc to use g++ to link in C++ libs. (matching unread)
> > > > > +  2009-11-17 [1/2]   Ingmar Vanhassel, Carl Worth  [notmuch] [PATCH] Typsos (inbox matching unread)
> > > > > +End of search results.
> > > > > +EOF
> > > > > +test_expect_equal_file OUTPUT EXPECTED
> > > > 
> > > > I am worried that this test would break because of changes in other
> > > > tests.  E.g. if a new test adds a new message which matches "tag:inbox
> > > > AND tags", this test would have to be updated.  I think we should avoid
> > > > this.  I see the following options here:
> > > > 
> > > > * Search for messages which are less likely to change, e.g. "from:carl".
> > > > 
> > > > * Rework the test to avoid using any fixed expected results, e.g.:
> > > > 
> > > >   - count all messages with tag:inbox
> > > > 
> > > >   - remove inbox tag, add some other distinct tag for all messages with
> > > >     tag:inbox
> > > > 
> > > >   - count all messages with tag:inbox again, check that it is 0
> > > > 
> > > >   - add the inbox tag back, remove the previously added tag, check the
> > > >     message count
> > > > 
> > > > I like the latter approach because it does not compare Emacs UI output
> > > > and hence would not break when that output changes.  What do you think?
> > > > 
> > > > Also, we should leave notmuch db in the same state as it was before the
> > > > test if possible.
> > > > 
> > > 
> > > Good point(s)!
> > > 
> > > How about this:
> > > 
> > >   #+begin_src sh
> > >     test_begin_subtest "Add/remove tags to/from all matching messages."
> > >     expected=$(notmuch count from:cworth AND tag:inbox)
> > >     test "${expected}" == "0" && expected="Need more matches!" # prevent false positives
> > >     test_emacs "(notmuch-search \"from:cworth AND tag:inbox\")
> > >             (notmuch-test-wait)
> > >             (notmuch-search-operate-all \"+matching\" \"-inbox\")"
> > >     output=$(notmuch count from:cworth AND tag:matching AND NOT tag:inbox)
> > >     notmuch tag -matching +inbox -- from:cworth AND tag:matching AND NOT tag:inbox # restore db state!
> > >     test_expect_equal "$output" "$expected"
> > >   #+end_src
> > > 
> > 
> > I think "from:cworth" is not needed here.  In my previous email I meant
> > this as an alternative option to testing using counts.
> > 
> 
> I know, but I decided to include it anyway, on the basis that it
> increases the probability of the search buffer containing threads with
> matched *as well as* unmatched messages, thus providing more insurance
> wrt `notmuch-search-operate-all' doing The Right Thing (TM).
> 
> 
> > Probably we should add more counts in the test?  E.g.:
> > 
> > * before tagging
> >   - count tag:matching, it must be zero
> > 
> > * after -inbox +matching
> >   - count tag:matching, it must be equal to $expected
> >   - count tag:inbox, it must be zero
> > 
> > * after +inbox -matching
> >   - count tag:matching, it must be zero
> >   - count tag:inbox, it must be equal to $expected
> > 
> > We can compare it in a single test_expect_equal call.  Actual and
> > expected results may be smth like:
> > 
> >   initial tag:matching count: n1
> >   tag:inbox count after tagging: n2
> >   tag:matching count after tagging: n3
> >   tag:inbox count after restoring: n4
> >   tag:matching count after restoring: n5
> > 
> > In case of failure, we should get a nice diff.
> > 
> 
> OK, how about this?:
> 

Looks good.  Minor comments below.

>   #+begin_src sh
>     test_begin_subtest "Add/remove tags to/from all matching messages."

We should add notmuch-search here, because similar functionality would
be available in notmuch-show soon.  Also, I suggest replacing
"add/remove tags to/from" with "change tags".  Consider:

  Change tags for all matching threads in notmuch-search.

>     old="inbox" ; new="xobni" ; filter="AND from:cworth"

I would prefer this to be on separate lines.

>     o1=$(notmuch count tag:"${old}" "${filter}") ; n1=$(notmuch count tag:"${new}" "${filter}")

I would definately prefer this to be on separate lines.

Also, please consider s/o1/old_count_1, s/n1/new_count_1/ and so on.

>     test "${o1}" == "0" && o1="Need more matches!" # prevent false positives
>     test_emacs "(notmuch-search \"tag:${old} ${filter}\")
>             (notmuch-test-wait)
>             (notmuch-search-operate-all \"+${new}\" \"-${old}\")"
>     o2=$(notmuch count tag:"${old}" "${filter}") ; n2=$(notmuch count tag:"${new}" "${filter}")

Same comment about separate lines.

>     notmuch tag -"${new}" +"${old}" -- tag:"${new}" "${filter}" AND NOT tag:"${old}"} # restore db state!

Why "AND NOT tag:$old" is needed here?

Since the line is too long, please move the comment to a separate line
above.

Since we are testing Emacs UI, should we restore db though Emacs as
well?

>     o3=$(notmuch count tag:"${old}" "${filter}") ; n3=$(notmuch count tag:"${new}" "${filter}")

Same comment about separate lines.

>     output="
>     before:   ${old}=$o1 ${new}=$n1
>     after:    ${old}=$o2 ${new}=$n2
>     restored: ${old}=$o3 ${new}=$n3"
>     expected="
>     before:   ${old}=$o1 ${new}=0
>     after:    ${old}=0 ${new}=$o1
>     restored: ${old}=$o1 ${new}=0"

I would change "=" to ":".

Regards,
  Dmitry

>     test_expect_equal "$output" "$expected"
>   #+end_src
> 
> 
> > Regards,
> >   Dmitry
> > 
> > > 
> > > > Regards,
> > > >   Dmitry
> > > > 
> > > > [1] id:"1327901644-15799-1-git-send-email-dmitry.kurochkin@gmail.com"
> > > > 
> > > > > +
> > > > >  test_begin_subtest "Message with .. in Message-Id:"
> > > > >  add_message [id]=123..456@example '[subject]="Message with .. in Message-Id"'
> > > > >  test_emacs '(notmuch-search "id:\"123..456@example\"")
> > > > > -- 
> > > > > 1.7.8.1
> > > > > 
> > > > _______________________________________________
> > > > notmuch mailing list
> > > > notmuch@notmuchmail.org
> > > > http://notmuchmail.org/mailman/listinfo/notmuch
> > > 
> > > 
> > > Peace
> > > 
> > > -- 
> > > Pieter
> 
> 
> Peace
> 
> -- 
> Pieter

Re: [PATCH v4] test: emacs: add test for `notmuch-search-operate-all'

[Pieter Praet]

On Thu, 02 Feb 2012 03:25:40 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > > [...]
> > 
> > OK, how about this?:
> > 
> 
> Looks good.  Minor comments below.
> 
> >   #+begin_src sh
> >     test_begin_subtest "Add/remove tags to/from all matching messages."
> 
> We should add notmuch-search here, because similar functionality would
> be available in notmuch-show soon.  Also, I suggest replacing
> "add/remove tags to/from" with "change tags".  Consider:
> 
>   Change tags for all matching threads in notmuch-search.
>

Agreed, though I think you meant "matching messages".


Although, Austin has suggested [1] having `notmuch-search-operate-all'
(s/operate/tag) work on threads (or all messages therein, at least),
as opposed to only on *matched* messages, with which I agree(d) [2].

This would also get rid of the regexp issue @ [3], allowing us to
fix the race condition without having to put `notmuch-search' on a
JSON diet first.

And OTOH, David has asked [4] for `notmuch-show-tag-all' to work only on
visible (uncollapsed) messages, so wrt keeping `notmuch-search-tag-all'
and `notmuch-show-tag-all' symmetrical, it seems we're at an impasse :)


> >     old="inbox" ; new="xobni" ; filter="AND from:cworth"
> 
> I would prefer this to be on separate lines.
> 
> >     o1=$(notmuch count tag:"${old}" "${filter}") ; n1=$(notmuch count tag:"${new}" "${filter}")
> 
> I would definately prefer this to be on separate lines.
> 
> Also, please consider s/o1/old_count_1, s/n1/new_count_1/ and so on.
> 
> >     test "${o1}" == "0" && o1="Need more matches!" # prevent false positives
> >     test_emacs "(notmuch-search \"tag:${old} ${filter}\")
> >             (notmuch-test-wait)
> >             (notmuch-search-operate-all \"+${new}\" \"-${old}\")"
> >     o2=$(notmuch count tag:"${old}" "${filter}") ; n2=$(notmuch count tag:"${new}" "${filter}")
> 
> Same comment about separate lines.
>
> >     notmuch tag -"${new}" +"${old}" -- tag:"${new}" "${filter}" AND NOT tag:"${old}"} # restore db state!
> 
> Why "AND NOT tag:$old" is needed here?
>

To be extra sure we're only touching the messages which were altered in
the previous segment, but it's probably rather useless.  Removed.


> Since the line is too long, please move the comment to a separate line
> above.
> 
> Since we are testing Emacs UI, should we restore db though Emacs as
> well?
>

Hmm, comes with a performance hit (which is why I avoided it initially),
but a fairly minor one at that.  Done.


> >     o3=$(notmuch count tag:"${old}" "${filter}") ; n3=$(notmuch count tag:"${new}" "${filter}")
> 
> Same comment about separate lines.
> 
> >     output="
> >     before:   ${old}=$o1 ${new}=$n1
> >     after:    ${old}=$o2 ${new}=$n2
> >     restored: ${old}=$o3 ${new}=$n3"
> >     expected="
> >     before:   ${old}=$o1 ${new}=0
> >     after:    ${old}=0 ${new}=$o1
> >     restored: ${old}=$o1 ${new}=0"
> 
> I would change "=" to ":".
>

Done, as well as everything related to separating lines and
renaming variables.

Fresh patch submitted in its original thread [5].


> Regards,
>   Dmitry
> 
> > [...]


Peace

-- 
Pieter

[1] id:"20111112163502.GE2658@mit.edu"
[2] id:"871ut8f9ya.fsf@praet.org"
[3] id:"1310416993-31031-1-git-send-email-pieter@praet.org"
[4] id:"87wr7xqpuf.fsf@rocinante.cs.unb.ca"
[5] id:"1329683908-5435-1-git-send-email-pieter@praet.org"