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.