[PATCH v3 0/3] Clean up reply's encoding story

[PATCH v3 0/3] Clean up reply's encoding story

[Austin Clements]

This is v3 of id:1376587658-19202-1-git-send-email-amdragon@mit.edu.
This addresses Jani's comment in id:87k3jl3ehe.fsf@nikula.org by
treating errors while retrieving the original message's References
header as if the original message didn't have a References header.  It
also adds a comment suggested by Jani on IRC.  The diff from v2 is
below.

diff --git a/notmuch-reply.c b/notmuch-reply.c
index bfd0f51..ac46d02 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -31,6 +31,7 @@ show_reply_headers (GMimeMessage *message)
     stream_stdout = g_mime_stream_file_new (stdout);
     if (stream_stdout) {
 	g_mime_stream_file_set_owner (GMIME_STREAM_FILE (stream_stdout), FALSE);
+	/* Output RFC 2822 formatted (and RFC 2047 encoded) headers. */
 	g_mime_object_write_to_stream (GMIME_OBJECT(message), stream_stdout);
 	g_object_unref(stream_stdout);
     }
@@ -526,14 +527,15 @@ create_reply_message(void *ctx,
 			      "In-Reply-To", in_reply_to);
 
     orig_references = notmuch_message_get_header (message, "references");
-    if (orig_references) {
-	references = talloc_asprintf (ctx, "%s%s%s",
-				      *orig_references ? orig_references : "",
-				      *orig_references ? " " : "",
-				      in_reply_to);
-	g_mime_object_set_header (GMIME_OBJECT (reply),
-				  "References", references);
-    }
+    if (!orig_references)
+	/* Treat errors like missing References headers. */
+	orig_references = "";
+    references = talloc_asprintf (ctx, "%s%s%s",
+				  *orig_references ? orig_references : "",
+				  *orig_references ? " " : "",
+				  in_reply_to);
+    g_mime_object_set_header (GMIME_OBJECT (reply),
+			      "References", references);
 
     return reply;
 }

[PATCH v3 1/3] reply: Test replying to messages with RFC 2047-encoded headers

[Austin Clements]

---
 test/reply |   56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/test/reply b/test/reply
index ee5d361..c877ffe 100755
--- a/test/reply
+++ b/test/reply
@@ -193,4 +193,60 @@ References: <${gen_msg_id}>
 On Tue, 05 Jan 2010 15:43:56 -0000, Sender <sender@example.com> wrote:
 > From guessing"
 
+test_begin_subtest "Reply with RFC 2047-encoded headers"
+add_message '[subject]="=?iso-8859-1?q?=e0=df=e7?="' \
+	    '[from]="=?utf-8?q?=e2=98=83?= <snowman@example.com>"' \
+	    '[date]="Tue, 05 Jan 2010 15:43:56 -0000"' \
+	    '[body]="Encoding"'
+
+output=$(notmuch reply id:${gen_msg_id})
+test_expect_equal "$output" "\
+From: Notmuch Test Suite <test_suite@notmuchmail.org>
+Subject: Re: àßç
+To: ☃ <snowman@example.com>
+In-Reply-To: <${gen_msg_id}>
+References: <${gen_msg_id}>
+
+On Tue, 05 Jan 2010 15:43:56 -0000, ☃ <snowman@example.com> wrote:
+> Encoding"
+
+test_begin_subtest "Reply with RFC 2047-encoded headers (JSON)"
+output=$(notmuch reply --format=json id:${gen_msg_id})
+test_expect_equal_json "$output" '
+{
+    "original": {
+        "body": [
+            {
+                "content": "Encoding\n",
+                "content-type": "text/plain",
+                "id": 1
+            }
+        ],
+        "date_relative": "2010-01-05",
+        "excluded": false,
+        "filename": "'${MAIL_DIR}'/msg-012",
+        "headers": {
+            "Date": "Tue, 05 Jan 2010 15:43:56 +0000",
+            "From": "\u2603 <snowman@example.com>",
+            "Subject": "\u00e0\u00df\u00e7",
+            "To": "Notmuch Test Suite <test_suite@notmuchmail.org>"
+        },
+        "id": "'${gen_msg_id}'",
+        "match": false,
+        "tags": [
+            "inbox",
+            "unread"
+        ],
+        "timestamp": 1262706236
+    },
+    "reply-headers": {
+        "From": "Notmuch Test Suite <test_suite@notmuchmail.org>",
+        "In-reply-to": "<'${gen_msg_id}'>",
+        "References": " <'${gen_msg_id}'>",
+        "Subject": "Re: \u00e0\u00df\u00e7",
+        "To": "\u2603 <snowman@example.com>"
+    }
+}'
+
+
 test_done
-- 
1.7.10.4

[PATCH v3 2/3] reply: Remove extraneous space from generated References

[Austin Clements]

Previously, the References header code seemed to assume
notmuch_message_get_header would return NULL if the header was not
present, but it actually returns "".  As a result of this, it was
inserting an unnecessary space when concatenating an empty or missing
original references header with the new reference.

This shows up in only two tests because the text reply format later
passes the whole reply template through g_mime_filter_headers, which
has the side effect of stripping out this extra space.
---
 notmuch-reply.c |    7 +++++--
 test/multipart  |    2 +-
 test/reply      |    2 +-
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/notmuch-reply.c b/notmuch-reply.c
index 3b2b58d..472a002 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -537,9 +537,12 @@ create_reply_message(void *ctx,
 			      "In-Reply-To", in_reply_to);
 
     orig_references = notmuch_message_get_header (message, "references");
+    if (!orig_references)
+	/* Treat errors like missing References headers. */
+	orig_references = "";
     references = talloc_asprintf (ctx, "%s%s%s",
-				  orig_references ? orig_references : "",
-				  orig_references ? " " : "",
+				  *orig_references ? orig_references : "",
+				  *orig_references ? " " : "",
 				  in_reply_to);
     g_mime_object_set_header (GMIME_OBJECT (reply),
 			      "References", references);
diff --git a/test/multipart b/test/multipart
index c974226..2033023 100755
--- a/test/multipart
+++ b/test/multipart
@@ -599,7 +599,7 @@ cat <<EOF >EXPECTED
  "From": "Notmuch Test Suite <test_suite@notmuchmail.org>",
  "To": "Carl Worth <cworth@cworth.org>, cworth@cworth.org",
  "In-reply-to": "<87liy5ap00.fsf@yoom.home.cworth.org>",
- "References": " <87liy5ap00.fsf@yoom.home.cworth.org>"},
+ "References": "<87liy5ap00.fsf@yoom.home.cworth.org>"},
  "original": {"id": "XXXXX",
  "match": false,
  "excluded": false,
diff --git a/test/reply b/test/reply
index c877ffe..a85ebe5 100755
--- a/test/reply
+++ b/test/reply
@@ -242,7 +242,7 @@ test_expect_equal_json "$output" '
     "reply-headers": {
         "From": "Notmuch Test Suite <test_suite@notmuchmail.org>",
         "In-reply-to": "<'${gen_msg_id}'>",
-        "References": " <'${gen_msg_id}'>",
+        "References": "<'${gen_msg_id}'>",
         "Subject": "Re: \u00e0\u00df\u00e7",
         "To": "\u2603 <snowman@example.com>"
     }
-- 
1.7.10.4

[PATCH v3 3/3] reply: Use RFC 2822/MIME wholly for text format template

[Austin Clements]

Previously, reply's default text format used an odd mix of RFC 2045
MIME encoding for the reply template's body and some made-up RFC
2822-like UTF-8 format for the headers.  The intent was to present the
headers to the user in a nice, un-encoded format, but this assumed
that whatever ultimately sent the email would RFC 2047-encode the
headers, while at the same time the body was already RFC 2045 encoded,
so it assumed that whatever sent the email would *not* re-encode the
body.

This can be fixed by either producing a fully decoded UTF-8 reply
template, or a fully encoded MIME-compliant RFC 2822 message.  This
patch does the latter because it is

a) Well-defined by RFC 2822 and MIME (while any UTF-8 format would be
   ad hoc).

b) Ready to be piped to sendmail.  The point of the text format is to
   be minimal, so a user should be able to pop up the template in
   whatever editor they want, edit it, and push it to sendmail.

c) Consistent with frontend capabilities.  If a frontend has the
   smarts to RFC 2047 encode the headers before sending the mail, it
   probably has the smarts to RFC 2047 decode them before presenting
   the template to a user for editing.

Also, as far as I know, nothing automated consumes the reply text
format, so changing this should not cause serious problems.  (And if
anything does still consume this format, it probably gets these
encoding issues wrong anyway.)
---
 Makefile.local           |    1 -
 gmime-filter-headers.c   |  263 ----------------------------------------------
 gmime-filter-headers.h   |   69 ------------
 man/man1/notmuch-reply.1 |    2 +-
 notmuch-reply.c          |   16 +--
 test/reply               |    9 +-
 test/reply-to-sender     |    4 +-
 7 files changed, 13 insertions(+), 351 deletions(-)
 delete mode 100644 gmime-filter-headers.c
 delete mode 100644 gmime-filter-headers.h

diff --git a/Makefile.local b/Makefile.local
index 84043fe..b7cd266 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -255,7 +255,6 @@ notmuch_client_srcs =		\
 	command-line-arguments.c\
 	debugger.c		\
 	gmime-filter-reply.c	\
-	gmime-filter-headers.c	\
 	hooks.c			\
 	notmuch.c		\
 	notmuch-config.c	\
diff --git a/gmime-filter-headers.c b/gmime-filter-headers.c
deleted file mode 100644
index 7db3779..0000000
--- a/gmime-filter-headers.c
+++ /dev/null
@@ -1,263 +0,0 @@
-/*
- * Copyright © 2009 Keith Packard <keithp@keithp.com>
- * Copyright © 2010 Michal Sojka <sojkam1@fel.cvut.cz>
- *
- * 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, write to the Free Software Foundation, Inc.,
- * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
- */
-
-#include "gmime-filter-headers.h"
-#include <string.h>
-#include <gmime/gmime-utils.h>
-#include <glib/gprintf.h>
-#include <stdlib.h>
-#include <xutil.h>
-
-/**
- * SECTION: gmime-filter-headers
- * @title: GMimeFilterHeaders
- * @short_description: Add/remove headers markers
- *
- * A #GMimeFilter for decoding rfc2047 encoded headers to UTF-8
- **/
-
-
-static void g_mime_filter_headers_class_init (GMimeFilterHeadersClass *klass);
-static void g_mime_filter_headers_init (GMimeFilterHeaders *filter, GMimeFilterHeadersClass *klass);
-static void g_mime_filter_headers_finalize (GObject *object);
-
-static GMimeFilter *filter_copy (GMimeFilter *filter);
-static void filter_filter (GMimeFilter *filter, char *in, size_t len, size_t prespace,
-			   char **out, size_t *outlen, size_t *outprespace);
-static void filter_complete (GMimeFilter *filter, char *in, size_t len, size_t prespace,
-			     char **out, size_t *outlen, size_t *outprespace);
-static void filter_reset (GMimeFilter *filter);
-
-
-static GMimeFilterClass *parent_class = NULL;
-
-GType
-g_mime_filter_headers_get_type (void)
-{
-	static GType type = 0;
-
-	if (!type) {
-		static const GTypeInfo info = {
-			sizeof (GMimeFilterHeadersClass),
-			NULL, /* base_class_init */
-			NULL, /* base_class_finalize */
-			(GClassInitFunc) g_mime_filter_headers_class_init,
-			NULL, /* class_finalize */
-			NULL, /* class_data */
-			sizeof (GMimeFilterHeaders),
-			0,    /* n_preallocs */
-			(GInstanceInitFunc) g_mime_filter_headers_init,
-			NULL	/* value_table */
-		};
-
-		type = g_type_register_static (GMIME_TYPE_FILTER, "GMimeFilterHeaders", &info, (GTypeFlags) 0);
-	}
-
-	return type;
-}
-
-
-static void
-g_mime_filter_headers_class_init (GMimeFilterHeadersClass *klass)
-{
-	GObjectClass *object_class = G_OBJECT_CLASS (klass);
-	GMimeFilterClass *filter_class = GMIME_FILTER_CLASS (klass);
-
-	parent_class = (GMimeFilterClass *) g_type_class_ref (GMIME_TYPE_FILTER);
-
-	object_class->finalize = g_mime_filter_headers_finalize;
-
-	filter_class->copy = filter_copy;
-	filter_class->filter = filter_filter;
-	filter_class->complete = filter_complete;
-	filter_class->reset = filter_reset;
-}
-
-static void
-g_mime_filter_headers_init (GMimeFilterHeaders *filter, GMimeFilterHeadersClass *klass)
-{
-	(void) klass;
-	filter->saw_nl = TRUE;
-	filter->line = NULL;
-	filter->line_size = 0;
-	filter->lineptr = NULL;
-}
-
-static void
-g_mime_filter_headers_finalize (GObject *object)
-{
-	free (GMIME_FILTER_HEADERS (object)->line);
-	G_OBJECT_CLASS (parent_class)->finalize (object);
-}
-
-
-static GMimeFilter *
-filter_copy (GMimeFilter *filter)
-{
-	(void) filter;
-	return g_mime_filter_headers_new ();
-}
-
-static void
-output_decoded_header (GMimeFilterHeaders *headers, char **outptr)
-{
-	char *colon, *name, *s, *decoded_value;
-	size_t offset;
-	gint ret;
-
-	colon = strchr (headers->line, ':');
-	if (colon == NULL)
-		return;
-
-	name = headers->line;
-	*colon = '\0';
-	s = colon + 1;
-	while (*s == ' ' || *s == '\t')
-		s++;
-	decoded_value = g_mime_utils_header_decode_text(s);
-	if (decoded_value == NULL)
-		return;
-	offset = *outptr - GMIME_FILTER (headers)->outbuf;
-	g_mime_filter_set_size (GMIME_FILTER (headers), strlen(name) + 2 +
-			       strlen(decoded_value) + 2, TRUE);
-	*outptr = GMIME_FILTER (headers)->outbuf + offset;
-	ret = g_sprintf (*outptr, "%s: %s\n", name, decoded_value);
-	if (ret > 0)
-		*outptr += ret;
-	free (decoded_value);
-}
-
-static void
-output_final_newline (GMimeFilterHeaders *headers, char **outptr)
-{
-	size_t offset;
-
-	offset = *outptr - GMIME_FILTER (headers)->outbuf;
-	g_mime_filter_set_size (GMIME_FILTER (headers), 1, TRUE);
-	*outptr = GMIME_FILTER (headers)->outbuf + offset;
-	*(*outptr)++ = '\n';
-}
-
-static void
-filter_filter (GMimeFilter *filter, char *inbuf, size_t inlen, size_t prespace,
-	       char **outbuf, size_t *outlen, size_t *outprespace)
-{
-	GMimeFilterHeaders *headers = (GMimeFilterHeaders *) filter;
-	register const char *inptr = inbuf;
-	const char *inend = inbuf + inlen;
-	char *lineptr, *lineend, *outptr;
-
-	(void) prespace;
-	if (headers->line == NULL) {
-		headers->line_size = 200;
-		headers->lineptr = headers->line = malloc (headers->line_size);
-	}
-	lineptr = headers->lineptr;
-	lineend = headers->line + headers->line_size - 1;
-	if (lineptr == NULL)
-		return;
-	outptr = filter->outbuf;
-	while (inptr < inend) {
-		if (*inptr == '\n') {
-			if (headers->saw_nl)
-				output_final_newline(headers, &outptr);
-			headers->saw_nl = TRUE;
-			inptr++;
-			continue;
-		}
-
-		if (lineptr == lineend) {
-			headers->line_size *= 2;
-			headers->line = xrealloc (headers->line, headers->line_size);
-			lineptr = headers->line + (headers->line_size / 2) - 1;
-			lineend = headers->line + headers->line_size - 1;
-		}
-
-		if (headers->saw_nl && *inptr != ' ' && *inptr != '\t') {
-			*lineptr = '\0';
-			output_decoded_header (headers, &outptr);
-			lineptr = headers->line;
-		}
-		if (headers->saw_nl && (*inptr == ' ' || *inptr == '\t')) {
-			*lineptr = ' ';
-			lineptr++;
-			while (inptr < inend && (*inptr == ' ' || *inptr == '\t'))
-				inptr++;
-			headers->saw_nl = FALSE;
-			continue;
-		}
-		headers->saw_nl = FALSE;
-
-		if (*inptr != '\r')
-			*lineptr++ = *inptr;
-		inptr++;
-	}
-	if (headers->saw_nl) {
-		*lineptr = '\0';
-		output_decoded_header (headers, &outptr);
-		lineptr = headers->line;
-	}
-	headers->lineptr = lineptr;
-	*outlen = outptr - filter->outbuf;
-	*outprespace = filter->outpre;
-	*outbuf = filter->outbuf;
-}
-
-static void
-filter_complete (GMimeFilter *filter, char *inbuf, size_t inlen, size_t prespace,
-		 char **outbuf, size_t *outlen, size_t *outprespace)
-{
-	if (inbuf && inlen)
-		filter_filter (filter, inbuf, inlen, prespace, outbuf, outlen, outprespace);
-}
-
-static void
-filter_reset (GMimeFilter *filter)
-{
-	GMimeFilterHeaders *headers = (GMimeFilterHeaders *) filter;
-
-	headers->saw_nl = TRUE;
-	free(headers->line);
-	headers->line = NULL;
-	headers->line_size = 0;
-}
-
-
-/**
- * g_mime_filter_headers_new:
- * @encode: %TRUE if the filter should encode or %FALSE otherwise
- * @dots: encode/decode dots (as for SMTP)
- *
- * Creates a new #GMimeFilterHeaders filter.
- *
- * If @encode is %TRUE, then all lines will be prefixed by "> ",
- * otherwise any lines starting with "> " will have that removed
- *
- * Returns: a new #GMimeFilterHeaders filter.
- **/
-GMimeFilter *
-g_mime_filter_headers_new (void)
-{
-	GMimeFilterHeaders *new_headers;
-
-	new_headers = (GMimeFilterHeaders *) g_object_newv (GMIME_TYPE_FILTER_HEADERS, 0, NULL);
-
-	return (GMimeFilter *) new_headers;
-}
-
diff --git a/gmime-filter-headers.h b/gmime-filter-headers.h
deleted file mode 100644
index 1d1a3eb..0000000
--- a/gmime-filter-headers.h
+++ /dev/null
@@ -1,69 +0,0 @@
-/*
- * Copyright © 2009 Keith Packard <keithp@keithp.com>
- * Copyright © 2010 Michal Sojka <sojkam1@fel.cvut.cz>
- *
- * 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, write to the Free Software Foundation, Inc.,
- * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
- */
-
-#ifndef _GMIME_FILTER_HEADERS_H_
-#define _GMIME_FILTER_HEADERS_H_
-
-#include <gmime/gmime-filter.h>
-
-G_BEGIN_DECLS
-
-#define GMIME_TYPE_FILTER_HEADERS            (g_mime_filter_headers_get_type ())
-#define GMIME_FILTER_HEADERS(obj)            (G_TYPE_CHECK_INSTANCE_CAST ((obj), GMIME_TYPE_FILTER_HEADERS, GMimeFilterHeaders))
-#define GMIME_FILTER_HEADERS_CLASS(klass)    (G_TYPE_CHECK_CLASS_CAST ((klass), GMIME_TYPE_FILTER_HEADERS, GMimeFilterHeadersClass))
-#define GMIME_IS_FILTER_HEADERS(obj)         (G_TYPE_CHECK_INSTANCE_TYPE ((obj), GMIME_TYPE_FILTER_HEADERS))
-#define GMIME_IS_FILTER_HEADERS_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), GMIME_TYPE_FILTER_HEADERS))
-#define GMIME_FILTER_HEADERS_GET_CLASS(obj)  (G_TYPE_INSTANCE_GET_CLASS ((obj), GMIME_TYPE_FILTER_HEADERS, GMimeFilterHeadersClass))
-
-typedef struct _GMimeFilterHeaders GMimeFilterHeaders;
-typedef struct _GMimeFilterHeadersClass GMimeFilterHeadersClass;
-
-/**
- * GMimeFilterHeaders:
- * @parent_object: parent #GMimeFilter
- * @saw_nl: previous char was a \n
- * @line: temporary buffer for line unfolding
- * @line_size: size of currently allocated memory for @line
- * @lineptr: pointer to the first unused character in @line
- *
- * A filter to decode rfc2047 encoded headers
- **/
-struct _GMimeFilterHeaders {
-	GMimeFilter parent_object;
-
-	gboolean saw_nl;
-	char *line;
-	size_t line_size;
-	char *lineptr;
-};
-
-struct _GMimeFilterHeadersClass {
-	GMimeFilterClass parent_class;
-
-};
-
-
-GType g_mime_filter_headers_get_type (void);
-
-GMimeFilter *g_mime_filter_headers_new (void);
-
-G_END_DECLS
-
-
-#endif /* _GMIME_FILTER_HEADERS_H_ */
diff --git a/man/man1/notmuch-reply.1 b/man/man1/notmuch-reply.1
index ac76b07..e553145 100644
--- a/man/man1/notmuch-reply.1
+++ b/man/man1/notmuch-reply.1
@@ -41,7 +41,7 @@ include
 .RS
 .TP 4
 .BR default
-Includes subject and quoted message body.
+Includes subject and quoted message body as an RFC 2822 message.
 .TP
 .BR json
 Produces JSON output containing headers for a reply message and the
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 472a002..ac46d02 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -21,28 +21,18 @@
  */
 
 #include "notmuch-client.h"
-#include "gmime-filter-headers.h"
 #include "sprinter.h"
 
 static void
 show_reply_headers (GMimeMessage *message)
 {
-    GMimeStream *stream_stdout = NULL, *stream_filter = NULL;
+    GMimeStream *stream_stdout = 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_object_write_to_stream will produce
-		// RFC2047-encoded headers, but we want to present the
-		// user with decoded headers and let whatever
-		// ultimately sends the mail do the RFC2047 encoding.
-		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);
-	}
+	/* Output RFC 2822 formatted (and RFC 2047 encoded) headers. */
+	g_mime_object_write_to_stream (GMIME_OBJECT(message), stream_stdout);
 	g_object_unref(stream_stdout);
     }
 }
diff --git a/test/reply b/test/reply
index a85ebe5..d4389cf 100755
--- a/test/reply
+++ b/test/reply
@@ -132,7 +132,9 @@ add_message '[subject]="This subject is exactly 200 bytes in length. Other than
 	    '[body]="200-byte header"'
 output=$(notmuch reply id:${gen_msg_id})
 test_expect_equal "$output" "From: Notmuch Test Suite <test_suite@notmuchmail.org>
-Subject: Re: This subject is exactly 200 bytes in length. Other than its length there is not much of note here. Note that the length of 200 bytes includes the Subject: and Re: prefixes with two spaces
+Subject: Re: This subject is exactly 200 bytes in length. Other than its
+ length there is not much of note here. Note that the length of 200 bytes
+ includes the Subject: and Re: prefixes with two spaces
 In-Reply-To: <${gen_msg_id}>
 References: <${gen_msg_id}>
 
@@ -200,10 +202,11 @@ add_message '[subject]="=?iso-8859-1?q?=e0=df=e7?="' \
 	    '[body]="Encoding"'
 
 output=$(notmuch reply id:${gen_msg_id})
+# Note that GMime changes from Q- to B-encoding
 test_expect_equal "$output" "\
 From: Notmuch Test Suite <test_suite@notmuchmail.org>
-Subject: Re: àßç
-To: ☃ <snowman@example.com>
+Subject: Re: =?iso-8859-1?b?4N/n?=
+To: =?UTF-8?b?4piD?= <snowman@example.com>
 In-Reply-To: <${gen_msg_id}>
 References: <${gen_msg_id}>
 
diff --git a/test/reply-to-sender b/test/reply-to-sender
index c7d15bb..30e5e38 100755
--- a/test/reply-to-sender
+++ b/test/reply-to-sender
@@ -200,7 +200,9 @@ add_message '[subject]="This subject is exactly 200 bytes in length. Other than
             '[body]="200-byte header"'
 output=$(notmuch reply  --reply-to=sender id:${gen_msg_id})
 test_expect_equal "$output" "From: Notmuch Test Suite <test_suite@notmuchmail.org>
-Subject: Re: This subject is exactly 200 bytes in length. Other than its length there is not much of note here. Note that the length of 200 bytes includes the Subject: and Re: prefixes with two spaces
+Subject: Re: This subject is exactly 200 bytes in length. Other than its
+ length there is not much of note here. Note that the length of 200 bytes
+ includes the Subject: and Re: prefixes with two spaces
 In-Reply-To: <${gen_msg_id}>
 References: <${gen_msg_id}>
 
-- 
1.7.10.4

Re: [PATCH v3 0/3] Clean up reply's encoding story

[Jani Nikula]

On Fri, 16 Aug 2013, Austin Clements <amdragon@MIT.EDU> wrote:
> This is v3 of id:1376587658-19202-1-git-send-email-amdragon@mit.edu.
> This addresses Jani's comment in id:87k3jl3ehe.fsf@nikula.org by
> treating errors while retrieving the original message's References
> header as if the original message didn't have a References header.  It
> also adds a comment suggested by Jani on IRC.  The diff from v2 is
> below.

The series LGTM, and I in particular welcome the diffstat in patch 3!

BR,
Jani.


>
> diff --git a/notmuch-reply.c b/notmuch-reply.c
> index bfd0f51..ac46d02 100644
> --- a/notmuch-reply.c
> +++ b/notmuch-reply.c
> @@ -31,6 +31,7 @@ show_reply_headers (GMimeMessage *message)
>      stream_stdout = g_mime_stream_file_new (stdout);
>      if (stream_stdout) {
>  	g_mime_stream_file_set_owner (GMIME_STREAM_FILE (stream_stdout), FALSE);
> +	/* Output RFC 2822 formatted (and RFC 2047 encoded) headers. */
>  	g_mime_object_write_to_stream (GMIME_OBJECT(message), stream_stdout);
>  	g_object_unref(stream_stdout);
>      }
> @@ -526,14 +527,15 @@ create_reply_message(void *ctx,
>  			      "In-Reply-To", in_reply_to);
>  
>      orig_references = notmuch_message_get_header (message, "references");
> -    if (orig_references) {
> -	references = talloc_asprintf (ctx, "%s%s%s",
> -				      *orig_references ? orig_references : "",
> -				      *orig_references ? " " : "",
> -				      in_reply_to);
> -	g_mime_object_set_header (GMIME_OBJECT (reply),
> -				  "References", references);
> -    }
> +    if (!orig_references)
> +	/* Treat errors like missing References headers. */
> +	orig_references = "";
> +    references = talloc_asprintf (ctx, "%s%s%s",
> +				  *orig_references ? orig_references : "",
> +				  *orig_references ? " " : "",
> +				  in_reply_to);
> +    g_mime_object_set_header (GMIME_OBJECT (reply),
> +			      "References", references);
>  
>      return reply;
>  }
>
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v3 0/3] Clean up reply's encoding story

[Tomi Ollila]

On Fri, Aug 16 2013, Jani Nikula <jani@nikula.org> wrote:

> On Fri, 16 Aug 2013, Austin Clements <amdragon@MIT.EDU> wrote:
>> This is v3 of id:1376587658-19202-1-git-send-email-amdragon@mit.edu.
>> This addresses Jani's comment in id:87k3jl3ehe.fsf@nikula.org by
>> treating errors while retrieving the original message's References
>> header as if the original message didn't have a References header.  It
>> also adds a comment suggested by Jani on IRC.  The diff from v2 is
>> below.
>
> The series LGTM, and I in particular welcome the diffstat in patch 3!

The series LGTM, and I in particular welcome the diffstat in patch 3!

> BR,
> Jani.

Tomi

>
>
>>
>> diff --git a/notmuch-reply.c b/notmuch-reply.c
>> index bfd0f51..ac46d02 100644
>> --- a/notmuch-reply.c
>> +++ b/notmuch-reply.c
>> @@ -31,6 +31,7 @@ show_reply_headers (GMimeMessage *message)
>>      stream_stdout = g_mime_stream_file_new (stdout);
>>      if (stream_stdout) {
>>  	g_mime_stream_file_set_owner (GMIME_STREAM_FILE (stream_stdout), FALSE);
>> +	/* Output RFC 2822 formatted (and RFC 2047 encoded) headers. */
>>  	g_mime_object_write_to_stream (GMIME_OBJECT(message), stream_stdout);
>>  	g_object_unref(stream_stdout);
>>      }
>> @@ -526,14 +527,15 @@ create_reply_message(void *ctx,
>>  			      "In-Reply-To", in_reply_to);
>>  
>>      orig_references = notmuch_message_get_header (message, "references");
>> -    if (orig_references) {
>> -	references = talloc_asprintf (ctx, "%s%s%s",
>> -				      *orig_references ? orig_references : "",
>> -				      *orig_references ? " " : "",
>> -				      in_reply_to);
>> -	g_mime_object_set_header (GMIME_OBJECT (reply),
>> -				  "References", references);
>> -    }
>> +    if (!orig_references)
>> +	/* Treat errors like missing References headers. */
>> +	orig_references = "";
>> +    references = talloc_asprintf (ctx, "%s%s%s",
>> +				  *orig_references ? orig_references : "",
>> +				  *orig_references ? " " : "",
>> +				  in_reply_to);
>> +    g_mime_object_set_header (GMIME_OBJECT (reply),
>> +			      "References", references);
>>  
>>      return reply;
>>  }
>>
>>
>> _______________________________________________
>> 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 v3 0/3] Clean up reply's encoding story

[David Bremner]

Austin Clements <amdragon@MIT.EDU> writes:

> This is v3 of id:1376587658-19202-1-git-send-email-amdragon@mit.edu.
> This addresses Jani's comment in id:87k3jl3ehe.fsf@nikula.org by
> treating errors while retrieving the original message's References
> header as if the original message didn't have a References header.  It
> also adds a comment suggested by Jani on IRC.  The diff from v2 is
> below.

pushed, 

d

Re: [PATCH v3 0/3] Clean up reply's encoding story

[Jani Nikula]

On Sat, 17 Aug 2013, David Bremner <david@tethera.net> wrote:
> Austin Clements <amdragon@MIT.EDU> writes:
>
>> This is v3 of id:1376587658-19202-1-git-send-email-amdragon@mit.edu.
>> This addresses Jani's comment in id:87k3jl3ehe.fsf@nikula.org by
>> treating errors while retrieving the original message's References
>> header as if the original message didn't have a References header.  It
>> also adds a comment suggested by Jani on IRC.  The diff from v2 is
>> below.
>
> pushed, 

'make test' now gives me:

 FAIL   Reply with RFC 2047-encoded headers
	--- reply.12.expected	2013-08-17 11:57:31.047782938 +0000
	+++ reply.12.output	2013-08-17 11:57:31.047782938 +0000
	@@ -1,5 +1,5 @@
	 From: Notmuch Test Suite <test_suite@notmuchmail.org>
	-Subject: Re: =?iso-8859-1?b?4N/n?=
	+Subject: Re: =?iso-8859-1?B?4N/n?=
	 To: =?UTF-8?b?4piD?= <snowman@example.com>
	 In-Reply-To: <msg-012@notmuch-test-suite>
	 References: <msg-012@notmuch-test-suite>

I'm using libgmime 2.6.16-1 from Debian testing.


BR,
Jani.

Re: [PATCH v3 0/3] Clean up reply's encoding story

[David Bremner]

Jani Nikula <jani@nikula.org> writes:
>
> 'make test' now gives me:
>
>  FAIL   Reply with RFC 2047-encoded headers
> 	--- reply.12.expected	2013-08-17 11:57:31.047782938 +0000
> 	+++ reply.12.output	2013-08-17 11:57:31.047782938 +0000
> 	@@ -1,5 +1,5 @@
> 	 From: Notmuch Test Suite <test_suite@notmuchmail.org>
> 	-Subject: Re: =?iso-8859-1?b?4N/n?=
> 	+Subject: Re: =?iso-8859-1?B?4N/n?=
> 	 To: =?UTF-8?b?4piD?= <snowman@example.com>
> 	 In-Reply-To: <msg-012@notmuch-test-suite>
> 	 References: <msg-012@notmuch-test-suite>
>
> I'm using libgmime 2.6.16-1 from Debian testing.
>

Oh hmm. now that I upgrade to 2.6.16-1 I see the same thing.  It seems
like something changed in gmime. I think the RFC specifies that ?b is
equivalent to ?B as an encoding marker, so maybe we should normalize
before comparing?

d