[Patch v2 0/4] Control selection of From: header when replying

classic Classic list List threaded Threaded
9 messages Options
Mark Walters Mark Walters
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate
star

[Patch v2 0/4] Control selection of From: header when replying


This is version 2 of the patch set first posted in
id:"[hidden email]"

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

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch
Mark Walters Mark Walters
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate
star

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

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

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch
Mark Walters Mark Walters
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate
star

[PATCH v2 2/4] cli: update man page for notmuch-reply --from:

In reply to this post by Mark Walters
---
 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

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch
Mark Walters Mark Walters
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate
star

[PATCH v2 3/4] emacs: Improve prompting for user address when replying.

In reply to this post by 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

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch
Mark Walters Mark Walters
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate
star

[PATCH v2 4/4] test: test the new --from option to notmuch-reply

In reply to this post by Mark Walters
---
 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 <[hidden email]> wrote:
 > 200-byte header"
+
+test_begin_subtest "reply --from=none"
+add_message '[from]="Sender <[hidden email]>"' \
+     [to]=[hidden email] \
+     [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 <[hidden email]>
+In-Reply-To: <${gen_msg_id}>
+References: <${gen_msg_id}>
+
+On Tue, 05 Jan 2010 15:43:56 -0000, Sender <[hidden email]> wrote:
+> reply --from=none test"
+
+test_begin_subtest "reply --from:fallback-none"
+add_message '[from]="Sender <[hidden email]>"' \
+    '[to]="Someone Else <[hidden email]>"' \
+     [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 <[hidden email]>, Someone Else <[hidden email]>
+In-Reply-To: <${gen_msg_id}>
+References: <${gen_msg_id}>
+
+On Tue, 05 Jan 2010 15:43:56 -0000, Sender <[hidden email]> wrote:
+> reply --from=fallback-none test"
+
+test_begin_subtest "reply default from:"
+add_message '[from]="Sender <[hidden email]>"' \
+    '[to]="Someone Else <[hidden email]>"' \
+     [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 <[hidden email]>
+Subject: Re: notmuch-reply-test
+To: Sender <[hidden email]>, Someone Else <[hidden email]>
+In-Reply-To: <${gen_msg_id}>
+References: <${gen_msg_id}>
+
+On Tue, 05 Jan 2010 15:43:56 -0000, Sender <[hidden email]> wrote:
+> reply default from: test"
+
+test_begin_subtest "Reply --from=primary"
+add_message '[from]="Sender <[hidden email]>"' \
+     [to]=[hidden email] \
+     [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 <[hidden email]>
+Subject: Re: notmuch-reply-test
+To: Sender <[hidden email]>
+In-Reply-To: <${gen_msg_id}>
+References: <${gen_msg_id}>
+
+On Tue, 05 Jan 2010 15:43:56 -0000, Sender <[hidden email]> wrote:
+> reply --from=primary"
+
 test_done
--
1.7.2.3

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch
Jani Nikula Jani Nikula
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate
star

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

In reply to this post by Mark Walters
On Sat,  4 Feb 2012 20:45:14 +0000, Mark Walters <[hidden email]> 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
> [hidden email]
> http://notmuchmail.org/mailman/listinfo/notmuch
_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch
Dmitry Kurochkin Dmitry Kurochkin
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate
star

Re: [Patch v2 0/4] Control selection of From: header when replying

In reply to this post by Mark Walters
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:"[hidden email]"
_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch
Mark Walters Mark Walters
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate
star

Re: [Patch v2 0/4] Control selection of From: header when replying

On Sun, 05 Feb 2012 10:58:04 +0400, Dmitry Kurochkin <[hidden email]> 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
_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch
Jani Nikula Jani Nikula
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate
star

Re: [Patch v2 0/4] Control selection of From: header when replying

In reply to this post by Dmitry Kurochkin
On Sun, 05 Feb 2012 10:58:04 +0400, Dmitry Kurochkin <[hidden email]> 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:"[hidden email]"
> _______________________________________________
> notmuch mailing list
> [hidden email]
> http://notmuchmail.org/mailman/listinfo/notmuch
_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch
Loading...