1 Return-Path: <tomi.ollila@iki.fi>
\r
2 X-Original-To: notmuch@notmuchmail.org
\r
3 Delivered-To: notmuch@notmuchmail.org
\r
4 Received: from localhost (localhost [127.0.0.1])
\r
5 by olra.theworths.org (Postfix) with ESMTP id 76FCC431FBC
\r
6 for <notmuch@notmuchmail.org>; Wed, 21 May 2014 11:27:22 -0700 (PDT)
\r
7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org
\r
11 X-Spam-Status: No, score=0 tagged_above=-999 required=5 tests=[none]
\r
13 Received: from olra.theworths.org ([127.0.0.1])
\r
14 by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)
\r
15 with ESMTP id LPYeizRPEoDt for <notmuch@notmuchmail.org>;
\r
16 Wed, 21 May 2014 11:27:15 -0700 (PDT)
\r
17 Received: from guru.guru-group.fi (guru.guru-group.fi [46.183.73.34])
\r
18 by olra.theworths.org (Postfix) with ESMTP id 93A8A431FAE
\r
19 for <notmuch@notmuchmail.org>; Wed, 21 May 2014 11:27:15 -0700 (PDT)
\r
20 Received: from guru.guru-group.fi (localhost [IPv6:::1])
\r
21 by guru.guru-group.fi (Postfix) with ESMTP id DCC8D100090;
\r
22 Wed, 21 May 2014 21:27:06 +0300 (EEST)
\r
23 From: Tomi Ollila <tomi.ollila@iki.fi>
\r
24 To: Mark Walters <markwalters1009@gmail.com>, notmuch@notmuchmail.org,
\r
25 David Edmondson <dme@dme.org>
\r
26 Subject: Re: [PATCH] emacs: make sure tagging on an empty query is harmless
\r
27 In-Reply-To: <1400666330-4363-1-git-send-email-markwalters1009@gmail.com>
\r
28 References: <87iop0ynl2.fsf@qmul.ac.uk>
\r
29 <1400666330-4363-1-git-send-email-markwalters1009@gmail.com>
\r
30 User-Agent: Notmuch/0.18+12~g9d41f94 (http://notmuchmail.org) Emacs/24.3.1
\r
31 (x86_64-unknown-linux-gnu)
\r
32 X-Face: HhBM'cA~<r"^Xv\KRN0P{vn'Y"Kd;zg_y3S[4)KSN~s?O\"QPoL
\r
33 $[Xv_BD:i/F$WiEWax}R(MPS`^UaptOGD`*/=@\1lKoVa9tnrg0TW?"r7aRtgk[F
\r
34 !)g;OY^,BjTbr)Np:%c_o'jj,Z
\r
35 Date: Wed, 21 May 2014 21:27:06 +0300
\r
36 Message-ID: <m2wqdfgf7p.fsf@guru.guru-group.fi>
\r
38 Content-Type: text/plain
\r
39 X-BeenThere: notmuch@notmuchmail.org
\r
40 X-Mailman-Version: 2.1.13
\r
42 List-Id: "Use and development of the notmuch mail system."
\r
43 <notmuch.notmuchmail.org>
\r
44 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,
\r
45 <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>
\r
46 List-Archive: <http://notmuchmail.org/pipermail/notmuch>
\r
47 List-Post: <mailto:notmuch@notmuchmail.org>
\r
48 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>
\r
49 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,
\r
50 <mailto:notmuch-request@notmuchmail.org?subject=subscribe>
\r
51 X-List-Received-Date: Wed, 21 May 2014 18:27:22 -0000
\r
53 On Wed, May 21 2014, Mark Walters <markwalters1009@gmail.com> wrote:
\r
55 > Currently notmuch-tag throws a "wrong-type-argument stringp nil" if
\r
56 > passed a nil query-string. Catch this and provide a more useful error
\r
57 > message. This fixes a case in notmuch-tree (if you try to tag when at
\r
58 > the end of the buffer).
\r
60 > Secondly, as pointed out by David (dme)
\r
61 > `notmuch-search-find-stable-query-region' can return the query string
\r
62 > () if there are no messages in the region. This gets passed to notmuch
\r
63 > tag, and due to interactions in the optimize_query code in
\r
64 > notmuch-tag.c becomes, in the case tag-change is -inbox, "( () ) and
\r
65 > (tag:inbox)". This query matches some strange collection of messages
\r
66 > which then get archived. This should probably be fixed, but in any
\r
67 > case make `notmuch-search-find-stable-query-region' return a nil
\r
68 > query-string in this case.
\r
70 > This avoids data-loss (random tag removal) in this case.
\r
73 +1 from me for 0.18.1, too.
\r
79 > This is my attempt to solve the same problem as the parent. I prefer
\r
80 > not throwing an error in n.s.f.s.q.r as it is difficult for the caller
\r
81 > to catch cleanly. Throwing it in notmuch-tag is fine as the caller can
\r
82 > trivially check for query-string being nil before calling notmuch-tag
\r
83 > if it wants to deal with it gracefully.
\r
85 > If people do prefer an error in n.s.f.s.q.r as in the parent patch
\r
86 > then I think we should update the error message. The first hunk of
\r
87 > this should also be applied to catch nil queries to notmuch-tag gracefully.
\r
89 > Although this has been present for a while I think it is a dataloss
\r
90 > issue so a fix should go in for 0.18.1
\r
99 > emacs/notmuch-tag.el | 2 ++
\r
100 > emacs/notmuch.el | 6 ++++--
\r
101 > 2 files changed, 6 insertions(+), 2 deletions(-)
\r
103 > diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
\r
104 > index 07c260e..f54aa9d 100644
\r
105 > --- a/emacs/notmuch-tag.el
\r
106 > +++ b/emacs/notmuch-tag.el
\r
107 > @@ -387,6 +387,8 @@ (defun notmuch-tag (query tag-changes)
\r
108 > (unless (string-match-p "^[-+]\\S-+$" tag-change)
\r
109 > (error "Tag must be of the form `+this_tag' or `-that_tag'")))
\r
112 > + (error "Nothing to tag!"))
\r
113 > (unless (null tag-changes)
\r
114 > (run-hooks 'notmuch-before-tag-hook)
\r
115 > (if (<= (length query) notmuch-tag-argument-limit)
\r
116 > diff --git a/emacs/notmuch.el b/emacs/notmuch.el
\r
117 > index 6c0bc1b..1adea9c 100644
\r
118 > --- a/emacs/notmuch.el
\r
119 > +++ b/emacs/notmuch.el
\r
120 > @@ -428,14 +428,16 @@ (defun notmuch-search-find-stable-query-region (beg end &optional only-matched)
\r
121 > "Return the stable query for the current region.
\r
123 > If ONLY-MATCHED is non-nil, include only matched messages. If it
\r
124 > -is nil, include both matched and unmatched messages."
\r
125 > +is nil, include both matched and unmatched messages. If there are
\r
126 > +no messages in the region then return nil."
\r
127 > (let ((query-list nil) (all (not only-matched)))
\r
128 > (dolist (queries (notmuch-search-properties-in-region :query beg end))
\r
129 > (when (first queries)
\r
130 > (push (first queries) query-list))
\r
131 > (when (and all (second queries))
\r
132 > (push (second queries) query-list)))
\r
133 > - (concat "(" (mapconcat 'identity query-list ") or (") ")")))
\r
134 > + (when query-list
\r
135 > + (concat "(" (mapconcat 'identity query-list ") or (") ")"))))
\r
137 > (defun notmuch-search-find-authors ()
\r
138 > "Return the authors for the current thread"
\r
142 > _______________________________________________
\r
143 > notmuch mailing list
\r
144 > notmuch@notmuchmail.org
\r
145 > http://notmuchmail.org/mailman/listinfo/notmuch
\r