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

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

Date: Sun, 08 Apr 2012 04:56:00 +0100

To: Jameson Graef Rollins, Notmuch Mail

Cc:

From: Mark Walters


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

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

seems to only do the funcall when the tags change.

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

Best wishes

Mark

Thread: