jao <jao@gnu.org> writes: > > > +(defun notmuch-mua--remove-dont-reply-to-names () > + (when-let ((nr (message-dont-reply-to-names))) Using when-let is fine with me, but I wonder if we should follow the advice in subr-x to do (eval-when-compile (require 'subr-x)) It's autoloaded, but maybe we can improve loading time a little? I noticed the other places we just do (require 'subr-x), but we're not always the most *cough* sophisticated elisp programmers. I'm a bit nervous about calling an undocumented defsubst here. It might be better to inline the 3 line definition. What do you think? > + (let ((nr-filter (if (functionp nr) > + (lambda (mail) (unless (funcall nr mail) mail)) > + (lambda (mail) (unless (string-match-p nr mail) mail))))) Inlining the definition would also make it more clear that the above code handles the case where message-dont-reply-to-names is list of regexps. Failing that maybe a comment. > + (dolist (header '("To" "Cc")) I found the re-use of v confusing in this 'when-let'. It reminds me of an exam question about scope :). > + (when-let ((v (message-fetch-field header))) > + (let* ((v (mapcar #'string-trim (message-tokenize-header v))) > + (vs (delq nil (mapcar nr-filter v))) > + (v (when vs (mapconcat #'identity vs ", ")))) I think 'and' is conventional for places where we care about what the expression evaluates to, and 'when' preferred when not. > + (message-replace-header header v))))))) > + > (defun notmuch-mua-mail (&optional to subject other-headers _continue > switch-function yank-action send-actions > return-action &rest ignored) > @@ -422,6 +434,7 @@ moved to the \"To:\" header." > (message-this-is-mail t)) > (message-setup-1 headers yank-action send-actions return-action)) > (notmuch-fcc-header-setup) > + (notmuch-mua--remove-dont-reply-to-names) > (message-sort-headers) > (message-hide-headers) > (set-buffer-modified-p nil) > diff --git a/test/T454-emacs-dont-reply-names.sh b/test/T454-emacs-dont-reply-names.sh > new file mode 100755 > index 00000000..e31141f9 > --- /dev/null > +++ b/test/T454-emacs-dont-reply-names.sh > @@ -0,0 +1,76 @@ > +#!/usr/bin/env bash > + > +test_description="emacs reply" I guess this description should match the name of the file and mention something like "don't reply"; this will make the individual test descriptions make more sense > +. $(dirname "$0")/test-lib.sh || exit 1 > +. $NOTMUCH_SRCDIR/test/test-lib-emacs.sh || exit 1 > + > +EXPECTED=$NOTMUCH_SRCDIR/test/emacs-show.expected-output > + > +test_require_emacs > + > +add_email_corpus default > + > +test_begin_subtest "regular expression" > +test_emacs '(let ((message-dont-reply-to-names "notmuchmail\\|noreply\\|harvard")) > + (notmuch-mua-new-reply > + "id:20091117203301.GV3165@dottiness.seas.harvard.edu" nil t) > + (test-visible-output "OUTPUT-FULL.raw"))' > + Maybe mention this tests Cc; I don't _think_ the other tests do? _______________________________________________ notmuch mailing list -- notmuch@notmuchmail.org To unsubscribe send an email to notmuch-leave@notmuchmail.org