[PATCH 0/2] Control guessing of the from-address when replying

[PATCH 0/2] Control guessing of the from-address when replying

[Mark Walters]

Hello

This patch sets is intended to allow the user more control over the
From: address when replying to emails. 

In notmuch-reply.c the current logic for the From: address looks as
the main headers, then at the delivery headers, and finally defaults
to the config file address. This means that the frontends (e.g. emacs)
cannot tell whether the users primary address is in the From: header
because the message being replied to was sent to that address, or
notmuch just fellback to using it as it had no better guess. 

The first patch allows the user to control which fallbacks are
used. The second implements this in emacs so that it does not fallback
to the primary address; thus, in this case, emacs can ask for the
address (if notmuch-always-prompt-for-sender is set). The set is not
heavily tested but appears to work.

There is some bikeshedding available as to what options we should be
able to pass to the --from= in notmuch-reply.c.

It obviously also needs some tests and some man pages updates.

Finally, we may want a defcustom option for emacs to control which it
uses (e.g., the user always wants to reply from ther primary address).


Best wishes

Mark

                  

[PATCH 1/2] cli: add --from option to reply to restrict guessing of the From: header.

[Mark Walters]

Add an option --from= to notmuch-reply.c to restrict guessing of the
From: header. The existing logic looks as the main headers, then at
the delivery headers, and finally defaults to the config file address.

This patch allows the user to restrict which of these guesses are
made.  Currently the supported values are:
       default|fallback-all    current behaviour
       fallback-received       fallback to delivery headers but not config file
       fallback-none	       only look at from/reply-to/to/cc/ headers
       none	 	       From: header is always left empty

If the code does not find an allowed address it outputs an empty From:
line and the caller can decide how to respond.
---
 notmuch-reply.c |   39 ++++++++++++++++++++++++++++++---------
 1 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/notmuch-reply.c b/notmuch-reply.c
index f55b1d2..f660749 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -24,6 +24,13 @@
 #include "gmime-filter-reply.h"
 #include "gmime-filter-headers.h"
 
+enum {
+    FROM_FALLBACK_ALL,
+    FROM_FALLBACK_RECEIVED,
+    FROM_FALLBACK_NONE,
+    FROM_NONE
+};
+
 static void
 reply_headers_message_part (GMimeMessage *message);
 
@@ -510,7 +517,8 @@ notmuch_reply_format_default(void *ctx,
 			     notmuch_config_t *config,
 			     notmuch_query_t *query,
 			     notmuch_show_params_t *params,
-			     notmuch_bool_t reply_all)
+			     notmuch_bool_t reply_all,
+			     int from_guess)
 {
     GMimeMessage *reply;
     notmuch_messages_t *messages;
@@ -542,15 +550,19 @@ notmuch_reply_format_default(void *ctx,
 	from_addr = add_recipients_from_message (reply, config, message,
 						 reply_all);
 
-	if (from_addr == NULL)
+	if ((from_addr == NULL) && (from_guess <= FROM_FALLBACK_RECEIVED))
 	    from_addr = guess_from_received_header (config, message);
 
-	if (from_addr == NULL)
+	if ((from_addr == NULL) && (from_guess <= FROM_FALLBACK_ALL ))
 	    from_addr = notmuch_config_get_user_primary_email (config);
 
-	from_addr = talloc_asprintf (ctx, "%s <%s>",
-				     notmuch_config_get_user_name (config),
-				     from_addr);
+	if ((from_addr != NULL) || (from_guess = FROM_NONE)) {
+	    from_addr = talloc_asprintf (ctx, "%s <%s>",
+					 notmuch_config_get_user_name (config),
+					 from_addr);
+	} else {
+	    from_addr = talloc_strdup (ctx, "");
+	}
 	g_mime_object_set_header (GMIME_OBJECT (reply),
 				  "From", from_addr);
 
@@ -590,7 +602,8 @@ notmuch_reply_format_headers_only(void *ctx,
 				  notmuch_config_t *config,
 				  notmuch_query_t *query,
 				  unused (notmuch_show_params_t *params),
-				  notmuch_bool_t reply_all)
+				  notmuch_bool_t reply_all,
+				  unused (int from_guess))
 {
     GMimeMessage *reply;
     notmuch_messages_t *messages;
@@ -657,10 +670,11 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
     notmuch_query_t *query;
     char *query_string;
     int opt_index, ret = 0;
-    int (*reply_format_func)(void *ctx, notmuch_config_t *config, notmuch_query_t *query, notmuch_show_params_t *params, notmuch_bool_t reply_all);
+    int (*reply_format_func)(void *ctx, notmuch_config_t *config, notmuch_query_t *query, notmuch_show_params_t *params, notmuch_bool_t reply_all, int from_guess);
     notmuch_show_params_t params = { .part = -1 };
     int format = FORMAT_DEFAULT;
     int reply_all = TRUE;
+    int from_guess = FROM_FALLBACK_ALL;
     notmuch_bool_t decrypt = FALSE;
 
     notmuch_opt_desc_t options[] = {
@@ -672,6 +686,13 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
 	  (notmuch_keyword_t []){ { "all", TRUE },
 				  { "sender", FALSE },
 				  { 0, 0 } } },
+	{ NOTMUCH_OPT_KEYWORD, &from_guess, "from", 'F',
+	  (notmuch_keyword_t []){ { "default", FROM_FALLBACK_ALL },
+				  { "fallback-all", FROM_FALLBACK_ALL },
+				  { "fallback-received", FROM_FALLBACK_RECEIVED },
+				  { "fallback-none", FROM_FALLBACK_NONE },
+				  { "none", FROM_NONE },
+				  { 0, 0 } } },
 	{ NOTMUCH_OPT_BOOLEAN, &decrypt, "decrypt", 'd', 0 },
 	{ 0, 0, 0, 0, 0 }
     };
@@ -732,7 +753,7 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
 	return 1;
     }
 
-    if (reply_format_func (ctx, config, query, &params, reply_all) != 0)
+    if (reply_format_func (ctx, config, query, &params, reply_all, from_guess) != 0)
 	return 1;
 
     notmuch_query_destroy (query);
-- 
1.7.2.3

Re: [PATCH 1/2] cli: add --from option to reply to restrict guessing of the From: header.

[Jani Nikula]

On Sat,  4 Feb 2012 17:09:09 +0000, Mark Walters <markwalters1009@gmail.com> wrote:
> Add an option --from= to notmuch-reply.c to restrict guessing of the
> From: header. The existing logic looks as the main headers, then at
> the delivery headers, and finally defaults to the config file address.
> 
> This patch allows the user to restrict which of these guesses are
> made.  Currently the supported values are:
>        default|fallback-all    current behaviour
>        fallback-received       fallback to delivery headers but not config file
>        fallback-none	       only look at from/reply-to/to/cc/ headers
>        none	 	       From: header is always left empty

As discussed on IRC, --from=primary is a natural extension to always use
the primary address from config, but that can be a later patch.

> 
> If the code does not find an allowed address it outputs an empty From:
> line and the caller can decide how to respond.
> ---
>  notmuch-reply.c |   39 ++++++++++++++++++++++++++++++---------
>  1 files changed, 30 insertions(+), 9 deletions(-)
> 
> diff --git a/notmuch-reply.c b/notmuch-reply.c
> index f55b1d2..f660749 100644
> --- a/notmuch-reply.c
> +++ b/notmuch-reply.c
> @@ -24,6 +24,13 @@
>  #include "gmime-filter-reply.h"
>  #include "gmime-filter-headers.h"
>  

I think it would be good to have a comment here reminding later
developers that the order matters in this enum.

> +enum {
> +    FROM_FALLBACK_ALL,
> +    FROM_FALLBACK_RECEIVED,
> +    FROM_FALLBACK_NONE,
> +    FROM_NONE
> +};
> +
>  static void
>  reply_headers_message_part (GMimeMessage *message);
>  
> @@ -510,7 +517,8 @@ notmuch_reply_format_default(void *ctx,
>  			     notmuch_config_t *config,
>  			     notmuch_query_t *query,
>  			     notmuch_show_params_t *params,
> -			     notmuch_bool_t reply_all)
> +			     notmuch_bool_t reply_all,
> +			     int from_guess)

Hrmh, I'd like this to sound more deterministic than
"guessing". from_select? from_use? from?

>  {
>      GMimeMessage *reply;
>      notmuch_messages_t *messages;
> @@ -542,15 +550,19 @@ notmuch_reply_format_default(void *ctx,
>  	from_addr = add_recipients_from_message (reply, config, message,
>  						 reply_all);
>  
> -	if (from_addr == NULL)
> +	if ((from_addr == NULL) && (from_guess <= FROM_FALLBACK_RECEIVED))

Please drop the extra braces here and below.

>  	    from_addr = guess_from_received_header (config, message);
>  
> -	if (from_addr == NULL)
> +	if ((from_addr == NULL) && (from_guess <= FROM_FALLBACK_ALL ))
>  	    from_addr = notmuch_config_get_user_primary_email (config);
>  
> -	from_addr = talloc_asprintf (ctx, "%s <%s>",
> -				     notmuch_config_get_user_name (config),
> -				     from_addr);
> +	if ((from_addr != NULL) || (from_guess = FROM_NONE)) {

Should that be (from_addr != NULL && from_guess != FROM_NONE)?
Definitely the assignment is an error!

> +	    from_addr = talloc_asprintf (ctx, "%s <%s>",
> +					 notmuch_config_get_user_name (config),
> +					 from_addr);
> +	} else {
> +	    from_addr = talloc_strdup (ctx, "");
> +	}
>  	g_mime_object_set_header (GMIME_OBJECT (reply),
>  				  "From", from_addr);
>  
> @@ -590,7 +602,8 @@ notmuch_reply_format_headers_only(void *ctx,
>  				  notmuch_config_t *config,
>  				  notmuch_query_t *query,
>  				  unused (notmuch_show_params_t *params),
> -				  notmuch_bool_t reply_all)
> +				  notmuch_bool_t reply_all,
> +				  unused (int from_guess))
>  {
>      GMimeMessage *reply;
>      notmuch_messages_t *messages;
> @@ -657,10 +670,11 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
>      notmuch_query_t *query;
>      char *query_string;
>      int opt_index, ret = 0;
> -    int (*reply_format_func)(void *ctx, notmuch_config_t *config, notmuch_query_t *query, notmuch_show_params_t *params, notmuch_bool_t reply_all);
> +    int (*reply_format_func)(void *ctx, notmuch_config_t *config, notmuch_query_t *query, notmuch_show_params_t *params, notmuch_bool_t reply_all, int from_guess);
>      notmuch_show_params_t params = { .part = -1 };
>      int format = FORMAT_DEFAULT;
>      int reply_all = TRUE;
> +    int from_guess = FROM_FALLBACK_ALL;
>      notmuch_bool_t decrypt = FALSE;
>  
>      notmuch_opt_desc_t options[] = {
> @@ -672,6 +686,13 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
>  	  (notmuch_keyword_t []){ { "all", TRUE },
>  				  { "sender", FALSE },
>  				  { 0, 0 } } },
> +	{ NOTMUCH_OPT_KEYWORD, &from_guess, "from", 'F',
> +	  (notmuch_keyword_t []){ { "default", FROM_FALLBACK_ALL },
> +				  { "fallback-all", FROM_FALLBACK_ALL },
> +				  { "fallback-received", FROM_FALLBACK_RECEIVED },
> +				  { "fallback-none", FROM_FALLBACK_NONE },
> +				  { "none", FROM_NONE },
> +				  { 0, 0 } } },
>  	{ NOTMUCH_OPT_BOOLEAN, &decrypt, "decrypt", 'd', 0 },
>  	{ 0, 0, 0, 0, 0 }
>      };
> @@ -732,7 +753,7 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
>  	return 1;
>      }
>  
> -    if (reply_format_func (ctx, config, query, &params, reply_all) != 0)
> +    if (reply_format_func (ctx, config, query, &params, reply_all, from_guess) != 0)
>  	return 1;
>  
>      notmuch_query_destroy (query);
> -- 
> 1.7.2.3
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 1/2] cli: add --from option to reply to restrict guessing of the From: header.

[Mark Walters]

On Sat, 04 Feb 2012 19:59:58 +0200, Jani Nikula <jani@nikula.org> wrote:
> On Sat,  4 Feb 2012 17:09:09 +0000, Mark Walters <markwalters1009@gmail.com> wrote:
> > Add an option --from= to notmuch-reply.c to restrict guessing of the
> > From: header. The existing logic looks as the main headers, then at
> > the delivery headers, and finally defaults to the config file address.
> > 
> > This patch allows the user to restrict which of these guesses are
> > made.  Currently the supported values are:
> >        default|fallback-all    current behaviour
> >        fallback-received       fallback to delivery headers but not config file
> >        fallback-none	       only look at from/reply-to/to/cc/ headers
> >        none	 	       From: header is always left empty
> 
> As discussed on IRC, --from=primary is a natural extension to always use
> the primary address from config, but that can be a later patch.

Ok, I will probably add that.

> > 
> > If the code does not find an allowed address it outputs an empty From:
> > line and the caller can decide how to respond.
> > ---
> >  notmuch-reply.c |   39 ++++++++++++++++++++++++++++++---------
> >  1 files changed, 30 insertions(+), 9 deletions(-)
> > 
> > diff --git a/notmuch-reply.c b/notmuch-reply.c
> > index f55b1d2..f660749 100644
> > --- a/notmuch-reply.c
> > +++ b/notmuch-reply.c
> > @@ -24,6 +24,13 @@
> >  #include "gmime-filter-reply.h"
> >  #include "gmime-filter-headers.h"
> >  
> 
> I think it would be good to have a comment here reminding later
> developers that the order matters in this enum.

Have fixed

> > +enum {
> > +    FROM_FALLBACK_ALL,
> > +    FROM_FALLBACK_RECEIVED,
> > +    FROM_FALLBACK_NONE,
> > +    FROM_NONE
> > +};
> > +
> >  static void
> >  reply_headers_message_part (GMimeMessage *message);
> >  
> > @@ -510,7 +517,8 @@ notmuch_reply_format_default(void *ctx,
> >  			     notmuch_config_t *config,
> >  			     notmuch_query_t *query,
> >  			     notmuch_show_params_t *params,
> > -			     notmuch_bool_t reply_all)
> > +			     notmuch_bool_t reply_all,
> > +			     int from_guess)
> 
> Hrmh, I'd like this to sound more deterministic than
> "guessing". from_select? from_use? from?

I have gone for from_select.

> >  {
> >      GMimeMessage *reply;
> >      notmuch_messages_t *messages;
> > @@ -542,15 +550,19 @@ notmuch_reply_format_default(void *ctx,
> >  	from_addr = add_recipients_from_message (reply, config, message,
> >  						 reply_all);
> >  
> > -	if (from_addr == NULL)
> > +	if ((from_addr == NULL) && (from_guess <= FROM_FALLBACK_RECEIVED))
> 
> Please drop the extra braces here and below.

Have fixed. (Indeed in a sense this was a bug see below!)
> 
> >  	    from_addr = guess_from_received_header (config, message);
> >  
> > -	if (from_addr == NULL)
> > +	if ((from_addr == NULL) && (from_guess <= FROM_FALLBACK_ALL ))
> >  	    from_addr = notmuch_config_get_user_primary_email (config);
> >  
> > -	from_addr = talloc_asprintf (ctx, "%s <%s>",
> > -				     notmuch_config_get_user_name (config),
> > -				     from_addr);
> > +	if ((from_addr != NULL) || (from_guess = FROM_NONE)) {
> 
> Should that be (from_addr != NULL && from_guess != FROM_NONE)?
> Definitely the assignment is an error!

You are quite right. And if I didn't have the superfluous brackets the
compiler would probably have given a warning. I have fixed it.

> 
> > +	    from_addr = talloc_asprintf (ctx, "%s <%s>",
> > +					 notmuch_config_get_user_name (config),
> > +					 from_addr);
> > +	} else {
> > +	    from_addr = talloc_strdup (ctx, "");
> > +	}
> >  	g_mime_object_set_header (GMIME_OBJECT (reply),
> >  				  "From", from_addr);
> >  
> > @@ -590,7 +602,8 @@ notmuch_reply_format_headers_only(void *ctx,
> >  				  notmuch_config_t *config,
> >  				  notmuch_query_t *query,
> >  				  unused (notmuch_show_params_t *params),
> > -				  notmuch_bool_t reply_all)
> > +				  notmuch_bool_t reply_all,
> > +				  unused (int from_guess))
> >  {
> >      GMimeMessage *reply;
> >      notmuch_messages_t *messages;
> > @@ -657,10 +670,11 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
> >      notmuch_query_t *query;
> >      char *query_string;
> >      int opt_index, ret = 0;
> > -    int (*reply_format_func)(void *ctx, notmuch_config_t *config, notmuch_query_t *query, notmuch_show_params_t *params, notmuch_bool_t reply_all);
> > +    int (*reply_format_func)(void *ctx, notmuch_config_t *config, notmuch_query_t *query, notmuch_show_params_t *params, notmuch_bool_t reply_all, int from_guess);
> >      notmuch_show_params_t params = { .part = -1 };
> >      int format = FORMAT_DEFAULT;
> >      int reply_all = TRUE;
> > +    int from_guess = FROM_FALLBACK_ALL;
> >      notmuch_bool_t decrypt = FALSE;
> >  
> >      notmuch_opt_desc_t options[] = {
> > @@ -672,6 +686,13 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
> >  	  (notmuch_keyword_t []){ { "all", TRUE },
> >  				  { "sender", FALSE },
> >  				  { 0, 0 } } },
> > +	{ NOTMUCH_OPT_KEYWORD, &from_guess, "from", 'F',
> > +	  (notmuch_keyword_t []){ { "default", FROM_FALLBACK_ALL },
> > +				  { "fallback-all", FROM_FALLBACK_ALL },
> > +				  { "fallback-received", FROM_FALLBACK_RECEIVED },
> > +				  { "fallback-none", FROM_FALLBACK_NONE },
> > +				  { "none", FROM_NONE },
> > +				  { 0, 0 } } },
> >  	{ NOTMUCH_OPT_BOOLEAN, &decrypt, "decrypt", 'd', 0 },
> >  	{ 0, 0, 0, 0, 0 }
> >      };
> > @@ -732,7 +753,7 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
> >  	return 1;
> >      }
> >  
> > -    if (reply_format_func (ctx, config, query, &params, reply_all) != 0)
> > +    if (reply_format_func (ctx, config, query, &params, reply_all, from_guess) != 0)
> >  	return 1;
> >  
> >      notmuch_query_destroy (query);
> > -- 
> > 1.7.2.3
> > 
> > _______________________________________________
> > notmuch mailing list
> > notmuch@notmuchmail.org
> > http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 2/2] emacs: Improve prompting for user address when replying.

[Mark Walters]

This patch uses the new --from option to notmuch reply to allow it to
prompt the user for the From: address in cases when the cli does not
know the "correct" from address. If the cli does not it either uses
the users default address or, if notmuch-always-prompt-for-sender
is set, prompts the user.
---
 emacs/notmuch-mua.el |   47 ++++++++++++++++++++++++++++-------------------
 1 files changed, 28 insertions(+), 19 deletions(-)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index 41f82c2..36e62f9 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -51,6 +51,24 @@ list."
 
 ;;
 
+(defcustom notmuch-identities nil
+  "Identities that can be used as the From: address when composing a new message.
+
+If this variable is left unset, then a list will be constructed from the
+name and addresses configured in the notmuch configuration file."
+  :type '(repeat string)
+  :group 'notmuch-send)
+
+(defcustom notmuch-always-prompt-for-sender nil
+  "Always prompt for the From: address when composing or forwarding a message.
+
+This is not taken into account when replying to a message, because in that case
+the From: header is already filled in by notmuch."
+  :type 'boolean
+  :group 'notmuch-send)
+
+(defvar notmuch-mua-sender-history nil)
+
 (defun notmuch-mua-user-agent-full ()
   "Generate a `User-Agent:' string suitable for notmuch."
   (concat (notmuch-mua-user-agent-notmuch)
@@ -75,7 +93,7 @@ list."
 (defun notmuch-mua-reply (query-string &optional sender reply-all)
   (let (headers
 	body
-	(args '("reply")))
+	(args '("reply" "--from=fallback-received")))
     (if notmuch-show-process-crypto
 	(setq args (append args '("--decrypt"))))
     (if reply-all
@@ -99,6 +117,15 @@ list."
     ;; If sender is non-nil, set the From: header to its value.
     (when sender
       (mail-header-set 'from sender headers))
+    ;; If we do not have a From: header yet it means that
+    ;; notmuch-reply.c was not able to make a useful guess so we fill
+    ;; it in ourselves.
+    (when (string= "" (mail-header 'from headers))
+      (if notmuch-always-prompt-for-sender
+	  (setq sender (notmuch-mua-prompt-for-sender))
+	(setq sender (concat
+		      (notmuch-user-name) " <" (notmuch-user-primary-email) ">")))
+      (mail-header-set 'from sender headers))
     (let
 	;; Overlay the composition window on that being used to read
 	;; the original message.
@@ -153,24 +180,6 @@ OTHER-ARGS are passed through to `message-mail'."
 
   (message-goto-to))
 
-(defcustom notmuch-identities nil
-  "Identities that can be used as the From: address when composing a new message.
-
-If this variable is left unset, then a list will be constructed from the
-name and addresses configured in the notmuch configuration file."
-  :type '(repeat string)
-  :group 'notmuch-send)
-
-(defcustom notmuch-always-prompt-for-sender nil
-  "Always prompt for the From: address when composing or forwarding a message.
-
-This is not taken into account when replying to a message, because in that case
-the From: header is already filled in by notmuch."
-  :type 'boolean
-  :group 'notmuch-send)
-
-(defvar notmuch-mua-sender-history nil)
-
 (defun notmuch-mua-prompt-for-sender ()
   (interactive)
   (let (name addresses one-name-only)
-- 
1.7.2.3

Re: [PATCH 2/2] emacs: Improve prompting for user address when replying.

[Jani Nikula]

On Sat,  4 Feb 2012 17:09:10 +0000, Mark Walters <markwalters1009@gmail.com> wrote:
> This patch uses the new --from option to notmuch reply to allow it to
> prompt the user for the From: address in cases when the cli does not
> know the "correct" from address. If the cli does not it either uses
> the users default address or, if notmuch-always-prompt-for-sender
> is set, prompts the user.
> ---
>  emacs/notmuch-mua.el |   47 ++++++++++++++++++++++++++++-------------------
>  1 files changed, 28 insertions(+), 19 deletions(-)
> 
> diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
> index 41f82c2..36e62f9 100644
> --- a/emacs/notmuch-mua.el
> +++ b/emacs/notmuch-mua.el
> @@ -51,6 +51,24 @@ list."
>  
>  ;;
>  
> +(defcustom notmuch-identities nil
> +  "Identities that can be used as the From: address when composing a new message.
> +
> +If this variable is left unset, then a list will be constructed from the
> +name and addresses configured in the notmuch configuration file."
> +  :type '(repeat string)
> +  :group 'notmuch-send)
> +
> +(defcustom notmuch-always-prompt-for-sender nil
> +  "Always prompt for the From: address when composing or forwarding a message.
> +
> +This is not taken into account when replying to a message, because in that case
> +the From: header is already filled in by notmuch."
> +  :type 'boolean
> +  :group 'notmuch-send)
> +
> +(defvar notmuch-mua-sender-history nil)
> +
>  (defun notmuch-mua-user-agent-full ()
>    "Generate a `User-Agent:' string suitable for notmuch."
>    (concat (notmuch-mua-user-agent-notmuch)
> @@ -75,7 +93,7 @@ list."
>  (defun notmuch-mua-reply (query-string &optional sender reply-all)
>    (let (headers
>  	body
> -	(args '("reply")))
> +	(args '("reply" "--from=fallback-received")))

There are better reviewers for the rest of the emacs bits, but wouldn't
it be better to just use the "notmuch reply" default when the user wants
the current behaviour?

BR,
Jani.


>      (if notmuch-show-process-crypto
>  	(setq args (append args '("--decrypt"))))
>      (if reply-all
> @@ -99,6 +117,15 @@ list."
>      ;; If sender is non-nil, set the From: header to its value.
>      (when sender
>        (mail-header-set 'from sender headers))
> +    ;; If we do not have a From: header yet it means that
> +    ;; notmuch-reply.c was not able to make a useful guess so we fill
> +    ;; it in ourselves.
> +    (when (string= "" (mail-header 'from headers))
> +      (if notmuch-always-prompt-for-sender
> +	  (setq sender (notmuch-mua-prompt-for-sender))
> +	(setq sender (concat
> +		      (notmuch-user-name) " <" (notmuch-user-primary-email) ">")))
> +      (mail-header-set 'from sender headers))
>      (let
>  	;; Overlay the composition window on that being used to read
>  	;; the original message.
> @@ -153,24 +180,6 @@ OTHER-ARGS are passed through to `message-mail'."
>  
>    (message-goto-to))
>  
> -(defcustom notmuch-identities nil
> -  "Identities that can be used as the From: address when composing a new message.
> -
> -If this variable is left unset, then a list will be constructed from the
> -name and addresses configured in the notmuch configuration file."
> -  :type '(repeat string)
> -  :group 'notmuch-send)
> -
> -(defcustom notmuch-always-prompt-for-sender nil
> -  "Always prompt for the From: address when composing or forwarding a message.
> -
> -This is not taken into account when replying to a message, because in that case
> -the From: header is already filled in by notmuch."
> -  :type 'boolean
> -  :group 'notmuch-send)
> -
> -(defvar notmuch-mua-sender-history nil)
> -
>  (defun notmuch-mua-prompt-for-sender ()
>    (interactive)
>    (let (name addresses one-name-only)
> -- 
> 1.7.2.3
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 2/2] emacs: Improve prompting for user address when replying.

[Mark Walters]

On Sat, 04 Feb 2012 20:03:13 +0200, Jani Nikula <jani@nikula.org> wrote:
> On Sat,  4 Feb 2012 17:09:10 +0000, Mark Walters <markwalters1009@gmail.com> wrote:
> > This patch uses the new --from option to notmuch reply to allow it to
> > prompt the user for the From: address in cases when the cli does not
> > know the "correct" from address. If the cli does not it either uses
> > the users default address or, if notmuch-always-prompt-for-sender
> > is set, prompts the user.
> > ---
> >  emacs/notmuch-mua.el |   47 ++++++++++++++++++++++++++++-------------------
> >  1 files changed, 28 insertions(+), 19 deletions(-)
> > 
> > diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
> > index 41f82c2..36e62f9 100644
> > --- a/emacs/notmuch-mua.el
> > +++ b/emacs/notmuch-mua.el
> > @@ -51,6 +51,24 @@ list."
> >  
> >  ;;
> >  
> > +(defcustom notmuch-identities nil
> > +  "Identities that can be used as the From: address when composing a new message.
> > +
> > +If this variable is left unset, then a list will be constructed from the
> > +name and addresses configured in the notmuch configuration file."
> > +  :type '(repeat string)
> > +  :group 'notmuch-send)
> > +
> > +(defcustom notmuch-always-prompt-for-sender nil
> > +  "Always prompt for the From: address when composing or forwarding a message.
> > +
> > +This is not taken into account when replying to a message, because in that case
> > +the From: header is already filled in by notmuch."
> > +  :type 'boolean
> > +  :group 'notmuch-send)
> > +
> > +(defvar notmuch-mua-sender-history nil)
> > +
> >  (defun notmuch-mua-user-agent-full ()
> >    "Generate a `User-Agent:' string suitable for notmuch."
> >    (concat (notmuch-mua-user-agent-notmuch)
> > @@ -75,7 +93,7 @@ list."
> >  (defun notmuch-mua-reply (query-string &optional sender reply-all)
> >    (let (headers
> >  	body
> > -	(args '("reply")))
> > +	(args '("reply" "--from=fallback-received")))
> 
> There are better reviewers for the rest of the emacs bits, but wouldn't
> it be better to just use the "notmuch reply" default when the user wants
> the current behaviour?

Does any user actually want to be prompted for a From address when
mailing/forwarding normally but not when replying and there is no reason
to choose one address over another?

What I would actually like is to consolidate the From address choosing
so that it occurs in one place in the emacs code, so the three
(mail/forward/reply) naturally behave the same.

Thanks for the rapid review (and the significant bug catch!)

Best wishes

Mark


> BR,
> Jani.
> 
> 
> >      (if notmuch-show-process-crypto
> >  	(setq args (append args '("--decrypt"))))
> >      (if reply-all
> > @@ -99,6 +117,15 @@ list."
> >      ;; If sender is non-nil, set the From: header to its value.
> >      (when sender
> >        (mail-header-set 'from sender headers))
> > +    ;; If we do not have a From: header yet it means that
> > +    ;; notmuch-reply.c was not able to make a useful guess so we fill
> > +    ;; it in ourselves.
> > +    (when (string= "" (mail-header 'from headers))
> > +      (if notmuch-always-prompt-for-sender
> > +	  (setq sender (notmuch-mua-prompt-for-sender))
> > +	(setq sender (concat
> > +		      (notmuch-user-name) " <" (notmuch-user-primary-email) ">")))
> > +      (mail-header-set 'from sender headers))
> >      (let
> >  	;; Overlay the composition window on that being used to read
> >  	;; the original message.
> > @@ -153,24 +180,6 @@ OTHER-ARGS are passed through to `message-mail'."
> >  
> >    (message-goto-to))
> >  
> > -(defcustom notmuch-identities nil
> > -  "Identities that can be used as the From: address when composing a new message.
> > -
> > -If this variable is left unset, then a list will be constructed from the
> > -name and addresses configured in the notmuch configuration file."
> > -  :type '(repeat string)
> > -  :group 'notmuch-send)
> > -
> > -(defcustom notmuch-always-prompt-for-sender nil
> > -  "Always prompt for the From: address when composing or forwarding a message.
> > -
> > -This is not taken into account when replying to a message, because in that case
> > -the From: header is already filled in by notmuch."
> > -  :type 'boolean
> > -  :group 'notmuch-send)
> > -
> > -(defvar notmuch-mua-sender-history nil)
> > -
> >  (defun notmuch-mua-prompt-for-sender ()
> >    (interactive)
> >    (let (name addresses one-name-only)
> > -- 
> > 1.7.2.3
> > 
> > _______________________________________________
> > notmuch mailing list
> > notmuch@notmuchmail.org
> > http://notmuchmail.org/mailman/listinfo/notmuch