From: Mark Walters <markwalters1009@gmail.com>
To: Austin Clements <amdragon@MIT.EDU>, notmuch@notmuchmail.org
Subject: Re: [PATCH 4/4] emacs: Simplify and fix
	`notmuch-mua-prompt-for-sender'
In-Reply-To: <1392923797-17045-5-git-send-email-amdragon@mit.edu>
References: <1392923797-17045-1-git-send-email-amdragon@mit.edu>
	<1392923797-17045-5-git-send-email-amdragon@mit.edu>
User-Agent: Notmuch/0.15.2+484~gfb59956 (http://notmuchmail.org) Emacs/23.4.1
	(x86_64-pc-linux-gnu)
Date: Thu, 20 Feb 2014 22:36:15 +0000
Message-ID: <87fvndfm3k.fsf@qmul.ac.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: bjonnh-nm@bjonnh.net
Precedence: list


Patches 1-3 look fine +1.

This one I am less sure about. I agree with the principle but for my use
case it is a little annoying:

I only use one name for all my addresses (Mark Walters), some addresses
are mark@.. and some walters@..

ido-completing-read is definitely less nice to use when all the addresses
match mark and walters.

I wonder if we could get the old behaviour in a more robust fashion. Two
possibilities we could consider are

1) if getting the information from the config file (when there is
necessarily a single name) then only complete the addresses.

2) make notmuch-identities a list of cons cells (name . email). Then
there is no parsing and the old method could be robust.

OTOH I can get used to the change.

Best wishes

Mark



On Thu, 20 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, but this has
> several problems.  First, 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>".  Second, though less serious, the
> prompt specialization means the user can't enter a different name like
> they can if their identities have different names.
>
> This patch rewrites `notmuch-mua-prompt-for-sender' to simply prompt
> for a full identity, where the list of identities is derived from
> either notmuch-identities or the user's Notmuch configuration.
>
> 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.
> ---
>  emacs/notmuch-mua.el | 33 +++++++++------------------------
>  1 file changed, 9 insertions(+), 24 deletions(-)
>
> diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
> index f2df770..4a485a4 100644
> --- a/emacs/notmuch-mua.el
> +++ b/emacs/notmuch-mua.el
> @@ -286,30 +286,15 @@ 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."
> +  (let ((identities (or notmuch-identities
> +			(let ((name (notmuch-user-name)))
> +			  (mapcar (lambda (addr) (concat name " <" addr ">"))
> +				  (cons (notmuch-user-primary-email)
> +					(notmuch-user-other-email)))))))
> +    (ido-completing-read "Send mail from: " identities
> +			 nil nil nil 'notmuch-mua-sender-history
> +			 (car identities))))
>  
>  (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
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch