[PATCH v2 3/3] show: Introduce mime_node formatter callback
Dmitry Kurochkin
dmitry.kurochkin at gmail.com
Mon Jan 23 14:37:30 PST 2012
On Sun, 22 Jan 2012 21:31:13 -0500, Austin Clements <amdragon at MIT.EDU> wrote:
> This callback is the gateway to the new mime_node_t-based formatters.
> This maintains backwards compatibility so the formatters can be
> transitioned one at a time. Once all formatters are converted, the
> formatter structure can be reduced to only message_set_{start,sep,end}
> and part, most of show_message can be deleted, and all of
> show-message.c can be deleted.
Few comments below.
> ---
> notmuch-client.h | 6 ++++++
> notmuch-reply.c | 2 +-
> notmuch-show.c | 23 +++++++++++++++++++----
> 3 files changed, 26 insertions(+), 5 deletions(-)
>
> diff --git a/notmuch-client.h b/notmuch-client.h
> index abfe5d3..59606b4 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -62,8 +62,14 @@
> #define STRINGIFY(s) STRINGIFY_(s)
> #define STRINGIFY_(s) #s
>
> +struct mime_node;
> +struct notmuch_show_params;
> +
> typedef struct notmuch_show_format {
> const char *message_set_start;
> + void (*part) (const void *ctx,
> + struct mime_node *node, int indent,
> + struct notmuch_show_params *params);
Can we make the params parameter const?
> const char *message_start;
> void (*message) (const void *ctx,
> notmuch_message_t *message,
> diff --git a/notmuch-reply.c b/notmuch-reply.c
> index bf67960..f55b1d2 100644
> --- a/notmuch-reply.c
> +++ b/notmuch-reply.c
> @@ -31,7 +31,7 @@ static void
> reply_part_content (GMimeObject *part);
>
> static const notmuch_show_format_t format_reply = {
> - "",
> + "", NULL,
> "", NULL,
> "", NULL, reply_headers_message_part, ">\n",
> "",
> diff --git a/notmuch-show.c b/notmuch-show.c
> index 682aa71..8185b02 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -42,7 +42,7 @@ static void
> format_part_end_text (GMimeObject *part);
>
> static const notmuch_show_format_t format_text = {
> - "",
> + "", NULL,
> "\fmessage{ ", format_message_text,
> "\fheader{\n", format_headers_text, format_headers_message_part_text, "\fheader}\n",
> "\fbody{\n",
> @@ -89,7 +89,7 @@ static void
> format_part_end_json (GMimeObject *part);
>
> static const notmuch_show_format_t format_json = {
> - "[",
> + "[", NULL,
> "{", format_message_json,
> "\"headers\": {", format_headers_json, format_headers_message_part_json, "}",
> ", \"body\": [",
> @@ -110,7 +110,7 @@ format_message_mbox (const void *ctx,
> unused (int indent));
>
> static const notmuch_show_format_t format_mbox = {
> - "",
> + "", NULL,
> "", format_message_mbox,
> "", NULL, NULL, "",
> "",
> @@ -129,7 +129,7 @@ static void
> format_part_content_raw (GMimeObject *part);
>
> static const notmuch_show_format_t format_raw = {
> - "",
> + "", NULL,
> "", NULL,
> "", NULL, format_headers_message_part_text, "\n",
> "",
> @@ -850,6 +850,21 @@ show_message (void *ctx,
> int indent,
> notmuch_show_params_t *params)
> {
> + if (format->part) {
> + void *local = talloc_new (ctx);
> + mime_node_t *root, *part;
> +
> + if (mime_node_open (local, message, params->cryptoctx, params->decrypt,
> + &root) != NOTMUCH_STATUS_SUCCESS)
> + goto DONE;
> + part = mime_node_seek_dfs (root, params->part < 0 ? 0 : params->part);
This should be done as a separate patch, but it looks like zero and
negative params->part is handled in the same way so we can change it to
always be non-negative.
> + if (part)
> + format->part (local, part, indent, params);
> + DONE:
> + talloc_free (local);
> + return;
Please move part assignment inside the if and remove the goto:
if (mime_node_open() == success && (part = seek()))
format->part();
Regards,
Dmitry
> + }
> +
> if (params->part <= 0) {
> fputs (format->message_start, stdout);
> if (format->message)
> --
> 1.7.7.3
>
> _______________________________________________
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
More information about the notmuch
mailing list