[PATCH v2 0/3] cli: mail-followup-to support

[PATCH v2 0/3] cli: mail-followup-to support

[Jani Nikula]

This is v2 of [1], rebased against master and with a better commit
message for patch 1/3. Patch 1/3 is trivial cleanup and IMO could just
be merged. There was debate on the actual stuff 2/3 and 3/3 [2].

I don't think adding a --reply-to=list option to notmuch reply is a good
idea. We should just do the right thing and keep the cli simple. Perhaps
we should drop the configuration (patch 2/3) altogether, and just
support mail-followup-to?

Or, perhaps notmuch reply should return *all* options in json, and let
the UI decide. Above all, let the UI change the reply style while
editing the reply. I'd probably like this approach most, more than these
patches, but I don't have the time to do it.

Here are the patches anyway; perhaps they could be an intermediate step,
or just a reference for anyone willing to do the above.


BR,
Jani.


[1] id:5eaa3acc22ee5513bdce5ab931b7a79ade880e06.1362254104.git.jani@nikula.org
[2] id:20130303120745.GA4884@hili.localdomain

Jani Nikula (3):
  cli: config: remove unnecessary braces from if blocks
  cli: add reply.honor_followup_to configuration option
  cli: support Mail-Followup-To: in notmuch reply

 notmuch-client.h |    7 +++++++
 notmuch-config.c |   56 ++++++++++++++++++++++++++++++++++++++++++------------
 notmuch-reply.c  |   25 ++++++++++++++++++++++--
 3 files changed, 74 insertions(+), 14 deletions(-)

-- 
1.7.10.4

[PATCH v2 1/3] cli: config: remove unnecessary braces from if blocks

[Jani Nikula]

Cosmetic change to drop unnecessary braces that don't even conform to
the prevailing coding style.
---
 notmuch-config.c |   13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/notmuch-config.c b/notmuch-config.c
index 48312e3..66a1cdf 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -395,39 +395,28 @@ notmuch_config_open (void *ctx,
      * the configuration file, we add some comments to help the user
      * understand what can be done. */
     if (config->is_new)
-    {
 	g_key_file_set_comment (config->key_file, NULL, NULL,
 				toplevel_config_comment, NULL);
-    }
 
     if (! file_had_database_group)
-    {
 	g_key_file_set_comment (config->key_file, "database", NULL,
 				database_config_comment, NULL);
-    }
 
     if (! file_had_new_group)
-    {
 	g_key_file_set_comment (config->key_file, "new", NULL,
 				new_config_comment, NULL);
-    }
 
     if (! file_had_user_group)
-    {
 	g_key_file_set_comment (config->key_file, "user", NULL,
 				user_config_comment, NULL);
-    }
 
     if (! file_had_maildir_group)
-    {
 	g_key_file_set_comment (config->key_file, "maildir", NULL,
 				maildir_config_comment, NULL);
-    }
 
-    if (! file_had_search_group) {
+    if (! file_had_search_group)
 	g_key_file_set_comment (config->key_file, "search", NULL,
 				search_config_comment, NULL);
-    }
 
     return config;
 }
-- 
1.7.10.4

[PATCH v2 2/3] cli: add reply.honor_followup_to configuration option

[Jani Nikula]

The reply.honor_followup_to configuration option determines whether
notmuch reply takes into account the Mail-Followup-To: header in
incoming messages.
---
 notmuch-client.h |    7 +++++++
 notmuch-config.c |   43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+)

diff --git a/notmuch-client.h b/notmuch-client.h
index 45749a6..0b7800c 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -301,6 +301,13 @@ notmuch_config_set_new_ignore (notmuch_config_t *config,
 			       size_t length);
 
 notmuch_bool_t
+notmuch_config_get_reply_honor_followup_to (notmuch_config_t *config);
+
+void
+notmuch_config_set_reply_honor_followup_to (notmuch_config_t *config,
+					    notmuch_bool_t honor_followup_to);
+
+notmuch_bool_t
 notmuch_config_get_maildir_synchronize_flags (notmuch_config_t *config);
 
 void
diff --git a/notmuch-config.c b/notmuch-config.c
index 66a1cdf..053038e 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -53,6 +53,17 @@ static const char new_config_comment[] =
     "\t	names will be ignored, independent of its depth/location\n"
     "\t	in the mail store.\n";
 
+static const char reply_config_comment[] =
+    " Configuration for \"notmuch reply\"\n"
+    "\n"
+    " The following option is supported here:\n"
+    "\n"
+    "\thonor_followup_to	Valid values are true and false.\n"
+    "\n"
+    "\tIf true, \"notmuch reply\" will honor the \"Mail-Followup-To:\""
+    "\theader in incoming messages. (This will not generate the headers"
+    "\tto outgoing messages.)\n";
+
 static const char user_config_comment[] =
     " User configuration\n"
     "\n"
@@ -115,6 +126,7 @@ struct _notmuch_config {
     size_t new_tags_length;
     const char **new_ignore;
     size_t new_ignore_length;
+    notmuch_bool_t reply_honor_followup_to;
     notmuch_bool_t maildir_synchronize_flags;
     const char **search_exclude_tags;
     size_t search_exclude_tags_length;
@@ -240,6 +252,7 @@ notmuch_config_open (void *ctx,
     char *notmuch_config_env = NULL;
     int file_had_database_group;
     int file_had_new_group;
+    int file_had_reply_group;
     int file_had_user_group;
     int file_had_maildir_group;
     int file_had_search_group;
@@ -273,6 +286,7 @@ notmuch_config_open (void *ctx,
     config->new_tags_length = 0;
     config->new_ignore = NULL;
     config->new_ignore_length = 0;
+    config->reply_honor_followup_to = FALSE;
     config->maildir_synchronize_flags = TRUE;
     config->search_exclude_tags = NULL;
     config->search_exclude_tags_length = 0;
@@ -316,6 +330,7 @@ notmuch_config_open (void *ctx,
     file_had_database_group = g_key_file_has_group (config->key_file,
 						    "database");
     file_had_new_group = g_key_file_has_group (config->key_file, "new");
+    file_had_reply_group = g_key_file_has_group (config->key_file, "reply");
     file_had_user_group = g_key_file_has_group (config->key_file, "user");
     file_had_maildir_group = g_key_file_has_group (config->key_file, "maildir");
     file_had_search_group = g_key_file_has_group (config->key_file, "search");
@@ -383,6 +398,15 @@ notmuch_config_open (void *ctx,
     }
 
     error = NULL;
+    config->reply_honor_followup_to =
+	g_key_file_get_boolean (config->key_file,
+				"reply", "honor_followup_to", &error);
+    if (error) {
+	notmuch_config_set_reply_honor_followup_to (config, FALSE);
+	g_error_free (error);
+    }
+
+    error = NULL;
     config->maildir_synchronize_flags =
 	g_key_file_get_boolean (config->key_file,
 				"maildir", "synchronize_flags", &error);
@@ -406,6 +430,10 @@ notmuch_config_open (void *ctx,
 	g_key_file_set_comment (config->key_file, "new", NULL,
 				new_config_comment, NULL);
 
+    if (! file_had_reply_group)
+	g_key_file_set_comment (config->key_file, "reply", NULL,
+				reply_config_comment, NULL);
+
     if (! file_had_user_group)
 	g_key_file_set_comment (config->key_file, "user", NULL,
 				user_config_comment, NULL);
@@ -845,6 +873,21 @@ notmuch_config_command (notmuch_config_t *config, int argc, char *argv[])
 }
 
 notmuch_bool_t
+notmuch_config_get_reply_honor_followup_to (notmuch_config_t *config)
+{
+    return config->reply_honor_followup_to;
+}
+
+void
+notmuch_config_set_reply_honor_followup_to (notmuch_config_t *config,
+					    notmuch_bool_t honor_followup_to)
+{
+    g_key_file_set_boolean (config->key_file,
+			    "reply", "honor_followup_to", honor_followup_to);
+    config->reply_honor_followup_to = honor_followup_to;
+}
+
+notmuch_bool_t
 notmuch_config_get_maildir_synchronize_flags (notmuch_config_t *config)
 {
     return config->maildir_synchronize_flags;
-- 
1.7.10.4

[PATCH v2 3/3] cli: support Mail-Followup-To: in notmuch reply

[Jani Nikula]

Use Mail-Followup-To header to determine recipients according to
http://cr.yp.to/proto/replyto.html if configured and present in the
message being replied to.
---
 notmuch-reply.c |   25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/notmuch-reply.c b/notmuch-reply.c
index e151f78..7391886 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -324,6 +324,7 @@ add_recipients_from_message (GMimeMessage *reply,
 	{ "bcc",        NULL, GMIME_RECIPIENT_TYPE_BCC }
     };
     const char *from_addr = NULL;
+    const char *recipients;
     unsigned int i;
     unsigned int n = 0;
 
@@ -343,9 +344,29 @@ add_recipients_from_message (GMimeMessage *reply,
 	reply_to_map[0].fallback = NULL;
     }
 
-    for (i = 0; i < ARRAY_SIZE (reply_to_map); i++) {
-	const char *recipients;
+    /* Use Mail-Followup-To header to determine recipients according
+     * to http://cr.yp.to/proto/replyto.html if configured and present
+     * in the message being replied to.
+     */
+    if (reply_all && notmuch_config_get_reply_honor_followup_to (config)) {
+	recipients = notmuch_message_get_header (message, "mail-followup-to");
+	if (recipients && *recipients) {
+	    n = scan_address_string (recipients, config, reply,
+				     GMIME_RECIPIENT_TYPE_TO, &from_addr);
+	    if (n) {
+		/* Same rationale as in the loop below. */
+		reply = NULL;
+
+		/* From address and some recipients are enough, bail out. */
+		if (from_addr)
+		    return from_addr;
+
+		/* Else need to find from address in other headers. */
+	    }
+	}
+    }
 
+    for (i = 0; i < ARRAY_SIZE (reply_to_map); i++) {
 	recipients = notmuch_message_get_header (message,
 						 reply_to_map[i].header);
 	if ((recipients == NULL || recipients[0] == '\0') && reply_to_map[i].fallback)
-- 
1.7.10.4

Re: [PATCH v2 1/3] cli: config: remove unnecessary braces from if blocks

[David Bremner]

Jani Nikula <jani@nikula.org> writes:

> Cosmetic change to drop unnecessary braces that don't even conform to
> the prevailing coding style.

pushed this one patch.

Re: [PATCH v2 0/3] cli: mail-followup-to support

[Mark Walters]

Hi

On Sat, 30 Mar 2013, Jani Nikula <jani@nikula.org> wrote:
> This is v2 of [1], rebased against master and with a better commit
> message for patch 1/3. Patch 1/3 is trivial cleanup and IMO could just
> be merged. There was debate on the actual stuff 2/3 and 3/3 [2].
>
> I don't think adding a --reply-to=list option to notmuch reply is a good
> idea. We should just do the right thing and keep the cli simple. Perhaps
> we should drop the configuration (patch 2/3) altogether, and just
> support mail-followup-to?

I think I don't like it as a config option: I would prefer it either as
no option or a command line option (or the JSON as you suggest). I think
we currently honour reply-to unconditionally and ignore mail-reply-to:
is that correct? Otherwise I think the patch looks fine.

> Or, perhaps notmuch reply should return *all* options in json, and let
> the UI decide. Above all, let the UI change the reply style while
> editing the reply. I'd probably like this approach most, more than these
> patches, but I don't have the time to do it.

I have some preliminary patches to do this: will post shortly.

Best wishes

Mark


> Here are the patches anyway; perhaps they could be an intermediate step,
> or just a reference for anyone willing to do the above.
>
>
> BR,
> Jani.
>
>
> [1] id:5eaa3acc22ee5513bdce5ab931b7a79ade880e06.1362254104.git.jani@nikula.org
> [2] id:20130303120745.GA4884@hili.localdomain
>
> Jani Nikula (3):
>   cli: config: remove unnecessary braces from if blocks
>   cli: add reply.honor_followup_to configuration option
>   cli: support Mail-Followup-To: in notmuch reply
>
>  notmuch-client.h |    7 +++++++
>  notmuch-config.c |   56 ++++++++++++++++++++++++++++++++++++++++++------------
>  notmuch-reply.c  |   25 ++++++++++++++++++++++--
>  3 files changed, 74 insertions(+), 14 deletions(-)
>
> -- 
> 1.7.10.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch