[Patch v8 0/6] Allow JSON to use non-entire thread, and use for elide

[Patch v8 0/6] Allow JSON to use non-entire thread, and use for elide

[Mark Walters]

This is version 8 of this patch series: the previous version is at
id:"1338723972-13063-1-git-send-email-markwalters1009@gmail.com". The
only change in this version is a rebase so that it applies to master.

I think this version addresses all comments made in reviews.

The error handling in the command-line parser is not ideal: (this
patch does not make it any worse). I posted a patch to fix this in
id:"1338906996-18720-1-git-send-email-markwalters1009@gmail.com". This
is not logically part of this series but is a natural follow up (and
touches lots of the same code).

Best wishes

Mark
  

Mark Walters (6):
  cli: command line parsing: allow default for keyword options
  cli: Let json output "null" messages for non --entire-thread
  cli: make --entire-thread=false work for format=json.
  Update devel/schemata for --entire-thread=false
  emacs: make elide messages use notmuch-show for omitting messages.
  cli: notmuch-show.c fix whitespace error

 command-line-arguments.c |   17 +++++++++++---
 devel/TODO               |    2 -
 devel/schemata           |    2 +-
 emacs/notmuch-show.el    |   18 ++++++++------
 notmuch-client.h         |    1 +
 notmuch-show.c           |   55 +++++++++++++++++++++++++++++++++++++--------
 6 files changed, 70 insertions(+), 25 deletions(-)

-- 
1.7.9.1

[Patch v8 1/6] cli: command line parsing: allow default for keyword options

[Mark Walters]

This changes the parsing for "keyword" options so that if the option
is specified with no argument the argument is parsed as if it were
passed an empty string. This make it easier to add options to existing
boolean arguments (the existing --option can default to TRUE).
---
 command-line-arguments.c |   17 +++++++++++++----
 1 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/command-line-arguments.c b/command-line-arguments.c
index 76b185f..b0a0dab 100644
--- a/command-line-arguments.c
+++ b/command-line-arguments.c
@@ -11,10 +11,15 @@
 */
 
 static notmuch_bool_t
-_process_keyword_arg (const notmuch_opt_desc_t *arg_desc, const char *arg_str) {
+_process_keyword_arg (const notmuch_opt_desc_t *arg_desc, char next, const char *arg_str) {
 
     const notmuch_keyword_t *keywords = arg_desc->keywords;
 
+    if (next == 0) {
+	/* No keyword given */
+	arg_str = "";
+    }
+
     while (keywords->name) {
 	if (strcmp (arg_str, keywords->name) == 0) {
 	    if (arg_desc->output_var) {
@@ -24,7 +29,10 @@ _process_keyword_arg (const notmuch_opt_desc_t *arg_desc, const char *arg_str) {
 	}
 	keywords++;
     }
-    fprintf (stderr, "unknown keyword: %s\n", arg_str);
+    if (next != 0)
+	fprintf (stderr, "unknown keyword: %s\n", arg_str);
+    else
+	fprintf (stderr, "option %s needs a keyword\n", arg_desc->name);
     return FALSE;
 }
 
@@ -99,7 +107,8 @@ parse_option (const char *arg,
 	     */
 	    if (next != '=' && next != ':' && next != 0) return FALSE;
 	    if (next == 0) {
-		if (try->opt_type != NOTMUCH_OPT_BOOLEAN)
+		if (try->opt_type != NOTMUCH_OPT_BOOLEAN &&
+		    try->opt_type != NOTMUCH_OPT_KEYWORD)
 		    return FALSE;
 	    } else {
 		if (value[0] == 0) return FALSE;
@@ -110,7 +119,7 @@ parse_option (const char *arg,
 
 	    switch (try->opt_type) {
 	    case NOTMUCH_OPT_KEYWORD:
-		return _process_keyword_arg (try, value);
+		return _process_keyword_arg (try, next, value);
 		break;
 	    case NOTMUCH_OPT_BOOLEAN:
 		return _process_boolean_arg (try, next, value);
-- 
1.7.9.1

[Patch v8 2/6] cli: Let json output "null" messages for non --entire-thread

[Mark Walters]

All formats except Json can output empty messages for non
entire-thread, but in Json format we output "null" to keep the other
elements (e.g. the replies to the omitted message) in the correct
place.
---
 notmuch-client.h |    1 +
 notmuch-show.c   |   20 ++++++++++++++++----
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index 9b63eae..0c17b79 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -75,6 +75,7 @@ typedef struct notmuch_show_format {
 			      const struct notmuch_show_params *params);
     const char *message_set_sep;
     const char *message_set_end;
+    const char *null_message;
 } notmuch_show_format_t;
 
 typedef struct notmuch_crypto {
diff --git a/notmuch-show.c b/notmuch-show.c
index 8247f1d..b004468 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -37,7 +37,8 @@ static const notmuch_show_format_t format_json = {
     .message_set_start = "[",
     .part = format_part_json_entry,
     .message_set_sep = ", ",
-    .message_set_end = "]"
+    .message_set_end = "]",
+    .null_message = "null"
 };
 
 static notmuch_status_t
@@ -800,6 +801,15 @@ format_part_raw (unused (const void *ctx), mime_node_t *node,
 }
 
 static notmuch_status_t
+show_null_message (const notmuch_show_format_t *format)
+{
+    /* Output a null message. Currently empty for all formats except Json */
+    if (format->null_message)
+	printf ("%s", format->null_message);
+    return NOTMUCH_STATUS_SUCCESS;
+}
+
+static notmuch_status_t
 show_message (void *ctx,
 	      const notmuch_show_format_t *format,
 	      notmuch_message_t *message,
@@ -861,11 +871,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),
-- 
1.7.9.1

[Patch v8 3/6] 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.

To do this the patch moves the --entire-thread option to be a keyword
option using the new command line parsing to allow the existing
--entire-thread to keep working.
---
 notmuch-show.c |   27 +++++++++++++++++++++++++--
 1 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index b004468..e6eb625 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -980,6 +980,12 @@ enum {
     NOTMUCH_FORMAT_RAW
 };
 
+enum {
+    ENTIRE_THREAD_DEFAULT,
+    ENTIRE_THREAD_TRUE,
+    ENTIRE_THREAD_FALSE,
+};
+
 /* The following is to allow future options to be added more easily */
 enum {
     EXCLUDE_TRUE,
@@ -1005,6 +1011,7 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
     };
     int format_sel = NOTMUCH_FORMAT_NOT_SPECIFIED;
     int exclude = EXCLUDE_TRUE;
+    int entire_thread = ENTIRE_THREAD_DEFAULT;
 
     notmuch_opt_desc_t options[] = {
 	{ NOTMUCH_OPT_KEYWORD, &format_sel, "format", 'f',
@@ -1017,8 +1024,12 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
           (notmuch_keyword_t []){ { "true", EXCLUDE_TRUE },
                                   { "false", EXCLUDE_FALSE },
                                   { 0, 0 } } },
+	{ NOTMUCH_OPT_KEYWORD, &entire_thread, "entire-thread", 't',
+	  (notmuch_keyword_t []){ { "true", ENTIRE_THREAD_TRUE },
+				  { "false", ENTIRE_THREAD_FALSE },
+				  { "", ENTIRE_THREAD_TRUE },
+				  { 0, 0 } } },
 	{ NOTMUCH_OPT_INT, &params.part, "part", 'p', 0 },
-	{ NOTMUCH_OPT_BOOLEAN, &params.entire_thread, "entire-thread", 't', 0 },
 	{ NOTMUCH_OPT_BOOLEAN, &params.crypto.decrypt, "decrypt", 'd', 0 },
 	{ NOTMUCH_OPT_BOOLEAN, &params.crypto.verify, "verify", 'v', 0 },
 	{ 0, 0, 0, 0, 0 }
@@ -1045,7 +1056,6 @@ 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;
 	break;
     case NOTMUCH_FORMAT_TEXT:
 	format = &format_text;
@@ -1068,6 +1078,19 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
 	break;
     }
 
+    /* Default is entire-thread = FALSE except for format=json. */
+    if (entire_thread == ENTIRE_THREAD_DEFAULT) {
+	if (format == &format_json)
+	    entire_thread = ENTIRE_THREAD_TRUE;
+	else
+	    entire_thread = ENTIRE_THREAD_FALSE;
+    }
+
+    if (entire_thread == ENTIRE_THREAD_TRUE)
+	params.entire_thread = TRUE;
+    else
+	params.entire_thread = FALSE;
+
     config = notmuch_config_open (ctx, NULL, NULL);
     if (config == NULL)
 	return 1;
-- 
1.7.9.1

[Patch v8 4/6] Update devel/schemata for --entire-thread=false

[Mark Walters]

Also remove the Json --entire-thread item from devel/TODO.
---
 devel/TODO     |    2 --
 devel/schemata |    2 +-
 2 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/devel/TODO b/devel/TODO
index 7b750af..eb757af 100644
--- a/devel/TODO
+++ b/devel/TODO
@@ -92,8 +92,6 @@ and email address in the From: line. We could also then easily support
 "notmuch compose --from <something>" to support getting at alternate
 email addresses.
 
-Fix the --format=json option to not imply --entire-thread.
-
 Implement "notmuch search --exclude-threads=<search-terms>" to allow
 for excluding muted threads, (and any other negative, thread-based
 filtering that the user wants to do).
diff --git a/devel/schemata b/devel/schemata
index 977cea7..8fcab8e 100644
--- a/devel/schemata
+++ b/devel/schemata
@@ -32,7 +32,7 @@ thread = [thread_node*]
 
 # A message and its replies (show_messages)
 thread_node = [
-    message?,                 # present if --entire-thread or matched
+    message?,                 # null if not matched and not --entire-thread
     [thread_node*]            # children of message
 ]
 
-- 
1.7.9.1

[Patch v8 5/6] 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 36cad93..b8fc9ef 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -958,9 +958,9 @@ message at DEPTH in the current thread."
   "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)
@@ -1041,16 +1041,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

[Patch v8 6/6] cli: notmuch-show.c fix whitespace error

[Mark Walters]

Fix an existing whitespace error since it is right next to
the changes of this series.
---
 notmuch-show.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index e6eb625..8f3c60e 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -1020,10 +1020,10 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
 				  { "mbox", NOTMUCH_FORMAT_MBOX },
 				  { "raw", NOTMUCH_FORMAT_RAW },
 				  { 0, 0 } } },
-        { NOTMUCH_OPT_KEYWORD, &exclude, "exclude", 'x',
-          (notmuch_keyword_t []){ { "true", EXCLUDE_TRUE },
-                                  { "false", EXCLUDE_FALSE },
-                                  { 0, 0 } } },
+	{ NOTMUCH_OPT_KEYWORD, &exclude, "exclude", 'x',
+	  (notmuch_keyword_t []){ { "true", EXCLUDE_TRUE },
+				  { "false", EXCLUDE_FALSE },
+				  { 0, 0 } } },
 	{ NOTMUCH_OPT_KEYWORD, &entire_thread, "entire-thread", 't',
 	  (notmuch_keyword_t []){ { "true", ENTIRE_THREAD_TRUE },
 				  { "false", ENTIRE_THREAD_FALSE },
-- 
1.7.9.1

Re: [Patch v8 2/6] cli: Let json output "null" messages for non --entire-thread

[Austin Clements]

The JSON format change LGTM.  This seems like the right way to do this.

On Sat, 16 Jun 2012, Mark Walters <markwalters1009@gmail.com> wrote:
> All formats except Json can output empty messages for non
> entire-thread, but in Json format we output "null" to keep the other
> elements (e.g. the replies to the omitted message) in the correct
> place.
> ---
>  notmuch-client.h |    1 +
>  notmuch-show.c   |   20 ++++++++++++++++----
>  2 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/notmuch-client.h b/notmuch-client.h
> index 9b63eae..0c17b79 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -75,6 +75,7 @@ typedef struct notmuch_show_format {
>  			      const struct notmuch_show_params *params);
>      const char *message_set_sep;
>      const char *message_set_end;
> +    const char *null_message;
>  } notmuch_show_format_t;
>  
>  typedef struct notmuch_crypto {
> diff --git a/notmuch-show.c b/notmuch-show.c
> index 8247f1d..b004468 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -37,7 +37,8 @@ static const notmuch_show_format_t format_json = {
>      .message_set_start = "[",
>      .part = format_part_json_entry,
>      .message_set_sep = ", ",
> -    .message_set_end = "]"
> +    .message_set_end = "]",
> +    .null_message = "null"
>  };
>  
>  static notmuch_status_t
> @@ -800,6 +801,15 @@ format_part_raw (unused (const void *ctx), mime_node_t *node,
>  }
>  
>  static notmuch_status_t
> +show_null_message (const notmuch_show_format_t *format)
> +{
> +    /* Output a null message. Currently empty for all formats except Json */
> +    if (format->null_message)
> +	printf ("%s", format->null_message);
> +    return NOTMUCH_STATUS_SUCCESS;
> +}
> +
> +static notmuch_status_t
>  show_message (void *ctx,
>  	      const notmuch_show_format_t *format,
>  	      notmuch_message_t *message,
> @@ -861,11 +871,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),
> -- 
> 1.7.9.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [Patch v8 4/6] Update devel/schemata for --entire-thread=false

[Austin Clements]

On Sat, 16 Jun 2012, Mark Walters <markwalters1009@gmail.com> wrote:
> Also remove the Json --entire-thread item from devel/TODO.
> ---
>  devel/TODO     |    2 --
>  devel/schemata |    2 +-
>  2 files changed, 1 insertions(+), 3 deletions(-)
>
> diff --git a/devel/TODO b/devel/TODO
> index 7b750af..eb757af 100644
> --- a/devel/TODO
> +++ b/devel/TODO
> @@ -92,8 +92,6 @@ and email address in the From: line. We could also then easily support
>  "notmuch compose --from <something>" to support getting at alternate
>  email addresses.
>  
> -Fix the --format=json option to not imply --entire-thread.
> -
>  Implement "notmuch search --exclude-threads=<search-terms>" to allow
>  for excluding muted threads, (and any other negative, thread-based
>  filtering that the user wants to do).
> diff --git a/devel/schemata b/devel/schemata
> index 977cea7..8fcab8e 100644
> --- a/devel/schemata
> +++ b/devel/schemata
> @@ -32,7 +32,7 @@ thread = [thread_node*]
>  
>  # A message and its replies (show_messages)
>  thread_node = [
> -    message?,                 # present if --entire-thread or matched
> +    message?,                 # null if not matched and not --entire-thread

To keep with the syntax of the rest of this file, this should be
    message|null,

If this is the only thing to change in this series, maybe it should just
be a follow-up patch.

>      [thread_node*]            # children of message
>  ]
>  
> -- 
> 1.7.9.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [Patch v8 0/6] Allow JSON to use non-entire thread, and use for elide

[David Bremner]

Mark Walters <markwalters1009@gmail.com> writes:

> This is version 8 of this patch series: the previous version is at
> id:"1338723972-13063-1-git-send-email-markwalters1009@gmail.com". The
> only change in this version is a rebase so that it applies to master.
>
> I think this version addresses all comments made in reviews.

I have tentatively marked this series as ready to push in nmbug. I'll
leave it in the ready queue for at least a few days.

d

[PATCH] Minor correction to devel/schemata

[Mark Walters]

---
In id:"87sjdm12d1.fsf@awakening.csail.mit.edu" Austin pointed out that
devel/schemata needs a slight correction with the new
--entire-thread=false option. This is that correction.

Best wishes

Mark

 devel/schemata |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/devel/schemata b/devel/schemata
index 8fcab8e..f7e1b69 100644
--- a/devel/schemata
+++ b/devel/schemata
@@ -32,7 +32,7 @@ thread = [thread_node*]
 
 # A message and its replies (show_messages)
 thread_node = [
-    message?,                 # null if not matched and not --entire-thread
+    message|null,             # null if not matched and not --entire-thread
     [thread_node*]            # children of message
 ]
 
-- 
1.7.9.1

[Patch v2] Minor correction to devel/schemata

[Mark Walters]

In id:"87sjdm12d1.fsf@awakening.csail.mit.edu" Austin pointed out that
devel/schemata needs a slight correction with the new
--entire-thread=false option. This is that correction.

---

Resend with better commit message

Best wishes

Mark

 devel/schemata |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/devel/schemata b/devel/schemata
index 8fcab8e..f7e1b69 100644
--- a/devel/schemata
+++ b/devel/schemata
@@ -32,7 +32,7 @@ thread = [thread_node*]
 
 # A message and its replies (show_messages)
 thread_node = [
-    message?,                 # null if not matched and not --entire-thread
+    message|null,             # null if not matched and not --entire-thread
     [thread_node*]            # children of message
 ]
 
-- 
1.7.9.1

Re: [Patch v2] Minor correction to devel/schemata

[Austin Clements]

LGTM.

On Sat, 30 Jun 2012, Mark Walters <markwalters1009@gmail.com> wrote:
> In id:"87sjdm12d1.fsf@awakening.csail.mit.edu" Austin pointed out that
> devel/schemata needs a slight correction with the new
> --entire-thread=false option. This is that correction.
>
> ---
>
> Resend with better commit message
>
> Best wishes
>
> Mark
>
>  devel/schemata |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/devel/schemata b/devel/schemata
> index 8fcab8e..f7e1b69 100644
> --- a/devel/schemata
> +++ b/devel/schemata
> @@ -32,7 +32,7 @@ thread = [thread_node*]
>  
>  # A message and its replies (show_messages)
>  thread_node = [
> -    message?,                 # null if not matched and not --entire-thread
> +    message|null,             # null if not matched and not --entire-thread
>      [thread_node*]            # children of message
>  ]
>  
> -- 
> 1.7.9.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch