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

[Matt Armstrong <marmstrong@google.com>]

Mark Walters <markwalters1009@gmail.com> writes:

>> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el

[...]

>> +(defun notmuch-show--build-queries ()

[...]

> We may want to call this from tree-mode later, so I wonder whether
> passing it notmuch-show-thread-id and notmuch-show-query-context as
> arguments would make sense. I quite like the idea of this function being
> self contained (ie not referencing global state variables). My instinct
> is that referencing truly global variables like customise variables is
> fine, but I am less keen on referencing buffer-local global variables.

Done.

Perhaps a useful rule of thumb is that functions referencing
buffer-local vars should be interactive?  I'm not sure that follows for
all cases, but I certainly agree that global and buffer-local vars
should probably not be referenced gratuitously, and pure functions are
generally good.


> However, some of this depends on what your later plans are.

Calling what I have in mind "plans" may be a stretch.  ;-) I'm not sure
this function is useful for notmuch-tree as written.  At minimum, I
would expect a shared function to have a different name.  But I think
you're right that there is probably opportunity for code sharing,
especially if notmuch grows more complex features with respect to
searching vs. expanding messages in threads.

Look for a v4 of this patch soon.  I appreciate your review so far.