[PATCH] emacs: search: allow command line args as part of query
Mark Walters
markwalters1009 at gmail.com
Tue Jun 4 14:12:12 PDT 2013
> On Tue, Jun 04 2013, Mark Walters <markwalters1009 at gmail.com> wrote:
>
>> This allows command line arguments for notmuch-search to be part of
>> the query-string.
>
> Nice feature -- some comments below:
>
Thanks for the review! replies below
>> The string must be of the form
>> [:blank:]*--cli-arguments -- query. I hope this doesn't clash with
>
> One problem requiring trailing '--' noticed after reviewed -- if one
> forgets that the whole string is going to be query string.
One easy fix would be to require any query starting with --something to
be prefixed by "-- ". We could also just assume the query starts if we
see a term not prefixed with -- or having a "--" term. What do you think?
>> xapian: I believe that queries shouldn't start with a "-".
>> The cli-arguments must be arguments in a whitelist of arguments: this
>> adds a slight maintenance burden but means we don't have to deal with
>> users who passed "--format=text" or other incompatible options.
>>
>> Correctly parsed example queries are
>> --sort=oldest-first -- tag:inbox
>> --exclude=false -- from:fred
>>
>> Some options (currently only sort-order) we parse in emacs, the rest
>> we just pass to the cli. In light testing it seems to work.
>>
>> A full custom parser would be nicer but at least here we are only parsing
>> the non-query part of a string which is relatively simple: indeed we
>> already do that in the c code.
>>
>> We could just implement the option for sort-order, but I thought for
>> interface consistency making all the sensible options (sort-order
>> exclude limit and offset) work was worth the extra hassle.
>> ---
>>
>> This is a slight change to
>> 1370292776-24535-1-git-send-email-markwalters1009 at gmail.com The change
>> is that we add a whitelist of allowed cli options; other options are
>> removed and the user is warned (but the query is not aborted).
>>
>> One other tiny change is that a query starting with "[[:blank:]]*-- "
>> is allowed: everything after the -- is part of the real qeury so if
>> any strange query is accidentally being misparsed the user can prefix
>> with "-- " and it will give the current behaviour.
>>
>> Best wishes
>>
>> Mark
>>
>>
>> emacs/notmuch-hello.el | 5 +++--
>> emacs/notmuch-lib.el | 44 ++++++++++++++++++++++++++++++++++++++++++++
>> emacs/notmuch.el | 36 +++++++++++++++++++++++++-----------
>> 3 files changed, 72 insertions(+), 13 deletions(-)
>>
>> diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
>> index c1c6f4b..bcc1843 100644
>> --- a/emacs/notmuch-hello.el
>> +++ b/emacs/notmuch-hello.el
>> @@ -383,10 +383,11 @@ options will be handled as specified for
>> `notmuch-hello-insert-searches'."
>> (with-temp-buffer
>> (dolist (elem query-alist nil)
>> - (let ((count-query (if (consp (cdr elem))
>> + (let* ((full-count-query (if (consp (cdr elem))
>> ;; do we have a different query for the message count?
>> (third elem)
>> - (cdr elem))))
>> + (cdr elem)))
>> + (count-query (car (notmuch-parse-query full-count-query))))
>> (insert
>> (notmuch-hello-filtered-query count-query
>> (or (plist-get options :filter-count)
>> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
>> index 28f78e0..65c489e 100644
>> --- a/emacs/notmuch-lib.el
>> +++ b/emacs/notmuch-lib.el
>> @@ -189,6 +189,50 @@ user-friendly queries."
>> "Return a query that matches the message with id ID."
>> (concat "id:" (notmuch-escape-boolean-term id)))
>>
>> +(defun notmuch-search-parse-sort-order (args oldest-first)
>> + (dolist (arg args nil)
>> + (when (equal arg "--sort=oldest-first")
>> + (setq oldest-first t))
>> + (when (equal arg "--sort=newest-first")
>> + (setq oldest-first nil)))
>> + (setq args (delete "--sort=oldest-first" args))
>> + (setq args (delete "--sort=newest-first" args))
>> + (cons oldest-first args))
>
> If one gave --sort=oldest-first --sort=newest-first oldest-first is
> chosen instead of newest-first as both are removed from args -- that
> should be pretty easy to fix by putting deletes inside whens.
I think this one is ok as the delete is outside the dolist.
>> +(defvar notmuch-parse-option-whitelist
>> + '("^--sort=oldest-first$"
>> + "^--sort=newest-first$"
>> + "^--exclude=true$"
>> + "^--exclude=false$"
>> + "^--exclude=flag$"
>> + "^--limit=[0-9]*$"
>> + "^--offset=[0-9]*$"
>> + "^--$"))
>
> is zero numbers of numbers for --limit & --offset good ([0-9]*) ?
You are quite right (ie + would be better).
>
>> +(defun notmuch-parse-in-whitelist-p (arg)
>> + (let ((allowed nil))
>> + (dolist (opt notmuch-parse-option-whitelist nil)
>> + (setq allowed (or allowed (string-match-p opt arg))))
>> + allowed))
>> +
>> +(defun notmuch-parse-query (query)
>> + "Parse a query into a search and cli arguments
>> +
>> +Returns a list consisting of query followed by the cli-args (as a
>> +list). If the string does not have cli-args then this will be nil."
>> +
>> + (if (or (string-match "^[[:blank:]]*--.*? -- " query)
>> + (string-match "^[[:blank:]]*-- " query))
>> + (let ((actual-query (substring query (match-end 0)))
>> + (args (split-string (match-string 0 query) " " t)))
>
> Should the whitespace matching in locations above be consistent:
> like "^[[:blank:]]*--.*?[[:blank:]]--[[:blank:]]". Blank matches
> space & tab (according to http://www.gnu.org/software/emacs/manual/html_node/elisp/Char-Classes.html#Char-Classes )
>
> Hmm, learned a bit:
>
> (split-string " foo bar ") -> ("foo" "bar")
> (split-string " foo bar " " ") -> ("" "" "foo" "" "" "bar" "" "")
> (split-string " foo bar " " " t) -> ("foo" "bar")
> (split-string " foo bar " " +") -> ("" "foo" "bar" "")
> (split-string " foo bar " " +" t) -> ("foo" "bar")
>
> -> ... (args (split-string (match-string 0 query) "[[:blank:]]" t)))
These are both clear improvements.
>> + (message "Parsing query")
>> + (dolist (arg args nil)
>> + (unless (notmuch-parse-in-whitelist-p arg)
>> + (setq args (delete arg args))
>> + (message "Removing unknown option %s" arg)))
>
> And finally, I'd suggest it is an error to encounter unknown
> option and in that case operation is aborted.
I think this would be better. I am not quite sure how to implement it,
and I wasn't sure what notmuch-hello should do when displaying counts of
an "illegal" saved search. What do you think?
Best wishes
Mark
>
> Tomi
>
>
>> + (cons actual-query args))
>> + ;; no cli arguments
>> + (list query)))
>> ;;
>>
>> (defun notmuch-common-do-stash (text)
>> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
>> index 7994d74..6a4052e 100644
>> --- a/emacs/notmuch.el
>> +++ b/emacs/notmuch.el
>> @@ -255,6 +255,7 @@ For a mouse binding, return nil."
>> (notmuch-common-do-stash (notmuch-search-find-thread-id)))
>>
>> (defvar notmuch-search-query-string)
>> +(defvar notmuch-search-query-args)
>> (defvar notmuch-search-target-thread)
>> (defvar notmuch-search-target-line)
>> (defvar notmuch-search-continuation)
>> @@ -409,6 +410,7 @@ Complete list of currently available key bindings:
>> (interactive)
>> (kill-all-local-variables)
>> (make-local-variable 'notmuch-search-query-string)
>> + (make-local-variable 'notmuch-search-query-args)
>> (make-local-variable 'notmuch-search-oldest-first)
>> (make-local-variable 'notmuch-search-target-thread)
>> (make-local-variable 'notmuch-search-target-line)
>> @@ -897,7 +899,7 @@ PROMPT is the string to prompt with."
>> 'notmuch-search-history nil nil)))))
>>
>> ;;;###autoload
>> -(defun notmuch-search (&optional query oldest-first target-thread target-line continuation)
>> +(defun notmuch-search (&optional query oldest-first target-thread target-line continuation cli-args)
>> "Run \"notmuch search\" with the given `query' and display results.
>>
>> If `query' is nil, it is read interactively from the minibuffer.
>> @@ -909,13 +911,20 @@ Other optional parameters are used as follows:
>> target-line: The line number to move to if the target thread does not
>> appear in the search results."
>> (interactive)
>> - (let* ((query (or query (notmuch-read-query "Notmuch search: ")))
>> + (let* ((full-query (or query (notmuch-read-query "Notmuch search: ")))
>> + (parsed-query (notmuch-parse-query full-query))
>> + (query (car parsed-query))
>> + (cli-args (or cli-args (cdr parsed-query)))
>> + (combined-order-query (notmuch-search-parse-sort-order cli-args oldest-first))
>> + (oldest-first (car combined-order-query))
>> + (cli-args (cdr combined-order-query))
>> (buffer (get-buffer-create (notmuch-search-buffer-title query))))
>> (switch-to-buffer buffer)
>> (notmuch-search-mode)
>> ;; Don't track undo information for this buffer
>> (set 'buffer-undo-list t)
>> (set 'notmuch-search-query-string query)
>> + (set 'notmuch-search-query-args cli-args)
>> (set 'notmuch-search-oldest-first oldest-first)
>> (set 'notmuch-search-target-thread target-thread)
>> (set 'notmuch-search-target-line target-line)
>> @@ -928,13 +937,13 @@ Other optional parameters are used as follows:
>> (erase-buffer)
>> (goto-char (point-min))
>> (save-excursion
>> - (let ((proc (notmuch-start-notmuch
>> + (let ((proc (apply #'notmuch-start-notmuch
>> "notmuch-search" buffer #'notmuch-search-process-sentinel
>> "search" "--format=sexp" "--format-version=1"
>> - (if oldest-first
>> + (if notmuch-search-oldest-first
>> "--sort=oldest-first"
>> "--sort=newest-first")
>> - query))
>> + (append cli-args (list query))))
>> ;; Use a scratch buffer to accumulate partial output.
>> ;; This buffer will be killed by the sentinel, which
>> ;; should be called no matter how the process dies.
>> @@ -957,9 +966,10 @@ same relative position within the new buffer."
>> (oldest-first notmuch-search-oldest-first)
>> (target-thread (notmuch-search-find-thread-id 'bare))
>> (query notmuch-search-query-string)
>> - (continuation notmuch-search-continuation))
>> + (continuation notmuch-search-continuation)
>> + (cli-args notmuch-search-query-args))
>> (notmuch-kill-this-buffer)
>> - (notmuch-search query oldest-first target-thread target-line continuation)
>> + (notmuch-search query oldest-first target-thread target-line continuation cli-args)
>> (goto-char (point-min))))
>>
>> (defcustom notmuch-poll-script nil
>> @@ -1024,18 +1034,22 @@ search."
>> (set 'notmuch-search-oldest-first (not notmuch-search-oldest-first))
>> (notmuch-search-refresh-view))
>>
>> -(defun notmuch-search-filter (query)
>> +(defun notmuch-search-filter (full-query)
>> "Filter the current search results based on an additional query string.
>>
>> Runs a new search matching only messages that match both the
>> current search results AND the additional query string provided."
>> (interactive (list (notmuch-read-query "Filter search: ")))
>> - (let ((grouped-query (if (string-match-p notmuch-search-disjunctive-regexp query)
>> + (let* ((parsed-query (notmuch-parse-query full-query))
>> + (query (car parsed-query))
>> + (grouped-query (if (string-match-p notmuch-search-disjunctive-regexp query)
>> (concat "( " query " )")
>> - query)))
>> + query))
>> + (extra-cli-args (cdr parsed-query))
>> + (cli-args (append notmuch-search-query-args extra-cli-args)))
>> (notmuch-search (if (string= notmuch-search-query-string "*")
>> grouped-query
>> - (concat notmuch-search-query-string " and " grouped-query)) notmuch-search-oldest-first)))
>> + (concat notmuch-search-query-string " and " grouped-query)) notmuch-search-oldest-first nil nil nil cli-args)))
>>
>> (defun notmuch-search-filter-by-tag (tag)
>> "Filter the current search results based on a single tag.
>> --
>> 1.7.9.1
>>
>> _______________________________________________
>> notmuch mailing list
>> notmuch at notmuchmail.org
>> http://notmuchmail.org/mailman/listinfo/notmuch
More information about the notmuch
mailing list