From cb83ea34da244f828beda04c0e242fe02986ec84 Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Wed, 13 Nov 2013 18:18:17 +1900 Subject: [PATCH] Re: [PATCH 8/8] emacs: Remove interactive behavior of `notmuch-tag' --- 74/7aadc0f76f7e2ee1e092ac46ebbf8ecb20cab9 | 204 ++++++++++++++++++++++ 1 file changed, 204 insertions(+) create mode 100644 74/7aadc0f76f7e2ee1e092ac46ebbf8ecb20cab9 diff --git a/74/7aadc0f76f7e2ee1e092ac46ebbf8ecb20cab9 b/74/7aadc0f76f7e2ee1e092ac46ebbf8ecb20cab9 new file mode 100644 index 000000000..8b5f758a7 --- /dev/null +++ b/74/7aadc0f76f7e2ee1e092ac46ebbf8ecb20cab9 @@ -0,0 +1,204 @@ +Return-Path: +X-Original-To: notmuch@notmuchmail.org +Delivered-To: notmuch@notmuchmail.org +Received: from localhost (localhost [127.0.0.1]) + by olra.theworths.org (Postfix) with ESMTP id ACDFD431FD7 + for ; Tue, 12 Nov 2013 15:18:26 -0800 (PST) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: -0.7 +X-Spam-Level: +X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5 + tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled +Received: from olra.theworths.org ([127.0.0.1]) + by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) + with ESMTP id XmqYAmR2v1iX for ; + Tue, 12 Nov 2013 15:18:21 -0800 (PST) +Received: from dmz-mailsec-scanner-6.mit.edu (dmz-mailsec-scanner-6.mit.edu + [18.7.68.35]) + (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) + (No client certificate requested) + by olra.theworths.org (Postfix) with ESMTPS id E1292431FCF + for ; Tue, 12 Nov 2013 15:18:20 -0800 (PST) +X-AuditID: 12074423-b7f2b6d000000ce1-d9-5282b73c0979 +Received: from mailhub-auth-1.mit.edu ( [18.9.21.35]) + (using TLS with cipher AES256-SHA (256/256 bits)) + (Client did not present a certificate) + by dmz-mailsec-scanner-6.mit.edu (Symantec Messaging Gateway) with SMTP + id D7.75.03297.C37B2825; Tue, 12 Nov 2013 18:18:20 -0500 (EST) +Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11]) + by mailhub-auth-1.mit.edu (8.13.8/8.9.2) with ESMTP id rACNIIHa012266; + Tue, 12 Nov 2013 18:18:19 -0500 +Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91]) + (authenticated bits=0) + (User authenticated as amdragon@ATHENA.MIT.EDU) + by outgoing.mit.edu (8.13.8/8.12.4) with ESMTP id rACNIHIZ001869 + (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT); + Tue, 12 Nov 2013 18:18:18 -0500 +Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.80) + (envelope-from ) + id 1VgNDt-0007DC-46; Tue, 12 Nov 2013 18:18:17 -0500 +Date: Tue, 12 Nov 2013 18:18:17 -0500 +From: Austin Clements +To: Jameson Graef Rollins +Subject: Re: [PATCH 8/8] emacs: Remove interactive behavior of `notmuch-tag' +Message-ID: <20131112231817.GE13399@mit.edu> +References: <1382471457-26056-1-git-send-email-amdragon@mit.edu> + <1382471457-26056-9-git-send-email-amdragon@mit.edu> + <8761s9gfpm.fsf@servo.finestructure.net> +MIME-Version: 1.0 +Content-Type: text/plain; charset=us-ascii +Content-Disposition: inline +In-Reply-To: <8761s9gfpm.fsf@servo.finestructure.net> +User-Agent: Mutt/1.5.21 (2010-09-15) +X-Brightmail-Tracker: + H4sIAAAAAAAAA+NgFmpileLIzCtJLcpLzFFi42IR4hRV1rXZ3hRkMG8Or8WefV4W12/OZHZg + 8rh7msvj2apbzAFMUVw2Kak5mWWpRfp2CVwZOz5/YC9o06mYd30uSwNjn3IXIyeHhICJxO1d + G5ggbDGJC/fWs3UxcnEICcxmkpi0+hM7hLORUeLfuh1QmdNMElseb2KGcJYAZY4eZQbpZxFQ + lfj+eTnYLDYBDYlt+5czgtgiAmYSPV/+gNnMAtIS3343A9VwcAgL+EhsmsQLEuYV0JE43b2b + CWLmIkaJM3+Xs0MkBCVOznzCAtGrJXHj30uwXpA5y/9xgIQ5BUwl9r7exgZiiwqoSEw5uY1t + AqPQLCTds5B0z0LoXsDIvIpRNiW3Sjc3MTOnODVZtzg5MS8vtUjXTC83s0QvNaV0EyMoqNld + lHcw/jmodIhRgINRiYfXIqYpSIg1say4MvcQoyQHk5Ior/xWoBBfUn5KZUZicUZ8UWlOavEh + RgkOZiUR3gMgOd6UxMqq1KJ8mJQ0B4uSOO8tDvsgIYH0xJLU7NTUgtQimKwMB4eSBG8KSKNg + UWp6akVaZk4JQpqJgxNkOA/Q8Daw4cUFibnFmekQ+VOMilLivEu3ASUEQBIZpXlwvbCk84pR + HOgVYV4GkCoeYMKC634FNJgJaLBFMdjgkkSElFQDI8tsv3aF7zW2Ow4qvlghNalK567jjkN2 + IheDz6rOeMRgcOBY+cYrzDEuygduavzSEPi/LiHNJFnSXPpE77LLNgv0Cq7s+Ne7spKF477w + FTm+Zz+489livZb4XJ1wKk13Qkn6j+45x/Tnsds1Vhnzbi60tRFZk/Dq4uumVYVSd1X6ov+o + StmtVmIpzkg01GIuKk4EAAr6cJ0VAwAA +Cc: notmuch@notmuchmail.org +X-BeenThere: notmuch@notmuchmail.org +X-Mailman-Version: 2.1.13 +Precedence: list +List-Id: "Use and development of the notmuch mail system." + +List-Unsubscribe: , + +List-Archive: +List-Post: +List-Help: +List-Subscribe: , + +X-List-Received-Date: Tue, 12 Nov 2013 23:18:26 -0000 + +Quoth Jameson Graef Rollins on Nov 03 at 4:42 pm: +> On Tue, Oct 22 2013, Austin Clements 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. + +The difficulty is that notmuch-tag used to play a dual role of both +parsing the space-separated string into a list of tag changes and +actually performing the tagging operation and, as a result, other +functions that were rooted in notmuch-tag also accepted both (though +functions that weren't, like notmuch-update-tags, required the list +form). This dual role was strange from an API perspective, but also, +because of other restructuring, notmuch-tag is no longer at the core +of many operations, so it's not in a good position to be a parser. + +Hence, while we could make `notmuch-tag' itself support the string +form, if we wanted this to work from other tagging entry-points, we'd +need to support both forms in all of the tagging functions. This +would be doable, but it would be messy, and without the single parsing +point it would be difficult to maintain. + +> 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. + +I'd rather keep the backwards compatibility removed, but you're +definitely right that we need a good NEWS item for it. I've posted a +NEWS item in id:1384296252-17884-1-git-send-email-amdragon@mit.edu, +but let me know if you think it can be clearer or more helpful. + +> 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. +> > +> > _______________________________________________ +> > notmuch mailing list +> > notmuch@notmuchmail.org +> > http://notmuchmail.org/mailman/listinfo/notmuch -- 2.26.2