[PATCH v2 0/3] composing patches

[PATCH v2 0/3] composing patches

[Felipe Contreras]

Hi,

I don't know how it works in gnus, but at least on the vim mode, the output
generated by 'notmuch reply' is not ready to be sent, at least the Message-ID
field is needed, and also is nice to have the User-Agent.

Besides, in order to avoid creating a new message by hand (possibly fetching
the info from notmuch config), it's more straightforward to have 'notmuch
compose' command.

In the future 'notmuch compose' might be used to replace 'notmuch reply' and
possily add a forward option too. It might also be possible to add mail aliases
when composing a message, and these aliases might be used while generating the
'notmuch search' output.

Although we are adding extra fields in notmuch reply, higher layers can just
override them easily.

Felipe Contreras (3):
  Add 'compose' command
  reply: add message-id header
  reply: add user-agent field

 Makefile.local    |    1 +
 notmuch-client.h  |    3 ++
 notmuch-compose.c |  111 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 notmuch-reply.c   |   17 ++++++++
 notmuch.c         |    5 +++
 5 files changed, 137 insertions(+)
 create mode 100644 notmuch-compose.c

-- 
1.7.10

[PATCH v2 1/3] Add 'compose' command

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@ngmail.com>
---
 Makefile.local    |    1 +
 notmuch-client.h  |    3 ++
 notmuch-compose.c |  111 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 notmuch.c         |    5 +++
 4 files changed, 120 insertions(+)
 create mode 100644 notmuch-compose.c

diff --git a/Makefile.local b/Makefile.local
index 53b4a0d..2c15ec2 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -279,6 +279,7 @@ notmuch_client_srcs =		\
 	gmime-filter-headers.c	\
 	hooks.c			\
 	notmuch.c		\
+	notmuch-compose.c	\
 	notmuch-config.c	\
 	notmuch-count.c		\
 	notmuch-dump.c		\
diff --git a/notmuch-client.h b/notmuch-client.h
index 19b7f01..1146cd1 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -126,6 +126,9 @@ int
 notmuch_reply_command (void *ctx, int argc, char *argv[]);
 
 int
+notmuch_compose_command (void *ctx, int argc, char *argv[]);
+
+int
 notmuch_restore_command (void *ctx, int argc, char *argv[]);
 
 int
diff --git a/notmuch-compose.c b/notmuch-compose.c
new file mode 100644
index 0000000..ac5ea95
--- /dev/null
+++ b/notmuch-compose.c
@@ -0,0 +1,111 @@
+/* notmuch - Not much of an email program, (just index and search)
+ *
+ * Copyright © 2009 Carl Worth
+ * Copyright © 2009 Keith Packard
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see http://www.gnu.org/licenses/ .
+ *
+ * Authors: Carl Worth <cworth@cworth.org>
+ *	    Keith Packard <keithp@keithp.com>
+ *	    Felipe Contreras <felipe.contreras@gmail.com>
+ */
+
+#include "notmuch-client.h"
+#include "gmime-filter-headers.h"
+
+static void
+show_message_headers (GMimeMessage *message)
+{
+    GMimeStream *stream_stdout = NULL, *stream_filter = NULL;
+
+    stream_stdout = g_mime_stream_file_new (stdout);
+    if (stream_stdout) {
+	g_mime_stream_file_set_owner (GMIME_STREAM_FILE (stream_stdout), FALSE);
+	stream_filter = g_mime_stream_filter_new(stream_stdout);
+	if (stream_filter) {
+		g_mime_stream_filter_add(GMIME_STREAM_FILTER(stream_filter),
+					 g_mime_filter_headers_new());
+		g_mime_object_write_to_stream(GMIME_OBJECT(message), stream_filter);
+		g_object_unref(stream_filter);
+	}
+	g_object_unref(stream_stdout);
+    }
+}
+
+static int
+notmuch_compose (void *ctx, notmuch_config_t *config)
+{
+    GMimeMessage *msg;
+    const char *from_addr = NULL;
+    const char *message_id, *user_agent;
+    char *simple_from;
+
+    /* The 1 means we want headers in a "pretty" order. */
+    msg = g_mime_message_new (1);
+    if (msg == NULL) {
+	fprintf (stderr, "Out of memory\n");
+	return 1;
+    }
+
+    g_mime_message_set_subject (msg, "");
+
+    g_mime_object_set_header (GMIME_OBJECT (msg), "To", "");
+
+    if (from_addr == NULL)
+	from_addr = notmuch_config_get_user_primary_email (config);
+
+    simple_from = talloc_strdup (ctx, from_addr);
+
+    from_addr = talloc_asprintf (ctx, "%s <%s>",
+				 notmuch_config_get_user_name (config),
+				 from_addr);
+    g_mime_object_set_header (GMIME_OBJECT (msg),
+			      "From", from_addr);
+
+    g_mime_object_set_header (GMIME_OBJECT (msg), "Bcc",
+			      notmuch_config_get_user_primary_email (config));
+
+    user_agent = talloc_asprintf (ctx, "notmuch %s",
+				  STRINGIFY(NOTMUCH_VERSION));
+    g_mime_object_set_header (GMIME_OBJECT (msg),
+			      "User-Agent", user_agent);
+
+    message_id = talloc_asprintf (ctx, "<%lu-notmuch-%s>",
+				  time(NULL),
+				  simple_from);
+    g_mime_object_set_header (GMIME_OBJECT (msg),
+			      "Message-ID", message_id);
+    talloc_free (simple_from);
+
+    show_message_headers (msg);
+
+    g_object_unref (G_OBJECT (msg));
+
+    return 0;
+}
+
+int
+notmuch_compose_command (void *ctx, unused (int argc), unused (char *argv[]))
+{
+    notmuch_config_t *config;
+    int ret = 0;
+
+    config = notmuch_config_open (ctx, NULL, NULL);
+    if (config == NULL)
+	return 1;
+
+    ret = notmuch_compose (ctx, config);
+
+    return ret;
+}
diff --git a/notmuch.c b/notmuch.c
index 477a09c..2b500d7 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -65,6 +65,11 @@ static command_t commands[] = {
     { "reply", notmuch_reply_command,
       "[options...] <search-terms> [...]",
       "Construct a reply template for a set of messages." },
+    { "compose", notmuch_compose_command,
+	NULL,
+	"Constructs an empty message.",
+	"\tConstructs a new empty message filling basic headers such as\n"
+	"\tFrom:, User-Agent: and Message-ID:." },
     { "tag", notmuch_tag_command,
       "+<tag>|-<tag> [...] [--] <search-terms> [...]" ,
       "Add/remove tags for all messages matching the search terms." },
-- 
1.7.10

[PATCH v2 2/3] reply: add message-id header

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 notmuch-reply.c |   11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/notmuch-reply.c b/notmuch-reply.c
index 0949d9f..d796bb2 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -464,6 +464,8 @@ create_reply_message(void *ctx,
 {
     const char *subject, *from_addr = NULL;
     const char *in_reply_to, *orig_references, *references;
+    const char *message_id;
+    char *simple_from;
 
     /* The 1 means we want headers in a "pretty" order. */
     GMimeMessage *reply = g_mime_message_new (1);
@@ -488,6 +490,8 @@ create_reply_message(void *ctx,
     if (from_addr == NULL)
 	from_addr = notmuch_config_get_user_primary_email (config);
 
+    simple_from = talloc_strdup (ctx, from_addr);
+
     from_addr = talloc_asprintf (ctx, "%s <%s>",
 				 notmuch_config_get_user_name (config),
 				 from_addr);
@@ -508,6 +512,13 @@ create_reply_message(void *ctx,
     g_mime_object_set_header (GMIME_OBJECT (reply),
 			      "References", references);
 
+    message_id = talloc_asprintf (ctx, "<%lu-notmuch-%s>",
+				  time(NULL),
+				  simple_from);
+    g_mime_object_set_header (GMIME_OBJECT (reply),
+			      "Message-ID", message_id);
+    talloc_free (simple_from);
+
     return reply;
 }
 
-- 
1.7.10

[PATCH v2 3/3] reply: add user-agent field

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 notmuch-reply.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/notmuch-reply.c b/notmuch-reply.c
index d796bb2..22838d5 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -464,7 +464,7 @@ create_reply_message(void *ctx,
 {
     const char *subject, *from_addr = NULL;
     const char *in_reply_to, *orig_references, *references;
-    const char *message_id;
+    const char *message_id, *user_agent;
     char *simple_from;
 
     /* The 1 means we want headers in a "pretty" order. */
@@ -512,6 +512,7 @@ create_reply_message(void *ctx,
     g_mime_object_set_header (GMIME_OBJECT (reply),
 			      "References", references);
 
+
     message_id = talloc_asprintf (ctx, "<%lu-notmuch-%s>",
 				  time(NULL),
 				  simple_from);
@@ -519,6 +520,11 @@ create_reply_message(void *ctx,
 			      "Message-ID", message_id);
     talloc_free (simple_from);
 
+    user_agent = talloc_asprintf (ctx, "notmuch %s",
+				  STRINGIFY(NOTMUCH_VERSION));
+    g_mime_object_set_header (GMIME_OBJECT (reply),
+			      "User-Agent", user_agent);
+
     return reply;
 }
 
-- 
1.7.10

Re: [PATCH v2 1/3] Add 'compose' command

[Jani Nikula]

On Wed, 18 Apr 2012 15:39:11 +0300, Felipe Contreras <felipe.contreras@gmail.com> wrote:
> Signed-off-by: Felipe Contreras <felipe.contreras@ngmail.com>
> ---
>  Makefile.local    |    1 +
>  notmuch-client.h  |    3 ++
>  notmuch-compose.c |  111 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  notmuch.c         |    5 +++
>  4 files changed, 120 insertions(+)
>  create mode 100644 notmuch-compose.c
> 
> diff --git a/Makefile.local b/Makefile.local
> index 53b4a0d..2c15ec2 100644
> --- a/Makefile.local
> +++ b/Makefile.local
> @@ -279,6 +279,7 @@ notmuch_client_srcs =		\
>  	gmime-filter-headers.c	\
>  	hooks.c			\
>  	notmuch.c		\
> +	notmuch-compose.c	\
>  	notmuch-config.c	\
>  	notmuch-count.c		\
>  	notmuch-dump.c		\
> diff --git a/notmuch-client.h b/notmuch-client.h
> index 19b7f01..1146cd1 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -126,6 +126,9 @@ int
>  notmuch_reply_command (void *ctx, int argc, char *argv[]);
>  
>  int
> +notmuch_compose_command (void *ctx, int argc, char *argv[]);
> +
> +int
>  notmuch_restore_command (void *ctx, int argc, char *argv[]);
>  
>  int
> diff --git a/notmuch-compose.c b/notmuch-compose.c
> new file mode 100644
> index 0000000..ac5ea95
> --- /dev/null
> +++ b/notmuch-compose.c
> @@ -0,0 +1,111 @@
> +/* notmuch - Not much of an email program, (just index and search)
> + *
> + * Copyright © 2009 Carl Worth
> + * Copyright © 2009 Keith Packard
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 3 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see http://www.gnu.org/licenses/ .
> + *
> + * Authors: Carl Worth <cworth@cworth.org>
> + *	    Keith Packard <keithp@keithp.com>
> + *	    Felipe Contreras <felipe.contreras@gmail.com>
> + */
> +
> +#include "notmuch-client.h"
> +#include "gmime-filter-headers.h"
> +
> +static void
> +show_message_headers (GMimeMessage *message)
> +{
> +    GMimeStream *stream_stdout = NULL, *stream_filter = NULL;
> +
> +    stream_stdout = g_mime_stream_file_new (stdout);
> +    if (stream_stdout) {
> +	g_mime_stream_file_set_owner (GMIME_STREAM_FILE (stream_stdout), FALSE);
> +	stream_filter = g_mime_stream_filter_new(stream_stdout);
> +	if (stream_filter) {
> +		g_mime_stream_filter_add(GMIME_STREAM_FILTER(stream_filter),
> +					 g_mime_filter_headers_new());
> +		g_mime_object_write_to_stream(GMIME_OBJECT(message), stream_filter);
> +		g_object_unref(stream_filter);
> +	}
> +	g_object_unref(stream_stdout);
> +    }
> +}
> +
> +static int
> +notmuch_compose (void *ctx, notmuch_config_t *config)
> +{
> +    GMimeMessage *msg;
> +    const char *from_addr = NULL;
> +    const char *message_id, *user_agent;
> +    char *simple_from;
> +
> +    /* The 1 means we want headers in a "pretty" order. */
> +    msg = g_mime_message_new (1);
> +    if (msg == NULL) {
> +	fprintf (stderr, "Out of memory\n");
> +	return 1;
> +    }
> +
> +    g_mime_message_set_subject (msg, "");
> +
> +    g_mime_object_set_header (GMIME_OBJECT (msg), "To", "");
> +
> +    if (from_addr == NULL)
> +	from_addr = notmuch_config_get_user_primary_email (config);
> +
> +    simple_from = talloc_strdup (ctx, from_addr);
> +
> +    from_addr = talloc_asprintf (ctx, "%s <%s>",
> +				 notmuch_config_get_user_name (config),
> +				 from_addr);
> +    g_mime_object_set_header (GMIME_OBJECT (msg),
> +			      "From", from_addr);
> +
> +    g_mime_object_set_header (GMIME_OBJECT (msg), "Bcc",
> +			      notmuch_config_get_user_primary_email (config));
> +
> +    user_agent = talloc_asprintf (ctx, "notmuch %s",
> +				  STRINGIFY(NOTMUCH_VERSION));
> +    g_mime_object_set_header (GMIME_OBJECT (msg),
> +			      "User-Agent", user_agent);
> +
> +    message_id = talloc_asprintf (ctx, "<%lu-notmuch-%s>",
> +				  time(NULL),
> +				  simple_from);

Running "notmuch compose" more than once within a second would result in
identical message ids for the messages, which is not a good idea. That's
not likely in interactive use, but the notmuch cli is highly scriptable,
so someone is bound to hit this.

Some paranoid might also be worried about "leaking" the time you run
"notmuch compose"... which may be different from the actual time you
send the message.


BR,
Jani.

> +    g_mime_object_set_header (GMIME_OBJECT (msg),
> +			      "Message-ID", message_id);
> +    talloc_free (simple_from);
> +
> +    show_message_headers (msg);
> +
> +    g_object_unref (G_OBJECT (msg));
> +
> +    return 0;
> +}
> +
> +int
> +notmuch_compose_command (void *ctx, unused (int argc), unused (char *argv[]))
> +{
> +    notmuch_config_t *config;
> +    int ret = 0;
> +
> +    config = notmuch_config_open (ctx, NULL, NULL);
> +    if (config == NULL)
> +	return 1;
> +
> +    ret = notmuch_compose (ctx, config);
> +
> +    return ret;
> +}
> diff --git a/notmuch.c b/notmuch.c
> index 477a09c..2b500d7 100644
> --- a/notmuch.c
> +++ b/notmuch.c
> @@ -65,6 +65,11 @@ static command_t commands[] = {
>      { "reply", notmuch_reply_command,
>        "[options...] <search-terms> [...]",
>        "Construct a reply template for a set of messages." },
> +    { "compose", notmuch_compose_command,
> +	NULL,
> +	"Constructs an empty message.",
> +	"\tConstructs a new empty message filling basic headers such as\n"
> +	"\tFrom:, User-Agent: and Message-ID:." },
>      { "tag", notmuch_tag_command,
>        "+<tag>|-<tag> [...] [--] <search-terms> [...]" ,
>        "Add/remove tags for all messages matching the search terms." },
> -- 
> 1.7.10
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2 2/3] reply: add message-id header

[Jani Nikula]

On Wed, 18 Apr 2012 15:39:12 +0300, Felipe Contreras <felipe.contreras@gmail.com> wrote:
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  notmuch-reply.c |   11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/notmuch-reply.c b/notmuch-reply.c
> index 0949d9f..d796bb2 100644
> --- a/notmuch-reply.c
> +++ b/notmuch-reply.c
> @@ -464,6 +464,8 @@ create_reply_message(void *ctx,
>  {
>      const char *subject, *from_addr = NULL;
>      const char *in_reply_to, *orig_references, *references;
> +    const char *message_id;
> +    char *simple_from;
>  
>      /* The 1 means we want headers in a "pretty" order. */
>      GMimeMessage *reply = g_mime_message_new (1);
> @@ -488,6 +490,8 @@ create_reply_message(void *ctx,
>      if (from_addr == NULL)
>  	from_addr = notmuch_config_get_user_primary_email (config);
>  
> +    simple_from = talloc_strdup (ctx, from_addr);
> +
>      from_addr = talloc_asprintf (ctx, "%s <%s>",
>  				 notmuch_config_get_user_name (config),
>  				 from_addr);
> @@ -508,6 +512,13 @@ create_reply_message(void *ctx,
>      g_mime_object_set_header (GMIME_OBJECT (reply),
>  			      "References", references);
>  
> +    message_id = talloc_asprintf (ctx, "<%lu-notmuch-%s>",
> +				  time(NULL),
> +				  simple_from);

This duplicates the problem in "notmuch compose", hinting that you
should probably have the message-id generation in one place only.

I do wonder whether the emacs ui / message mode will add a duplicate
message-id rather than override this.


BR,
Jani.

> +    g_mime_object_set_header (GMIME_OBJECT (reply),
> +			      "Message-ID", message_id);
> +    talloc_free (simple_from);
> +
>      return reply;
>  }
>  
> -- 
> 1.7.10
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2 1/3] Add 'compose' command

[Felipe Contreras]

On Wed, Apr 18, 2012 at 4:06 PM, Jani Nikula <jani@nikula.org> wrote:

> Running "notmuch compose" more than once within a second would result in
> identical message ids for the messages, which is not a good idea. That's
> not likely in interactive use, but the notmuch cli is highly scriptable,
> so someone is bound to hit this.
>
> Some paranoid might also be worried about "leaking" the time you run
> "notmuch compose"... which may be different from the actual time you
> send the message.

It's still better than the current situation; nothing. In any case,
people that have not needed this would not be affected; their UI would
override the Message-ID.

So do you have a better suggestion for a Message-ID?

-- 
Felipe Contreras

Re: [PATCH v2 1/3] Add 'compose' command

[Dmitry Kurochkin]

Hi Felipe.

Felipe Contreras <felipe.contreras@gmail.com> writes:

> On Wed, Apr 18, 2012 at 4:06 PM, Jani Nikula <jani@nikula.org> wrote:
>
>> Running "notmuch compose" more than once within a second would result in
>> identical message ids for the messages, which is not a good idea. That's
>> not likely in interactive use, but the notmuch cli is highly scriptable,
>> so someone is bound to hit this.
>>
>> Some paranoid might also be worried about "leaking" the time you run
>> "notmuch compose"... which may be different from the actual time you
>> send the message.
>
> It's still better than the current situation; nothing. In any case,
> people that have not needed this would not be affected; their UI would
> override the Message-ID.
>

I disagree.  If notmuch CLI generates a Message-ID, it must be a good
one.  Otherwise we make users falsely believe that they do have a proper
Message-ID while in fact they do not.  And that would bite them sooner
or later.

Regards,
  Dmitry

> So do you have a better suggestion for a Message-ID?
>
> -- 
> Felipe Contreras
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2 1/3] Add 'compose' command

[Felipe Contreras]

On Wed, Apr 18, 2012 at 4:43 PM, Dmitry Kurochkin
<dmitry.kurochkin@gmail.com> wrote:
> Hi Felipe.
>
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On Wed, Apr 18, 2012 at 4:06 PM, Jani Nikula <jani@nikula.org> wrote:
>>
>>> Running "notmuch compose" more than once within a second would result in
>>> identical message ids for the messages, which is not a good idea. That's
>>> not likely in interactive use, but the notmuch cli is highly scriptable,
>>> so someone is bound to hit this.
>>>
>>> Some paranoid might also be worried about "leaking" the time you run
>>> "notmuch compose"... which may be different from the actual time you
>>> send the message.
>>
>> It's still better than the current situation; nothing. In any case,
>> people that have not needed this would not be affected; their UI would
>> override the Message-ID.
>>
>
> I disagree.  If notmuch CLI generates a Message-ID, it must be a good
> one.  Otherwise we make users falsely believe that they do have a proper
> Message-ID while in fact they do not.  And that would bite them sooner
> or later.

And then they'll report it, and we would fix it.

Anyway, everything comes from a patch, so, do you have a patch,
pseudo-code, or even a suggestion?

-- 
Felipe Contreras

Re: [PATCH v2 1/3] Add 'compose' command

[Jani Nikula]

On Wed, 18 Apr 2012 16:34:30 +0300, Felipe Contreras <felipe.contreras@gmail.com> wrote:
> On Wed, Apr 18, 2012 at 4:06 PM, Jani Nikula <jani@nikula.org> wrote:
> 
> > Running "notmuch compose" more than once within a second would result in
> > identical message ids for the messages, which is not a good idea. That's
> > not likely in interactive use, but the notmuch cli is highly scriptable,
> > so someone is bound to hit this.
> >
> > Some paranoid might also be worried about "leaking" the time you run
> > "notmuch compose"... which may be different from the actual time you
> > send the message.
> 
> It's still better than the current situation; nothing. In any case,
> people that have not needed this would not be affected; their UI would
> override the Message-ID.
> 
> So do you have a better suggestion for a Message-ID?

The easy way would be to just use g_mime_utils_generate_message_id()
[1]. It doesn't give you any control of the part before @, but I'm not
sure if that really matters.

Alternatively you can write your own according to e.g. [2]. Glib appears
to have decent and portable support for pseudo random number
generation. But why bother? I'd go with gmime.

BR,
Jani.


[1] http://developer.gnome.org/gmime/stable/gmime-gmime-utils.html#g-mime-utils-generate-message-id
[2] http://www.jwz.org/doc/mid.html

Re: [PATCH v2 1/3] Add 'compose' command

[Dmitry Kurochkin]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> On Wed, Apr 18, 2012 at 4:43 PM, Dmitry Kurochkin
> <dmitry.kurochkin@gmail.com> wrote:
>> Hi Felipe.
>>
>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>
>>> On Wed, Apr 18, 2012 at 4:06 PM, Jani Nikula <jani@nikula.org> wrote:
>>>
>>>> Running "notmuch compose" more than once within a second would result in
>>>> identical message ids for the messages, which is not a good idea. That's
>>>> not likely in interactive use, but the notmuch cli is highly scriptable,
>>>> so someone is bound to hit this.
>>>>
>>>> Some paranoid might also be worried about "leaking" the time you run
>>>> "notmuch compose"... which may be different from the actual time you
>>>> send the message.
>>>
>>> It's still better than the current situation; nothing. In any case,
>>> people that have not needed this would not be affected; their UI would
>>> override the Message-ID.
>>>
>>
>> I disagree.  If notmuch CLI generates a Message-ID, it must be a good
>> one.  Otherwise we make users falsely believe that they do have a proper
>> Message-ID while in fact they do not.  And that would bite them sooner
>> or later.
>
> And then they'll report it, and we would fix it.
>
> Anyway, everything comes from a patch, so, do you have a patch,
> pseudo-code, or even a suggestion?
>

A patch needs some positive reviews to be accepted.  Replying to
comments with "make a better patch" may not be the best strategy for
getting your patches accepted.

I do not have a patch.  As for a suggestion, I would start with some
googling.  If you did that, you would probably find the gmime function
kindly pointed out by Jani.

Regards,
  Dmitry

> -- 
> Felipe Contreras

Re: [PATCH v2 1/3] Add 'compose' command

[Felipe Contreras]

On Wed, Apr 18, 2012 at 5:20 PM, Jani Nikula <jani@nikula.org> wrote:
> On Wed, 18 Apr 2012 16:34:30 +0300, Felipe Contreras <felipe.contreras@gmail.com> wrote:
>> On Wed, Apr 18, 2012 at 4:06 PM, Jani Nikula <jani@nikula.org> wrote:
>>
>> > Running "notmuch compose" more than once within a second would result in
>> > identical message ids for the messages, which is not a good idea. That's
>> > not likely in interactive use, but the notmuch cli is highly scriptable,
>> > so someone is bound to hit this.
>> >
>> > Some paranoid might also be worried about "leaking" the time you run
>> > "notmuch compose"... which may be different from the actual time you
>> > send the message.
>>
>> It's still better than the current situation; nothing. In any case,
>> people that have not needed this would not be affected; their UI would
>> override the Message-ID.
>>
>> So do you have a better suggestion for a Message-ID?
>
> The easy way would be to just use g_mime_utils_generate_message_id()
> [1]. It doesn't give you any control of the part before @, but I'm not
> sure if that really matters.

This is what gmime does:
g_strdup_printf ("%lu.%lu.%lu@%s", (unsigned long int) time (NULL),
(unsigned long int) getpid (), count++, fqdn);

Which actually has some of the issues you mentioned.

I can do the same if you want (add pid and count). The advantage of
using our own format is that not only would it be more unique, but it
would not have "@fqdn".

> Alternatively you can write your own according to e.g. [2]. Glib appears
> to have decent and portable support for pseudo random number
> generation. But why bother? I'd go with gmime.

But gmime doesn't have anything random. I would actually prefer to
have something random though.

-- 
Felipe Contreras

Re: [PATCH v2 1/3] Add 'compose' command

[Felipe Contreras]

On Wed, Apr 18, 2012 at 5:41 PM, Dmitry Kurochkin
<dmitry.kurochkin@gmail.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On Wed, Apr 18, 2012 at 4:43 PM, Dmitry Kurochkin
>> <dmitry.kurochkin@gmail.com> wrote:
>>> Hi Felipe.
>>>
>>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>>
>>>> On Wed, Apr 18, 2012 at 4:06 PM, Jani Nikula <jani@nikula.org> wrote:
>>>>
>>>>> Running "notmuch compose" more than once within a second would result in
>>>>> identical message ids for the messages, which is not a good idea. That's
>>>>> not likely in interactive use, but the notmuch cli is highly scriptable,
>>>>> so someone is bound to hit this.
>>>>>
>>>>> Some paranoid might also be worried about "leaking" the time you run
>>>>> "notmuch compose"... which may be different from the actual time you
>>>>> send the message.
>>>>
>>>> It's still better than the current situation; nothing. In any case,
>>>> people that have not needed this would not be affected; their UI would
>>>> override the Message-ID.
>>>>
>>>
>>> I disagree.  If notmuch CLI generates a Message-ID, it must be a good
>>> one.  Otherwise we make users falsely believe that they do have a proper
>>> Message-ID while in fact they do not.  And that would bite them sooner
>>> or later.
>>
>> And then they'll report it, and we would fix it.
>>
>> Anyway, everything comes from a patch, so, do you have a patch,
>> pseudo-code, or even a suggestion?
>>
>
> A patch needs some positive reviews to be accepted.  Replying to
> comments with "make a better patch" may not be the best strategy for
> getting your patches accepted.

I did not say "make a better patch" or anything of the sort. You made
comments, I replied to those comments.

Your argument is essentially:

a) we don't have feature X
b) this patch applies feature X imperfectly
c) I don't know how to implement X perfectly

I don't think "patch is not perfect" is a valid argument. No patch is
perfect, if you want a better patch, then *at least* you should
suggest what would make it better.

Cheers.

-- 
Felipe Contreras

Re: [PATCH v2 1/3] Add 'compose' command

[Tomi Ollila]

On Wed, Apr 18 2012, Felipe Contreras <felipe.contreras@gmail.com> wrote:

> On Wed, Apr 18, 2012 at 5:20 PM, Jani Nikula <jani@nikula.org> wrote:
>> On Wed, 18 Apr 2012 16:34:30 +0300, Felipe Contreras <felipe.contreras@gmail.com> wrote:
>>> On Wed, Apr 18, 2012 at 4:06 PM, Jani Nikula <jani@nikula.org> wrote:
>>>
>>> > Running "notmuch compose" more than once within a second would result in
>>> > identical message ids for the messages, which is not a good idea. That's
>>> > not likely in interactive use, but the notmuch cli is highly scriptable,
>>> > so someone is bound to hit this.
>>> >
>>> > Some paranoid might also be worried about "leaking" the time you run
>>> > "notmuch compose"... which may be different from the actual time you
>>> > send the message.
>>>
>>> It's still better than the current situation; nothing. In any case,
>>> people that have not needed this would not be affected; their UI would
>>> override the Message-ID.
>>>
>>> So do you have a better suggestion for a Message-ID?
>>
>> The easy way would be to just use g_mime_utils_generate_message_id()
>> [1]. It doesn't give you any control of the part before @, but I'm not
>> sure if that really matters.
>
> This is what gmime does:
> g_strdup_printf ("%lu.%lu.%lu@%s", (unsigned long int) time (NULL),
> (unsigned long int) getpid (), count++, fqdn);
>
> Which actually has some of the issues you mentioned.
>
> I can do the same if you want (add pid and count). The advantage of
> using our own format is that not only would it be more unique, but it
> would not have "@fqdn".

getpid() is good. I guess count is useless (always one). Now some ideas
how to obfuscate time(NULL) (nonce + hash (crc32 good enough?) ?

Tomi

>
>> Alternatively you can write your own according to e.g. [2]. Glib appears
>> to have decent and portable support for pseudo random number
>> generation. But why bother? I'd go with gmime.
>
> But gmime doesn't have anything random. I would actually prefer to
> have something random though.



>
> -- 
> Felipe Contreras
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2 1/3] Add 'compose' command

[Dmitry Kurochkin]

Tomi Ollila <tomi.ollila@iki.fi> writes:

> On Wed, Apr 18 2012, Felipe Contreras <felipe.contreras@gmail.com> wrote:
>
>> On Wed, Apr 18, 2012 at 5:20 PM, Jani Nikula <jani@nikula.org> wrote:
>>> On Wed, 18 Apr 2012 16:34:30 +0300, Felipe Contreras <felipe.contreras@gmail.com> wrote:
>>>> On Wed, Apr 18, 2012 at 4:06 PM, Jani Nikula <jani@nikula.org> wrote:
>>>>
>>>> > Running "notmuch compose" more than once within a second would result in
>>>> > identical message ids for the messages, which is not a good idea. That's
>>>> > not likely in interactive use, but the notmuch cli is highly scriptable,
>>>> > so someone is bound to hit this.
>>>> >
>>>> > Some paranoid might also be worried about "leaking" the time you run
>>>> > "notmuch compose"... which may be different from the actual time you
>>>> > send the message.
>>>>
>>>> It's still better than the current situation; nothing. In any case,
>>>> people that have not needed this would not be affected; their UI would
>>>> override the Message-ID.
>>>>
>>>> So do you have a better suggestion for a Message-ID?
>>>
>>> The easy way would be to just use g_mime_utils_generate_message_id()
>>> [1]. It doesn't give you any control of the part before @, but I'm not
>>> sure if that really matters.
>>
>> This is what gmime does:
>> g_strdup_printf ("%lu.%lu.%lu@%s", (unsigned long int) time (NULL),
>> (unsigned long int) getpid (), count++, fqdn);
>>
>> Which actually has some of the issues you mentioned.
>>
>> I can do the same if you want (add pid and count). The advantage of
>> using our own format is that not only would it be more unique, but it
>> would not have "@fqdn".
>
> getpid() is good. I guess count is useless (always one). Now some ideas
> how to obfuscate time(NULL) (nonce + hash (crc32 good enough?) ?
>

I think the best would be for notmuch to use the gmime function and
change gmime (open a bug, may be provide a patch) to follow the best
practice for Message-ID generation (I guess that would be the document
pointed by Jani [1]).

Regards,
  Dmitry

[1] http://www.jwz.org/doc/mid.html

> Tomi
>
>>
>>> Alternatively you can write your own according to e.g. [2]. Glib appears
>>> to have decent and portable support for pseudo random number
>>> generation. But why bother? I'd go with gmime.
>>
>> But gmime doesn't have anything random. I would actually prefer to
>> have something random though.
>
>
>
>>
>> -- 
>> Felipe Contreras
>> _______________________________________________
>> notmuch mailing list
>> notmuch@notmuchmail.org
>> http://notmuchmail.org/mailman/listinfo/notmuch
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2 1/3] Add 'compose' command

[Jani Nikula]

On Wed, 18 Apr 2012, Felipe Contreras <felipe.contreras@gmail.com> wrote:
> On Wed, Apr 18, 2012 at 5:20 PM, Jani Nikula <jani@nikula.org> wrote:
>> On Wed, 18 Apr 2012 16:34:30 +0300, Felipe Contreras <felipe.contreras@gmail.com> wrote:
>>> On Wed, Apr 18, 2012 at 4:06 PM, Jani Nikula <jani@nikula.org> wrote:
>>>
>>> > Running "notmuch compose" more than once within a second would result in
>>> > identical message ids for the messages, which is not a good idea. That's
>>> > not likely in interactive use, but the notmuch cli is highly scriptable,
>>> > so someone is bound to hit this.
>>> >
>>> > Some paranoid might also be worried about "leaking" the time you run
>>> > "notmuch compose"... which may be different from the actual time you
>>> > send the message.
>>>
>>> It's still better than the current situation; nothing. In any case,
>>> people that have not needed this would not be affected; their UI would
>>> override the Message-ID.
>>>
>>> So do you have a better suggestion for a Message-ID?
>>
>> The easy way would be to just use g_mime_utils_generate_message_id()
>> [1]. It doesn't give you any control of the part before @, but I'm not
>> sure if that really matters.
>
> This is what gmime does:
> g_strdup_printf ("%lu.%lu.%lu@%s", (unsigned long int) time (NULL),
> (unsigned long int) getpid (), count++, fqdn);
>
> Which actually has some of the issues you mentioned.

Thanks for looking into gmime source. The implementation is a bit of a
disappointment.

> I can do the same if you want (add pid and count). The advantage of
> using our own format is that not only would it be more unique, but it
> would not have "@fqdn".

I'm starting to think doing our own would be the best, although I
wouldn't object to using the gmime implementation "for now".

>> Alternatively you can write your own according to e.g. [2]. Glib appears
>> to have decent and portable support for pseudo random number
>> generation. But why bother? I'd go with gmime.
>
> But gmime doesn't have anything random. I would actually prefer to
> have something random though.

Agreed.


BR,
Jani.

Re: [PATCH v2 2/3] reply: add message-id header

[Adam Wolfe Gordon]

On Wed, Apr 18, 2012 at 07:09, Jani Nikula <jani@nikula.org> wrote:
> I do wonder whether the emacs ui / message mode will add a duplicate
> message-id rather than override this.

I think the emacs reply code will replace Message-ID and User-Agent
headers correctly, as it does with the From header when selecting a
different identity within emacs. But, this should be tested.

Ideally, the emacs tests should catch any problems here. As things are
right now, I think we would catch problems with the User-Agent header
in the reply tests: the tests replace the emacs and notmuch versions
with XXX but otherwise leave the header intact, so it should differ
from the one generated by notmuch compose.

The Message-ID is trickier, since in emacs it's generated when the
message is sent, not when the message is created, and thus the reply
tests don't see it at all. The "Sending a message via (fake) SMTP"
test could potentially catch problems, but currently the tests don't
try to predict the Message-ID emacs will give the message, so they
replace it with XXX.

-- Adam

Re: [PATCH v2 0/3] composing patches

[Adam Wolfe Gordon]

Hi Felipe,

On Wed, Apr 18, 2012 at 06:39, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> I don't know how it works in gnus, but at least on the vim mode, the output
> generated by 'notmuch reply' is not ready to be sent, at least the Message-ID
> field is needed, and also is nice to have the User-Agent.

In the emacs interface, the Message-ID header is generated when the
message is sent, so it never shows up in the reply buffer. The
User-Agent header is created by the emacs reply code.

> Besides, in order to avoid creating a new message by hand (possibly fetching
> the info from notmuch config), it's more straightforward to have 'notmuch
> compose' command.
>
> In the future 'notmuch compose' might be used to replace 'notmuch reply' and
> possily add a forward option too. It might also be possible to add mail aliases
> when composing a message, and these aliases might be used while generating the
> 'notmuch search' output.

I can see how this functionality is useful, and I like the idea of
consolidating message creation, forwarding, and reply. However, if
this is intended to replace or share code with reply (and I think it
should), it should support all the existing reply formats: default,
json, and headers-only.

In fact, I think it might make more sense to make this work an
extension of the existing reply code rather than a rewrite.
notmuch-reply.c could become notmuch-compose.c, and provide the
compose command (with flags for creating a reply), and also the reply
command (which would just be an alias to the compose functionality).

>  Makefile.local    |    1 +
>  notmuch-client.h  |    3 ++
>  notmuch-compose.c |  111 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  notmuch-reply.c   |   17 ++++++++
>  notmuch.c         |    5 +++

Such a big new features should definitely have some tests.

-- Adam

Re: [PATCH v2 2/3] reply: add message-id header

[Felipe Contreras]

On Wed, Apr 18, 2012 at 4:09 PM, Jani Nikula <jani@nikula.org> wrote:
> On Wed, 18 Apr 2012 15:39:12 +0300, Felipe Contreras <felipe.contreras@gmail.com> wrote:
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>> ---
>>  notmuch-reply.c |   11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/notmuch-reply.c b/notmuch-reply.c
>> index 0949d9f..d796bb2 100644
>> --- a/notmuch-reply.c
>> +++ b/notmuch-reply.c
>> @@ -464,6 +464,8 @@ create_reply_message(void *ctx,
>>  {
>>      const char *subject, *from_addr = NULL;
>>      const char *in_reply_to, *orig_references, *references;
>> +    const char *message_id;
>> +    char *simple_from;
>>
>>      /* The 1 means we want headers in a "pretty" order. */
>>      GMimeMessage *reply = g_mime_message_new (1);
>> @@ -488,6 +490,8 @@ create_reply_message(void *ctx,
>>      if (from_addr == NULL)
>>       from_addr = notmuch_config_get_user_primary_email (config);
>>
>> +    simple_from = talloc_strdup (ctx, from_addr);
>> +
>>      from_addr = talloc_asprintf (ctx, "%s <%s>",
>>                                notmuch_config_get_user_name (config),
>>                                from_addr);
>> @@ -508,6 +512,13 @@ create_reply_message(void *ctx,
>>      g_mime_object_set_header (GMIME_OBJECT (reply),
>>                             "References", references);
>>
>> +    message_id = talloc_asprintf (ctx, "<%lu-notmuch-%s>",
>> +                               time(NULL),
>> +                               simple_from);
>
> This duplicates the problem in "notmuch compose", hinting that you
> should probably have the message-id generation in one place only.

Hopefully in the future 'notmuch reply' would be merged onto 'notmuch
compose --reply' or something. So this Message-ID code would be in one
place.

-- 
Felipe Contreras

Re: [PATCH v2 1/3] Add 'compose' command

[Tomi Ollila]

On Wed, Apr 18 2012, Jani Nikula <jani@nikula.org> wrote:

> On Wed, 18 Apr 2012, Felipe Contreras <felipe.contreras@gmail.com> wrote:
>> On Wed, Apr 18, 2012 at 5:20 PM, Jani Nikula <jani@nikula.org> wrote:
>>> On Wed, 18 Apr 2012 16:34:30 +0300, Felipe Contreras <felipe.contreras@gmail.com> wrote:
>>>> On Wed, Apr 18, 2012 at 4:06 PM, Jani Nikula <jani@nikula.org> wrote:
>>>>
>>>> > Running "notmuch compose" more than once within a second would result in
>>>> > identical message ids for the messages, which is not a good idea. That's
>>>> > not likely in interactive use, but the notmuch cli is highly scriptable,
>>>> > so someone is bound to hit this.
>>>> >
>>>> > Some paranoid might also be worried about "leaking" the time you run
>>>> > "notmuch compose"... which may be different from the actual time you
>>>> > send the message.
>>>>
>>>> It's still better than the current situation; nothing. In any case,
>>>> people that have not needed this would not be affected; their UI would
>>>> override the Message-ID.
>>>>
>>>> So do you have a better suggestion for a Message-ID?
>>>
>>> The easy way would be to just use g_mime_utils_generate_message_id()
>>> [1]. It doesn't give you any control of the part before @, but I'm not
>>> sure if that really matters.
>>
>> This is what gmime does:
>> g_strdup_printf ("%lu.%lu.%lu@%s", (unsigned long int) time (NULL),
>> (unsigned long int) getpid (), count++, fqdn);
>>
>> Which actually has some of the issues you mentioned.
>
> Thanks for looking into gmime source. The implementation is a bit of a
> disappointment.
>
>> I can do the same if you want (add pid and count). The advantage of
>> using our own format is that not only would it be more unique, but it
>> would not have "@fqdn".
>
> I'm starting to think doing our own would be the best, although I
> wouldn't object to using the gmime implementation "for now".

I think I would be disappointed if I had to use message id:s generated
by gmime -- just that "time leakage" problem. I guess the message-id
is usually generated just before the mail is sent so the date in message
id and Date: header are about equal. If message id is generated one time
and Date: another then the time taking to write an email leaks... (except
if Date: is also generate at 'notmuch compose' execution time (uhh ;/) 

Anyway, gmime implementation or something having time(NULL).getpid()
could be used "for now".

>>> Alternatively you can write your own according to e.g. [2]. Glib appears
>>> to have decent and portable support for pseudo random number
>>> generation. But why bother? I'd go with gmime.
>>
>> But gmime doesn't have anything random. I would actually prefer to
>> have something random though.
>
> Agreed.

Me too.

>
>
> BR,
> Jani.

Tomi

Re: [PATCH v2 1/3] Add 'compose' command

[Felipe Contreras]

On Thu, Apr 19, 2012 at 12:31 PM, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> On Wed, Apr 18 2012, Jani Nikula <jani@nikula.org> wrote:
>
>> On Wed, 18 Apr 2012, Felipe Contreras <felipe.contreras@gmail.com> wrote:
>>> On Wed, Apr 18, 2012 at 5:20 PM, Jani Nikula <jani@nikula.org> wrote:
>>>> On Wed, 18 Apr 2012 16:34:30 +0300, Felipe Contreras <felipe.contreras@gmail.com> wrote:
>>>>> On Wed, Apr 18, 2012 at 4:06 PM, Jani Nikula <jani@nikula.org> wrote:
>>>>>
>>>>> > Running "notmuch compose" more than once within a second would result in
>>>>> > identical message ids for the messages, which is not a good idea. That's
>>>>> > not likely in interactive use, but the notmuch cli is highly scriptable,
>>>>> > so someone is bound to hit this.
>>>>> >
>>>>> > Some paranoid might also be worried about "leaking" the time you run
>>>>> > "notmuch compose"... which may be different from the actual time you
>>>>> > send the message.
>>>>>
>>>>> It's still better than the current situation; nothing. In any case,
>>>>> people that have not needed this would not be affected; their UI would
>>>>> override the Message-ID.
>>>>>
>>>>> So do you have a better suggestion for a Message-ID?
>>>>
>>>> The easy way would be to just use g_mime_utils_generate_message_id()
>>>> [1]. It doesn't give you any control of the part before @, but I'm not
>>>> sure if that really matters.
>>>
>>> This is what gmime does:
>>> g_strdup_printf ("%lu.%lu.%lu@%s", (unsigned long int) time (NULL),
>>> (unsigned long int) getpid (), count++, fqdn);
>>>
>>> Which actually has some of the issues you mentioned.
>>
>> Thanks for looking into gmime source. The implementation is a bit of a
>> disappointment.
>>
>>> I can do the same if you want (add pid and count). The advantage of
>>> using our own format is that not only would it be more unique, but it
>>> would not have "@fqdn".
>>
>> I'm starting to think doing our own would be the best, although I
>> wouldn't object to using the gmime implementation "for now".
>
> I think I would be disappointed if I had to use message id:s generated
> by gmime -- just that "time leakage" problem. I guess the message-id
> is usually generated just before the mail is sent so the date in message
> id and Date: header are about equal. If message id is generated one time
> and Date: another then the time taking to write an email leaks... (except
> if Date: is also generate at 'notmuch compose' execution time (uhh ;/)
>
> Anyway, gmime implementation or something having time(NULL).getpid()
> could be used "for now".

I agree. I was going to send a first batch of patches using gmime's
implementation for now, but since apparently I have to add a bunch of
features to my notmuch-compose patch, I'm most likely going to wait
since I'm having trouble getting even a working setup right now.

Cheers.

-- 
Felipe Contreras