unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH 0/4] Allow specifying alternate names for addresses in other_email
@ 2016-08-09 20:55 Shea Levy
  2016-08-09 20:55 ` [PATCH 1/4] Add user.other_name property to associate names with other_email Shea Levy
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Shea Levy @ 2016-08-09 20:55 UTC (permalink / raw)
  To: notmuch

Currently, while notmuch-reply will recognize email addresses other than
the main address with user.other_email, it always sets the name part of
the address in the envelope-from and From headers to user.name. This
patchset enables specifying names on a per-address basis with a new
user.other_name property. Presumably other users of user.other_email
may want to use this as well, but those are not updated currently.

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

* [PATCH 1/4] Add user.other_name property to associate names with other_email.
  2016-08-09 20:55 [PATCH 0/4] Allow specifying alternate names for addresses in other_email Shea Levy
@ 2016-08-09 20:55 ` Shea Levy
  2016-08-09 20:55 ` [PATCH 2/4] notmuch-reply: respect users.other_name in From Shea Levy
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Shea Levy @ 2016-08-09 20:55 UTC (permalink / raw)
  To: notmuch

Entries are paired with the email address at the same index in
other_email. To use user.name for a given other_email, leave that
entry blank or leave user.other_name shorter than the relevant index
altogether.
---
 notmuch-client.h |  9 +++++++++
 notmuch-config.c | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/notmuch-client.h b/notmuch-client.h
index ebc092b..69c83a2 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -304,6 +304,15 @@ notmuch_config_set_user_other_email (notmuch_config_t *config,
 				     size_t length);
 
 const char **
+notmuch_config_get_user_other_name (notmuch_config_t *config,
+				    size_t *length);
+
+void
+notmuch_config_set_user_other_name (notmuch_config_t *config,
+				    const char *other_name[],
+				    size_t length);
+
+const char **
 notmuch_config_get_new_tags (notmuch_config_t *config,
 			     size_t *length);
 void
diff --git a/notmuch-config.c b/notmuch-config.c
index e5d42a0..4ac16b7 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -63,6 +63,8 @@ static const char user_config_comment[] =
     "\tprimary_email	Your primary email address.\n"
     "\tother_email	A list (separated by ';') of other email addresses\n"
     "\t		at which you receive email.\n"
+    "\tother_name	A list (separated by ';') of names corresponding to\n"
+    "\t 	other_email addresses. Leave an entry blank to use default\n"
     "\n"
     " Notmuch will use the various email addresses configured here when\n"
     " formatting replies. It will avoid including your own addresses in the\n"
@@ -120,6 +122,8 @@ struct _notmuch_config {
     char *user_primary_email;
     const char **user_other_email;
     size_t user_other_email_length;
+    const char **user_other_name;
+    size_t user_other_name_length;
     const char **new_tags;
     size_t new_tags_length;
     const char **new_ignore;
@@ -237,6 +241,8 @@ get_username_from_passwd_file (void *ctx)
  *
  *		user_other_email:	Not set.
  *
+ *		user_other_name:	Not set.
+ *
  *	The default configuration also contains comments to guide the
  *	user in editing the file directly.
  */
@@ -280,6 +286,8 @@ notmuch_config_open (void *ctx,
     config->user_primary_email = NULL;
     config->user_other_email = NULL;
     config->user_other_email_length = 0;
+    config->user_other_name = NULL;
+    config->user_other_name_length = 0;
     config->new_tags = NULL;
     config->new_tags_length = 0;
     config->new_ignore = NULL;
@@ -674,6 +682,23 @@ notmuch_config_set_user_other_email (notmuch_config_t *config,
 		     &(config->user_other_email));
 }
 
+const char **
+notmuch_config_get_user_other_name (notmuch_config_t *config, size_t *length)
+{
+    return _config_get_list (config, "user", "other_name",
+			     &(config->user_other_name),
+			     &(config->user_other_name_length), length);
+}
+
+void
+notmuch_config_set_user_other_name (notmuch_config_t *config,
+				    const char *list[],
+				    size_t length)
+{
+    _config_set_list (config, "user", "other_name", list, length,
+		     &(config->user_other_name));
+}
+
 void
 notmuch_config_set_new_tags (notmuch_config_t *config,
 				     const char *list[],
@@ -790,6 +815,13 @@ notmuch_config_command_get (notmuch_config_t *config, char *item)
 	other_email = notmuch_config_get_user_other_email (config, &length);
 	for (i = 0; i < length; i++)
 	    printf ("%s\n", other_email[i]);
+    } else if (strcmp(item, "user.other_name") == 0) {
+	const char **other_name;
+	size_t i, length;
+
+	other_name = notmuch_config_get_user_other_name (config, &length);
+	for (i = 0; i < length; i++)
+	    printf ("%s\n", other_name[i]);
     } else if (strcmp(item, "new.tags") == 0) {
 	const char **tags;
 	size_t i, length;
-- 
2.7.4

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

* [PATCH 2/4] notmuch-reply: respect users.other_name in From
  2016-08-09 20:55 [PATCH 0/4] Allow specifying alternate names for addresses in other_email Shea Levy
  2016-08-09 20:55 ` [PATCH 1/4] Add user.other_name property to associate names with other_email Shea Levy
@ 2016-08-09 20:55 ` Shea Levy
  2016-08-09 20:55 ` [PATCH 3/4] Add documentation for user.other_name Shea Levy
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Shea Levy @ 2016-08-09 20:55 UTC (permalink / raw)
  To: notmuch

---
 notmuch-reply.c | 129 ++++++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 92 insertions(+), 37 deletions(-)

diff --git a/notmuch-reply.c b/notmuch-reply.c
index 4951373..1c205f9 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -118,25 +118,41 @@ match_address (const char *str, const char *address, address_match_t mode)
 }
 
 /* Match given string against user's configured "primary" and "other"
- * addresses according to mode. */
+ * addresses according to mode.
+ *
+ * If 'user_from_name' is non-NULL and the address is found,
+ * *user_from_name is set to the corresponding name.
+ */
 static const char *
-address_match (const char *str, notmuch_config_t *config, address_match_t mode)
+address_match (const char *str, notmuch_config_t *config, const char **user_from_name, address_match_t mode)
 {
     const char *primary;
     const char **other;
-    size_t i, other_len;
+    const char **other_name;
+    size_t i, other_len, other_name_len;
 
     if (!str || *str == '\0')
 	return NULL;
 
     primary = notmuch_config_get_user_primary_email (config);
-    if (match_address (str, primary, mode))
+    if (match_address (str, primary, mode)) {
+	if (user_from_name != NULL)
+	    *user_from_name = notmuch_config_get_user_name (config);
 	return primary;
+    }
 
     other = notmuch_config_get_user_other_email (config, &other_len);
     for (i = 0; i < other_len; i++) {
-	if (match_address (str, other[i], mode))
+	if (match_address (str, other[i], mode)) {
+	    if (user_from_name != NULL) {
+		other_name = notmuch_config_get_user_other_name (config, &other_name_len);
+		if (i < other_name_len)
+		    *user_from_name = other_name[i];
+		else
+		    *user_from_name = NULL;
+	    }
 	    return other[i];
+	}
     }
 
     return NULL;
@@ -144,28 +160,41 @@ address_match (const char *str, notmuch_config_t *config, address_match_t mode)
 
 /* Does the given string contain an address configured as one of the
  * user's "primary" or "other" addresses. If so, return the matching
- * address, NULL otherwise. */
+ * address, NULL otherwise.
+ *
+ * If 'user_from_name' is non-NULL and the address is the user's,
+ * *user_from_name is set to the corresponding name.
+ */
 static const char *
-user_address_in_string (const char *str, notmuch_config_t *config)
+user_address_in_string (const char *str, notmuch_config_t *config, const char **user_from_name)
 {
-    return address_match (str, config, USER_ADDRESS_IN_STRING);
+    return address_match (str, config, user_from_name, USER_ADDRESS_IN_STRING);
 }
 
 /* Do any of the addresses configured as one of the user's "primary"
  * or "other" addresses contain the given string. If so, return the
- * matching address, NULL otherwise. */
+ * matching address, NULL otherwise.
+ *
+ * If 'user_from_name' is non-NULL and the address is the user's,
+ * *user_from_name is set to the corresponding name.
+ */
 static const char *
-string_in_user_address (const char *str, notmuch_config_t *config)
+string_in_user_address (const char *str, notmuch_config_t *config, const char **user_from_name)
 {
-    return address_match (str, config, STRING_IN_USER_ADDRESS);
+    return address_match (str, config, user_from_name, STRING_IN_USER_ADDRESS);
 }
 
 /* Is the given address configured as one of the user's "primary" or
- * "other" addresses. */
+ * "other" addresses.
+ *
+ * If 'user_from_name' is non-NULL and the address is the user's,
+ * *user_from_name is set to the corresponding name.
+ *
+ */
 static notmuch_bool_t
-address_is_users (const char *address, notmuch_config_t *config)
+address_is_users (const char *address, notmuch_config_t *config, const char **user_from_name)
 {
-    return address_match (address, config, STRING_IS_USER_ADDRESS) != NULL;
+    return address_match (address, config, user_from_name, STRING_IS_USER_ADDRESS) != NULL;
 }
 
 /* Scan addresses in 'list'.
@@ -178,6 +207,9 @@ address_is_users (const char *address, notmuch_config_t *config)
  * be set to the first address encountered in 'list' that is the
  * user's address.
  *
+ * If 'user_from_name' is non-NULL and *user_from is set by this call,
+ * *user_from_name will be set to the name corresponding to *user_from.
+ *
  * Return the number of addresses added to 'message'. (If 'message' is
  * NULL, the function returns 0 by definition.)
  */
@@ -186,7 +218,8 @@ scan_address_list (InternetAddressList *list,
 		   notmuch_config_t *config,
 		   GMimeMessage *message,
 		   GMimeRecipientType type,
-		   const char **user_from)
+		   const char **user_from,
+		   const char **user_from_name)
 {
     InternetAddress *address;
     int i;
@@ -203,7 +236,7 @@ scan_address_list (InternetAddressList *list,
 	    if (group_list == NULL)
 		continue;
 
-	    n += scan_address_list (group_list, config, message, type, user_from);
+	    n += scan_address_list (group_list, config, message, type, user_from, user_from_name);
 	} else {
 	    InternetAddressMailbox *mailbox;
 	    const char *name;
@@ -214,7 +247,7 @@ scan_address_list (InternetAddressList *list,
 	    name = internet_address_get_name (address);
 	    addr = internet_address_mailbox_get_addr (mailbox);
 
-	    if (address_is_users (addr, config)) {
+	    if (address_is_users (addr, config, user_from_name)) {
 		if (user_from && *user_from == NULL)
 		    *user_from = addr;
 	    } else if (message) {
@@ -238,7 +271,8 @@ scan_address_string (const char *recipients,
 		     notmuch_config_t *config,
 		     GMimeMessage *message,
 		     GMimeRecipientType type,
-		     const char **user_from)
+		     const char **user_from,
+		     const char **user_from_name)
 {
     InternetAddressList *list;
 
@@ -249,7 +283,7 @@ scan_address_string (const char *recipients,
     if (list == NULL)
 	return 0;
 
-    return scan_address_list (list, config, message, type, user_from);
+    return scan_address_list (list, config, message, type, user_from, user_from_name);
 }
 
 /* Does the address in the Reply-To header of 'message' already appear
@@ -300,6 +334,10 @@ reply_to_header_is_redundant (notmuch_message_t *message)
  * (typically this would be reply-to-sender, but also handles reply to
  * user's own message in a sensible way).
  *
+ * If 'from_name' is non-NULL and any of the user's addresses were found
+ * in these headers and the user specified an alternate name for that
+ * address, it will be set to the corresponding value.
+ *
  * If any of the user's addresses were found in these headers, the
  * first of these returned, otherwise NULL is returned.
  */
@@ -307,7 +345,8 @@ static const char *
 add_recipients_from_message (GMimeMessage *reply,
 			     notmuch_config_t *config,
 			     notmuch_message_t *message,
-			     notmuch_bool_t reply_all)
+			     notmuch_bool_t reply_all,
+			     const char **from_name)
 {
     struct {
 	const char *header;
@@ -349,7 +388,7 @@ add_recipients_from_message (GMimeMessage *reply,
 						     reply_to_map[i].fallback);
 
 	n += scan_address_string (recipients, config, reply,
-				  reply_to_map[i].recipient_type, &from_addr);
+				  reply_to_map[i].recipient_type, &from_addr, from_name);
 
 	if (!reply_all && n) {
 	    /* Stop adding new recipients in reply-to-sender mode if
@@ -375,10 +414,13 @@ add_recipients_from_message (GMimeMessage *reply,
  * Look for the user's address in " for <email@add.res>" in the
  * received headers.
  *
+ * If 'user_from_name' is non-NULL and the address is found,
+ * *user_from_name is set to the corresponding name.
+ *
  * Return the address that was found, if any, and NULL otherwise.
  */
 static const char *
-guess_from_in_received_for (notmuch_config_t *config, const char *received)
+guess_from_in_received_for (notmuch_config_t *config, const char *received, const char **user_from_name)
 {
     const char *ptr;
 
@@ -386,7 +428,7 @@ guess_from_in_received_for (notmuch_config_t *config, const char *received)
     if (! ptr)
 	return NULL;
 
-    return user_address_in_string (ptr, config);
+    return user_address_in_string (ptr, config, user_from_name);
 }
 
 /*
@@ -399,10 +441,13 @@ guess_from_in_received_for (notmuch_config_t *config, const char *received)
  * - and then assume that the first whitespace delimited token that
  * follows is the receiving system in this step of the receive chain.
  *
+ * If 'user_from_name' is non-NULL and the address is found,
+ * *user_from_name is set to the corresponding name.
+ *
  * Return the address that was found, if any, and NULL otherwise.
  */
 static const char *
-guess_from_in_received_by (notmuch_config_t *config, const char *received)
+guess_from_in_received_by (notmuch_config_t *config, const char *received, const char **user_from_name)
 {
     const char *addr;
     const char *by = received;
@@ -440,7 +485,7 @@ guess_from_in_received_by (notmuch_config_t *config, const char *received)
 	     */
 	    *(tld - 1) = '.';
 
-	    addr = string_in_user_address (domain, config);
+	    addr = string_in_user_address (domain, config, user_from_name);
 	    if (addr) {
 		free (mta);
 		return addr;
@@ -460,11 +505,15 @@ guess_from_in_received_by (notmuch_config_t *config, const char *received)
  * The Received: header is special in our get_header function and is
  * always concatenated.
  *
+ * If 'user_from_name' is non-NULL and the address is found,
+ * *user_from_name is set to the corresponding name.
+ *
  * Return the address that was found, if any, and NULL otherwise.
  */
 static const char *
 guess_from_in_received_headers (notmuch_config_t *config,
-				notmuch_message_t *message)
+				notmuch_message_t *message,
+				const char **user_from_name)
 {
     const char *received, *addr;
     char *sanitized;
@@ -477,9 +526,9 @@ guess_from_in_received_headers (notmuch_config_t *config,
     if (! sanitized)
 	return NULL;
 
-    addr = guess_from_in_received_for (config, sanitized);
+    addr = guess_from_in_received_for (config, sanitized, user_from_name);
     if (! addr)
-	addr = guess_from_in_received_by (config, sanitized);
+	addr = guess_from_in_received_by (config, sanitized, user_from_name);
 
     talloc_free (sanitized);
 
@@ -491,10 +540,13 @@ guess_from_in_received_headers (notmuch_config_t *config,
  * headers: Envelope-To, X-Original-To, and Delivered-To (searched in
  * that order).
  *
+ * If 'user_from_name' is non-NULL and the address is found,
+ * *user_from_name is set to the corresponding name.
+ *
  * Return the address that was found, if any, and NULL otherwise.
  */
 static const char *
-get_from_in_to_headers (notmuch_config_t *config, notmuch_message_t *message)
+get_from_in_to_headers (notmuch_config_t *config, notmuch_message_t *message, const char **user_from_name)
 {
     size_t i;
     const char *tohdr, *addr;
@@ -508,7 +560,7 @@ get_from_in_to_headers (notmuch_config_t *config, notmuch_message_t *message)
 	tohdr = notmuch_message_get_header (message, to_headers[i]);
 
 	/* Note: tohdr potentially contains a list of email addresses. */
-	addr = user_address_in_string (tohdr, config);
+	addr = user_address_in_string (tohdr, config, user_from_name);
 	if (addr)
 	    return addr;
     }
@@ -523,7 +575,7 @@ create_reply_message(void *ctx,
 		     notmuch_bool_t reply_all)
 {
     const char *subject, *from_addr = NULL;
-    const char *in_reply_to, *orig_references, *references;
+    const char *in_reply_to, *orig_references, *references, *from_name;
 
     /* The 1 means we want headers in a "pretty" order. */
     GMimeMessage *reply = g_mime_message_new (1);
@@ -540,7 +592,7 @@ create_reply_message(void *ctx,
     }
 
     from_addr = add_recipients_from_message (reply, config,
-					     message, reply_all);
+					     message, reply_all, &from_name);
 
     /*
      * Sadly, there is no standard way to find out to which email
@@ -555,7 +607,7 @@ create_reply_message(void *ctx,
      * Delivered-To: headers.
      */
     if (from_addr == NULL)
-	from_addr = get_from_in_to_headers (config, message);
+	from_addr = get_from_in_to_headers (config, message, &from_name);
 
     /*
      * Check for a (for <email@add.res>) clause in Received: headers,
@@ -563,14 +615,17 @@ create_reply_message(void *ctx,
      * of Received: headers
      */
     if (from_addr == NULL)
-	from_addr = guess_from_in_received_headers (config, message);
+	from_addr = guess_from_in_received_headers (config, message, &from_name);
 
     /* Default to user's primary address. */
-    if (from_addr == NULL)
+    if (from_addr == NULL) {
 	from_addr = notmuch_config_get_user_primary_email (config);
+	from_name = notmuch_config_get_user_name (config);
+    } else if (from_name == NULL || from_name[0] == '\0')
+	from_name = notmuch_config_get_user_name (config);
 
     from_addr = talloc_asprintf (ctx, "%s <%s>",
-				 notmuch_config_get_user_name (config),
+				 from_name,
 				 from_addr);
     g_mime_object_set_header (GMIME_OBJECT (reply),
 			      "From", from_addr);
@@ -751,7 +806,7 @@ notmuch_reply_format_headers_only(void *ctx,
 	g_mime_object_set_header (GMIME_OBJECT (reply),
 				  "References", references);
 
-	(void)add_recipients_from_message (reply, config, message, reply_all);
+	(void)add_recipients_from_message (reply, config, message, reply_all, NULL);
 
 	reply_headers = g_mime_object_to_string (GMIME_OBJECT (reply));
 	printf ("%s", reply_headers);
-- 
2.7.4

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

* [PATCH 3/4] Add documentation for user.other_name
  2016-08-09 20:55 [PATCH 0/4] Allow specifying alternate names for addresses in other_email Shea Levy
  2016-08-09 20:55 ` [PATCH 1/4] Add user.other_name property to associate names with other_email Shea Levy
  2016-08-09 20:55 ` [PATCH 2/4] notmuch-reply: respect users.other_name in From Shea Levy
@ 2016-08-09 20:55 ` Shea Levy
  2016-08-09 20:55 ` [PATCH 4/4] Update NEWS " Shea Levy
  2016-08-13 11:58 ` [PATCH 0/4] Allow specifying alternate names for addresses in other_email Jani Nikula
  4 siblings, 0 replies; 10+ messages in thread
From: Shea Levy @ 2016-08-09 20:55 UTC (permalink / raw)
  To: notmuch

---
 doc/man1/notmuch-config.rst | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/doc/man1/notmuch-config.rst b/doc/man1/notmuch-config.rst
index 5a517eb..6844c8d 100644
--- a/doc/man1/notmuch-config.rst
+++ b/doc/man1/notmuch-config.rst
@@ -68,6 +68,16 @@ The available configuration items are described below.
 
         Default: not set.
 
+    **user.other\_name**
+        A list of other names associated with addresses in
+        **user.other\_email**. Leave an entry empty to use
+        **user.name** for the corresponding address. If the
+        list of other names is shorter than the list of other
+        addresses, all addresses after the last listed name will
+        be associated with **user.name**.
+
+        Default: not set.
+
     **new.tags**
         A list of tags that will be added to all messages incorporated
         by **notmuch new**.
-- 
2.7.4

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

* [PATCH 4/4] Update NEWS for user.other_name
  2016-08-09 20:55 [PATCH 0/4] Allow specifying alternate names for addresses in other_email Shea Levy
                   ` (2 preceding siblings ...)
  2016-08-09 20:55 ` [PATCH 3/4] Add documentation for user.other_name Shea Levy
@ 2016-08-09 20:55 ` Shea Levy
  2016-08-13 11:58 ` [PATCH 0/4] Allow specifying alternate names for addresses in other_email Jani Nikula
  4 siblings, 0 replies; 10+ messages in thread
From: Shea Levy @ 2016-08-09 20:55 UTC (permalink / raw)
  To: notmuch

---
 NEWS | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/NEWS b/NEWS
index 3a9c8d3..fdf9c81 100644
--- a/NEWS
+++ b/NEWS
@@ -16,6 +16,11 @@ Ruby Bindings
 
 Add support for `notmuch_database_get_all_tags`
 
+General
+-------
+
+Add the `user.other_name` configuration setting
+
 Notmuch 0.22.1 (2016-07-19)
 ===========================
 
-- 
2.7.4

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

* Re: [PATCH 0/4] Allow specifying alternate names for addresses in other_email
  2016-08-09 20:55 [PATCH 0/4] Allow specifying alternate names for addresses in other_email Shea Levy
                   ` (3 preceding siblings ...)
  2016-08-09 20:55 ` [PATCH 4/4] Update NEWS " Shea Levy
@ 2016-08-13 11:58 ` Jani Nikula
  2016-08-14 12:43   ` Shea Levy
  4 siblings, 1 reply; 10+ messages in thread
From: Jani Nikula @ 2016-08-13 11:58 UTC (permalink / raw)
  To: Shea Levy, notmuch

On Tue, 09 Aug 2016, Shea Levy <shea@shealevy.com> wrote:
> Currently, while notmuch-reply will recognize email addresses other than
> the main address with user.other_email, it always sets the name part of
> the address in the envelope-from and From headers to user.name. This
> patchset enables specifying names on a per-address basis with a new
> user.other_name property. Presumably other users of user.other_email
> may want to use this as well, but those are not updated currently.

I am not convinved by adding another configuration option, especially
when it has to be in sync with another configuration option (ordering in
user.other_name having to match user.other_email). I would much prefer
allowing (but not requiring) "Name <user@example.org>" style addresses
both in user.primary_email and user.other_email.

With a cursory glance at the implementation, I wonder if you could just
pick the name based on the address you've picked earlier, and leave the
address matching mostly as it is. Would save some passing of parameters
around. Maybe.

Additionally, I'd very much like to have my series [1] merged
first. It'll be *much* easier to rebase your series on top than the
other way around...

BR,
Jani.


[1] id:cover.1471088022.git.jani@nikula.org

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

* Re: [PATCH 0/4] Allow specifying alternate names for addresses in other_email
  2016-08-13 11:58 ` [PATCH 0/4] Allow specifying alternate names for addresses in other_email Jani Nikula
@ 2016-08-14 12:43   ` Shea Levy
  2016-08-14 13:35     ` Jani Nikula
  0 siblings, 1 reply; 10+ messages in thread
From: Shea Levy @ 2016-08-14 12:43 UTC (permalink / raw)
  To: Jani Nikula, notmuch

Hi Jani,

Jani Nikula <jani@nikula.org> writes:

> On Tue, 09 Aug 2016, Shea Levy <shea@shealevy.com> wrote:
>> Currently, while notmuch-reply will recognize email addresses other than
>> the main address with user.other_email, it always sets the name part of
>> the address in the envelope-from and From headers to user.name. This
>> patchset enables specifying names on a per-address basis with a new
>> user.other_name property. Presumably other users of user.other_email
>> may want to use this as well, but those are not updated currently.
>
> I am not convinved by adding another configuration option, especially
> when it has to be in sync with another configuration option (ordering in
> user.other_name having to match user.other_email). I would much prefer
> allowing (but not requiring) "Name <user@example.org>" style addresses
> both in user.primary_email and user.other_email.
>

This would be fine with me. Is there already code in place to separate
out the name and email values from that kind of email address?

>
> With a cursory glance at the implementation, I wonder if you could just
> pick the name based on the address you've picked earlier, and leave the
> address matching mostly as it is. Would save some passing of parameters
> around. Maybe.
>

Hmm, I'm not sure what you mean by this, sorry. Can you expand?

>
> Additionally, I'd very much like to have my series [1] merged
> first. It'll be *much* easier to rebase your series on top than the
> other way around...
>

Happy to rebase my work on yours.

>
> BR,
> Jani.
>
>
> [1] id:cover.1471088022.git.jani@nikula.org

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

* Re: [PATCH 0/4] Allow specifying alternate names for addresses in other_email
  2016-08-14 12:43   ` Shea Levy
@ 2016-08-14 13:35     ` Jani Nikula
  2016-08-14 13:45       ` Jani Nikula
  2016-08-18 10:52       ` David Bremner
  0 siblings, 2 replies; 10+ messages in thread
From: Jani Nikula @ 2016-08-14 13:35 UTC (permalink / raw)
  To: Shea Levy, notmuch

On Sun, 14 Aug 2016, Shea Levy <shea@shealevy.com> wrote:
> Jani Nikula <jani@nikula.org> writes:
>
>> On Tue, 09 Aug 2016, Shea Levy <shea@shealevy.com> wrote:
>>> Currently, while notmuch-reply will recognize email addresses other than
>>> the main address with user.other_email, it always sets the name part of
>>> the address in the envelope-from and From headers to user.name. This
>>> patchset enables specifying names on a per-address basis with a new
>>> user.other_name property. Presumably other users of user.other_email
>>> may want to use this as well, but those are not updated currently.
>>
>> I am not convinved by adding another configuration option, especially
>> when it has to be in sync with another configuration option (ordering in
>> user.other_name having to match user.other_email). I would much prefer
>> allowing (but not requiring) "Name <user@example.org>" style addresses
>> both in user.primary_email and user.other_email.
>>
>
> This would be fine with me. Is there already code in place to separate
> out the name and email values from that kind of email address?

I think we should use gmime for this, and expect the configuration to be
a comma separated list of addresses, specifically in the format that
internet_address_list_parse_string() parses [2]. On the plus side, it'll
handle most of the weird corner cases about (lists of) email addresses,
and we can probably safely ignore the ones it doesn't handle. We already
use that stuff in notmuch-show.c and notmuch-reply.c. On the minus side,
using the functions is about as much fun as string manipulation
generally is in C.

Then there's the annoying detail that this'll change the format of the
config from a semicolon separated list to a comma separated list. This
means switching from using g_key_file_get_string_list() to
g_key_file_get_string(). But we also need to have backward compatibility
somehow. One option is to add a new config option (didn't I just frown
on adding new ones?) that would replace all of user.name,
user.primary_email, and user.other_email, making the first in the list
the primary one. So we could check if, say, user.email is present, and
use that for all of the name/primary/other configuration, falling back
to the separate fields otherwise. This is also a safe option in case
some other software uses the configuration options directly.

Anyway, this will be slightly tedious to code, and some of the changes
might be a bit controversial, so I suggest waiting until we get feedback
from others first. (David, I'm looking at you!)

>> With a cursory glance at the implementation, I wonder if you could just
>> pick the name based on the address you've picked earlier, and leave the
>> address matching mostly as it is. Would save some passing of parameters
>> around. Maybe.
>>
>
> Hmm, I'm not sure what you mean by this, sorry. Can you expand?

Only match the *address* part, and when you've found a match, find the
corresponding name part. Or do you need to be able to distinguish
between different names with the same address?

No matter what, I think the first thing should be changing just the
config, and then building the rest on top afterwards.

>> Additionally, I'd very much like to have my series [1] merged
>> first. It'll be *much* easier to rebase your series on top than the
>> other way around...
>>
>
> Happy to rebase my work on yours.

Of course, lacking review, there's no guarantee my series actually gets
merged. It's open source, first come, first served!


BR,
Jani.


>>
>> [1] id:cover.1471088022.git.jani@nikula.org

[2] https://developer.gnome.org/gmime/stable/InternetAddressList.html

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

* Re: [PATCH 0/4] Allow specifying alternate names for addresses in other_email
  2016-08-14 13:35     ` Jani Nikula
@ 2016-08-14 13:45       ` Jani Nikula
  2016-08-18 10:52       ` David Bremner
  1 sibling, 0 replies; 10+ messages in thread
From: Jani Nikula @ 2016-08-14 13:45 UTC (permalink / raw)
  To: Shea Levy, notmuch

On Sun, 14 Aug 2016, Jani Nikula <jani@nikula.org> wrote:
> I think we should use gmime for this, and expect the configuration to be
> a comma separated list of addresses, specifically in the format that
> internet_address_list_parse_string() parses [2].

Here's a little sample to get started and play around. Build using
something along the lines of:

gcc $(pkg-config --cflags gmime-2.6 --libs gmime-2.6) internetaddress.c

You can try e.g.

$ ./a.out "J. Random Hacker <jrh@example.org>, dude@example.org, \"Hacker, J. Random\" <hacker@example.com>"
name[0]: J. Random Hacker
addr[0]: jrh@example.org
name[1]: (null)
addr[1]: dude@example.org
name[2]: Hacker, J. Random
addr[2]: hacker@example.com

----------->%----------

#include <stdio.h>
#include <stdlib.h>
#include <stdbool.h>

#include <gmime/gmime.h>

int main(int argc, char *argv[])
{
	InternetAddressList *list;
	InternetAddress *mailbox;
	int i;

	if (argc != 2) {
		fprintf(stderr, "usage: %s address-list\n", argv[0]);
		return EXIT_FAILURE;
	}

	list = internet_address_list_parse_string(argv[1]);

	for (i = 0; i < internet_address_list_length(list); i++) {
		InternetAddress *addr;
		InternetAddressMailbox *mbox;

		addr = internet_address_list_get_address(list, i);

		if (INTERNET_ADDRESS_IS_GROUP(addr)) {
			printf("[%d] is a group, ignoring\n", i);
			continue;
		}

		mbox = INTERNET_ADDRESS_MAILBOX(addr);

		printf("name[%d]: %s\n", i, internet_address_get_name(addr));
		printf("addr[%d]: %s\n", i, internet_address_mailbox_get_addr(mbox));
	}

	return EXIT_SUCCESS;	
}

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

* Re: [PATCH 0/4] Allow specifying alternate names for addresses in other_email
  2016-08-14 13:35     ` Jani Nikula
  2016-08-14 13:45       ` Jani Nikula
@ 2016-08-18 10:52       ` David Bremner
  1 sibling, 0 replies; 10+ messages in thread
From: David Bremner @ 2016-08-18 10:52 UTC (permalink / raw)
  To: Jani Nikula, Shea Levy, notmuch

Jani Nikula <jani@nikula.org> writes:

> Then there's the annoying detail that this'll change the format of the
> config from a semicolon separated list to a comma separated list. This
> means switching from using g_key_file_get_string_list() to
> g_key_file_get_string(). But we also need to have backward compatibility
> somehow. One option is to add a new config option (didn't I just frown
> on adding new ones?) that would replace all of user.name,
> user.primary_email, and user.other_email, making the first in the list
> the primary one. So we could check if, say, user.email is present, and
> use that for all of the name/primary/other configuration, falling back
> to the separate fields otherwise.

I guess if you wanted to be very friendly, you could support "virtual
keys" for notmuch config so that "notmuch config get user.email" still
works. Maybe that's already what you intend to suggest there. We already
have config keys that are not stored in .notmuch-config

I do agree that have two parallel arrays that the user is supposed to
keep in sync is a classic bad interface design (we basically introduce
structs/OOP by saying that's bad ;)). Other than that I'm open to what
the config options look like, since setting them up is a rare operation.

I think the only places in notmuch these config options are used is in
notmuch-reply, and in the emacs client.

It occurs to me that another option is some kind of versioning of config
files, and yet another upgrade process (since we rewrite the whole
config file anyway). This might be even more tedious to write, but at
least the logic of dealing with different config file versions would all
be in one place.

> This is also a safe option in case some other software uses the
> configuration options directly.

I guess there's no safe option if people read the file directly, but I
keep telling people not to do that ;). 

> Anyway, this will be slightly tedious to code, and some of the changes
> might be a bit controversial, so I suggest waiting until we get feedback
> from others first. (David, I'm looking at you!)

not sure if these are the droids^Wfeedbacks you are looking for.

d

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

end of thread, other threads:[~2016-08-18 10:53 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-09 20:55 [PATCH 0/4] Allow specifying alternate names for addresses in other_email Shea Levy
2016-08-09 20:55 ` [PATCH 1/4] Add user.other_name property to associate names with other_email Shea Levy
2016-08-09 20:55 ` [PATCH 2/4] notmuch-reply: respect users.other_name in From Shea Levy
2016-08-09 20:55 ` [PATCH 3/4] Add documentation for user.other_name Shea Levy
2016-08-09 20:55 ` [PATCH 4/4] Update NEWS " Shea Levy
2016-08-13 11:58 ` [PATCH 0/4] Allow specifying alternate names for addresses in other_email Jani Nikula
2016-08-14 12:43   ` Shea Levy
2016-08-14 13:35     ` Jani Nikula
2016-08-14 13:45       ` Jani Nikula
2016-08-18 10:52       ` David Bremner

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