From: Jameson Graef Rollins <jrollins@finestructure.net>
To: Austin Clements <amdragon@MIT.EDU>, notmuch@notmuchmail.org
Subject: Re: [PATCH 8/8] emacs: Remove interactive behavior of `notmuch-tag'
In-Reply-To: <1382471457-26056-9-git-send-email-amdragon@mit.edu>
References: <1382471457-26056-1-git-send-email-amdragon@mit.edu>
	<1382471457-26056-9-git-send-email-amdragon@mit.edu>
User-Agent: Notmuch/0.16+120~gfd733a4 (http://notmuchmail.org) Emacs/24.3.1
	(x86_64-pc-linux-gnu)
Date: Sun, 03 Nov 2013 16:42:29 -0800
Message-ID: <8761s9gfpm.fsf@servo.finestructure.net>
MIME-Version: 1.0
Content-Type: multipart/signed; boundary="=-=-=";
	micalg=pgp-sha256; protocol="application/pgp-signature"
Precedence: list

--=-=-=
Content-Type: text/plain
Content-Transfer-Encoding: quoted-printable

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<)))
>=20=20
> -(defun notmuch-tag (query &optional tag-changes)
> +(defun notmuch-tag (query tag-changes)
>    "Add/remove tags in TAG-CHANGES to messages matching QUERY.
>=20=20
>  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.
>=20=20
>  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=3D tag-changes "-") (string=3D tag-changes "+") (n=
ull 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=20a 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)))
>=20=20
>  (defun notmuch-tag-change-list (tags &optional reverse)
>    "Convert TAGS into a list of tag changes.
> --=20
> 1.8.4.rc3
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

--=-=-=
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.15 (GNU/Linux)

iQIcBAEBCAAGBQJSdu12AAoJEO00zqvie6q8qm4P/iNt9rQEue2PXr+S64TIqiWE
0eotrTAfGwcGvu2HrIuRUWELc9Nvix0ZIwT+COOxPB/iNn0l8ONTeQ+ATcOrZcOV
m+H/wBWSFPOP45mYhSCbYQuWlhfO+oKM+9dOIVaHB+RIDkSfKnBgQQVusZVwp7Kw
69LJ+DPTD3J0QnYAqhHblMwEsTc8ESETKbwa72BTugBdQWlQX3A8gBS2ws9V8NM+
PBAT0eiHvnA3Dw2NM7BIqiS1MbvFqCeiHVL5b9Zt0HNEa1/nN6Rkp8th8RhSAWrQ
8kHbUlCpBFdRteXqzgS9ZpfLbiMZPmrDK8RSxL7hbkgWTKKvy7fgYHQ2W7g+pXhm
vMxuMFWhE/KJMVf4pj18NPMOaViU2843CHSYxN+sbKxHr5NrljNOvRBKNCy9nygE
TfLGUqLUI7lXEPis0Izs4X373RJBIM2HqYKgdv3IGmuuKjoslwBzFeJHSwbtJ/f8
6ZLEgu9CL3GC7FLjkjLlGE/wqExSq2slItti7do8ZhGlvJq5fmz0DSeqCIhNM9Z4
SwpvUAHRcF3Z9JXHjHDl1o9+UDFIc0pkIHEs482Vkjlrr9ymtgiSTS4Fzg0GBmbv
jR+gwYDz0FyHU00FL27rF/SGlgMAgHfFU4qt3wb88JUv1LTXikvWqQLCMNjpJJpx
b7X33elCGage5rj/zV5F
=MV+R
-----END PGP SIGNATURE-----
--=-=-=--