* [PATCH v2] emacs: Add notmuch-show--build-queries. @ 2016-10-11 17:41 Matt Armstrong 2016-10-11 17:41 ` [PATCH v2] " Matt Armstrong 0 siblings, 1 reply; 4+ messages in thread From: Matt Armstrong @ 2016-10-11 17:41 UTC (permalink / raw) To: notmuch This supercedes id:1474003701-19831-1-git-send-email-marmstrong@google.com with a much simpler patch. My goal here is to make it easier to tweak notmuch-show behavior without hacking on notmuch-show--build-buffer itself, since it is responsible for a host of other tasks. I'm still working (slowly) on coming up with a nice mental model for the way notmuch's Emacs interface treats threads -vs- messages with respect to "expansion" in show and tree results. Although, to be honest, wiht this patch and the following piece of advice, notmuch-show buffers are doing what I want in all cases: (defun my-notmuch-show--prepend-tag-unread (queries) "Prepend a tag:unread query to QUERIES. Intended to be used as :filter-return advice on `notmuch-show--build-queries'." (cons `(,notmuch-show-thread-id "and tag:unread") queries)) (advice-add 'notmuch-show--build-queries :filter-return #'my-notmuch-show--prepend-tag-unread) ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2] Add notmuch-show--build-queries. 2016-10-11 17:41 [PATCH v2] emacs: Add notmuch-show--build-queries Matt Armstrong @ 2016-10-11 17:41 ` Matt Armstrong 2016-10-13 16:46 ` Mark Walters 0 siblings, 1 reply; 4+ messages in thread From: Matt Armstrong @ 2016-10-11 17:41 UTC (permalink / raw) To: notmuch notmuch-show--build-buffer now queries a list of queries built by the former. This simplifies the logic. It also provides an easy place to experiment with alternate sets of queries for given notmuch-show-* variables (e.g. users can use advice-add to do so in a surgical way). --- emacs/notmuch-show.el | 37 +++++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el index f2487ab..0727319 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -1261,6 +1261,20 @@ matched." (message "No messages matched the query!") nil)))) +(defun notmuch-show--build-queries () + "Return a list of queries to try for this search. + +If `notmuch-show-query-context` is not nil, the first query is +the conjunction of it and `notmuch-show-thread-id`. The next +query is `notmuch-show-thread-id` alone, and serves as a fallback +if the prior matches no messages." + (let* ((thread notmuch-show-thread-id) + (context notmuch-show-query-context) + (queries `((,thread)))) + (if context + (setq queries (cons `(,thread "and (" ,context ")") queries))) + queries)) + (defun notmuch-show--build-buffer (&optional state) "Display messages matching the current buffer context. @@ -1268,25 +1282,20 @@ Apply the previously saved STATE if supplied, otherwise show the first relevant message. If no messages match the query return NIL." - (let* ((basic-args (list notmuch-show-thread-id)) - (args (if notmuch-show-query-context - (append (list "\'") basic-args - (list "and (" notmuch-show-query-context ")\'")) - (append (list "\'") basic-args (list "\'")))) - (cli-args (cons "--exclude=false" + (let* ((cli-args (cons "--exclude=false" (when notmuch-show-elide-non-matching-messages (list "--entire-thread=false")))) - - (forest (or (notmuch-query-get-threads (append cli-args args)) - ;; If a query context reduced the number of - ;; results to zero, try again without it. - (and notmuch-show-query-context - (notmuch-query-get-threads (append cli-args basic-args))))) - + (queries (notmuch-show--build-queries)) + (forest nil) ;; Must be reset every time we are going to start inserting ;; messages into the buffer. (notmuch-show-previous-subject "")) - + ;; Use results from the first query that returns some. + (while (and (consp queries) + (not (setq forest + (notmuch-query-get-threads + `(,@cli-args "'" ,@(car queries) "'"))))) + (setq queries (cdr queries))) (when forest (notmuch-show-insert-forest forest) -- 2.8.0.rc3.226.g39d4020 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] Add notmuch-show--build-queries. 2016-10-11 17:41 ` [PATCH v2] " Matt Armstrong @ 2016-10-13 16:46 ` Mark Walters 2016-10-13 19:34 ` Matt Armstrong 0 siblings, 1 reply; 4+ messages in thread From: Mark Walters @ 2016-10-13 16:46 UTC (permalink / raw) To: Matt Armstrong, notmuch On Tue, 11 Oct 2016, Matt Armstrong <marmstrong@google.com> wrote: > notmuch-show--build-buffer now queries a list of queries built by the > former. This simplifies the logic. It also provides an easy place to > experiment with alternate sets of queries for given notmuch-show-* > variables (e.g. users can use advice-add to do so in a surgical way). Hi I think I like the overall logic, but I do have some comments below. > --- > emacs/notmuch-show.el | 37 +++++++++++++++++++++++-------------- > 1 file changed, 23 insertions(+), 14 deletions(-) > > diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el > index f2487ab..0727319 100644 > --- a/emacs/notmuch-show.el > +++ b/emacs/notmuch-show.el > @@ -1261,6 +1261,20 @@ matched." > (message "No messages matched the query!") > nil)))) > > +(defun notmuch-show--build-queries () > + "Return a list of queries to try for this search. > + > +If `notmuch-show-query-context` is not nil, the first query is > +the conjunction of it and `notmuch-show-thread-id`. The next > +query is `notmuch-show-thread-id` alone, and serves as a fallback > +if the prior matches no messages." > + (let* ((thread notmuch-show-thread-id) > + (context notmuch-show-query-context) > + (queries `((,thread)))) > + (if context > + (setq queries (cons `(,thread "and (" ,context ")") queries))) > + queries)) I think it would be better to keep closer to the existing style -- using list and append. At least do that as a first patch and then do a change as above with ` and , so we can review them separately. Note I find the list append style much easier to read, and I am guessing someone else did (perhaps cworth?) as they wrote it that way. I think I would go for something like (let* ((... queries)) (push (list thread) queries) (if context (push (list thread "and (" context ")") queries)) queries) > + > (defun notmuch-show--build-buffer (&optional state) > "Display messages matching the current buffer context. > > @@ -1268,25 +1282,20 @@ Apply the previously saved STATE if supplied, otherwise show the > first relevant message. > > If no messages match the query return NIL." > - (let* ((basic-args (list notmuch-show-thread-id)) > - (args (if notmuch-show-query-context > - (append (list "\'") basic-args > - (list "and (" notmuch-show-query-context ")\'")) > - (append (list "\'") basic-args (list "\'")))) > - (cli-args (cons "--exclude=false" > + (let* ((cli-args (cons "--exclude=false" > (when notmuch-show-elide-non-matching-messages > (list "--entire-thread=false")))) > - > - (forest (or (notmuch-query-get-threads (append cli-args args)) > - ;; If a query context reduced the number of > - ;; results to zero, try again without it. > - (and notmuch-show-query-context > - (notmuch-query-get-threads (append cli-args basic-args))))) > - > + (queries (notmuch-show--build-queries)) > + (forest nil) > ;; Must be reset every time we are going to start inserting > ;; messages into the buffer. > (notmuch-show-previous-subject "")) > - > + ;; Use results from the first query that returns some. > + (while (and (consp queries) > + (not (setq forest > + (notmuch-query-get-threads > + `(,@cli-args "'" ,@(car queries) "'"))))) > + (setq queries (cdr queries))) Similarly here I would avoid the ` , @ . I think I would also suggest making pushing the setq outside the while condition. Something like (while (and (consp queries) (not forest)) (setq forest (notmuch-query-get-threads (append cli-args (list "'") (car queries) (list "'")))) Best wishes Mark ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] Add notmuch-show--build-queries. 2016-10-13 16:46 ` Mark Walters @ 2016-10-13 19:34 ` Matt Armstrong 0 siblings, 0 replies; 4+ messages in thread From: Matt Armstrong @ 2016-10-13 19:34 UTC (permalink / raw) To: Mark Walters, notmuch Mark, thanks for the review. I agree with your comments. I'll send another patch. As you might guess, I'm new to elisp. Use of push makes some of this fall out nicely. For what it is worth, I did experiment with using loop to find the first functional "forest". There are several other calls to loop in this file. It was something like: (let* ((cli-args (....)) (forest (loop for query in (notmuch-show--build-queries) for forest = (notmuch-query-get-threads (append cli-args (list "'") query (list "'"))) until forest finally return forest)) ...) ...) I won't go that route in my next patch (I'm not convinced it is better). But I also am not particularly invested in any one way to do this. Mark Walters <markwalters1009@gmail.com> writes: > On Tue, 11 Oct 2016, Matt Armstrong <marmstrong@google.com> wrote: >> notmuch-show--build-buffer now queries a list of queries built by the >> former. This simplifies the logic. It also provides an easy place to >> experiment with alternate sets of queries for given notmuch-show-* >> variables (e.g. users can use advice-add to do so in a surgical way). > > Hi > > I think I like the overall logic, but I do have some comments below. > >> --- >> emacs/notmuch-show.el | 37 +++++++++++++++++++++++-------------- >> 1 file changed, 23 insertions(+), 14 deletions(-) >> >> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el >> index f2487ab..0727319 100644 >> --- a/emacs/notmuch-show.el >> +++ b/emacs/notmuch-show.el >> @@ -1261,6 +1261,20 @@ matched." >> (message "No messages matched the query!") >> nil)))) >> >> +(defun notmuch-show--build-queries () >> + "Return a list of queries to try for this search. >> + >> +If `notmuch-show-query-context` is not nil, the first query is >> +the conjunction of it and `notmuch-show-thread-id`. The next >> +query is `notmuch-show-thread-id` alone, and serves as a fallback >> +if the prior matches no messages." >> + (let* ((thread notmuch-show-thread-id) >> + (context notmuch-show-query-context) >> + (queries `((,thread)))) >> + (if context >> + (setq queries (cons `(,thread "and (" ,context ")") queries))) >> + queries)) > > I think it would be better to keep closer to the existing style -- using > list and append. At least do that as a first patch and then do a change > as above with ` and , so we can review them separately. Note I find the > list append style much easier to read, and I am guessing someone else > did (perhaps cworth?) as they wrote it that way. > > I think I would go for something like > > (let* ((... > queries)) > (push (list thread) queries) > (if context (push (list thread "and (" context ")") queries)) > queries) >> (defun notmuch-show--build-buffer (&optional state) >> "Display messages matching the current buffer context. >> >> @@ -1268,25 +1282,20 @@ Apply the previously saved STATE if supplied, otherwise show the >> first relevant message. >> >> If no messages match the query return NIL." >> - (let* ((basic-args (list notmuch-show-thread-id)) >> - (args (if notmuch-show-query-context >> - (append (list "\'") basic-args >> - (list "and (" notmuch-show-query-context ")\'")) >> - (append (list "\'") basic-args (list "\'")))) >> - (cli-args (cons "--exclude=false" >> + (let* ((cli-args (cons "--exclude=false" >> (when notmuch-show-elide-non-matching-messages >> (list "--entire-thread=false")))) >> - >> - (forest (or (notmuch-query-get-threads (append cli-args args)) >> - ;; If a query context reduced the number of >> - ;; results to zero, try again without it. >> - (and notmuch-show-query-context >> - (notmuch-query-get-threads (append cli-args basic-args))))) >> - >> + (queries (notmuch-show--build-queries)) >> + (forest nil) >> ;; Must be reset every time we are going to start inserting >> ;; messages into the buffer. >> (notmuch-show-previous-subject "")) >> - >> + ;; Use results from the first query that returns some. >> + (while (and (consp queries) >> + (not (setq forest >> + (notmuch-query-get-threads >> + `(,@cli-args "'" ,@(car queries) "'"))))) >> + (setq queries (cdr queries))) > > Similarly here I would avoid the ` , @ . I think I would also suggest > making pushing the setq outside the while condition. Something like > > (while (and (consp queries) > (not forest)) > (setq forest (notmuch-query-get-threads > (append cli-args (list "'") (car queries) (list "'")))) ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-10-13 19:34 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-10-11 17:41 [PATCH v2] emacs: Add notmuch-show--build-queries Matt Armstrong 2016-10-11 17:41 ` [PATCH v2] " Matt Armstrong 2016-10-13 16:46 ` Mark Walters 2016-10-13 19:34 ` Matt Armstrong
Code repositories for project(s) associated with this public inbox https://yhetil.org/notmuch.git/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).