unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [Patch v2 0/4] Control selection of From: header when replying
@ 2012-02-04 20:45 Mark Walters
  2012-02-04 20:45 ` [PATCH v2 1/4] cli: add --from option to reply to restrict guessing of the From: header Mark Walters
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Mark Walters @ 2012-02-04 20:45 UTC (permalink / raw)
  To: notmuch


This is version 2 of the patch set first posted in
id:"1328375350-10352-1-git-send-email-markwalters1009@gmail.com"

This fixes the bug Jani pointed out as well as the more minor
criticisms. It also has some tests and the man pages are updated.

Best wishes

Mark

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 1/4] cli: add --from option to reply to restrict guessing of the From: header.
  2012-02-04 20:45 [Patch v2 0/4] Control selection of From: header when replying Mark Walters
@ 2012-02-04 20:45 ` Mark Walters
  2012-02-04 22:07   ` Jani Nikula
  2012-02-04 20:45 ` [PATCH v2 2/4] cli: update man page for notmuch-reply --from: Mark Walters
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Mark Walters @ 2012-02-04 20:45 UTC (permalink / raw)
  To: notmuch

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 |   45 ++++++++++++++++++++++++++++++++++++---------
 1 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/notmuch-reply.c b/notmuch-reply.c
index f55b1d2..8c73cb7 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -24,6 +24,15 @@
 #include "gmime-filter-reply.h"
 #include "gmime-filter-headers.h"
 
+/* The order here matters as we use '<' when deciding how to behave. */
+enum {
+    FROM_FALLBACK_ALL,
+    FROM_FALLBACK_RECEIVED,
+    FROM_FALLBACK_NONE,
+    FROM_NONE,
+    FROM_PRIMARY
+};
+
 static void
 reply_headers_message_part (GMimeMessage *message);
 
@@ -510,7 +519,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_select)
 {
     GMimeMessage *reply;
     notmuch_messages_t *messages;
@@ -542,15 +552,22 @@ 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_select <= FROM_FALLBACK_RECEIVED)
 	    from_addr = guess_from_received_header (config, message);
 
-	if (from_addr == NULL)
+	if ((from_addr == NULL && from_select <= FROM_FALLBACK_ALL) ||
+	    from_select == FROM_PRIMARY)
 	    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 we have an address and we want an address print
+	 * it. Otherwise set an empty From: header. */
+	if (from_addr != NULL && from_select != 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 +607,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_select))
 {
     GMimeMessage *reply;
     notmuch_messages_t *messages;
@@ -657,10 +675,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_select);
     notmuch_show_params_t params = { .part = -1 };
     int format = FORMAT_DEFAULT;
     int reply_all = TRUE;
+    int from_select = FROM_FALLBACK_ALL;
     notmuch_bool_t decrypt = FALSE;
 
     notmuch_opt_desc_t options[] = {
@@ -672,6 +691,14 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
 	  (notmuch_keyword_t []){ { "all", TRUE },
 				  { "sender", FALSE },
 				  { 0, 0 } } },
+	{ NOTMUCH_OPT_KEYWORD, &from_select, "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 },
+				  { "primary", FROM_PRIMARY },
+				  { 0, 0 } } },
 	{ NOTMUCH_OPT_BOOLEAN, &decrypt, "decrypt", 'd', 0 },
 	{ 0, 0, 0, 0, 0 }
     };
@@ -732,7 +759,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_select) != 0)
 	return 1;
 
     notmuch_query_destroy (query);
-- 
1.7.2.3

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 2/4] cli: update man page for notmuch-reply --from:
  2012-02-04 20:45 [Patch v2 0/4] Control selection of From: header when replying Mark Walters
  2012-02-04 20:45 ` [PATCH v2 1/4] cli: add --from option to reply to restrict guessing of the From: header Mark Walters
@ 2012-02-04 20:45 ` Mark Walters
  2012-02-04 20:45 ` [PATCH v2 3/4] emacs: Improve prompting for user address when replying Mark Walters
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Mark Walters @ 2012-02-04 20:45 UTC (permalink / raw)
  To: notmuch

---
 man/man1/notmuch-reply.1 |   27 +++++++++++++++++++++++++++
 1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/man/man1/notmuch-reply.1 b/man/man1/notmuch-reply.1
index 5160ece..101ed47 100644
--- a/man/man1/notmuch-reply.1
+++ b/man/man1/notmuch-reply.1
@@ -62,6 +62,33 @@ addresses), try To:, Cc:, and Bcc: headers in this order, and copy
 values from the first that contains something other than only the
 user's addresses.
 .RE
+.TP 4
+.BR \-\-from= ( default | fallback-all | fallback-received | fallback-none | none | primary )
+.RS
+.TP 4
+Controls the choice of the From: header returned.
+.TP 4
+.BR default | fallback-all
+Chooses the From: address by looking at the reply-to/from/to/cc
+headers of the message being replied to, if no user address is found
+look in the delivery headers, if still no user address is found return
+the user's primary address.
+.TP 4
+.BR fallback-received
+As above but if no user address is found after looking in the delivery
+headers returns an empty From: header.
+.TP 4
+.BR fallback-none
+Only look at the reply-to/from/to/cc of the original message. If no
+user address is found return an empty From: header.
+.TP 4
+.BR none
+Always return an empty From: header.
+.TP 4
+.BR primary
+Always return the users primary address as the From: header.
+.RE
+.RE
 .RE
 
 See \fBnotmuch-search-terms\fR(7)
-- 
1.7.2.3

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 3/4] emacs: Improve prompting for user address when replying.
  2012-02-04 20:45 [Patch v2 0/4] Control selection of From: header when replying Mark Walters
  2012-02-04 20:45 ` [PATCH v2 1/4] cli: add --from option to reply to restrict guessing of the From: header Mark Walters
  2012-02-04 20:45 ` [PATCH v2 2/4] cli: update man page for notmuch-reply --from: Mark Walters
@ 2012-02-04 20:45 ` Mark Walters
  2012-02-04 20:45 ` [PATCH v2 4/4] test: test the new --from option to notmuch-reply Mark Walters
  2012-02-05  6:58 ` [Patch v2 0/4] Control selection of From: header when replying Dmitry Kurochkin
  4 siblings, 0 replies; 9+ messages in thread
From: Mark Walters @ 2012-02-04 20:45 UTC (permalink / raw)
  To: notmuch

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 4/4] test: test the new --from option to notmuch-reply
  2012-02-04 20:45 [Patch v2 0/4] Control selection of From: header when replying Mark Walters
                   ` (2 preceding siblings ...)
  2012-02-04 20:45 ` [PATCH v2 3/4] emacs: Improve prompting for user address when replying Mark Walters
@ 2012-02-04 20:45 ` Mark Walters
  2012-02-05  6:58 ` [Patch v2 0/4] Control selection of From: header when replying Dmitry Kurochkin
  4 siblings, 0 replies; 9+ messages in thread
From: Mark Walters @ 2012-02-04 20:45 UTC (permalink / raw)
  To: notmuch

---
 test/reply |   69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 69 insertions(+), 0 deletions(-)

diff --git a/test/reply b/test/reply
index 00f4bea..0bb22bb 100755
--- a/test/reply
+++ b/test/reply
@@ -138,4 +138,73 @@ References: <${gen_msg_id}>
 
 On Tue, 05 Jan 2010 15:43:56 -0000, Notmuch Test Suite <test_suite@notmuchmail.org> wrote:
 > 200-byte header"
+
+test_begin_subtest "reply --from=none"
+add_message '[from]="Sender <sender@example.com>"' \
+	     [to]=test_suite@notmuchmail.org \
+	     [subject]=notmuch-reply-test \
+	    '[date]="Tue, 05 Jan 2010 15:43:56 -0000"' \
+	    '[body]="reply --from=none test"'
+
+output=$(notmuch reply --from=none id:${gen_msg_id})
+test_expect_equal "$output" "From: 
+Subject: Re: notmuch-reply-test
+To: Sender <sender@example.com>
+In-Reply-To: <${gen_msg_id}>
+References: <${gen_msg_id}>
+
+On Tue, 05 Jan 2010 15:43:56 -0000, Sender <sender@example.com> wrote:
+> reply --from=none test"
+
+test_begin_subtest "reply --from:fallback-none"
+add_message '[from]="Sender <sender@example.com>"' \
+	    '[to]="Someone Else <someone@example.com>"' \
+	     [subject]=notmuch-reply-test \
+	    '[date]="Tue, 05 Jan 2010 15:43:56 -0000"' \
+	    '[body]="reply --from=fallback-none test"'
+
+output=$(notmuch reply --from=fallback-none id:${gen_msg_id})
+test_expect_equal "$output" "From: 
+Subject: Re: notmuch-reply-test
+To: Sender <sender@example.com>, Someone Else <someone@example.com>
+In-Reply-To: <${gen_msg_id}>
+References: <${gen_msg_id}>
+
+On Tue, 05 Jan 2010 15:43:56 -0000, Sender <sender@example.com> wrote:
+> reply --from=fallback-none test"
+
+test_begin_subtest "reply default from:"
+add_message '[from]="Sender <sender@example.com>"' \
+	    '[to]="Someone Else <someone@example.com>"' \
+	     [subject]=notmuch-reply-test \
+	    '[date]="Tue, 05 Jan 2010 15:43:56 -0000"' \
+	    '[body]="reply default from: test"'
+
+output=$(notmuch reply id:${gen_msg_id})
+test_expect_equal "$output" "From: Notmuch Test Suite <test_suite@notmuchmail.org>
+Subject: Re: notmuch-reply-test
+To: Sender <sender@example.com>, Someone Else <someone@example.com>
+In-Reply-To: <${gen_msg_id}>
+References: <${gen_msg_id}>
+
+On Tue, 05 Jan 2010 15:43:56 -0000, Sender <sender@example.com> wrote:
+> reply default from: test"
+
+test_begin_subtest "Reply --from=primary"
+add_message '[from]="Sender <sender@example.com>"' \
+	     [to]=test_suite_other@notmuchmail.org \
+	     [subject]=notmuch-reply-test \
+	    '[date]="Tue, 05 Jan 2010 15:43:56 -0000"' \
+	    '[body]="reply --from=primary"'
+
+output=$(notmuch reply --from=primary id:${gen_msg_id})
+test_expect_equal "$output" "From: Notmuch Test Suite <test_suite@notmuchmail.org>
+Subject: Re: notmuch-reply-test
+To: Sender <sender@example.com>
+In-Reply-To: <${gen_msg_id}>
+References: <${gen_msg_id}>
+
+On Tue, 05 Jan 2010 15:43:56 -0000, Sender <sender@example.com> wrote:
+> reply --from=primary"
+
 test_done
-- 
1.7.2.3

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/4] cli: add --from option to reply to restrict guessing of the From: header.
  2012-02-04 20:45 ` [PATCH v2 1/4] cli: add --from option to reply to restrict guessing of the From: header Mark Walters
@ 2012-02-04 22:07   ` Jani Nikula
  0 siblings, 0 replies; 9+ messages in thread
From: Jani Nikula @ 2012-02-04 22:07 UTC (permalink / raw)
  To: Mark Walters, notmuch

On Sat,  4 Feb 2012 20:45:14 +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

The patch looks good. The "primary" option added in v2 is missing from
the commit message, but no need to send a new version because of that.

I didn't check the other patches in the series.

BR,
Jani.


> 
> 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 |   45 ++++++++++++++++++++++++++++++++++++---------
>  1 files changed, 36 insertions(+), 9 deletions(-)
> 
> diff --git a/notmuch-reply.c b/notmuch-reply.c
> index f55b1d2..8c73cb7 100644
> --- a/notmuch-reply.c
> +++ b/notmuch-reply.c
> @@ -24,6 +24,15 @@
>  #include "gmime-filter-reply.h"
>  #include "gmime-filter-headers.h"
>  
> +/* The order here matters as we use '<' when deciding how to behave. */
> +enum {
> +    FROM_FALLBACK_ALL,
> +    FROM_FALLBACK_RECEIVED,
> +    FROM_FALLBACK_NONE,
> +    FROM_NONE,
> +    FROM_PRIMARY
> +};
> +
>  static void
>  reply_headers_message_part (GMimeMessage *message);
>  
> @@ -510,7 +519,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_select)
>  {
>      GMimeMessage *reply;
>      notmuch_messages_t *messages;
> @@ -542,15 +552,22 @@ 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_select <= FROM_FALLBACK_RECEIVED)
>  	    from_addr = guess_from_received_header (config, message);
>  
> -	if (from_addr == NULL)
> +	if ((from_addr == NULL && from_select <= FROM_FALLBACK_ALL) ||
> +	    from_select == FROM_PRIMARY)
>  	    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 we have an address and we want an address print
> +	 * it. Otherwise set an empty From: header. */
> +	if (from_addr != NULL && from_select != 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 +607,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_select))
>  {
>      GMimeMessage *reply;
>      notmuch_messages_t *messages;
> @@ -657,10 +675,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_select);
>      notmuch_show_params_t params = { .part = -1 };
>      int format = FORMAT_DEFAULT;
>      int reply_all = TRUE;
> +    int from_select = FROM_FALLBACK_ALL;
>      notmuch_bool_t decrypt = FALSE;
>  
>      notmuch_opt_desc_t options[] = {
> @@ -672,6 +691,14 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
>  	  (notmuch_keyword_t []){ { "all", TRUE },
>  				  { "sender", FALSE },
>  				  { 0, 0 } } },
> +	{ NOTMUCH_OPT_KEYWORD, &from_select, "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 },
> +				  { "primary", FROM_PRIMARY },
> +				  { 0, 0 } } },
>  	{ NOTMUCH_OPT_BOOLEAN, &decrypt, "decrypt", 'd', 0 },
>  	{ 0, 0, 0, 0, 0 }
>      };
> @@ -732,7 +759,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_select) != 0)
>  	return 1;
>  
>      notmuch_query_destroy (query);
> -- 
> 1.7.2.3
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Patch v2 0/4] Control selection of From: header when replying
  2012-02-04 20:45 [Patch v2 0/4] Control selection of From: header when replying Mark Walters
                   ` (3 preceding siblings ...)
  2012-02-04 20:45 ` [PATCH v2 4/4] test: test the new --from option to notmuch-reply Mark Walters
@ 2012-02-05  6:58 ` Dmitry Kurochkin
  2012-02-05 12:50   ` Mark Walters
  2012-02-05 13:12   ` Jani Nikula
  4 siblings, 2 replies; 9+ messages in thread
From: Dmitry Kurochkin @ 2012-02-05  6:58 UTC (permalink / raw)
  To: Mark Walters, notmuch

Hi Mark.

I am not sure I like this solution.  My concerns are:

* New option looks too complex, too specific.

* There are more aspects of notmuch reply behavior which users would
  like to change (e.g. which part to quote).  If we add an option for
  each, we complicate both nomtuch show UI and code.

The problem is that notmuch show output format is too limiting.  Instead
of providing myriad of options for tweaking notmuch show text format
behavior, we should add JSON format for notmuch reply similar to nomtuch
show.  That would allow notmuch reply to produce structured output with
required additional information, which should be enough for users to
construct whatever reply they want.

In this particular case, notmuch reply JSON format could have
"from-source" attribute that would indicate how it was guessed.

Now the best part.  Not so long ago, Adam (in Cc) provided a patch for
improving nomtuch reply for HTML-only emails.  At first he added an
option for notmuch reply, like you did for from-guessing.  I suggested
him to implement it based on the JSON format instead and he did.  AFAIK
the latest version of his patches is [1].  I did not look at the code
though.  It seems that it is waiting for more review.

So, instead of adding more nomtuch show options, I think a better
solution is to work with Adam to get the notmuch reply JSON format to
master and then fix the from-guessing issue by adding an attribute to
notmuch reply JSON format.

Regards,
  Dmitry

[1] id:"1326995217-27423-1-git-send-email-awg+notmuch@xvx.ca"

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Patch v2 0/4] Control selection of From: header when replying
  2012-02-05  6:58 ` [Patch v2 0/4] Control selection of From: header when replying Dmitry Kurochkin
@ 2012-02-05 12:50   ` Mark Walters
  2012-02-05 13:12   ` Jani Nikula
  1 sibling, 0 replies; 9+ messages in thread
From: Mark Walters @ 2012-02-05 12:50 UTC (permalink / raw)
  To: Dmitry Kurochkin, notmuch

On Sun, 05 Feb 2012 10:58:04 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> Hi Mark.
> 
> I am not sure I like this solution.  My concerns are:
> 
> * New option looks too complex, too specific.
> 
> * There are more aspects of notmuch reply behavior which users would
>   like to change (e.g. which part to quote).  If we add an option for
>   each, we complicate both nomtuch show UI and code.
> 
> The problem is that notmuch show output format is too limiting.  Instead
> of providing myriad of options for tweaking notmuch show text format
> behavior, we should add JSON format for notmuch reply similar to nomtuch
> show.  That would allow notmuch reply to produce structured output with
> required additional information, which should be enough for users to
> construct whatever reply they want.
> 
> In this particular case, notmuch reply JSON format could have
> "from-source" attribute that would indicate how it was guessed.
> 
> Now the best part.  Not so long ago, Adam (in Cc) provided a patch for
> improving nomtuch reply for HTML-only emails.  At first he added an
> option for notmuch reply, like you did for from-guessing.  I suggested
> him to implement it based on the JSON format instead and he did.  AFAIK
> the latest version of his patches is [1].  I did not look at the code
> though.  It seems that it is waiting for more review.
> 
> So, instead of adding more nomtuch show options, I think a better
> solution is to work with Adam to get the notmuch reply JSON format to
> master and then fix the from-guessing issue by adding an attribute to
> notmuch reply JSON format.

Yes I think you are quite right: this would be a better solution
(particularly as Adam has already done all the hard work!)

Many thanks

Mark

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Patch v2 0/4] Control selection of From: header when replying
  2012-02-05  6:58 ` [Patch v2 0/4] Control selection of From: header when replying Dmitry Kurochkin
  2012-02-05 12:50   ` Mark Walters
@ 2012-02-05 13:12   ` Jani Nikula
  1 sibling, 0 replies; 9+ messages in thread
From: Jani Nikula @ 2012-02-05 13:12 UTC (permalink / raw)
  To: Dmitry Kurochkin, Mark Walters, notmuch

On Sun, 05 Feb 2012 10:58:04 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> Hi Mark.
> 
> I am not sure I like this solution.  My concerns are:
> 
> * New option looks too complex, too specific.

*shrug* The --from parameter is simple to implement, simple to test, and
simple to use.

> * There are more aspects of notmuch reply behavior which users would
>   like to change (e.g. which part to quote).  If we add an option for
>   each, we complicate both nomtuch show UI and code.

This I agree is more of an issue.

> The problem is that notmuch show output format is too limiting.  Instead
> of providing myriad of options for tweaking notmuch show text format
> behavior, we should add JSON format for notmuch reply similar to nomtuch
> show.  That would allow notmuch reply to produce structured output with
> required additional information, which should be enough for users to
> construct whatever reply they want.

Heh, when I told Mark on IRC to just send the patches and not discuss
and worry about it too much, I added "...and then someone will come up
with an approach we failed to think of, and scrap the whole
thing". Thanks Dmitry! ;)

> In this particular case, notmuch reply JSON format could have
> "from-source" attribute that would indicate how it was guessed.

My first thought is that it's offloading things that are trivial in the
cli to the users of the cli where it might be slightly more complicated,
but...

> Now the best part.  Not so long ago, Adam (in Cc) provided a patch for
> improving nomtuch reply for HTML-only emails.  At first he added an
> option for notmuch reply, like you did for from-guessing.  I suggested
> him to implement it based on the JSON format instead and he did.  AFAIK
> the latest version of his patches is [1].  I did not look at the code
> though.  It seems that it is waiting for more review.
> 
> So, instead of adding more nomtuch show options, I think a better
> solution is to work with Adam to get the notmuch reply JSON format to
> master and then fix the from-guessing issue by adding an attribute to
> notmuch reply JSON format.

...no matter what the solution will be in the end, I agree it's best to
get Adam's work merged first, and see how easily this can be handled
using JSON vs. the parameter.

Thanks for your insights.


BR,
Jani.


> Regards,
>   Dmitry
> 
> [1] id:"1326995217-27423-1-git-send-email-awg+notmuch@xvx.ca"
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2012-02-05 13:12 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-02-04 20:45 [Patch v2 0/4] Control selection of From: header when replying Mark Walters
2012-02-04 20:45 ` [PATCH v2 1/4] cli: add --from option to reply to restrict guessing of the From: header Mark Walters
2012-02-04 22:07   ` Jani Nikula
2012-02-04 20:45 ` [PATCH v2 2/4] cli: update man page for notmuch-reply --from: Mark Walters
2012-02-04 20:45 ` [PATCH v2 3/4] emacs: Improve prompting for user address when replying Mark Walters
2012-02-04 20:45 ` [PATCH v2 4/4] test: test the new --from option to notmuch-reply Mark Walters
2012-02-05  6:58 ` [Patch v2 0/4] Control selection of From: header when replying Dmitry Kurochkin
2012-02-05 12:50   ` Mark Walters
2012-02-05 13:12   ` Jani Nikula

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).