Re: [PATCH 2/2] cli/show: add content-disposition to structured output message parts

[Mark Walters <markwalters1009@gmail.com>]

On Sat, 28 Jan 2017, Jani Nikula <jani@nikula.org> wrote:
> Help the clients decide how to display parts.

This series looks good to me. I would say +1, but I am not sure my
knowledge of C is really up to that.

There are a bunch of tests to update; I will send a patch to do that in
shortly.

> I'm not sure if this should bump the version in schemata, because the
> comment in notmuch-client.h says new map fields can be added without
> increasing NOTMUCH_FORMAT_CUR. Yet the schemata version history says
> thread_summary.query was added in v2... which looks like a change that
> shouldn't need a version bump. Confused.

I think the reason for the bump before was that the client needed to
know it had got the new format, so it could either bail out, or at least
work around it, if it got the old format.

In this case though, I think any sane program has to treat
content-disposition nil as being "do whatever the default is" (since I
don't think all parts have a content-disposition).

Thus, I don't think we need a schemata bump, as the client can just use
the extra information if present, and otherwise do what it normally would.

Best wishes

Mark

>
> The problem could be avoided by conflating both this change and
> id:20170110201929.21875-1-jani@nikula.org into v3... ;)
> ---
>  devel/schemata | 1 +
>  notmuch-show.c | 6 ++++++
>  2 files changed, 7 insertions(+)
>
> diff --git a/devel/schemata b/devel/schemata
> index 41dc4a60fff3..c94459eb783a 100644
> --- a/devel/schemata
> +++ b/devel/schemata
> @@ -76,6 +76,7 @@ part = {
>      sigstatus?:     sigstatus,
>  
>      content-type:   string,
> +    content-disposition?:       string,
>      content-id?:    string,
>      # if content-type starts with "multipart/":
>      content:        [part*],
> diff --git a/notmuch-show.c b/notmuch-show.c
> index 8b38fe6db136..8e69b3465886 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -582,6 +582,7 @@ format_part_sprinter (const void *ctx, sprinter_t *sp, mime_node_t *node,
>      GMimeObject *meta = node->envelope_part ?
>  	GMIME_OBJECT (node->envelope_part) : node->part;
>      GMimeContentType *content_type = g_mime_object_get_content_type (meta);
> +    const char *disposition = _get_disposition (meta);
>      const char *cid = g_mime_object_get_content_id (meta);
>      const char *filename = GMIME_IS_PART (node->part) ?
>  	g_mime_part_get_filename (GMIME_PART (node->part)) : NULL;
> @@ -611,6 +612,11 @@ format_part_sprinter (const void *ctx, sprinter_t *sp, mime_node_t *node,
>      sp->map_key (sp, "content-type");
>      sp->string (sp, g_mime_content_type_to_string (content_type));
>  
> +    if (disposition) {
> +	sp->map_key (sp, "content-disposition");
> +	sp->string (sp, disposition);
> +    }
> +
>      if (cid) {
>  	sp->map_key (sp, "content-id");
>  	sp->string (sp, cid);
> -- 
> 2.11.0