Re: [PATCH v2] emacs: wrap current search in parens when filtering

Subject: Re: [PATCH v2] emacs: wrap current search in parens when filtering

Date: Sat, 05 Sep 2015 23:35:56 -0300

To: Uli Scholler, notmuch@notmuchmail.org

Cc:

From: David Bremner

Some pretty fussy comments follow. Probably I could have fixed these in
the time it took to write this message ;).

Uli Scholler <uli@scholler.net> writes:
> +  (let ((grouped-query (notmuch-maybe-group-query-string query))
> +	(grouped-search-query (notmuch-maybe-group-query-string notmuch-search-query-string)))

- I didn't find it very obvious which of these introduced variables was
  which. I thought maybe "grouped-original-query" for the second
  one. It's pretty subjective though, so your call.

- The lines get pretty long here. We try to keep code to 80 columns.

- Your revised patch isn't in quite the right format for git am;
  the actual commit message get's lost. The unintuitive trick is to add
  commentary in the patch after the ---
  
> +    (notmuch-search (if (string= grouped-search-query "*")
>  			grouped-query
> -		      (concat notmuch-search-query-string " and " grouped-query)) notmuch-search-oldest-first)))
> +		      (concat grouped-search-query " and " grouped-query)) notmuch-search-oldest-first)))

Thread: