[PATCH v3 0/2] Allow JSON to use non-entire thread, and use for elide

[PATCH v3 0/2] Allow JSON to use non-entire thread, and use for elide

[Mark Walters]

This is a rebased version of [1] with the bugfix [2] rolled in and the
style change suggested by Adam in [3].

I haven't added any tests yet: there do not seem to be any tests of
threading in JSON currently. I intend to rebase my show-tests [4] (and
can include/add tests for this). Alternatively there are Pieter's
tests [5] of the elide functionality which would test both patches.
Any suggestions on the preferred approach are gratefuly received!


Description of these patches from the previous version:

The first patch allows --entire-thread=false for notmuch-show.c when
the output format is JSON. In the previous version [2] Austin
suggested that we should output an empty message (i.e., {}) for
non-matching messages rather than just omitting them. This version
does that.

Note the first patch is entirely functional without the second.

The second patch uses the first to implement the "elide" functionality
in the emacs interface on the cli-side rather than on the emacs
side. This is substantially faster in some cases. In current emacs
show view it is a relatively small speed-up which is only noticable
with large threads. However, it will be used by notmuch-pick [3] and
there the speed up will be important. (I have a current version of
notmuch-pick which I will submit in the near future.)

[1] id:"1334077496-9172-1-git-send-email-markwalters1009@gmail.com"
[2] id:"1334090884-13001-1-git-send-email-markwalters1009@gmail.com"
[3] id:"CAMoJFUuXmLQR+awJnPKg8ZJAKbHbPAmSrEUVoFj_=dZ76WiKzw@mail.gmail.com"
[4] id:"1332171061-27983-1-git-send-email-markwalters1009@gmail.com"
[5] id:"1329684990-12504-3-git-send-email-pieter@praet.org"

Mark Walters (2):
  cli: make --entire-thread=false work for format=json.
  emacs: make elide messages use notmuch-show for omitting messages.

 emacs/notmuch-show.el |   18 ++++++++++--------
 notmuch-show.c        |   34 +++++++++++++++++++++++++++++-----
 2 files changed, 39 insertions(+), 13 deletions(-)

-- 
1.7.9.1

[PATCH v3 1/2] cli: make --entire-thread=false work for format=json.

[Mark Walters]

The --entire-thread option in notmuch-show.c defaults to true when
format=json. Previously there was no way to turn this off. This patch
makes it respect --entire-thread=false.

The one subtlety is that we initialise a notmuch_bool_t to -1 to
indicate that the option parsing has not set it. This allows the code
to distinguish between the option being omitted from the command line,
and the option being set to false on the command line.

Finally, all formats except Json can output empty messages for non
entire-thread, but in Json format we need to output {} to keep the
other elements (e.g. the replies to this message) in the correct
place.
---
 notmuch-show.c |   34 +++++++++++++++++++++++++++++-----
 1 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index da4a797..327c263 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -800,6 +800,16 @@ format_part_raw (unused (const void *ctx), mime_node_t *node,
 }
 
 static notmuch_status_t
+show_null_message (const notmuch_show_format_t *format)
+{
+    /* For all formats except json an empty message output is valid;
+     * for json we need the braces.*/
+    if (format == &format_json)
+	printf ("{}");
+    return NOTMUCH_STATUS_SUCCESS;
+}
+
+static notmuch_status_t
 show_message (void *ctx,
 	      const notmuch_show_format_t *format,
 	      notmuch_message_t *message,
@@ -862,11 +872,13 @@ show_messages (void *ctx,
 	    if (status && !res)
 		res = status;
 	    next_indent = indent + 1;
-
-	    if (!status && format->message_set_sep)
-		fputs (format->message_set_sep, stdout);
+	} else {
+	    status = show_null_message (format);
 	}
 
+	if (!status && format->message_set_sep)
+	    fputs (format->message_set_sep, stdout);
+
 	status = show_messages (ctx,
 				format,
 				notmuch_message_get_replies (message),
@@ -984,7 +996,13 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
     char *query_string;
     int opt_index, ret;
     const notmuch_show_format_t *format = &format_text;
-    notmuch_show_params_t params = { .part = -1, .omit_excluded = TRUE };
+
+    /* We abuse the notmuch_bool_t variable params.entire-thread by
+     * setting it to -1 to denote that the command line parsing has
+     * not set it. We ensure it is set to TRUE or FALSE before passing
+     * it to any other function.*/
+    notmuch_show_params_t params = { .part = -1, .entire_thread = -1 };
+
     int format_sel = NOTMUCH_FORMAT_NOT_SPECIFIED;
     notmuch_bool_t verify = FALSE;
     int exclude = EXCLUDE_TRUE;
@@ -1024,7 +1042,9 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
     switch (format_sel) {
     case NOTMUCH_FORMAT_JSON:
 	format = &format_json;
-	params.entire_thread = TRUE;
+	/* JSON defaults to entire-thread TRUE */
+	if (params.entire_thread == -1)
+	    params.entire_thread = TRUE;
 	break;
     case NOTMUCH_FORMAT_TEXT:
 	format = &format_text;
@@ -1046,6 +1066,10 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
 	params.raw = TRUE;
 	break;
     }
+    /* Default is entire-thread = FALSE except for format=json which
+     * is dealt with above. */
+    if (params.entire_thread == -1)
+	params.entire_thread = FALSE;
 
     if (params.decrypt || verify) {
 #ifdef GMIME_ATLEAST_26
-- 
1.7.9.1

[PATCH v3 2/2] emacs: make elide messages use notmuch-show for omitting messages.

[Mark Walters]

Previously the elide messages code got the entire-thread from
notmuch-show.c and then threw away all non-matching messages. This
version calls notmuch-show.c without the --entire-thread flag so
it never receives the non-matching messages in the first place.

This makes it substantially faster.
---
 emacs/notmuch-show.el |   18 ++++++++++--------
 1 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 4ee4290..347f90c 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -976,9 +976,9 @@ current buffer, if possible."
   "Insert the message tree TREE at depth DEPTH in the current thread."
   (let ((msg (car tree))
 	(replies (cadr tree)))
-    (if (or (not notmuch-show-elide-non-matching-messages)
-	    (plist-get msg :match))
-	(notmuch-show-insert-msg msg depth))
+    ;; We test whether there is a message or just some replies.
+    (when msg
+      (notmuch-show-insert-msg msg depth))
     (notmuch-show-insert-thread replies (1+ depth))))
 
 (defun notmuch-show-insert-thread (thread depth)
@@ -1059,16 +1059,18 @@ function is used."
 	     (args (if notmuch-show-query-context
 		       (append (list "\'") basic-args
 			       (list "and (" notmuch-show-query-context ")\'"))
-		     (append (list "\'") basic-args (list "\'")))))
-	(notmuch-show-insert-forest (notmuch-query-get-threads
-				     (cons "--exclude=false" args)))
+		     (append (list "\'") basic-args (list "\'"))))
+	     (cli-args (cons "--exclude=false"
+			     (when notmuch-show-elide-non-matching-messages
+			       (list "--entire-thread=false")))))
+
+	(notmuch-show-insert-forest (notmuch-query-get-threads (append cli-args args)))
 	;; If the query context reduced the results to nothing, run
 	;; the basic query.
 	(when (and (eq (buffer-size) 0)
 		   notmuch-show-query-context)
 	  (notmuch-show-insert-forest
-	   (notmuch-query-get-threads
-	    (cons "--exclude=false" basic-args)))))
+	   (notmuch-query-get-threads (append cli-args basic-args)))))
 
       (jit-lock-register #'notmuch-show-buttonise-links)
 
-- 
1.7.9.1

Re: [PATCH v3 1/2] cli: make --entire-thread=false work for format=json.

[Adam Wolfe Gordon]

On Sat, Apr 21, 2012 at 03:15, Mark Walters <markwalters1009@gmail.com> wrote:
> The --entire-thread option in notmuch-show.c defaults to true when
> format=json. Previously there was no way to turn this off. This patch
> makes it respect --entire-thread=false.
>
> The one subtlety is that we initialise a notmuch_bool_t to -1 to
> indicate that the option parsing has not set it. This allows the code
> to distinguish between the option being omitted from the command line,
> and the option being set to false on the command line.
>
> Finally, all formats except Json can output empty messages for non
> entire-thread, but in Json format we need to output {} to keep the
> other elements (e.g. the replies to this message) in the correct
> place.

LGTM. You can probably remove the relevant item from devel/TODO as well!

-- Adam

Re: [PATCH v3 1/2] cli: make --entire-thread=false work for format=json.

[Austin Clements]

Quoth Mark Walters on Apr 21 at 10:15 am:
> The --entire-thread option in notmuch-show.c defaults to true when
> format=json. Previously there was no way to turn this off. This patch
> makes it respect --entire-thread=false.
> 
> The one subtlety is that we initialise a notmuch_bool_t to -1 to
> indicate that the option parsing has not set it. This allows the code
> to distinguish between the option being omitted from the command line,
> and the option being set to false on the command line.
> 
> Finally, all formats except Json can output empty messages for non
> entire-thread, but in Json format we need to output {} to keep the
> other elements (e.g. the replies to this message) in the correct
> place.
> ---
>  notmuch-show.c |   34 +++++++++++++++++++++++++++++-----
>  1 files changed, 29 insertions(+), 5 deletions(-)
> 
> diff --git a/notmuch-show.c b/notmuch-show.c
> index da4a797..327c263 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -800,6 +800,16 @@ format_part_raw (unused (const void *ctx), mime_node_t *node,
>  }
>  
>  static notmuch_status_t
> +show_null_message (const notmuch_show_format_t *format)
> +{
> +    /* For all formats except json an empty message output is valid;
> +     * for json we need the braces.*/
> +    if (format == &format_json)

This should probably be specified as a field in the format, rather
than using a hard-coded dispatch.  It could even be a plain string
field that you could fputs in show_messages.

> +	printf ("{}");

This is a definite improvement over the current strangeness, but I
think 'null' would be better than an empty object.  null is a clearly
distinguished value, rather than something that initially has a
message-like type, but then isn't a message.

You should also update devel/schemata to reflect this change.

> +    return NOTMUCH_STATUS_SUCCESS;
> +}
> +
> +static notmuch_status_t
>  show_message (void *ctx,
>  	      const notmuch_show_format_t *format,
>  	      notmuch_message_t *message,
> @@ -862,11 +872,13 @@ show_messages (void *ctx,
>  	    if (status && !res)
>  		res = status;
>  	    next_indent = indent + 1;
> -
> -	    if (!status && format->message_set_sep)
> -		fputs (format->message_set_sep, stdout);
> +	} else {
> +	    status = show_null_message (format);
>  	}
>  
> +	if (!status && format->message_set_sep)
> +	    fputs (format->message_set_sep, stdout);
> +
>  	status = show_messages (ctx,
>  				format,
>  				notmuch_message_get_replies (message),

I believe the stuff above could be a separate patch from the stuff
below.

> @@ -984,7 +996,13 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
>      char *query_string;
>      int opt_index, ret;
>      const notmuch_show_format_t *format = &format_text;
> -    notmuch_show_params_t params = { .part = -1, .omit_excluded = TRUE };
> +
> +    /* We abuse the notmuch_bool_t variable params.entire-thread by
> +     * setting it to -1 to denote that the command line parsing has
> +     * not set it. We ensure it is set to TRUE or FALSE before passing
> +     * it to any other function.*/
> +    notmuch_show_params_t params = { .part = -1, .entire_thread = -1 };
> +
>      int format_sel = NOTMUCH_FORMAT_NOT_SPECIFIED;
>      notmuch_bool_t verify = FALSE;
>      int exclude = EXCLUDE_TRUE;
> @@ -1024,7 +1042,9 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
>      switch (format_sel) {
>      case NOTMUCH_FORMAT_JSON:
>  	format = &format_json;
> -	params.entire_thread = TRUE;
> +	/* JSON defaults to entire-thread TRUE */
> +	if (params.entire_thread == -1)
> +	    params.entire_thread = TRUE;
>  	break;
>      case NOTMUCH_FORMAT_TEXT:
>  	format = &format_text;
> @@ -1046,6 +1066,10 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
>  	params.raw = TRUE;
>  	break;
>      }
> +    /* Default is entire-thread = FALSE except for format=json which
> +     * is dealt with above. */
> +    if (params.entire_thread == -1)
> +	params.entire_thread = FALSE;
>  
>      if (params.decrypt || verify) {
>  #ifdef GMIME_ATLEAST_26