From: Jani Nikula <jani@nikula.org>
To: Austin Clements <amdragon@MIT.EDU>, notmuch@notmuchmail.org
Subject: Re: [PATCH v2 2/3] reply: Remove extraneous space from generated
	References
In-Reply-To: <1376587658-19202-3-git-send-email-amdragon@mit.edu>
References: <1376587658-19202-1-git-send-email-amdragon@mit.edu>
	<1376587658-19202-3-git-send-email-amdragon@mit.edu>
User-Agent: Notmuch/0.15.2+227~g40b2846 (http://notmuchmail.org) Emacs/24.3.1
	(x86_64-pc-linux-gnu)
Date: Fri, 16 Aug 2013 17:19:25 +0300
Message-ID: <87k3jl3ehe.fsf@nikula.org>
MIME-Version: 1.0
Content-Type: text/plain
Cc: tomi.ollila@iki.fi
Precedence: list

On Thu, 15 Aug 2013, Austin Clements <amdragon@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?

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@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
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch