[PATCH 0/6] Finish show rewrite

[PATCH 0/6] Finish show rewrite

[Austin Clements]

The long-awaited and oft-belated conclusion of the show rewrite.  All
of the formatters have been converted to the new style, so this series
just rips out unused code and does a little cleanup.

[PATCH 1/6] Sync schemata with current code structure

[Austin Clements]

The schema itself hasn't changed, but many of the references to
functions in notmuch-show.c were out of date.
---
 devel/schemata |   21 +++++++--------------
 1 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/devel/schemata b/devel/schemata
index 728a46f..977cea7 100644
--- a/devel/schemata
+++ b/devel/schemata
@@ -36,7 +36,7 @@ thread_node = [
     [thread_node*]            # children of message
 ]
 
-# A message (show_message)
+# A message (format_part_json)
 message = {
     # (format_message_json)
     id:             messageid,
@@ -50,18 +50,13 @@ message = {
     body:           [part]
 }
 
-# A MIME part (show_message_body)
+# A MIME part (format_part_json)
 part = {
-    # format_part_start_json
     id:             int|string, # part id (currently DFS part number)
 
-    # format_part_encstatus_json
     encstatus?:     encstatus,
-
-    # format_part_sigstatus_json
     sigstatus?:     sigstatus,
 
-    # format_part_content_json
     content-type:   string,
     content-id?:    string,
     # if content-type starts with "multipart/":
@@ -77,9 +72,7 @@ part = {
     content?:       string
 }
 
-# The headers of a message (format_headers_json with raw headers
-# and reply = FALSE) or a part (format_headers_message_part_json
-# with pretty-printed headers)
+# The headers of a message or part (format_headers_json with reply = FALSE)
 headers = {
     Subject:        string,
     From:           string,
@@ -89,14 +82,14 @@ headers = {
     Date:           string
 }
 
-# Encryption status (format_part_encstatus_json)
+# Encryption status (format_part_json)
 encstatus = [{status: "good"|"bad"}]
 
 # Signature status (format_part_sigstatus_json)
 sigstatus = [signature*]
 
 signature = {
-    # signature_status_to_string
+    # (signature_status_to_string)
     status:         "none"|"good"|"bad"|"error"|"unknown",
     # if status is "good":
     fingerprint?:   string,
@@ -142,14 +135,14 @@ notmuch reply schema
 --------------------
 
 reply = {
-    # The headers of the constructed reply (format_headers_json with
-    # raw headers and reply = TRUE)
+    # The headers of the constructed reply
     reply-headers: reply_headers,
 
     # As in the show format (format_part_json)
     original: message
 }
 
+# Reply headers (format_headers_json with reply = TRUE)
 reply_headers = {
     Subject:        string,
     From:           string,
-- 
1.7.9.1

[PATCH 2/6] show: Remove support for old-style formatters in show_message

[Austin Clements]

show_message used to have a compatibility path for old-style
formatters.  This removes that.
---
 notmuch-show.c |   52 +++++++++++++---------------------------------------
 1 files changed, 13 insertions(+), 39 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index 0bf5e21..5a71115 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -815,46 +815,20 @@ show_message (void *ctx,
 	      int indent,
 	      notmuch_show_params_t *params)
 {
-    if (format->part) {
-	void *local = talloc_new (ctx);
-	mime_node_t *root, *part;
-	notmuch_status_t status;
-
-	status = mime_node_open (local, message, params->cryptoctx,
-				 params->decrypt, &root);
-	if (status)
-	    goto DONE;
-	part = mime_node_seek_dfs (root, (params->part < 0 ? 0 : params->part));
-	if (part)
-	    status = format->part (local, part, indent, params);
-      DONE:
-	talloc_free (local);
-	return status;
-    }
-
-    if (params->part <= 0) {
-	fputs (format->message_start, stdout);
-	if (format->message)
-	    format->message(ctx, message, indent);
-
-	fputs (format->header_start, stdout);
-	if (format->header)
-	    format->header(ctx, message);
-	fputs (format->header_end, stdout);
-
-	fputs (format->body_start, stdout);
-    }
-
-    if (format->part_content)
-	show_message_body (message, format, params);
-
-    if (params->part <= 0) {
-	fputs (format->body_end, stdout);
-
-	fputs (format->message_end, stdout);
-    }
+    void *local = talloc_new (ctx);
+    mime_node_t *root, *part;
+    notmuch_status_t status;
 
-    return NOTMUCH_STATUS_SUCCESS;
+    status = mime_node_open (local, message, params->cryptoctx,
+			     params->decrypt, &root);
+    if (status)
+	goto DONE;
+    part = mime_node_seek_dfs (root, (params->part < 0 ? 0 : params->part));
+    if (part)
+	status = format->part (local, part, indent, params);
+  DONE:
+    talloc_free (local);
+    return status;
 }
 
 static notmuch_status_t
-- 
1.7.9.1

[PATCH 3/6] Remove show-message.c

[Austin Clements]

There are no more calls to show_message_body.
---
 Makefile.local   |    1 -
 notmuch-client.h |    5 ---
 show-message.c   |  106 ------------------------------------------------------
 3 files changed, 0 insertions(+), 112 deletions(-)
 delete mode 100644 show-message.c

diff --git a/Makefile.local b/Makefile.local
index 1131dea..935f0f1 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -301,7 +301,6 @@ notmuch_client_srcs =		\
 	notmuch-tag.c		\
 	notmuch-time.c		\
 	query-string.c		\
-	show-message.c		\
 	mime-node.c		\
 	json.c
 
diff --git a/notmuch-client.h b/notmuch-client.h
index 203ac49..34155fc 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -184,11 +184,6 @@ char *
 query_string_from_args (void *ctx, int argc, char *argv[]);
 
 notmuch_status_t
-show_message_body (notmuch_message_t *message,
-		   const notmuch_show_format_t *format,
-		   notmuch_show_params_t *params);
-
-notmuch_status_t
 show_one_part (const char *filename, int part);
 
 void
diff --git a/show-message.c b/show-message.c
deleted file mode 100644
index 83ecf81..0000000
--- a/show-message.c
+++ /dev/null
@@ -1,106 +0,0 @@
-/* notmuch - Not much of an email program, (just index and search)
- *
- * Copyright © 2009 Carl Worth
- * Copyright © 2009 Keith Packard
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see http://www.gnu.org/licenses/ .
- *
- * Authors: Carl Worth <cworth@cworth.org>
- *	    Keith Packard <keithp@keithp.com>
- */
-
-#include "notmuch-client.h"
-
-typedef struct show_message_state {
-    int part_count;
-} show_message_state_t;
-
-static void
-show_message_part (mime_node_t *node,
-		   show_message_state_t *state,
-		   const notmuch_show_format_t *format,
-		   int first)
-{
-    /* Formatters expect the envelope for embedded message parts */
-    GMimeObject *part = node->envelope_part ?
-	GMIME_OBJECT (node->envelope_part) : node->part;
-    int i;
-
-    if (!first)
-	fputs (format->part_sep, stdout);
-
-    /* Format this part */
-    if (format->part_start)
-	format->part_start (part, &(state->part_count));
-
-    if (node->decrypt_attempted && format->part_encstatus)
-	format->part_encstatus (node->decrypt_success);
-
-    if (node->verify_attempted && format->part_sigstatus)
-#ifdef GMIME_ATLEAST_26
-	format->part_sigstatus (node->sig_list);
-#else
-	format->part_sigstatus (node->sig_validity);
-#endif
-
-    format->part_content (part);
-
-    if (node->envelope_part) {
-	fputs (format->header_start, stdout);
-	if (format->header_message_part)
-	    format->header_message_part (GMIME_MESSAGE (node->part));
-	fputs (format->header_end, stdout);
-
-	fputs (format->body_start, stdout);
-    }
-
-    /* Recurse over the children */
-    state->part_count += 1;
-    for (i = 0; i < node->nchildren; i++)
-	show_message_part (mime_node_child (node, i), state, format, i == 0);
-
-    /* Finish this part */
-    if (node->envelope_part)
-	fputs (format->body_end, stdout);
-
-    if (format->part_end)
-	format->part_end (part);
-}
-
-notmuch_status_t
-show_message_body (notmuch_message_t *message,
-		   const notmuch_show_format_t *format,
-		   notmuch_show_params_t *params)
-{
-    notmuch_status_t ret;
-    show_message_state_t state;
-    mime_node_t *root, *part;
-
-    ret = mime_node_open (NULL, message, params->cryptoctx, params->decrypt,
-			  &root);
-    if (ret)
-	return ret;
-
-    /* The caller of show_message_body has already handled the
-     * outermost envelope, so skip it. */
-    state.part_count = MAX (params->part, 1);
-
-    part = mime_node_seek_dfs (root, state.part_count);
-    if (part)
-	show_message_part (part, &state, format, TRUE);
-
-    talloc_free (root);
-
-    return NOTMUCH_STATUS_SUCCESS;
-}
-- 
1.7.9.1

[PATCH 4/6] show: Remove unused fields from notmuch_show_format

[Austin Clements]

These fields were only used by old-style formatters.
---
 notmuch-client.h |   23 -----------------------
 1 files changed, 0 insertions(+), 23 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index 34155fc..f0fb748 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -70,29 +70,6 @@ typedef struct notmuch_show_format {
     notmuch_status_t (*part) (const void *ctx,
 			      struct mime_node *node, int indent,
 			      const struct notmuch_show_params *params);
-    const char *message_start;
-    void (*message) (const void *ctx,
-		     notmuch_message_t *message,
-		     int indent);
-    const char *header_start;
-    void (*header) (const void *ctx,
-		    notmuch_message_t *message);
-    void (*header_message_part) (GMimeMessage *message);
-    const char *header_end;
-    const char *body_start;
-    void (*part_start) (GMimeObject *part,
-			int *part_count);
-    void (*part_encstatus) (int status);
-#ifdef GMIME_ATLEAST_26
-    void (*part_sigstatus) (GMimeSignatureList* siglist);
-#else
-    void (*part_sigstatus) (const GMimeSignatureValidity* validity);
-#endif
-    void (*part_content) (GMimeObject *part);
-    void (*part_end) (GMimeObject *part);
-    const char *part_sep;
-    const char *body_end;
-    const char *message_end;
     const char *message_set_sep;
     const char *message_set_end;
 } notmuch_show_format_t;
-- 
1.7.9.1

[PATCH 5/6] show: Support NULL values for message_set_{start, sep, end}

[Austin Clements]

Many formats don't need these, so it's more convenient if they don't
have to set them at all.
---
 notmuch-show.c |   24 +++++++++++++++---------
 1 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index 5a71115..ef26ad2 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -844,17 +844,19 @@ show_messages (void *ctx,
     int next_indent;
     notmuch_status_t status, res = NOTMUCH_STATUS_SUCCESS;
 
-    fputs (format->message_set_start, stdout);
+    if (format->message_set_start)
+	fputs (format->message_set_start, stdout);
 
     for (;
 	 notmuch_messages_valid (messages);
 	 notmuch_messages_move_to_next (messages))
     {
-	if (!first_set)
+	if (!first_set && format->message_set_sep)
 	    fputs (format->message_set_sep, stdout);
 	first_set = 0;
 
-	fputs (format->message_set_start, stdout);
+	if (format->message_set_start)
+	    fputs (format->message_set_start, stdout);
 
 	message = notmuch_messages_get (messages);
 
@@ -868,7 +870,7 @@ show_messages (void *ctx,
 		res = status;
 	    next_indent = indent + 1;
 
-	    if (!status)
+	    if (!status && format->message_set_sep)
 		fputs (format->message_set_sep, stdout);
 	}
 
@@ -882,10 +884,12 @@ show_messages (void *ctx,
 
 	notmuch_message_destroy (message);
 
-	fputs (format->message_set_end, stdout);
+	if (format->message_set_end)
+	    fputs (format->message_set_end, stdout);
     }
 
-    fputs (format->message_set_end, stdout);
+    if (format->message_set_end)
+	fputs (format->message_set_end, stdout);
 
     return res;
 }
@@ -931,7 +935,8 @@ do_show (void *ctx,
     int first_toplevel = 1;
     notmuch_status_t status, res = NOTMUCH_STATUS_SUCCESS;
 
-    fputs (format->message_set_start, stdout);
+    if (format->message_set_start)
+	fputs (format->message_set_start, stdout);
 
     for (threads = notmuch_query_search_threads (query);
 	 notmuch_threads_valid (threads);
@@ -945,7 +950,7 @@ do_show (void *ctx,
 	    INTERNAL_ERROR ("Thread %s has no toplevel messages.\n",
 			    notmuch_thread_get_thread_id (thread));
 
-	if (!first_toplevel)
+	if (!first_toplevel && format->message_set_sep)
 	    fputs (format->message_set_sep, stdout);
 	first_toplevel = 0;
 
@@ -957,7 +962,8 @@ do_show (void *ctx,
 
     }
 
-    fputs (format->message_set_end, stdout);
+    if (format->message_set_end)
+	fputs (format->message_set_end, stdout);
 
     return res != NOTMUCH_STATUS_SUCCESS;
 }
-- 
1.7.9.1

[PATCH 6/6] show: Remove empty message_set_{start,sep,end} fields

[Austin Clements]

Setting these to NULL is equivalent to the empty string now.
---
 notmuch-show.c |    9 ---------
 1 files changed, 0 insertions(+), 9 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index ef26ad2..7929476 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -26,10 +26,7 @@ format_part_text (const void *ctx, mime_node_t *node,
 		  int indent, const notmuch_show_params_t *params);
 
 static const notmuch_show_format_t format_text = {
-    .message_set_start = "",
     .part = format_part_text,
-    .message_set_sep = "",
-    .message_set_end = ""
 };
 
 static notmuch_status_t
@@ -48,10 +45,7 @@ format_part_mbox (const void *ctx, mime_node_t *node,
 		  int indent, const notmuch_show_params_t *params);
 
 static const notmuch_show_format_t format_mbox = {
-    .message_set_start = "",
     .part = format_part_mbox,
-    .message_set_sep = "",
-    .message_set_end = ""
 };
 
 static notmuch_status_t
@@ -60,10 +54,7 @@ format_part_raw (unused (const void *ctx), mime_node_t *node,
 		 unused (const notmuch_show_params_t *params));
 
 static const notmuch_show_format_t format_raw = {
-    .message_set_start = "",
     .part = format_part_raw,
-    .message_set_sep = "",
-    .message_set_end = ""
 };
 
 static const char *
-- 
1.7.9.1

Re: [PATCH 0/6] Finish show rewrite

[Tomi Ollila]

On Sun, Apr 08 2012, Austin Clements wrote:

> The long-awaited and oft-belated conclusion of the show rewrite.  All
> of the formatters have been converted to the new style, so this series
> just rips out unused code and does a little cleanup.

Looks good, tests pass.

+1

Tomi

Re: [PATCH 0/6] Finish show rewrite

[Mark Walters]

On Sun, 08 Apr 2012, Austin Clements <amdragon@MIT.EDU> wrote:
> The long-awaited and oft-belated conclusion of the show rewrite.  All
> of the formatters have been converted to the new style, so this series
> just rips out unused code and does a little cleanup.

This looks good to me: I don't know the code well but almost all of it
looks like any bugs would give compile time failures. 

Best wishes

Mark

Re: [PATCH 0/6] Finish show rewrite

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 531 bytes --]

On Sat, Apr 07 2012, Austin Clements <amdragon@MIT.EDU> wrote:
> The long-awaited and oft-belated conclusion of the show rewrite.  All
> of the formatters have been converted to the new style, so this series
> just rips out unused code and does a little cleanup.

LGTM, tested, and +1.

This has been a truly epic endeavor, Austin, and the result is
phenomenal.  The show code is much cleaner, more efficient, and better
functioning.  Thank you so much for working through it all and pushing
it through to the end.  Kudos.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH 0/6] Finish show rewrite

[David Bremner]

Austin Clements <amdragon@MIT.EDU> writes:

> The long-awaited and oft-belated conclusion of the show rewrite.  All
> of the formatters have been converted to the new style, so this series
> just rips out unused code and does a little cleanup.
>

pushed,

d