Thanks for contributing to Notmuch. Some generic comments: 1) Please consider a more comprehensive commit message [1]. The "why" here is maybe obvious, but consider pointing out whether this makes it more consistent with other parts of the UI (or not). Also, a (bit more extended) of the changes is always helpful, both to help read the diff and to warn of any potential breaking changes. 2) Please update doc/notmuch-emacs.rst. You can re-use docstrings; let me know if you need help doing that. We haven't been strict about this is in the past, but the emacs docs are really lagging. 3) We generally want at least one test for a new feature. test/T460-emacs-tree.sh has the existing tree related tests. Again, if you need help with the test suite, let us know. 4) Please use notmuch-tree-- as a prefix for new private symbols that users should not rely on. I'm not sure about which symbols that applies to here. [1] https://notmuchmail.org/contributing/#index5h2 Pierre Neidhardt <mail@ambrevar.xyz> writes: > +(defun notmuch-tree-foreach-result (beg end fn) > + (lexical-let ((pos (notmuch-tree-result-beginning beg)) As an aside, I'm curious if we can profitably start to use file level lexical-binding for parts of notmuch-emacs. > + (notmuch-tree-foreach-result beg end > + (lambda (pos) > + (save-mark-and-excursion I did wonder if notmuch-tree-foreach-result should be a macro. I'm not sure if the small gain in code compactness from just passing a "body" in the style of notmuch-show--with-currently-shown-message is worth it. _______________________________________________ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch