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

[Austin Clements]

Quoth Jani Nikula on Aug 16 at  5:19 pm:
> On Thu, 15 Aug 2013, Austin Clements <amdragon at MIT.EDU> wrote:
> > 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 |   14 ++++++++------
> >  test/multipart  |    2 +-
> >  test/reply      |    2 +-
> >  3 files changed, 10 insertions(+), 8 deletions(-)
> >
> > diff --git a/notmuch-reply.c b/notmuch-reply.c
> > index 3b2b58d..0f3b9cd 100644
> > --- a/notmuch-reply.c
> > +++ b/notmuch-reply.c
> > @@ -537,12 +537,14 @@ create_reply_message(void *ctx,
> >  			      "In-Reply-To", in_reply_to);
> >  
> >      orig_references = notmuch_message_get_header (message, "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) {
> > +	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 turned out to be NULL, wouldn't this then fail to add
> a References: header with in_reply_to in it?

That's true.  It's not clear what the best course of action is if
orig_references is NULL, but you're right that failing to produce a
References header altogether is probably not it.  I'll make it treat
errors and missing/empty References headers identically.

> Jani.
> 
> 
> >  
> >      return reply;
> >  }
> > 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 at notmuchmail.org>",
> >   "To": "Carl Worth <cworth at cworth.org>, cworth at cworth.org",
> >   "In-reply-to": "<87liy5ap00.fsf at yoom.home.cworth.org>",
> > - "References": " <87liy5ap00.fsf at yoom.home.cworth.org>"},
> > + "References": "<87liy5ap00.fsf at 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 at notmuchmail.org>",
> >          "In-reply-to": "<'${gen_msg_id}'>",
> > -        "References": " <'${gen_msg_id}'>",
> > +        "References": "<'${gen_msg_id}'>",
> >          "Subject": "Re: \u00e0\u00df\u00e7",
> >          "To": "\u2603 <snowman at example.com>"
> >      }