Re: [PATCH 8/8] emacs: Remove interactive behavior of `notmuch-tag'

[Jameson Graef Rollins <jrollins@finestructure.net>]

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

On Tue, Oct 22 2013, Austin Clements <amdragon@MIT.EDU> wrote:
> We no longer use this, since we've lifted all interactive behavior to
> the appropriate interactive entry points.  Because of this,
> `notmuch-tag' also no longer needs to return the tag changes list,
> since the caller always passes it in.
> ---
>  emacs/notmuch-tag.el | 19 ++++---------------
>  1 file changed, 4 insertions(+), 15 deletions(-)
>
> diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
> index f9c1740..feee17c 100644
> --- a/emacs/notmuch-tag.el
> +++ b/emacs/notmuch-tag.el
> @@ -249,27 +249,18 @@ from TAGS if present."
>  	   (error "Changed tag must be of the form `+this_tag' or `-that_tag'")))))
>      (sort result-tags 'string<)))
>  
> -(defun notmuch-tag (query &optional tag-changes)
> +(defun notmuch-tag (query tag-changes)
>    "Add/remove tags in TAG-CHANGES to messages matching QUERY.
>  
>  QUERY should be a string containing the search-terms.
> -TAG-CHANGES can take multiple forms.  If TAG-CHANGES is a list of
> -strings of the form \"+tag\" or \"-tag\" then those are the tag
> -changes applied.  If TAG-CHANGES is a string then it is
> -interpreted as a single tag change.  If TAG-CHANGES is the string
> -\"-\" or \"+\", or null, then the user is prompted to enter the
> -tag changes.
> +TAG-CHANGES is a list of strings of the form \"+tag\" or
> +\"-tag\" to add or remove tags, respectively.
>  
>  Note: Other code should always use this function alter tags of
>  messages instead of running (notmuch-call-notmuch-process \"tag\" ..)
>  directly, so that hooks specified in notmuch-before-tag-hook and
>  notmuch-after-tag-hook will be run."
>    ;; Perform some validation
> -  (if (string-or-null-p tag-changes)
> -      (if (or (string= tag-changes "-") (string= tag-changes "+") (null tag-changes))
> -	  (setq tag-changes (notmuch-read-tag-changes
> -			     (notmuch-tag-completions query) nil tag-changes))
> -	(setq tag-changes (list tag-changes))))

Hey folks.  After a recent upgrade to 0.16+120~gfd733a4+1 I found that
my custom tagging functions weren't working, which I traced back to the
removal of this section.

Previously notmuch-tag accepted tag changes in two forms: as a list of
tag changes:

(list "+foo" "-bar")

or as a space-separated string:

"+foo -bar"

This removed section is what would turn the space-separated string into
a list.

I have custom tagging functions that were written like this:

(notmuch-search-tag "+spam")

notmuch-search-tag passes the TAG-CHANGES to notmuch-tag, which was
changing it to a list in this removed section.  Since its removal, all
of my custom functions started throwing the following error:

Wrong type argument: stringp, 43

I didn't pay super close attention to the rest of the patch series, but
From a backwards compatibility point of view, do we really need to
remove this section?  Is there a problem with specifying tag changes as
a string?  I think it was actually me that last modified notmuch-tag,
and I vaguely remember spending time thinking about how to preserve that
particular way to specify the changes, so that things would be simpler
for simple tag operations.

In any event, some amount of backwards compatibility has been removed,
so if we do want to keep it removed, we should add a good NEWS item to
explain that peoples custom bindings might break.

jamie.

>    (mapc (lambda (tag-change)
>  	  (unless (string-match-p "^[-+]\\S-+$" tag-change)
>  	    (error "Tag must be of the form `+this_tag' or `-that_tag'")))
> @@ -278,9 +269,7 @@ notmuch-after-tag-hook will be run."
>      (run-hooks 'notmuch-before-tag-hook)
>      (apply 'notmuch-call-notmuch-process "tag"
>  	   (append tag-changes (list "--" query)))
> -    (run-hooks 'notmuch-after-tag-hook))
> -  ;; in all cases we return tag-changes as a list
> -  tag-changes)
> +    (run-hooks 'notmuch-after-tag-hook)))
>  
>  (defun notmuch-tag-change-list (tags &optional reverse)
>    "Convert TAGS into a list of tag changes.
> -- 
> 1.8.4.rc3
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

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