jao <j...@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.gv3...@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-le...@notmuchmail.org
