[PATCH 0/4] Bug fixes for identity handling in Emacs

[PATCH 0/4] Bug fixes for identity handling in Emacs

[Austin Clements]

This series fixes several bugs surrounding identity handling in Emacs.
It was inspired by a problem that bjonnh on IRC had where forwarding
from an address where the full name matched the mailbox would crash
`notmuch-mua-new-forward-message'.  While it was possible to work
around that specific problem with a simple fix, it revealed that the
code was subtly broken in many ways.

The patches in this series are all technically independent and can be
pushed in any order, but they're all related problems.

[PATCH 1/4] emacs: Build forwarded message buffer more directly

[Austin Clements]

Previously, we used `message-forward' to build forwarded messages, but
this function is simply too high-level to be a good fit for some of
what we do.

First, since `message-forward' builds a full forward message buffer
given the message to forward, we have to duplicate much of the logic
in `notmuch-mua-mail' to patch the notmuch-y things into the built
buffer.

Second, `message-forward' constructs the From header from
user-full-name and user-mail-address.  As a result, if we prompt the
user for an identity, we have to parse it into name and address
components, just to have it put back together by `message-forward'.
This process is not entirely loss-less because
`mail-extract-address-components' does a lot of canonicalization
(since it's intended for displaying addresses, not for parsing them).

To fix these problems, don't use `message-forward' at all.
`message-forward' itself is basically just a call to `message-mail'
and `message-forward-make-body'.  Do this ourselves, but call
`notmuch-mua-mail' instead of `message-mail' so we can directly build
a notmuch-y message and control the From header.

This also fixes a bug that was a direct consequence of our use of
`mail-extract-address-components': if the user chose an identity that
had no name part (or the name part matched the mailbox), we would bind
user-full-name to nil, which would cause an exception in the bowels of
message-mode because user-full-name is expected to always be a string
(even if it's just "").
---
 emacs/notmuch-mua.el | 33 +++++++++++----------------------
 1 file changed, 11 insertions(+), 22 deletions(-)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index 481abd7..f2df770 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -231,21 +231,6 @@ list."
   (message-goto-body)
   (set-buffer-modified-p nil))
 
-(defun notmuch-mua-forward-message ()
-  (funcall (notmuch-mua-get-switch-function) (current-buffer))
-  (message-forward)
-
-  (when notmuch-mua-user-agent-function
-    (let ((user-agent (funcall notmuch-mua-user-agent-function)))
-      (when (not (string= "" user-agent))
-	(message-add-header (format "User-Agent: %s" user-agent)))))
-  (message-sort-headers)
-  (message-hide-headers)
-  (set-buffer-modified-p nil)
-  (notmuch-mua-maybe-set-window-dedicated)
-
-  (message-goto-to))
-
 (defun notmuch-mua-mail (&optional to subject other-headers &rest other-args)
   "Invoke the notmuch mail composition window.
 
@@ -345,13 +330,17 @@ The current buffer must contain an RFC2822 message to forward.
 
 If PROMPT-FOR-SENDER is non-nil, the user will be prompted for
 the From: address first."
-  (if (or prompt-for-sender notmuch-always-prompt-for-sender)
-      (let* ((sender (notmuch-mua-prompt-for-sender))
-	     (address-components (mail-extract-address-components sender))
-	     (user-full-name (car address-components))
-	     (user-mail-address (cadr address-components)))
-	(notmuch-mua-forward-message))
-    (notmuch-mua-forward-message)))
+  (let* ((cur (current-buffer))
+	 (message-forward-decoded-p nil)
+	 (subject (message-make-forward-subject))
+	 (other-headers
+	  (when (or prompt-for-sender notmuch-always-prompt-for-sender)
+	    (list (cons 'From (notmuch-mua-prompt-for-sender))))))
+    (notmuch-mua-mail nil subject other-headers nil (notmuch-mua-get-switch-function))
+    (message-forward-make-body cur)
+    ;; `message-forward-make-body' shows the User-agent header.  Hide
+    ;; it again.
+    (message-hide-headers)))
 
 (defun notmuch-mua-new-reply (query-string &optional prompt-for-sender reply-all)
   "Compose a reply to the message identified by QUERY-STRING.
-- 
1.8.4.rc3

[PATCH 2/4] emacs: Fix exception when fetching empty or unconfigured settings

[Austin Clements]

When "notmuch config" is called with the name of an empty or
unconfigured setting, it prints nothing (not even a new line).
Previously, `notmuch-config-get' assumed it would always print a
newline.  As a result, when `notmuch-config-get' was called with the
name of an empty of unconfigured setting, it would attempt to
(substring "" 0 -1) to strip the newline, which would fail with a
(args-out-of-range "" 0 -1) exception.

Fix this by only stripping the newline if there actually is one.
---
 emacs/notmuch-lib.el | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index fa35fa9..09110b5 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -198,8 +198,13 @@ on the command line, and then retry your notmuch command")))
 
 (defun notmuch-config-get (item)
   "Return a value from the notmuch configuration."
-  ;; Trim off the trailing newline
-  (substring (notmuch-command-to-string "config" "get" item) 0 -1))
+  (let* ((val (notmuch-command-to-string "config" "get" item))
+	 (len (length val)))
+    ;; Trim off the trailing newline (if the value is empty or not
+    ;; configured, there will be no newline)
+    (if (and (> len 0) (= (aref val (- len 1)) ?\n))
+	(substring val 0 -1)
+      val)))
 
 (defun notmuch-database-path ()
   "Return the database.path value from the notmuch configuration."
-- 
1.8.4.rc3

[PATCH 3/4] emacs: Fix `notmuch-user-other-email' when no other emails are configured

[Austin Clements]

Thanks to the previous patch, this no longer crashes in this
situation, but now would return ("").  Fix it to return () when no
emails are configured.
---
 emacs/notmuch-lib.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 09110b5..2fefdad 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -220,7 +220,7 @@ on the command line, and then retry your notmuch command")))
 
 (defun notmuch-user-other-email ()
   "Return the user.other_email value (as a list) from the notmuch configuration."
-  (split-string (notmuch-config-get "user.other_email") "\n"))
+  (split-string (notmuch-config-get "user.other_email") "\n" t))
 
 (defun notmuch-poll ()
   "Run \"notmuch new\" or an external script to import mail.
-- 
1.8.4.rc3

[PATCH 4/4] emacs: Simplify and fix `notmuch-mua-prompt-for-sender'

[Austin Clements]

`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

Re: [PATCH 4/4] emacs: Simplify and fix `notmuch-mua-prompt-for-sender'

[Mark Walters]

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

Re: [PATCH 4/4] emacs: Simplify and fix `notmuch-mua-prompt-for-sender'

[Austin Clements]

Quoth Mark Walters on Feb 20 at 10:36 pm:
> 
> 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.

Ah, interesting.  I'd figured ido would make this change *less*
jarring because you could enter just the email substring.

> 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.

This would be easy enough.  I'd chosen not to do this partly because I
thought the user may want to enter an address in "Name <email>" form
to override the name, especially if they only have one configured
identity (and since Emacs somehow lacks a address parser that can
extract the name without also transforming it to death, this is hard
to detect).  But I'm happy to ignore that situation.

> 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)
> >
> > _______________________________________________
> > notmuch mailing list
> > notmuch@notmuchmail.org
> > http://notmuchmail.org/mailman/listinfo/notmuch

-- 
Austin Clements                                      MIT/'06/PhD/CSAIL
amdragon@mit.edu                           http://web.mit.edu/amdragon
       Somewhere in the dream we call reality you will find me,
              searching for the reality we call dreams.

Re: [PATCH 4/4] emacs: Simplify and fix `notmuch-mua-prompt-for-sender'

[Tomi Ollila]

On Thu, Feb 20 2014, Austin Clements <amdragon@MIT.EDU> wrote:

This seris LGTM. I don't know about patch 4 as I have not used
the features (hand-edited From: always...)  maybe I should...

Tomi


> 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(-)
>

Re: [PATCH 0/4] Bug fixes for identity handling in Emacs

[David Bremner]

Austin Clements <amdragon@MIT.EDU> writes:

> This series fixes several bugs surrounding identity handling in Emacs.
> It was inspired by a problem that bjonnh on IRC had where forwarding
> from an address where the full name matched the mailbox would crash
> `notmuch-mua-new-forward-message'.  While it was possible to work
> around that specific problem with a simple fix, it revealed that the
> code was subtly broken in many ways.

I pushed the first 3. I wasn't sure what the concencus was on patch 4.

d

[PATCH v2] emacs: Simplify and fix `notmuch-mua-prompt-for-sender'

[Austin Clements]

`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

Re: [PATCH v2] emacs: Simplify and fix `notmuch-mua-prompt-for-sender'

[Mark Walters]

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

Re: [PATCH v2] emacs: Simplify and fix `notmuch-mua-prompt-for-sender'

[David Bremner]

Austin Clements <amdragon@MIT.EDU> writes:
>
> Mark, is this better?
>

hopefully so, because I pushed it.

d