Re: Status of Vim client

[David Bremner]

Nick Howell  writes:

> Nick Howell wrote:
>> I have a few patchsets adding:
>
>> - attachment support in compose
>> - mailcap support for multipart messages
>
> Mildly clean (imo), independent versions of these have been sent to
> the mailing list; they are meant to be applied on-top of felipec's
> repository. (I was unaware of, and haven't looked at, the imain repo.)

For whatever reason, I didn't see those messages on the (i.e. this) mailing 
list.

> p.s. please let me know if I broke some rule of patch mailing
> ettiquette, e.g. should I have included "vim:" or based the patches
> off of notmuch.git?

Yes, from our point of view notmuch.git is upstream, so all patches for
potential inclusion should be based off of that.

It would be handy to include vim: in the subject line, but it's not
absolutely crucial.

d
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2] Add notmuch-show--build-queries.

[Mark Walters]

On Tue, 11 Oct 2016, Matt Armstrong  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 ( 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
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: Status of Vim client

[nlhowell]

David Bremner wrote:
> Nick Howell  writes:
> > Nick Howell wrote:
> >> I have a few patchsets adding:
> >
> >> - attachment support in compose - mailcap support for multipart
> >> messages
> >
> > Mildly clean (imo), independent versions of these have been sent
> > to the mailing list; they are meant to be applied on-top of
> > felipec's repository. (I was unaware of, and haven't looked at,
> > the imain repo.)
> 
> For whatever reason, I didn't see those messages on the (i.e. this)
> mailing list.

Strange. I will make ettiquette adjustments and re-send. Here's my
smtp log:
> Oct 10 17:32:12 host=smtp.gmail.com tls=on auth=on user=nlhowell 
> from=nlhow...@gmail.com recipients=notmuch@notmuchmail.org,nlhow...@gmail.com 
> mailsize=1023 smtpstatus=250 smtpmsg='250 2.0.0 OK 1476142322 
> m128sm382316ioa.41 - gsmtp' exitcode=EX_OK
> Oct 10 17:32:27 host=smtp.gmail.com tls=on auth=on user=nlhowell 
> from=nlhow...@gmail.com recipients=notmuch@notmuchmail.org,nlhow...@gmail.com 
> mailsize=4032 smtpstatus=250 smtpmsg='250 2.0.0 OK 1476142337 
> b133sm7677395iti.21 - gsmtp' exitcode=EX_OK
> Oct 10 17:32:43 host=smtp.gmail.com tls=on auth=on user=nlhowell 
> from=nlhow...@gmail.com recipients=notmuch@notmuchmail.org,nlhow...@gmail.com 
> mailsize=4085 smtpstatus=250 smtpmsg='250 2.0.0 OK 1476142353 
> n69sm7704802ita.0 - gsmtp' exitcode=EX_OK
> Oct 10 18:25:13 host=smtp.gmail.com tls=on auth=on user=nlhowell 
> from=nlhow...@gmail.com recipients=notmuch@notmuchmail.org,nlhow...@gmail.com 
> mailsize=1149 smtpstatus=250 smtpmsg='250 2.0.0 OK 1476145503 
> fi6sm530779pac.20 - gsmtp' exitcode=EX_OK
> Oct 10 18:25:18 host=smtp.gmail.com tls=on auth=on user=nlhowell 
> from=nlhow...@gmail.com recipients=notmuch@notmuchmail.org,nlhow...@gmail.com 
> mailsize=3608 smtpstatus=250 smtpmsg='250 2.0.0 OK 1476145508 
> n77sm507192pfi.82 - gsmtp' exitcode=EX_OK
> Oct 10 18:25:23 host=smtp.gmail.com tls=on auth=on user=nlhowell 
> from=nlhow...@gmail.com recipients=notmuch@notmuchmail.org,nlhow...@gmail.com 
> mailsize=1071 smtpstatus=250 smtpmsg='250 2.0.0 OK 1476145514 
> e1sm542594pap.11 - gsmtp' exitcode=EX_OK
> Oct 10 18:25:29 host=smtp.gmail.com tls=on auth=on user=nlhowell 
> from=nlhow...@gmail.com recipients=notmuch@notmuchmail.org,nlhow...@gmail.com 
> mailsize=1422 smtpstatus=250 smtpmsg='250 2.0.0 OK 1476145519 
> k67sm507280pfb.86 - gsmtp' exitcode=EX_OK
> Oct 10 18:25:34 host=smtp.gmail.com tls=on auth=on user=nlhowell 
> from=nlhow...@gmail.com recipients=notmuch@notmuchmail.org,nlhow...@gmail.com 
> mailsize=970 smtpstatus=250 smtpmsg='250 2.0.0 OK 1476145524 n2sm286444pfa.75 
> - gsmtp' exitcode=EX_OK
> Oct 10 18:25:39 host=smtp.gmail.com tls=on auth=on user=nlhowell 
> from=nlhow...@gmail.com recipients=notmuch@notmuchmail.org,nlhow...@gmail.com 
> mailsize=4136 smtpstatus=250 smtpmsg='250 2.0.0 OK 1476145530 
> j6sm497374paa.44 - gsmtp' exitcode=EX_OK
> Oct 10 18:25:44 host=smtp.gmail.com tls=on auth=on user=nlhowell 
> from=nlhow...@gmail.com recipients=notmuch@notmuchmail.org,nlhow...@gmail.com 
> mailsize=1653 smtpstatus=250 smtpmsg='250 2.0.0 OK 1476145535 
> g9sm503882paw.40 - gsmtp' exitcode=EX_OK
> Oct 10 18:40:12 host=smtp.gmail.com tls=on auth=on user=nlhowell 
> from=nlhow...@gmail.com 
> recipients=notmuch@notmuchmail.org,nlhow...@gmail.com,nlhow...@gmail.com 
> mailsize=2058 smtpstatus=250 smtpmsg='250 2.0.0 OK 1476146402 
> n7sm558994pfn.62 - gsmtp' exitcode=EX_OK

I assumed they were getting moderated, but perhaps they got dropped?

> > p.s. please let me know if I broke some rule of patch mailing
> > ettiquette, e.g. should I have included "vim:" or based the
> > patches off of notmuch.git?
> 
> Yes, from our point of view notmuch.git is upstream, so all patches
> for potential inclusion should be based off of that.
> 
> It would be handy to include vim: in the subject line, but it's not
> absolutely crucial.

I'll make these two corrections.

Cheers,
Nick
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2] Add notmuch-show--build-queries.

[Matt Armstrong]

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  writes:

> On Tue, 11 Oct 2016, Matt Armstrong  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 ( 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 "'"

___
notmuch mailing list
notmuch@notmuchmail.org

[no subject]

[Matt Armstrong]

This supercedes
id:1476207707-21827-1-git-send-email-marmstr...@google.com with
changes steming from Mark's helpful feedback.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v3] Add notmuch-show--build-queries.

[Matt Armstrong]

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 | 36 ++--
 1 file changed, 22 insertions(+), 14 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index f2487ab..b393c11 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)
+(push (list thread) queries)
+(if context (push (list thread "and (" context ")") queries))
+queries))
+
 (defun notmuch-show--build-buffer ( state)
   "Display messages matching the current buffer context.
 
@@ -1268,25 +1282,19 @@ 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 (not forest) (consp queries))
+  (setq forest (notmuch-query-get-threads
+   (append cli-args (list "'") (car queries) (list "'"
+  (setq queries (cdr queries)))
 (when forest
   (notmuch-show-insert-forest forest)
 
-- 
2.8.0.rc3.226.g39d4020

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch