On Sun, Sep 06 2015, David Bremner <da...@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 <u...@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@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch _______________________________________________ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
