[PATCH v2] emacs: wrap current search in parens when filtering
Tomi Ollila
tomi.ollila at iki.fi
Sun Sep 6 02:19:07 PDT 2015
On Sun, Sep 06 2015, David Bremner <david at tethera.net> wrote:
> Some pretty fussy comments follow. Probably I could have fixed these in
> the time it took to write this message ;).
>
> Uli Scholler <uli at 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.
Yes, naming is hard. I have an additional suggestion:
notmuch-maybe-group-query-string could be written as
notmuch-group-disjunctive-query-string
> - The lines get pretty long here. We try to keep code to 80 columns.
In this case the status quo did not change -- e.g. one line that was
replaced was even longer. Anyway, shorter lines would be welcome
improvements.
> - 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 ---
Yes. A good custom is to try to git-am the file before sending if it has
been edited after git format-patch is used to create it -- or even if it
not edited, to catch any indesired "whitespace" etc. additions..
Tomi
>
>> + (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)))
> _______________________________________________
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
More information about the notmuch
mailing list