From: Mark Walters <markwalters1009@gmail.com>
To: Austin Clements <amdragon@MIT.EDU>, notmuch@notmuchmail.org
Subject: Re: [PATCH v2] emacs: Simplify and fix
 `notmuch-mua-prompt-for-sender'
In-Reply-To: <1393524654-20502-1-git-send-email-amdragon@mit.edu>
References: <87fvndfm3k.fsf@qmul.ac.uk>
	<1393524654-20502-1-git-send-email-amdragon@mit.edu>
User-Agent: Notmuch/0.15.2+562~g339f306 (http://notmuchmail.org) Emacs/23.4.1
	(x86_64-pc-linux-gnu)
Date: Sat, 01 Mar 2014 09:37:22 +0000
Message-ID: <87ob1q2ral.fsf@qmul.ac.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list


This version is fine (thanks!). And the patch looks good to me too +1.

Best wishes

Mark


On Thu, 27 Feb 2014, Austin Clements <amdragon@MIT.EDU> wrote:
> `notmuch-mua-prompt-for-sender' is over-engineered and often wrong.
> It attempts to detect when all identities have the same name and
> specialize the prompt to just the email address part.  However, to do
> this it uses `mail-extract-address-components', which is meant for
> displaying email addresses, not general-purpose parsing, and hence
> performs many canonicalizations that can interfere with this use.  For
> example, configuring notmuch-identities to ("Austin
> <austin@example.com>"), will cause `notmuch-mua-prompt-for-sender' to
> lose the name part entirely and return " <austin@example.com>".
>
> This patch rewrites `notmuch-mua-prompt-for-sender' to simply prompt
> for a full identity when notmuch-identities is configured, or to
> prompt for a sender address when it isn't.
>
> The original code also did several strange things, like using `eval'
> and specifying that this function was interactive.  As a side-effect,
> this patch fixes these problems.  And it adds a docstring.
> ---
>
> Mark, is this better?
>
> emacs/notmuch-mua.el | 37 +++++++++++++------------------------
>  1 file changed, 13 insertions(+), 24 deletions(-)
>
> diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
> index f2df770..b16a10e 100644
> --- a/emacs/notmuch-mua.el
> +++ b/emacs/notmuch-mua.el
> @@ -286,30 +286,19 @@ the From: header is already filled in by notmuch."
>        (ad-activate 'ido-completing-read)))
>  
>  (defun notmuch-mua-prompt-for-sender ()
> -  (interactive)
> -  (let (name addresses one-name-only)
> -    ;; If notmuch-identities is non-nil, check if there is a fixed user name.
> -    (if notmuch-identities
> -	(let ((components (mapcar 'mail-extract-address-components notmuch-identities)))
> -	  (setq name          (caar components)
> -		addresses     (mapcar 'cadr components)
> -		one-name-only (eval
> -			       (cons 'and
> -				     (mapcar (lambda (identity)
> -					       (string-equal name (car identity)))
> -					     components)))))
> -      ;; If notmuch-identities is nil, use values from the notmuch configuration file.
> -      (setq name          (notmuch-user-name)
> -	    addresses     (cons (notmuch-user-primary-email) (notmuch-user-other-email))
> -	    one-name-only t))
> -    ;; Now prompt the user, either for an email address only or for a full identity.
> -    (if one-name-only
> -	(let ((address
> -	       (ido-completing-read (concat "Sender address for " name ": ") addresses
> -				    nil nil nil 'notmuch-mua-sender-history (car addresses))))
> -	  (concat name " <" address ">"))
> -      (ido-completing-read "Send mail From: " notmuch-identities
> -			   nil nil nil 'notmuch-mua-sender-history (car notmuch-identities)))))
> +  "Prompt for a sender from the user's configured identities."
> +  (if notmuch-identities
> +      (ido-completing-read "Send mail from: " notmuch-identities
> +			   nil nil nil 'notmuch-mua-sender-history
> +			   (car notmuch-identities))
> +    (let* ((name (notmuch-user-name))
> +	   (addrs (cons (notmuch-user-primary-email)
> +			(notmuch-user-other-email)))
> +	   (address
> +	    (ido-completing-read (concat "Sender address for " name ": ") addrs
> +				 nil nil nil 'notmuch-mua-sender-history
> +				 (car addrs))))
> +      (concat name " <" address ">"))))
>  
>  (put 'notmuch-mua-new-mail 'notmuch-prefix-doc "... and prompt for sender")
>  (defun notmuch-mua-new-mail (&optional prompt-for-sender)
> -- 
> 1.8.4.rc3