1 Return-Path: <m.walters@qmul.ac.uk>
\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 B3911431FB6
\r
6 for <notmuch@notmuchmail.org>; Tue, 4 Jun 2013 14:12:30 -0700 (PDT)
\r
7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org
\r
11 X-Spam-Status: No, score=1.401 tagged_above=-999 required=5
\r
12 tests=[DKIM_ADSP_CUSTOM_MED=0.001, FREEMAIL_FROM=0.001,
\r
13 FREEMAIL_REPLY=2.499, NML_ADSP_CUSTOM_MED=1.2, RCVD_IN_DNSWL_MED=-2.3]
\r
15 Received: from olra.theworths.org ([127.0.0.1])
\r
16 by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)
\r
17 with ESMTP id I7Me27Q0KajJ for <notmuch@notmuchmail.org>;
\r
18 Tue, 4 Jun 2013 14:12:23 -0700 (PDT)
\r
19 Received: from mail2.qmul.ac.uk (mail2.qmul.ac.uk [138.37.6.6])
\r
20 (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits))
\r
21 (No client certificate requested)
\r
22 by olra.theworths.org (Postfix) with ESMTPS id F351C431FAE
\r
23 for <notmuch@notmuchmail.org>; Tue, 4 Jun 2013 14:12:22 -0700 (PDT)
\r
24 Received: from smtp.qmul.ac.uk ([138.37.6.40])
\r
25 by mail2.qmul.ac.uk with esmtp (Exim 4.71)
\r
26 (envelope-from <m.walters@qmul.ac.uk>)
\r
27 id 1UjyWc-0003Ff-Nt; Tue, 04 Jun 2013 22:12:17 +0100
\r
28 Received: from 93-97-24-31.zone5.bethere.co.uk ([93.97.24.31] helo=localhost)
\r
29 by smtp.qmul.ac.uk with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.71)
\r
30 (envelope-from <m.walters@qmul.ac.uk>)
\r
31 id 1UjyWc-0003O5-2J; Tue, 04 Jun 2013 22:12:14 +0100
\r
32 From: Mark Walters <markwalters1009@gmail.com>
\r
33 To: Tomi Ollila <tomi.ollila@iki.fi>, notmuch@notmuchmail.org
\r
34 Subject: Re: [PATCH] emacs: search: allow command line args as part of query
\r
35 In-Reply-To: <m28v2p7jg9.fsf@guru.guru-group.fi>
\r
36 References: <1370292776-24535-1-git-send-email-markwalters1009@gmail.com>
\r
37 <1370359319-2140-1-git-send-email-markwalters1009@gmail.com>
\r
38 <m28v2p7jg9.fsf@guru.guru-group.fi>
\r
39 User-Agent: Notmuch/0.14+255~gff3cc55 (http://notmuchmail.org) Emacs/23.4.1
\r
40 (x86_64-pc-linux-gnu)
\r
41 Date: Tue, 04 Jun 2013 22:12:12 +0100
\r
42 Message-ID: <87bo7loa1f.fsf@qmul.ac.uk>
\r
44 Content-Type: text/plain; charset=us-ascii
\r
45 X-Sender-Host-Address: 93.97.24.31
\r
46 X-QM-SPAM-Info: Sender has good ham record. :)
\r
47 X-QM-Body-MD5: 1a032ec29fed26dbff244980b766a925 (of first 20000 bytes)
\r
48 X-SpamAssassin-Score: 0.6
\r
49 X-SpamAssassin-SpamBar: /
\r
50 X-SpamAssassin-Report: The QM spam filters have analysed this message to
\r
52 spam. We require at least 5.0 points to mark a message as spam.
\r
53 This message scored 0.6 points. Summary of the scoring:
\r
54 * 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail
\r
55 provider * (markwalters1009[at]gmail.com)
\r
56 * 1.0 FREEMAIL_REPLY From and body contain different freemails
\r
57 * -0.4 AWL AWL: From: address is in the auto white-list
\r
58 X-QM-Scan-Virus: ClamAV says the message is clean
\r
59 X-BeenThere: notmuch@notmuchmail.org
\r
60 X-Mailman-Version: 2.1.13
\r
62 List-Id: "Use and development of the notmuch mail system."
\r
63 <notmuch.notmuchmail.org>
\r
64 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,
\r
65 <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>
\r
66 List-Archive: <http://notmuchmail.org/pipermail/notmuch>
\r
67 List-Post: <mailto:notmuch@notmuchmail.org>
\r
68 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>
\r
69 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,
\r
70 <mailto:notmuch-request@notmuchmail.org?subject=subscribe>
\r
71 X-List-Received-Date: Tue, 04 Jun 2013 21:12:30 -0000
\r
75 > On Tue, Jun 04 2013, Mark Walters <markwalters1009@gmail.com> wrote:
\r
77 >> This allows command line arguments for notmuch-search to be part of
\r
78 >> the query-string.
\r
80 > Nice feature -- some comments below:
\r
83 Thanks for the review! replies below
\r
85 >> The string must be of the form
\r
86 >> [:blank:]*--cli-arguments -- query. I hope this doesn't clash with
\r
88 > One problem requiring trailing '--' noticed after reviewed -- if one
\r
89 > forgets that the whole string is going to be query string.
\r
91 One easy fix would be to require any query starting with --something to
\r
92 be prefixed by "-- ". We could also just assume the query starts if we
\r
93 see a term not prefixed with -- or having a "--" term. What do you think?
\r
95 >> xapian: I believe that queries shouldn't start with a "-".
\r
96 >> The cli-arguments must be arguments in a whitelist of arguments: this
\r
97 >> adds a slight maintenance burden but means we don't have to deal with
\r
98 >> users who passed "--format=text" or other incompatible options.
\r
100 >> Correctly parsed example queries are
\r
101 >> --sort=oldest-first -- tag:inbox
\r
102 >> --exclude=false -- from:fred
\r
104 >> Some options (currently only sort-order) we parse in emacs, the rest
\r
105 >> we just pass to the cli. In light testing it seems to work.
\r
107 >> A full custom parser would be nicer but at least here we are only parsing
\r
108 >> the non-query part of a string which is relatively simple: indeed we
\r
109 >> already do that in the c code.
\r
111 >> We could just implement the option for sort-order, but I thought for
\r
112 >> interface consistency making all the sensible options (sort-order
\r
113 >> exclude limit and offset) work was worth the extra hassle.
\r
116 >> This is a slight change to
\r
117 >> 1370292776-24535-1-git-send-email-markwalters1009@gmail.com The change
\r
118 >> is that we add a whitelist of allowed cli options; other options are
\r
119 >> removed and the user is warned (but the query is not aborted).
\r
121 >> One other tiny change is that a query starting with "[[:blank:]]*-- "
\r
122 >> is allowed: everything after the -- is part of the real qeury so if
\r
123 >> any strange query is accidentally being misparsed the user can prefix
\r
124 >> with "-- " and it will give the current behaviour.
\r
131 >> emacs/notmuch-hello.el | 5 +++--
\r
132 >> emacs/notmuch-lib.el | 44 ++++++++++++++++++++++++++++++++++++++++++++
\r
133 >> emacs/notmuch.el | 36 +++++++++++++++++++++++++-----------
\r
134 >> 3 files changed, 72 insertions(+), 13 deletions(-)
\r
136 >> diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
\r
137 >> index c1c6f4b..bcc1843 100644
\r
138 >> --- a/emacs/notmuch-hello.el
\r
139 >> +++ b/emacs/notmuch-hello.el
\r
140 >> @@ -383,10 +383,11 @@ options will be handled as specified for
\r
141 >> `notmuch-hello-insert-searches'."
\r
142 >> (with-temp-buffer
\r
143 >> (dolist (elem query-alist nil)
\r
144 >> - (let ((count-query (if (consp (cdr elem))
\r
145 >> + (let* ((full-count-query (if (consp (cdr elem))
\r
146 >> ;; do we have a different query for the message count?
\r
150 >> + (count-query (car (notmuch-parse-query full-count-query))))
\r
152 >> (notmuch-hello-filtered-query count-query
\r
153 >> (or (plist-get options :filter-count)
\r
154 >> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
\r
155 >> index 28f78e0..65c489e 100644
\r
156 >> --- a/emacs/notmuch-lib.el
\r
157 >> +++ b/emacs/notmuch-lib.el
\r
158 >> @@ -189,6 +189,50 @@ user-friendly queries."
\r
159 >> "Return a query that matches the message with id ID."
\r
160 >> (concat "id:" (notmuch-escape-boolean-term id)))
\r
162 >> +(defun notmuch-search-parse-sort-order (args oldest-first)
\r
163 >> + (dolist (arg args nil)
\r
164 >> + (when (equal arg "--sort=oldest-first")
\r
165 >> + (setq oldest-first t))
\r
166 >> + (when (equal arg "--sort=newest-first")
\r
167 >> + (setq oldest-first nil)))
\r
168 >> + (setq args (delete "--sort=oldest-first" args))
\r
169 >> + (setq args (delete "--sort=newest-first" args))
\r
170 >> + (cons oldest-first args))
\r
172 > If one gave --sort=oldest-first --sort=newest-first oldest-first is
\r
173 > chosen instead of newest-first as both are removed from args -- that
\r
174 > should be pretty easy to fix by putting deletes inside whens.
\r
176 I think this one is ok as the delete is outside the dolist.
\r
178 >> +(defvar notmuch-parse-option-whitelist
\r
179 >> + '("^--sort=oldest-first$"
\r
180 >> + "^--sort=newest-first$"
\r
181 >> + "^--exclude=true$"
\r
182 >> + "^--exclude=false$"
\r
183 >> + "^--exclude=flag$"
\r
184 >> + "^--limit=[0-9]*$"
\r
185 >> + "^--offset=[0-9]*$"
\r
188 > is zero numbers of numbers for --limit & --offset good ([0-9]*) ?
\r
190 You are quite right (ie + would be better).
\r
193 >> +(defun notmuch-parse-in-whitelist-p (arg)
\r
194 >> + (let ((allowed nil))
\r
195 >> + (dolist (opt notmuch-parse-option-whitelist nil)
\r
196 >> + (setq allowed (or allowed (string-match-p opt arg))))
\r
199 >> +(defun notmuch-parse-query (query)
\r
200 >> + "Parse a query into a search and cli arguments
\r
202 >> +Returns a list consisting of query followed by the cli-args (as a
\r
203 >> +list). If the string does not have cli-args then this will be nil."
\r
205 >> + (if (or (string-match "^[[:blank:]]*--.*? -- " query)
\r
206 >> + (string-match "^[[:blank:]]*-- " query))
\r
207 >> + (let ((actual-query (substring query (match-end 0)))
\r
208 >> + (args (split-string (match-string 0 query) " " t)))
\r
210 > Should the whitespace matching in locations above be consistent:
\r
211 > like "^[[:blank:]]*--.*?[[:blank:]]--[[:blank:]]". Blank matches
\r
212 > space & tab (according to http://www.gnu.org/software/emacs/manual/html_node/elisp/Char-Classes.html#Char-Classes )
\r
214 > Hmm, learned a bit:
\r
216 > (split-string " foo bar ") -> ("foo" "bar")
\r
217 > (split-string " foo bar " " ") -> ("" "" "foo" "" "" "bar" "" "")
\r
218 > (split-string " foo bar " " " t) -> ("foo" "bar")
\r
219 > (split-string " foo bar " " +") -> ("" "foo" "bar" "")
\r
220 > (split-string " foo bar " " +" t) -> ("foo" "bar")
\r
222 > -> ... (args (split-string (match-string 0 query) "[[:blank:]]" t)))
\r
224 These are both clear improvements.
\r
226 >> + (message "Parsing query")
\r
227 >> + (dolist (arg args nil)
\r
228 >> + (unless (notmuch-parse-in-whitelist-p arg)
\r
229 >> + (setq args (delete arg args))
\r
230 >> + (message "Removing unknown option %s" arg)))
\r
232 > And finally, I'd suggest it is an error to encounter unknown
\r
233 > option and in that case operation is aborted.
\r
235 I think this would be better. I am not quite sure how to implement it,
\r
236 and I wasn't sure what notmuch-hello should do when displaying counts of
\r
237 an "illegal" saved search. What do you think?
\r
251 >> + (cons actual-query args))
\r
252 >> + ;; no cli arguments
\r
253 >> + (list query)))
\r
256 >> (defun notmuch-common-do-stash (text)
\r
257 >> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
\r
258 >> index 7994d74..6a4052e 100644
\r
259 >> --- a/emacs/notmuch.el
\r
260 >> +++ b/emacs/notmuch.el
\r
261 >> @@ -255,6 +255,7 @@ For a mouse binding, return nil."
\r
262 >> (notmuch-common-do-stash (notmuch-search-find-thread-id)))
\r
264 >> (defvar notmuch-search-query-string)
\r
265 >> +(defvar notmuch-search-query-args)
\r
266 >> (defvar notmuch-search-target-thread)
\r
267 >> (defvar notmuch-search-target-line)
\r
268 >> (defvar notmuch-search-continuation)
\r
269 >> @@ -409,6 +410,7 @@ Complete list of currently available key bindings:
\r
271 >> (kill-all-local-variables)
\r
272 >> (make-local-variable 'notmuch-search-query-string)
\r
273 >> + (make-local-variable 'notmuch-search-query-args)
\r
274 >> (make-local-variable 'notmuch-search-oldest-first)
\r
275 >> (make-local-variable 'notmuch-search-target-thread)
\r
276 >> (make-local-variable 'notmuch-search-target-line)
\r
277 >> @@ -897,7 +899,7 @@ PROMPT is the string to prompt with."
\r
278 >> 'notmuch-search-history nil nil)))))
\r
281 >> -(defun notmuch-search (&optional query oldest-first target-thread target-line continuation)
\r
282 >> +(defun notmuch-search (&optional query oldest-first target-thread target-line continuation cli-args)
\r
283 >> "Run \"notmuch search\" with the given `query' and display results.
\r
285 >> If `query' is nil, it is read interactively from the minibuffer.
\r
286 >> @@ -909,13 +911,20 @@ Other optional parameters are used as follows:
\r
287 >> target-line: The line number to move to if the target thread does not
\r
288 >> appear in the search results."
\r
290 >> - (let* ((query (or query (notmuch-read-query "Notmuch search: ")))
\r
291 >> + (let* ((full-query (or query (notmuch-read-query "Notmuch search: ")))
\r
292 >> + (parsed-query (notmuch-parse-query full-query))
\r
293 >> + (query (car parsed-query))
\r
294 >> + (cli-args (or cli-args (cdr parsed-query)))
\r
295 >> + (combined-order-query (notmuch-search-parse-sort-order cli-args oldest-first))
\r
296 >> + (oldest-first (car combined-order-query))
\r
297 >> + (cli-args (cdr combined-order-query))
\r
298 >> (buffer (get-buffer-create (notmuch-search-buffer-title query))))
\r
299 >> (switch-to-buffer buffer)
\r
300 >> (notmuch-search-mode)
\r
301 >> ;; Don't track undo information for this buffer
\r
302 >> (set 'buffer-undo-list t)
\r
303 >> (set 'notmuch-search-query-string query)
\r
304 >> + (set 'notmuch-search-query-args cli-args)
\r
305 >> (set 'notmuch-search-oldest-first oldest-first)
\r
306 >> (set 'notmuch-search-target-thread target-thread)
\r
307 >> (set 'notmuch-search-target-line target-line)
\r
308 >> @@ -928,13 +937,13 @@ Other optional parameters are used as follows:
\r
310 >> (goto-char (point-min))
\r
312 >> - (let ((proc (notmuch-start-notmuch
\r
313 >> + (let ((proc (apply #'notmuch-start-notmuch
\r
314 >> "notmuch-search" buffer #'notmuch-search-process-sentinel
\r
315 >> "search" "--format=sexp" "--format-version=1"
\r
316 >> - (if oldest-first
\r
317 >> + (if notmuch-search-oldest-first
\r
318 >> "--sort=oldest-first"
\r
319 >> "--sort=newest-first")
\r
321 >> + (append cli-args (list query))))
\r
322 >> ;; Use a scratch buffer to accumulate partial output.
\r
323 >> ;; This buffer will be killed by the sentinel, which
\r
324 >> ;; should be called no matter how the process dies.
\r
325 >> @@ -957,9 +966,10 @@ same relative position within the new buffer."
\r
326 >> (oldest-first notmuch-search-oldest-first)
\r
327 >> (target-thread (notmuch-search-find-thread-id 'bare))
\r
328 >> (query notmuch-search-query-string)
\r
329 >> - (continuation notmuch-search-continuation))
\r
330 >> + (continuation notmuch-search-continuation)
\r
331 >> + (cli-args notmuch-search-query-args))
\r
332 >> (notmuch-kill-this-buffer)
\r
333 >> - (notmuch-search query oldest-first target-thread target-line continuation)
\r
334 >> + (notmuch-search query oldest-first target-thread target-line continuation cli-args)
\r
335 >> (goto-char (point-min))))
\r
337 >> (defcustom notmuch-poll-script nil
\r
338 >> @@ -1024,18 +1034,22 @@ search."
\r
339 >> (set 'notmuch-search-oldest-first (not notmuch-search-oldest-first))
\r
340 >> (notmuch-search-refresh-view))
\r
342 >> -(defun notmuch-search-filter (query)
\r
343 >> +(defun notmuch-search-filter (full-query)
\r
344 >> "Filter the current search results based on an additional query string.
\r
346 >> Runs a new search matching only messages that match both the
\r
347 >> current search results AND the additional query string provided."
\r
348 >> (interactive (list (notmuch-read-query "Filter search: ")))
\r
349 >> - (let ((grouped-query (if (string-match-p notmuch-search-disjunctive-regexp query)
\r
350 >> + (let* ((parsed-query (notmuch-parse-query full-query))
\r
351 >> + (query (car parsed-query))
\r
352 >> + (grouped-query (if (string-match-p notmuch-search-disjunctive-regexp query)
\r
353 >> (concat "( " query " )")
\r
356 >> + (extra-cli-args (cdr parsed-query))
\r
357 >> + (cli-args (append notmuch-search-query-args extra-cli-args)))
\r
358 >> (notmuch-search (if (string= notmuch-search-query-string "*")
\r
360 >> - (concat notmuch-search-query-string " and " grouped-query)) notmuch-search-oldest-first)))
\r
361 >> + (concat notmuch-search-query-string " and " grouped-query)) notmuch-search-oldest-first nil nil nil cli-args)))
\r
363 >> (defun notmuch-search-filter-by-tag (tag)
\r
364 >> "Filter the current search results based on a single tag.
\r
368 >> _______________________________________________
\r
369 >> notmuch mailing list
\r
370 >> notmuch@notmuchmail.org
\r
371 >> http://notmuchmail.org/mailman/listinfo/notmuch
\r