Re: [PATCH v3 1/2] cli/show: add --format=pretty

[David Bremner <david@tethera.net>]

Hannu Hartikainen <hannu@hrtk.in> writes:


> +static notmuch_status_t
> +format_part_pretty (const void *ctx, sprinter_t *sp, mime_node_t *node,
> +		    int indent, const notmuch_show_params_t *params)
> +{
> +    /* The disposition and content-type metadata are associated with
> +     * the envelope for message parts */
> +    GMimeObject *meta = node->envelope_part ? (
> +	GMIME_OBJECT (node->envelope_part) ) : node->part;
> +    GMimeContentType *content_type = g_mime_object_get_content_type (meta);
> +    GMimeStream *stream = params->out_stream;
> +    int i;
> +
> +    if (GMIME_IS_MESSAGE (node->part)) {
> +	GMimeMessage *message = GMIME_MESSAGE (node->part);
> +	char *recipients_string;
> +	char *date_string;
> +
> +	g_mime_stream_printf (stream, "Subject: %s\n", g_mime_message_get_subject (message));
> +	g_mime_stream_printf (stream, "From: %s\n", g_mime_message_get_from_string (message));
> +	recipients_string = g_mime_message_get_address_string (message, GMIME_ADDRESS_TYPE_TO);
> +	if (recipients_string)
> +	    g_mime_stream_printf (stream, "To: %s\n", recipients_string);
> +	g_free (recipients_string);
> +	recipients_string = g_mime_message_get_address_string (message, GMIME_ADDRESS_TYPE_CC);
> +	if (recipients_string)
> +	    g_mime_stream_printf (stream, "Cc: %s\n", recipients_string);
> +	g_free (recipients_string);
> +	date_string = g_mime_message_get_date_string (node, message);
> +	g_mime_stream_printf (stream, "Date: %s\n", date_string);
> +	g_mime_stream_printf (stream, "Message-ID: <%s>\n\n", g_mime_message_get_message_id (
> +				  message));

I don't know what g_mime_message_get_message_id will return if there is
no message-id, but that case can and does arise. Notmuch constructs a
message-id by sha1 hashing the message. That message-id is in the
database, so I think you should probably print out the message-id from
the database.

> +static const notmuch_show_format_t format_pretty = {
> +    .new_sprinter = sprinter_text_create,
> +    .part = format_part_pretty,
> +};

I think this deserves a comment to explain briefly why it works.

> diff --git a/test/T520-show.sh b/test/T520-show.sh
> index 6f42ca12..c9bcf81d 100755
> --- a/test/T520-show.sh
> +++ b/test/T520-show.sh
> @@ -27,4 +27,36 @@ notmuch show --entire-thread=true --sort=newest-first $QUERY > EXPECTED
>  notmuch show --entire-thread=true --sort=oldest-first $QUERY > OUTPUT
>  test_expect_equal_file EXPECTED OUTPUT
>  
> +test_begin_subtest "--format=pretty"
> +output=$(notmuch show --format=pretty id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com 2>&1 && echo OK)
> +test_expect_equal "$output" "Subject: [notmuch] preliminary FreeBSD support
> +From: Alex Botero-Lowry <alex.boterolowry@gmail.com>
> +To: notmuch@notmuchmail.org
> +Date: Tue, 17 Nov 2009 11:36:14 -0800
> +Message-ID: <cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com>
> +

I would prefer the
cat <<EOF > EXPECTED
...
EOF

style here rather then the multiline strings, finishing with
"test_expect_equal_file EXPECTED OUTPUT".