From: Tomi Ollila <tomi.ollila@iki.fi>
To: John Lenz <lenz@math.uic.edu>, notmuch@notmuchmail.org
Subject: Re: cli: add --include-html option to notmuch show
In-Reply-To: <notmuch-web-1372724382.450184839@www.wuzzeb.org>
References: <notmuch-web-1372724382.450184839@www.wuzzeb.org>
User-Agent: Notmuch/0.15.2+225~g6062adf (http://notmuchmail.org) Emacs/24.3.1
	(x86_64-unknown-linux-gnu)
Date: Sun, 21 Jul 2013 23:23:58 +0300
Message-ID: <m27ggj1x29.fsf@guru.guru-group.fi>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: list

On Tue, Jul 02 2013, John Lenz <lenz@math.uic.edu> wrote:

> For my client, the largest bottleneck for displaying large threads is
> exporting each html part individually since by default notmuch will not
> show the json parts.  For large threads there can be quite a few parts and
> each must be exported and decoded one by one.  Also, I then have to deal
> with all the crazy charsets which I can do through a library but is a
> pain.

This looks like a useful option. I just wonder what effect does different
charsets do to the output (is text/html content output verbatim (with just
json/sexp escaping of '"' -characters). 

If you added test(s) showing what happens with different charsets
(like one message having 3 text/html parts, one us-ascii, one iso-8859-1
and one utf-8) that would make things clearer and (also) protect us from 
regressions.

Tomi


> Therefore, this patch adds an --include-html option that causes the
> text/html parts to be included as part of the output of show.
>
>
> diff man/man1/notmuch-show.1
> --- a/man/man1/notmuch-show.1	Sun Jun 23 14:24:02 2013 +1000
> +++ b/man/man1/notmuch-show.1	Mon Jul 01 18:51:13 2013 -0500
> @@ -207,6 +207,20 @@
>  output is much faster and substantially smaller.
>  .RE
>  
> +.RS 4
> +.TP 4
> +.B \-\-include-html
> +
> +Include "text/html" parts as part of the output (currently only supported with
> +--format=json and --format=sexp).
> +By default, unless
> +.B --part=N
> +is used to select a specific part or
> +.B --include-html
> +is used to include all "text/html" parts, no part with content type "text/html"
> +is included in the output.
> +.RE
> +
>  A common use of
>  .B notmuch show
>  is to display a single thread of email messages. For this, use a
> diff notmuch-client.h
> --- a/notmuch-client.h	Sun Jun 23 14:24:02 2013 +1000
> +++ b/notmuch-client.h	Mon Jul 01 18:51:13 2013 -0500
> @@ -89,6 +89,7 @@
>      notmuch_bool_t raw;
>      int part;
>      notmuch_crypto_t crypto;
> +    notmuch_bool_t include_html;
>  } notmuch_show_params_t;
>  
>  /* There's no point in continuing when we've detected that we've done
> @@ -220,7 +221,8 @@
>  
>  void
>  format_part_sprinter (const void *ctx, struct sprinter *sp, mime_node_t *node,
> -		      notmuch_bool_t first, notmuch_bool_t output_body);
> +		      notmuch_bool_t first, notmuch_bool_t output_body,
> +		      notmuch_bool_t include_html);
>  
>  void
>  format_headers_sprinter (struct sprinter *sp, GMimeMessage *message,
> diff notmuch-reply.c
> --- a/notmuch-reply.c	Sun Jun 23 14:24:02 2013 +1000
> +++ b/notmuch-reply.c	Mon Jul 01 18:51:13 2013 -0500
> @@ -624,7 +624,7 @@
>  
>      /* Start the original */
>      sp->map_key (sp, "original");
> -    format_part_sprinter (ctx, sp, node, TRUE, TRUE);
> +    format_part_sprinter (ctx, sp, node, TRUE, TRUE, FALSE);
>  
>      /* End */
>      sp->end (sp);
> diff notmuch-show.c
> --- a/notmuch-show.c	Sun Jun 23 14:24:02 2013 +1000
> +++ b/notmuch-show.c	Mon Jul 01 18:51:13 2013 -0500
> @@ -630,7 +630,8 @@
>  
>  void
>  format_part_sprinter (const void *ctx, sprinter_t *sp, mime_node_t *node,
> -		      notmuch_bool_t first, notmuch_bool_t output_body)
> +		      notmuch_bool_t first, notmuch_bool_t output_body,
> +		      notmuch_bool_t include_html)
>  {
>      /* Any changes to the JSON or S-Expression format should be
>       * reflected in the file devel/schemata. */
> @@ -645,7 +646,7 @@
>  	if (output_body) {
>  	    sp->map_key (sp, "body");
>  	    sp->begin_list (sp);
> -	    format_part_sprinter (ctx, sp, mime_node_child (node, 0), first, TRUE);
> +	    format_part_sprinter (ctx, sp, mime_node_child (node, 0), first, TRUE, include_html);
>  	    sp->end (sp);
>  	}
>  	sp->end (sp);
> @@ -700,14 +701,15 @@
>  	/* For non-HTML text parts, we include the content in the
>  	 * JSON. Since JSON must be Unicode, we handle charset
>  	 * decoding here and do not report a charset to the caller.
> -	 * For text/html parts, we do not include the content. If a
> -	 * caller is interested in text/html parts, it should retrieve
> -	 * them separately and they will not be decoded. Since this
> -	 * makes charset decoding the responsibility on the caller, we
> +	 * For text/html parts, we do not include the content unless
> +	 * the --include-html option has been passed. If a html part
> +	 * is not included, it can be requested directly. This makes
> +	 * charset decoding the responsibility on the caller so we
>  	 * report the charset for text/html parts.
>  	 */
>  	if (g_mime_content_type_is_type (content_type, "text", "*") &&
> -	    ! g_mime_content_type_is_type (content_type, "text", "html"))
> +	    (include_html ||
> +	     ! g_mime_content_type_is_type (content_type, "text", "html")))
>  	{
>  	    GMimeStream *stream_memory = g_mime_stream_mem_new ();
>  	    GByteArray *part_content;
> @@ -737,7 +739,7 @@
>      }
>  
>      for (i = 0; i < node->nchildren; i++)
> -	format_part_sprinter (ctx, sp, mime_node_child (node, i), i == 0, TRUE);
> +	format_part_sprinter (ctx, sp, mime_node_child (node, i), i == 0, TRUE, include_html);
>  
>      /* Close content structures */
>      for (i = 0; i < nclose; i++)
> @@ -751,7 +753,7 @@
>  			    mime_node_t *node, unused (int indent),
>  			    const notmuch_show_params_t *params)
>  {
> -    format_part_sprinter (ctx, sp, node, TRUE, params->output_body);
> +    format_part_sprinter (ctx, sp, node, TRUE, params->output_body, params->include_html);
>  
>      return NOTMUCH_STATUS_SUCCESS;
>  }
> @@ -1077,7 +1079,8 @@
>  	.crypto = {
>  	    .verify = FALSE,
>  	    .decrypt = FALSE
> -	}
> +	},
> +	.include_html = FALSE
>      };
>      int format_sel = NOTMUCH_FORMAT_NOT_SPECIFIED;
>      int exclude = EXCLUDE_TRUE;
> @@ -1105,6 +1108,7 @@
>  	{ NOTMUCH_OPT_BOOLEAN, &params.crypto.decrypt, "decrypt", 'd', 0 },
>  	{ NOTMUCH_OPT_BOOLEAN, &params.crypto.verify, "verify", 'v', 0 },
>  	{ NOTMUCH_OPT_BOOLEAN, &params.output_body, "body", 'b', 0 },
> +	{ NOTMUCH_OPT_BOOLEAN, &params.include_html, "include-html", 0, 0 },
>  	{ 0, 0, 0, 0, 0 }
>      };
>  
> @@ -1176,6 +1180,11 @@
>  	}
>      }
>  
> +    if (params.include_html &&
> +        (format_sel != NOTMUCH_FORMAT_JSON && format_sel != NOTMUCH_FORMAT_SEXP)) {
> +	fprintf (stderr, "Warning: --include-html only implemented for format=json and format=sexp\n");
> +    }
> +
>      if (entire_thread == ENTIRE_THREAD_TRUE)
>  	params.entire_thread = TRUE;
>      else
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch