--- /dev/null
+Return-Path: <dmitry.kurochkin@gmail.com>\r
+X-Original-To: notmuch@notmuchmail.org\r
+Delivered-To: notmuch@notmuchmail.org\r
+Received: from localhost (localhost [127.0.0.1])\r
+ by olra.theworths.org (Postfix) with ESMTP id 0A18C429E54\r
+ for <notmuch@notmuchmail.org>; Mon, 23 Jan 2012 14:38:38 -0800 (PST)\r
+X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
+X-Spam-Flag: NO\r
+X-Spam-Score: -0.799\r
+X-Spam-Level: \r
+X-Spam-Status: No, score=-0.799 tagged_above=-999 required=5\r
+ tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1,\r
+ FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\r
+Received: from olra.theworths.org ([127.0.0.1])\r
+ by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
+ with ESMTP id 2zvMBLeQFHfm for <notmuch@notmuchmail.org>;\r
+ Mon, 23 Jan 2012 14:38:37 -0800 (PST)\r
+Received: from mail-bk0-f53.google.com (mail-bk0-f53.google.com\r
+ [209.85.214.53]) (using TLSv1 with cipher RC4-SHA (128/128 bits))\r
+ (No client certificate requested)\r
+ by olra.theworths.org (Postfix) with ESMTPS id 202F3429E21\r
+ for <notmuch@notmuchmail.org>; Mon, 23 Jan 2012 14:38:36 -0800 (PST)\r
+Received: by bkbzt19 with SMTP id zt19so1782240bkb.26\r
+ for <notmuch@notmuchmail.org>; Mon, 23 Jan 2012 14:38:35 -0800 (PST)\r
+DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma;\r
+ h=from:to:subject:in-reply-to:references:user-agent:date:message-id\r
+ :mime-version:content-type;\r
+ bh=zVi4hNELe+v81INSGDuYzniUcsTy8U9IJATbr4MermE=;\r
+ b=wm+5ljffm6M1BZAynbdo7lUZ7Q1CmfUkMIAZLLuaziEHMSb7gcQhzNShG92UDV527Y\r
+ yXDNbQgbGf1zPfCUkyNpupF8uq9ZHONl8BGOtYUVY/9jd92+sFH2j6LarJgoLOcvWDGw\r
+ Slc0w4M7bpy7HfPsDbd+LaZDWrKgyP1NmbLs4=\r
+Received: by 10.204.154.211 with SMTP id p19mr4072050bkw.130.1327358315691;\r
+ Mon, 23 Jan 2012 14:38:35 -0800 (PST)\r
+Received: from localhost ([91.144.186.21])\r
+ by mx.google.com with ESMTPS id ga13sm31546122bkc.5.2012.01.23.14.38.34\r
+ (version=TLSv1/SSLv3 cipher=OTHER);\r
+ Mon, 23 Jan 2012 14:38:34 -0800 (PST)\r
+From: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>\r
+To: Austin Clements <amdragon@MIT.EDU>, notmuch@notmuchmail.org\r
+Subject: Re: [PATCH v2 3/3] show: Introduce mime_node formatter callback\r
+In-Reply-To: <1327285873-4713-4-git-send-email-amdragon@mit.edu>\r
+References: <1326918507-28033-1-git-send-email-amdragon@mit.edu>\r
+ <1327285873-4713-1-git-send-email-amdragon@mit.edu>\r
+ <1327285873-4713-4-git-send-email-amdragon@mit.edu>\r
+User-Agent: Notmuch/0.11+100~gd650abf (http://notmuchmail.org) Emacs/23.3.1\r
+ (x86_64-pc-linux-gnu)\r
+Date: Tue, 24 Jan 2012 02:37:30 +0400\r
+Message-ID: <8739b6niz9.fsf@gmail.com>\r
+MIME-Version: 1.0\r
+Content-Type: text/plain; charset=us-ascii\r
+X-BeenThere: notmuch@notmuchmail.org\r
+X-Mailman-Version: 2.1.13\r
+Precedence: list\r
+List-Id: "Use and development of the notmuch mail system."\r
+ <notmuch.notmuchmail.org>\r
+List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
+ <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
+List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
+List-Post: <mailto:notmuch@notmuchmail.org>\r
+List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
+List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
+ <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
+X-List-Received-Date: Mon, 23 Jan 2012 22:38:38 -0000\r
+\r
+On Sun, 22 Jan 2012 21:31:13 -0500, Austin Clements <amdragon@MIT.EDU> wrote:\r
+> This callback is the gateway to the new mime_node_t-based formatters.\r
+> This maintains backwards compatibility so the formatters can be\r
+> transitioned one at a time. Once all formatters are converted, the\r
+> formatter structure can be reduced to only message_set_{start,sep,end}\r
+> and part, most of show_message can be deleted, and all of\r
+> show-message.c can be deleted.\r
+\r
+Few comments below.\r
+\r
+> ---\r
+> notmuch-client.h | 6 ++++++\r
+> notmuch-reply.c | 2 +-\r
+> notmuch-show.c | 23 +++++++++++++++++++----\r
+> 3 files changed, 26 insertions(+), 5 deletions(-)\r
+> \r
+> diff --git a/notmuch-client.h b/notmuch-client.h\r
+> index abfe5d3..59606b4 100644\r
+> --- a/notmuch-client.h\r
+> +++ b/notmuch-client.h\r
+> @@ -62,8 +62,14 @@\r
+> #define STRINGIFY(s) STRINGIFY_(s)\r
+> #define STRINGIFY_(s) #s\r
+> \r
+> +struct mime_node;\r
+> +struct notmuch_show_params;\r
+> +\r
+> typedef struct notmuch_show_format {\r
+> const char *message_set_start;\r
+> + void (*part) (const void *ctx,\r
+> + struct mime_node *node, int indent,\r
+> + struct notmuch_show_params *params);\r
+\r
+Can we make the params parameter const?\r
+\r
+> const char *message_start;\r
+> void (*message) (const void *ctx,\r
+> notmuch_message_t *message,\r
+> diff --git a/notmuch-reply.c b/notmuch-reply.c\r
+> index bf67960..f55b1d2 100644\r
+> --- a/notmuch-reply.c\r
+> +++ b/notmuch-reply.c\r
+> @@ -31,7 +31,7 @@ static void\r
+> reply_part_content (GMimeObject *part);\r
+> \r
+> static const notmuch_show_format_t format_reply = {\r
+> - "",\r
+> + "", NULL,\r
+> "", NULL,\r
+> "", NULL, reply_headers_message_part, ">\n",\r
+> "",\r
+> diff --git a/notmuch-show.c b/notmuch-show.c\r
+> index 682aa71..8185b02 100644\r
+> --- a/notmuch-show.c\r
+> +++ b/notmuch-show.c\r
+> @@ -42,7 +42,7 @@ static void\r
+> format_part_end_text (GMimeObject *part);\r
+> \r
+> static const notmuch_show_format_t format_text = {\r
+> - "",\r
+> + "", NULL,\r
+> "\fmessage{ ", format_message_text,\r
+> "\fheader{\n", format_headers_text, format_headers_message_part_text, "\fheader}\n",\r
+> "\fbody{\n",\r
+> @@ -89,7 +89,7 @@ static void\r
+> format_part_end_json (GMimeObject *part);\r
+> \r
+> static const notmuch_show_format_t format_json = {\r
+> - "[",\r
+> + "[", NULL,\r
+> "{", format_message_json,\r
+> "\"headers\": {", format_headers_json, format_headers_message_part_json, "}",\r
+> ", \"body\": [",\r
+> @@ -110,7 +110,7 @@ format_message_mbox (const void *ctx,\r
+> unused (int indent));\r
+> \r
+> static const notmuch_show_format_t format_mbox = {\r
+> - "",\r
+> + "", NULL,\r
+> "", format_message_mbox,\r
+> "", NULL, NULL, "",\r
+> "",\r
+> @@ -129,7 +129,7 @@ static void\r
+> format_part_content_raw (GMimeObject *part);\r
+> \r
+> static const notmuch_show_format_t format_raw = {\r
+> - "",\r
+> + "", NULL,\r
+> "", NULL,\r
+> "", NULL, format_headers_message_part_text, "\n",\r
+> "",\r
+> @@ -850,6 +850,21 @@ show_message (void *ctx,\r
+> int indent,\r
+> notmuch_show_params_t *params)\r
+> {\r
+> + if (format->part) {\r
+> + void *local = talloc_new (ctx);\r
+> + mime_node_t *root, *part;\r
+> +\r
+> + if (mime_node_open (local, message, params->cryptoctx, params->decrypt,\r
+> + &root) != NOTMUCH_STATUS_SUCCESS)\r
+> + goto DONE;\r
+> + part = mime_node_seek_dfs (root, params->part < 0 ? 0 : params->part);\r
+\r
+This should be done as a separate patch, but it looks like zero and\r
+negative params->part is handled in the same way so we can change it to\r
+always be non-negative.\r
+\r
+> + if (part)\r
+> + format->part (local, part, indent, params);\r
+> + DONE:\r
+> + talloc_free (local);\r
+> + return;\r
+\r
+Please move part assignment inside the if and remove the goto:\r
+\r
+ if (mime_node_open() == success && (part = seek()))\r
+ format->part();\r
+\r
+Regards,\r
+ Dmitry\r
+\r
+> + }\r
+> +\r
+> if (params->part <= 0) {\r
+> fputs (format->message_start, stdout);\r
+> if (format->message)\r
+> -- \r
+> 1.7.7.3\r
+> \r
+> _______________________________________________\r
+> notmuch mailing list\r
+> notmuch@notmuchmail.org\r
+> http://notmuchmail.org/mailman/listinfo/notmuch\r