* [PATCH] emacs: add completion to "tag all" operation ("*" binding)
@ 2012-01-26 1:12 Dmitry Kurochkin
2012-01-26 1:26 ` [PATCH v2] " Dmitry Kurochkin
` (4 more replies)
0 siblings, 5 replies; 24+ messages in thread
From: Dmitry Kurochkin @ 2012-01-26 1:12 UTC (permalink / raw)
To: notmuch
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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2] emacs: add completion to "tag all" operation ("*" binding)
2012-01-26 1:12 [PATCH] emacs: add completion to "tag all" operation ("*" binding) Dmitry Kurochkin
@ 2012-01-26 1:26 ` Dmitry Kurochkin
2012-01-26 1:37 ` [PATCH] " Austin Clements
` (3 subsequent siblings)
4 siblings, 0 replies; 24+ messages in thread
From: Dmitry Kurochkin @ 2012-01-26 1:26 UTC (permalink / raw)
To: notmuch
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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH] emacs: add completion to "tag all" operation ("*" binding)
2012-01-26 1:12 [PATCH] emacs: add completion to "tag all" operation ("*" binding) Dmitry Kurochkin
2012-01-26 1:26 ` [PATCH v2] " Dmitry Kurochkin
@ 2012-01-26 1:37 ` Austin Clements
2012-01-26 1:47 ` Dmitry Kurochkin
2012-01-26 2:45 ` [PATCH v3] " Dmitry Kurochkin
` (2 subsequent siblings)
4 siblings, 1 reply; 24+ messages in thread
From: Austin Clements @ 2012-01-26 1:37 UTC (permalink / raw)
To: Dmitry Kurochkin; +Cc: notmuch
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."
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] emacs: add completion to "tag all" operation ("*" binding)
2012-01-26 1:37 ` [PATCH] " Austin Clements
@ 2012-01-26 1:47 ` Dmitry Kurochkin
2012-01-26 8:46 ` David Edmondson
0 siblings, 1 reply; 24+ messages in thread
From: Dmitry Kurochkin @ 2012-01-26 1:47 UTC (permalink / raw)
To: Austin Clements, notmuch
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."
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3] emacs: add completion to "tag all" operation ("*" binding)
2012-01-26 1:12 [PATCH] emacs: add completion to "tag all" operation ("*" binding) Dmitry Kurochkin
2012-01-26 1:26 ` [PATCH v2] " Dmitry Kurochkin
2012-01-26 1:37 ` [PATCH] " Austin Clements
@ 2012-01-26 2:45 ` Dmitry Kurochkin
2012-01-26 5:06 ` [PATCH v4] " Dmitry Kurochkin
2012-01-26 17:34 ` [PATCH v5 0/2] emacs: add completion to "tag all" operation Dmitry Kurochkin
4 siblings, 0 replies; 24+ messages in thread
From: Dmitry Kurochkin @ 2012-01-26 2:45 UTC (permalink / raw)
To: notmuch
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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v4] emacs: add completion to "tag all" operation ("*" binding)
2012-01-26 1:12 [PATCH] emacs: add completion to "tag all" operation ("*" binding) Dmitry Kurochkin
` (2 preceding siblings ...)
2012-01-26 2:45 ` [PATCH v3] " Dmitry Kurochkin
@ 2012-01-26 5:06 ` Dmitry Kurochkin
2012-01-26 7:04 ` Austin Clements
2012-01-26 17:34 ` [PATCH v5 0/2] emacs: add completion to "tag all" operation Dmitry Kurochkin
4 siblings, 1 reply; 24+ messages in thread
From: Dmitry Kurochkin @ 2012-01-26 5:06 UTC (permalink / raw)
To: notmuch
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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v4] emacs: add completion to "tag all" operation ("*" binding)
2012-01-26 5:06 ` [PATCH v4] " Dmitry Kurochkin
@ 2012-01-26 7:04 ` Austin Clements
2012-01-26 17:37 ` Dmitry Kurochkin
0 siblings, 1 reply; 24+ messages in thread
From: Austin Clements @ 2012-01-26 7:04 UTC (permalink / raw)
To: Dmitry Kurochkin; +Cc: notmuch
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."
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] emacs: add completion to "tag all" operation ("*" binding)
2012-01-26 1:47 ` Dmitry Kurochkin
@ 2012-01-26 8:46 ` David Edmondson
2012-01-26 17:39 ` Dmitry Kurochkin
0 siblings, 1 reply; 24+ messages in thread
From: David Edmondson @ 2012-01-26 8:46 UTC (permalink / raw)
To: Dmitry Kurochkin, Austin Clements, notmuch
[-- 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 --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v5 0/2] emacs: add completion to "tag all" operation
2012-01-26 1:12 [PATCH] emacs: add completion to "tag all" operation ("*" binding) Dmitry Kurochkin
` (3 preceding siblings ...)
2012-01-26 5:06 ` [PATCH v4] " Dmitry Kurochkin
@ 2012-01-26 17:34 ` Dmitry Kurochkin
2012-01-26 17:34 ` [PATCH v5 1/2] emacs: add completion to "tag all" operation ("*" binding) Dmitry Kurochkin
` (4 more replies)
4 siblings, 5 replies; 24+ messages in thread
From: Dmitry Kurochkin @ 2012-01-26 17:34 UTC (permalink / raw)
To: notmuch
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"
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v5 1/2] emacs: add completion to "tag all" operation ("*" binding)
2012-01-26 17:34 ` [PATCH v5 0/2] emacs: add completion to "tag all" operation Dmitry Kurochkin
@ 2012-01-26 17:34 ` Dmitry Kurochkin
2012-01-27 11:55 ` David Bremner
2012-01-26 17:34 ` [PATCH v5 2/2] emacs: `notmuch-search-operate-all' code cleanup, no functional changes Dmitry Kurochkin
` (3 subsequent siblings)
4 siblings, 1 reply; 24+ messages in thread
From: Dmitry Kurochkin @ 2012-01-26 17:34 UTC (permalink / raw)
To: notmuch
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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v5 2/2] emacs: `notmuch-search-operate-all' code cleanup, no functional changes
2012-01-26 17:34 ` [PATCH v5 0/2] emacs: add completion to "tag all" operation Dmitry Kurochkin
2012-01-26 17:34 ` [PATCH v5 1/2] emacs: add completion to "tag all" operation ("*" binding) Dmitry Kurochkin
@ 2012-01-26 17:34 ` Dmitry Kurochkin
2012-01-27 11:56 ` David Bremner
2012-01-26 18:18 ` [PATCH v5 0/2] emacs: add completion to "tag all" operation David Edmondson
` (2 subsequent siblings)
4 siblings, 1 reply; 24+ messages in thread
From: Dmitry Kurochkin @ 2012-01-26 17:34 UTC (permalink / raw)
To: notmuch
---
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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v4] emacs: add completion to "tag all" operation ("*" binding)
2012-01-26 7:04 ` Austin Clements
@ 2012-01-26 17:37 ` Dmitry Kurochkin
0 siblings, 0 replies; 24+ messages in thread
From: Dmitry Kurochkin @ 2012-01-26 17:37 UTC (permalink / raw)
To: Austin Clements; +Cc: notmuch
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."
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] emacs: add completion to "tag all" operation ("*" binding)
2012-01-26 8:46 ` David Edmondson
@ 2012-01-26 17:39 ` Dmitry Kurochkin
0 siblings, 0 replies; 24+ messages in thread
From: Dmitry Kurochkin @ 2012-01-26 17:39 UTC (permalink / raw)
To: David Edmondson, Austin Clements, notmuch
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5 0/2] emacs: add completion to "tag all" operation
2012-01-26 17:34 ` [PATCH v5 0/2] emacs: add completion to "tag all" operation Dmitry Kurochkin
2012-01-26 17:34 ` [PATCH v5 1/2] emacs: add completion to "tag all" operation ("*" binding) Dmitry Kurochkin
2012-01-26 17:34 ` [PATCH v5 2/2] emacs: `notmuch-search-operate-all' code cleanup, no functional changes Dmitry Kurochkin
@ 2012-01-26 18:18 ` David Edmondson
2012-01-26 18:35 ` Austin Clements
2012-01-30 7:45 ` [PATCH v4] test: emacs: add test for `notmuch-search-operate-all' Pieter Praet
4 siblings, 0 replies; 24+ messages in thread
From: David Edmondson @ 2012-01-26 18:18 UTC (permalink / raw)
To: Dmitry Kurochkin, notmuch
[-- Attachment #1: Type: text/plain, Size: 28 bytes --]
The series look good to me.
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5 0/2] emacs: add completion to "tag all" operation
2012-01-26 17:34 ` [PATCH v5 0/2] emacs: add completion to "tag all" operation Dmitry Kurochkin
` (2 preceding siblings ...)
2012-01-26 18:18 ` [PATCH v5 0/2] emacs: add completion to "tag all" operation David Edmondson
@ 2012-01-26 18:35 ` Austin Clements
2012-01-30 7:45 ` [PATCH v4] test: emacs: add test for `notmuch-search-operate-all' Pieter Praet
4 siblings, 0 replies; 24+ messages in thread
From: Austin Clements @ 2012-01-26 18:35 UTC (permalink / raw)
To: Dmitry Kurochkin; +Cc: notmuch
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.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5 1/2] emacs: add completion to "tag all" operation ("*" binding)
2012-01-26 17:34 ` [PATCH v5 1/2] emacs: add completion to "tag all" operation ("*" binding) Dmitry Kurochkin
@ 2012-01-27 11:55 ` David Bremner
0 siblings, 0 replies; 24+ messages in thread
From: David Bremner @ 2012-01-27 11:55 UTC (permalink / raw)
To: Dmitry Kurochkin, notmuch
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5 2/2] emacs: `notmuch-search-operate-all' code cleanup, no functional changes
2012-01-26 17:34 ` [PATCH v5 2/2] emacs: `notmuch-search-operate-all' code cleanup, no functional changes Dmitry Kurochkin
@ 2012-01-27 11:56 ` David Bremner
0 siblings, 0 replies; 24+ messages in thread
From: David Bremner @ 2012-01-27 11:56 UTC (permalink / raw)
To: Dmitry Kurochkin, notmuch
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.
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v4] test: emacs: add test for `notmuch-search-operate-all'
2012-01-26 17:34 ` [PATCH v5 0/2] emacs: add completion to "tag all" operation Dmitry Kurochkin
` (3 preceding siblings ...)
2012-01-26 18:35 ` Austin Clements
@ 2012-01-30 7:45 ` Pieter Praet
2012-01-30 8:13 ` Dmitry Kurochkin
4 siblings, 1 reply; 24+ messages in thread
From: Pieter Praet @ 2012-01-30 7:45 UTC (permalink / raw)
To: Dmitry Kurochkin; +Cc: Notmuch Mail
`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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v4] test: emacs: add test for `notmuch-search-operate-all'
2012-01-30 7:45 ` [PATCH v4] test: emacs: add test for `notmuch-search-operate-all' Pieter Praet
@ 2012-01-30 8:13 ` Dmitry Kurochkin
2012-02-01 13:44 ` Pieter Praet
0 siblings, 1 reply; 24+ messages in thread
From: Dmitry Kurochkin @ 2012-01-30 8:13 UTC (permalink / raw)
To: Pieter Praet; +Cc: Notmuch Mail
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
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v4] test: emacs: add test for `notmuch-search-operate-all'
2012-01-30 8:13 ` Dmitry Kurochkin
@ 2012-02-01 13:44 ` Pieter Praet
2012-02-01 14:06 ` Dmitry Kurochkin
0 siblings, 1 reply; 24+ messages in thread
From: Pieter Praet @ 2012-02-01 13:44 UTC (permalink / raw)
To: Dmitry Kurochkin; +Cc: Notmuch Mail
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v4] test: emacs: add test for `notmuch-search-operate-all'
2012-02-01 13:44 ` Pieter Praet
@ 2012-02-01 14:06 ` Dmitry Kurochkin
2012-02-01 20:32 ` Pieter Praet
0 siblings, 1 reply; 24+ messages in thread
From: Dmitry Kurochkin @ 2012-02-01 14:06 UTC (permalink / raw)
To: Pieter Praet; +Cc: Notmuch Mail
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v4] test: emacs: add test for `notmuch-search-operate-all'
2012-02-01 14:06 ` Dmitry Kurochkin
@ 2012-02-01 20:32 ` Pieter Praet
2012-02-01 23:25 ` Dmitry Kurochkin
0 siblings, 1 reply; 24+ messages in thread
From: Pieter Praet @ 2012-02-01 20:32 UTC (permalink / raw)
To: Dmitry Kurochkin; +Cc: Notmuch Mail
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v4] test: emacs: add test for `notmuch-search-operate-all'
2012-02-01 20:32 ` Pieter Praet
@ 2012-02-01 23:25 ` Dmitry Kurochkin
2012-02-19 20:42 ` Pieter Praet
0 siblings, 1 reply; 24+ messages in thread
From: Dmitry Kurochkin @ 2012-02-01 23:25 UTC (permalink / raw)
To: Pieter Praet; +Cc: Notmuch Mail
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v4] test: emacs: add test for `notmuch-search-operate-all'
2012-02-01 23:25 ` Dmitry Kurochkin
@ 2012-02-19 20:42 ` Pieter Praet
0 siblings, 0 replies; 24+ messages in thread
From: Pieter Praet @ 2012-02-19 20:42 UTC (permalink / raw)
To: Dmitry Kurochkin; +Cc: Notmuch Mail
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"
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2012-02-19 20:45 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-26 1:12 [PATCH] emacs: add completion to "tag all" operation ("*" binding) Dmitry Kurochkin
2012-01-26 1:26 ` [PATCH v2] " Dmitry Kurochkin
2012-01-26 1:37 ` [PATCH] " Austin Clements
2012-01-26 1:47 ` Dmitry Kurochkin
2012-01-26 8:46 ` David Edmondson
2012-01-26 17:39 ` Dmitry Kurochkin
2012-01-26 2:45 ` [PATCH v3] " Dmitry Kurochkin
2012-01-26 5:06 ` [PATCH v4] " Dmitry Kurochkin
2012-01-26 7:04 ` Austin Clements
2012-01-26 17:37 ` Dmitry Kurochkin
2012-01-26 17:34 ` [PATCH v5 0/2] emacs: add completion to "tag all" operation Dmitry Kurochkin
2012-01-26 17:34 ` [PATCH v5 1/2] emacs: add completion to "tag all" operation ("*" binding) Dmitry Kurochkin
2012-01-27 11:55 ` David Bremner
2012-01-26 17:34 ` [PATCH v5 2/2] emacs: `notmuch-search-operate-all' code cleanup, no functional changes Dmitry Kurochkin
2012-01-27 11:56 ` David Bremner
2012-01-26 18:18 ` [PATCH v5 0/2] emacs: add completion to "tag all" operation David Edmondson
2012-01-26 18:35 ` Austin Clements
2012-01-30 7:45 ` [PATCH v4] test: emacs: add test for `notmuch-search-operate-all' Pieter Praet
2012-01-30 8:13 ` Dmitry Kurochkin
2012-02-01 13:44 ` Pieter Praet
2012-02-01 14:06 ` Dmitry Kurochkin
2012-02-01 20:32 ` Pieter Praet
2012-02-01 23:25 ` Dmitry Kurochkin
2012-02-19 20:42 ` Pieter Praet
Code repositories for project(s) associated with this public inbox
https://yhetil.org/notmuch.git/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).