[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

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[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

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[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."
_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Dmitry Kurochkin]

On Wed, 25 Jan 2012 20:37:27 -0500, Austin Clements <[hidden email]> 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."
_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[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:"[hidden email]"

 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

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[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:"[hidden email]"

 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

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[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:"[hidden email]"
>
>  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."

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[David Edmondson]

On Thu, 26 Jan 2012 05:47:07 +0400, Dmitry Kurochkin <[hidden email]> 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.

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[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:"[hidden email]"
[2] id:"[hidden email]"

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[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

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[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

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Dmitry Kurochkin]

On Thu, 26 Jan 2012 02:04:39 -0500, Austin Clements <[hidden email]> 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:"[hidden email]"
> >
> >  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."

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Dmitry Kurochkin]

On Thu, 26 Jan 2012 08:46:00 +0000, David Edmondson <[hidden email]> wrote:

> On Thu, 26 Jan 2012 05:47:07 +0400, Dmitry Kurochkin <[hidden email]> 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
_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[David Edmondson]

The series look good to me.

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[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.
_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[David Bremner-2]

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

pushed,

d
_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[David Bremner-2]

On Thu, 26 Jan 2012 21:34:49 +0400, Dmitry Kurochkin <[hidden email]> wrote:
> ---
>  emacs/notmuch.el |   11 +++++------
>  1 files changed, 5 insertions(+), 6 deletions(-)

Pushed,

d.

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[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:"[hidden email]"
- id:"[hidden email]"
- id:"[hidden email]"


 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

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Dmitry Kurochkin]

Hi Pieter.

On Mon, 30 Jan 2012 08:45:50 +0100, Pieter Praet <[hidden email]> 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:"[hidden email]"
> - id:"[hidden email]"
> - id:"[hidden email]"
>

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:"[hidden email]"

> +
>  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
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Pieter Praet]

On Mon, 30 Jan 2012 12:13:48 +0400, Dmitry Kurochkin <[hidden email]> wrote:

> Hi Pieter.
>
> On Mon, 30 Jan 2012 08:45:50 +0100, Pieter Praet <[hidden email]> 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:"[hidden email]"
> > - id:"[hidden email]"
> > - id:"[hidden email]"
> >
>
> 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:"[hidden email]"
>
> > +
> >  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
> [hidden email]
> http://notmuchmail.org/mailman/listinfo/notmuch

Peace

--
Pieter
_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch