unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH 01/10] cli: Framework for structured output versioning
@ 2012-12-02  2:39 Austin Clements
  2012-12-02  2:39 ` [PATCH 02/10] search: Support --use-schema Austin Clements
                   ` (9 more replies)
  0 siblings, 10 replies; 17+ messages in thread
From: Austin Clements @ 2012-12-02  2:39 UTC (permalink / raw)
  To: notmuch

Currently there is a period of pain whenever we make
backward-incompatible changes to the structured output format.  This
series of patches introduces a way for CLI callers to request a
specific schema version on the command line and to determine if the
CLI does not supported the requested version (and perhaps present a
useful diagnostic to the user).  Since the caller requests a schema
version, it's also possible for the CLI to support multiple
incompatible versions simultaneously, unlike the alternate approach of
including version information in the output.

This patch lays the groundwork by introducing a versioning convention,
standard exit codes, and a utility function to check the requested
version and produce standardized diagnostic messages and exit
statuses.
---
 Makefile.local   |    1 +
 notmuch-client.h |   46 ++++++++++++++++++++++++++++++++++++++++++++++
 notmuch.c        |    3 +++
 schema.c         |   46 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 96 insertions(+)
 create mode 100644 schema.c

diff --git a/Makefile.local b/Makefile.local
index 2b91946..bfed50e 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -274,6 +274,7 @@ notmuch_client_srcs =		\
 	query-string.c		\
 	mime-node.c		\
 	crypto.c		\
+	schema.c		\
 
 notmuch_client_modules = $(notmuch_client_srcs:.c=.o)
 
diff --git a/notmuch-client.h b/notmuch-client.h
index ae9344b..14e7363 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -117,6 +117,52 @@ chomp_newline (char *str)
 	str[strlen(str)-1] = '\0';
 }
 
+/* Exit status code indicating the requested schema version is too old
+ * (support for that version has been dropped).  CLI code should use
+ * notmuch_exit_if_unsupported_schema rather than directly exiting
+ * with this code.
+ */
+#define NOTMUCH_EXIT_SCHEMA_TOO_OLD 20
+/* Exit status code indicating the requested schema version is newer
+ * than the version supported by the CLI.  CLI code should use
+ * notmuch_exit_if_unsupported_schema rather than directly exiting
+ * with this code.
+ */
+#define NOTMUCH_EXIT_SCHEMA_TOO_NEW 21
+
+/* The version number of the current structured output schema.
+ * Requests for schema versions above this will return an error.
+ * Backwards-incompatible changes such as removing map fields,
+ * changing the meaning of map fields, or changing the meanings of
+ * list elements should increase this.  New map fields can be added
+ * without increasing this.  Note that different notmuch commands may
+ * support different backwards-compatible version numbers, but the
+ * "current" version is consistent across all parts of the schema.
+ */
+#define NOTMUCH_SCHEMA_CUR 1
+
+/* The schema version requested by the caller on the command line.  If
+ * no schema version is requested, this should be set to
+ * NOTMUCH_SCHEMA_CUR.  This is global---rather than per
+ * command---because commands can share structured output code.
+ */
+extern int notmuch_schema_version;
+
+/* Commands that support structured output should support the
+ * following argument
+ *  { NOTMUCH_OPT_INT, &notmuch_schema_version, "use-schema", 0, 0 }
+ * and should invoke notmuch_exit_if_unsupported_schema to check
+ * against the per-command minimum supported schema version and the
+ * global maximum supported schema version.  If notmuch_schema_version
+ * is outside the supported range, this will print a detailed
+ * diagnostic message for the user and exit with
+ * NOTMUCH_EXIT_SCHEMA_TOO_{OLD,NEW} to inform the invoking program of
+ * the problem.
+ */
+void
+notmuch_exit_if_unsupported_schema (const char *command,
+				    int min_schema_version);
+
 notmuch_crypto_context_t *
 notmuch_crypto_get_context (notmuch_crypto_t *crypto, const char *protocol);
 
diff --git a/notmuch.c b/notmuch.c
index 477a09c..ed860f2 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -242,6 +242,9 @@ main (int argc, char *argv[])
     g_mime_init (0);
     g_type_init ();
 
+    /* Globally default to the current schema version. */
+    notmuch_schema_version = NOTMUCH_SCHEMA_CUR;
+
     if (argc == 1)
 	return notmuch (local);
 
diff --git a/schema.c b/schema.c
new file mode 100644
index 0000000..4b6c4fa
--- /dev/null
+++ b/schema.c
@@ -0,0 +1,46 @@
+/* notmuch - Not much of an email program, (just index and search)
+ *
+ * This file is part of notmuch.
+ *
+ * Copyright © 2012 Austin Clements
+ *
+ * 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/ .
+ *
+ * Author: Austin Clements <aclements@csail.mit.edu>
+ */
+
+#include "notmuch-client.h"
+
+int notmuch_schema_version;
+
+void
+notmuch_exit_if_unsupported_schema (const char *command,
+				    int min_schema_version)
+{
+    if (notmuch_schema_version > NOTMUCH_SCHEMA_CUR) {
+	fprintf (stderr, "\
+A caller requested a newer output format (schema version %d) than\n\
+supported by the notmuch CLI (which supports up to version %d).  You\n\
+may need to upgrade your notmuch CLI.\n",
+		 notmuch_schema_version, NOTMUCH_SCHEMA_CUR);
+	exit (NOTMUCH_EXIT_SCHEMA_TOO_NEW);
+    } else if (notmuch_schema_version < min_schema_version) {
+	fprintf (stderr, "\
+A caller requested an older output format (schema version %d) than\n\
+supported by the %s command of the notmuch CLI (which requires at\n\
+least version %d).  You may need to upgrade your notmuch front-end.\n",
+		 notmuch_schema_version, command, min_schema_version);
+	exit (NOTMUCH_EXIT_SCHEMA_TOO_OLD);
+    }
+}
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 02/10] search: Support --use-schema
  2012-12-02  2:39 [PATCH 01/10] cli: Framework for structured output versioning Austin Clements
@ 2012-12-02  2:39 ` Austin Clements
  2012-12-02  2:39 ` [PATCH 03/10] show: " Austin Clements
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: Austin Clements @ 2012-12-02  2:39 UTC (permalink / raw)
  To: notmuch

---
 man/man1/notmuch-search.1 |   20 ++++++++++++++++++++
 notmuch-client.h          |    2 ++
 notmuch-search.c          |    3 +++
 3 files changed, 25 insertions(+)

diff --git a/man/man1/notmuch-search.1 b/man/man1/notmuch-search.1
index 6ccd3b8..79dc1fe 100644
--- a/man/man1/notmuch-search.1
+++ b/man/man1/notmuch-search.1
@@ -32,6 +32,15 @@ Presents the results in either JSON or plain-text (default).
 
 .RS 4
 .TP 4
+.BR \-\-use-schema=N
+
+Use the specified structured output schema version.  This is intended
+for programs that invoke \fBnotmuch\fR(1) internally.  If omitted, the
+latest supported version will be used.
+.RE
+
+.RS 4
+.TP 4
 .B \-\-output=(summary|threads|messages|files|tags)
 
 .RS 4
@@ -125,6 +134,17 @@ In this case all matching threads are returned but the "match count"
 is the number of matching non-excluded messages in the thread.
 .RE
 
+.SH EXIT STATUS
+
+This command supports the following special exit status codes
+
+.TP
+.B 20
+The requested schema version is too old.
+.TP
+.B 21
+The requested schema version is too new.
+
 .SH SEE ALSO
 
 \fBnotmuch\fR(1), \fBnotmuch-config\fR(1), \fBnotmuch-count\fR(1),
diff --git a/notmuch-client.h b/notmuch-client.h
index 14e7363..95c4dd7 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -140,6 +140,8 @@ chomp_newline (char *str)
  * "current" version is consistent across all parts of the schema.
  */
 #define NOTMUCH_SCHEMA_CUR 1
+/* The minimum schema version supported by the "search" command. */
+#define NOTMUCH_SCHEMA_SEARCH 1
 
 /* The schema version requested by the caller on the command line.  If
  * no schema version is requested, this should be set to
diff --git a/notmuch-search.c b/notmuch-search.c
index 830c4e4..477f9eb 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -317,6 +317,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
 	  (notmuch_keyword_t []){ { "json", NOTMUCH_FORMAT_JSON },
 				  { "text", NOTMUCH_FORMAT_TEXT },
 				  { 0, 0 } } },
+	{ NOTMUCH_OPT_INT, &notmuch_schema_version, "use-schema", 0, 0 },
 	{ NOTMUCH_OPT_KEYWORD, &output, "output", 'o',
 	  (notmuch_keyword_t []){ { "summary", OUTPUT_SUMMARY },
 				  { "threads", OUTPUT_THREADS },
@@ -352,6 +353,8 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
 	INTERNAL_ERROR("no output format selected");
     }
 
+    notmuch_exit_if_unsupported_schema ("search", NOTMUCH_SCHEMA_SEARCH);
+
     config = notmuch_config_open (ctx, NULL, NULL);
     if (config == NULL)
 	return 1;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 03/10] show: Support --use-schema
  2012-12-02  2:39 [PATCH 01/10] cli: Framework for structured output versioning Austin Clements
  2012-12-02  2:39 ` [PATCH 02/10] search: Support --use-schema Austin Clements
@ 2012-12-02  2:39 ` Austin Clements
  2012-12-02  2:39 ` [PATCH 04/10] reply: " Austin Clements
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: Austin Clements @ 2012-12-02  2:39 UTC (permalink / raw)
  To: notmuch

---
 man/man1/notmuch-show.1 |   20 ++++++++++++++++++++
 notmuch-client.h        |    2 ++
 notmuch-show.c          |    3 +++
 3 files changed, 25 insertions(+)

diff --git a/man/man1/notmuch-show.1 b/man/man1/notmuch-show.1
index 4481f21..e2fd0c2 100644
--- a/man/man1/notmuch-show.1
+++ b/man/man1/notmuch-show.1
@@ -108,6 +108,15 @@ message.
 
 .RS 4
 .TP 4
+.BR \-\-use-schema=N
+
+Use the specified structured output schema version.  This is intended
+for programs that invoke \fBnotmuch\fR(1) internally.  If omitted, the
+latest supported version will be used.
+.RE
+
+.RS 4
+.TP 4
 .B \-\-part=N
 
 Output the single decoded MIME part N of a single message.  The search
@@ -181,6 +190,17 @@ column of output from the
 .B notmuch search
 command.
 
+.SH EXIT STATUS
+
+This command supports the following special exit status codes
+
+.TP
+.B 20
+The requested schema version is too old.
+.TP
+.B 21
+The requested schema version is too new.
+
 .SH SEE ALSO
 
 \fBnotmuch\fR(1), \fBnotmuch-config\fR(1), \fBnotmuch-count\fR(1),
diff --git a/notmuch-client.h b/notmuch-client.h
index 95c4dd7..d931bbf 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -142,6 +142,8 @@ chomp_newline (char *str)
 #define NOTMUCH_SCHEMA_CUR 1
 /* The minimum schema version supported by the "search" command. */
 #define NOTMUCH_SCHEMA_SEARCH 1
+/* The minimum schema version supported by the "show" command. */
+#define NOTMUCH_SCHEMA_SHOW 1
 
 /* The schema version requested by the caller on the command line.  If
  * no schema version is requested, this should be set to
diff --git a/notmuch-show.c b/notmuch-show.c
index 2fa2292..9d1f7c7 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -1059,6 +1059,7 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
 				  { "mbox", NOTMUCH_FORMAT_MBOX },
 				  { "raw", NOTMUCH_FORMAT_RAW },
 				  { 0, 0 } } },
+	{ NOTMUCH_OPT_INT, &notmuch_schema_version, "use-schema", 0, 0 },
 	{ NOTMUCH_OPT_KEYWORD, &exclude, "exclude", 'x',
 	  (notmuch_keyword_t []){ { "true", EXCLUDE_TRUE },
 				  { "false", EXCLUDE_FALSE },
@@ -1118,6 +1119,8 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
 	break;
     }
 
+    notmuch_exit_if_unsupported_schema ("show", NOTMUCH_SCHEMA_SHOW);
+
     /* Default is entire-thread = FALSE except for format=json. */
     if (entire_thread == ENTIRE_THREAD_DEFAULT) {
 	if (format == &format_json)
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 04/10] reply: Support --use-schema
  2012-12-02  2:39 [PATCH 01/10] cli: Framework for structured output versioning Austin Clements
  2012-12-02  2:39 ` [PATCH 02/10] search: Support --use-schema Austin Clements
  2012-12-02  2:39 ` [PATCH 03/10] show: " Austin Clements
@ 2012-12-02  2:39 ` Austin Clements
  2012-12-02  2:39 ` [PATCH 05/10] test: Sanity tests for --use-schema argument Austin Clements
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: Austin Clements @ 2012-12-02  2:39 UTC (permalink / raw)
  To: notmuch

---
 man/man1/notmuch-reply.1 |   21 +++++++++++++++++++++
 notmuch-client.h         |    4 ++++
 notmuch-reply.c          |    3 +++
 3 files changed, 28 insertions(+)

diff --git a/man/man1/notmuch-reply.1 b/man/man1/notmuch-reply.1
index d264060..71ff569 100644
--- a/man/man1/notmuch-reply.1
+++ b/man/man1/notmuch-reply.1
@@ -52,6 +52,16 @@ to create a reply message intelligently.
 Only produces In\-Reply\-To, References, To, Cc, and Bcc headers.
 .RE
 .RE
+
+.RS
+.TP 4
+.BR \-\-use-schema=N
+
+Use the specified structured output schema version.  This is intended
+for programs that invoke \fBnotmuch\fR(1) internally.  If omitted, the
+latest supported version will be used.
+.RE
+
 .RS
 .TP 4
 .BR \-\-reply\-to= ( all | sender )
@@ -93,6 +103,17 @@ replying to multiple messages at once, but the JSON format does not.
 .RE
 .RE
 
+.SH EXIT STATUS
+
+This command supports the following special exit status codes
+
+.TP
+.B 20
+The requested schema version is too old.
+.TP
+.B 21
+The requested schema version is too new.
+
 .SH SEE ALSO
 
 \fBnotmuch\fR(1), \fBnotmuch-config\fR(1), \fBnotmuch-count\fR(1),
diff --git a/notmuch-client.h b/notmuch-client.h
index d931bbf..0eb6b36 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -144,6 +144,10 @@ chomp_newline (char *str)
 #define NOTMUCH_SCHEMA_SEARCH 1
 /* The minimum schema version supported by the "show" command. */
 #define NOTMUCH_SCHEMA_SHOW 1
+/* The minimum schema version supported by the "reply" command.  Reply
+ * uses show, so in general this must be >= NOTMUCH_SCHEMA_SHOW.
+ */
+#define NOTMUCH_SCHEMA_REPLY 1
 
 /* The schema version requested by the caller on the command line.  If
  * no schema version is requested, this should be set to
diff --git a/notmuch-reply.c b/notmuch-reply.c
index e60a264..059a412 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -724,6 +724,7 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
 				  { "json", FORMAT_JSON },
 				  { "headers-only", FORMAT_HEADERS_ONLY },
 				  { 0, 0 } } },
+	{ NOTMUCH_OPT_INT, &notmuch_schema_version, "use-schema", 0, 0 },
 	{ NOTMUCH_OPT_KEYWORD, &reply_all, "reply-to", 'r',
 	  (notmuch_keyword_t []){ { "all", TRUE },
 				  { "sender", FALSE },
@@ -745,6 +746,8 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
     else
 	reply_format_func = notmuch_reply_format_default;
 
+    notmuch_exit_if_unsupported_schema ("reply", NOTMUCH_SCHEMA_REPLY);
+
     config = notmuch_config_open (ctx, NULL, NULL);
     if (config == NULL)
 	return 1;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 05/10] test: Sanity tests for --use-schema argument
  2012-12-02  2:39 [PATCH 01/10] cli: Framework for structured output versioning Austin Clements
                   ` (2 preceding siblings ...)
  2012-12-02  2:39 ` [PATCH 04/10] reply: " Austin Clements
@ 2012-12-02  2:39 ` Austin Clements
  2012-12-02  2:39 ` [PATCH 06/10] emacs: Fix bug in resynchronizing after a JSON parse error Austin Clements
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: Austin Clements @ 2012-12-02  2:39 UTC (permalink / raw)
  To: notmuch

---
 test/json |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/test/json b/test/json
index bfafd55..65e007c 100755
--- a/test/json
+++ b/test/json
@@ -60,4 +60,10 @@ test_expect_equal_json "$output" "[{\"thread\": \"XXX\",
  \"tags\": [\"inbox\",
  \"unread\"]}]"
 
+test_expect_code 20 "Schema version: too low" \
+    "notmuch search --use-schema=0 *"
+
+test_expect_code 21 "Schema version: too high" \
+    "notmuch search --use-schema=999 *"
+
 test_done
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 06/10] emacs: Fix bug in resynchronizing after a JSON parse error
  2012-12-02  2:39 [PATCH 01/10] cli: Framework for structured output versioning Austin Clements
                   ` (3 preceding siblings ...)
  2012-12-02  2:39 ` [PATCH 05/10] test: Sanity tests for --use-schema argument Austin Clements
@ 2012-12-02  2:39 ` Austin Clements
  2012-12-08  8:32   ` Mark Walters
  2012-12-02  2:39 ` [PATCH 07/10] emacs: Use --use-schema for search Austin Clements
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 17+ messages in thread
From: Austin Clements @ 2012-12-02  2:39 UTC (permalink / raw)
  To: notmuch

Previously, if the input stream consisted only of an error message,
notmuch-json-begin-compound would signal a (wrong-type-argument
number-or-marker-p nil) error when reaching the end of the error
message.  This happened because notmuch-json-scan-to-value would think
that it reached a value and put the parser into the 'value state.
Even after notmuch-json-begin-compound signaled the syntax error, the
parser would remain in this state and when the resynchronization logic
reached the end of the buffer, the parser would fail because the
'value state indicates that characters are available.

This fixes this problem by restoring the parser's previous state if it
encounters a syntax error.
---
 emacs/notmuch-lib.el |   22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 1d0ec17..9402456 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -474,15 +474,19 @@ Entering JSON objects is currently unimplemented."
   (with-current-buffer (notmuch-json-buffer jp)
     ;; Disallow terminators
     (setf (notmuch-json-allow-term jp) nil)
-    (or (notmuch-json-scan-to-value jp)
-	(if (/= (char-after) ?\[)
-	    (signal 'json-readtable-error (list "expected '['"))
-	  (forward-char)
-	  (push ?\] (notmuch-json-term-stack jp))
-	  ;; Expect a value or terminator next
-	  (setf (notmuch-json-next jp) 'expect-value
-		(notmuch-json-allow-term jp) t)
-	  t))))
+    ;; Save "next" so we can restore it if there's a syntax error
+    (let ((saved-next (notmuch-json-next jp)))
+      (or (notmuch-json-scan-to-value jp)
+	  (if (/= (char-after) ?\[)
+	      (progn
+		(setf (notmuch-json-next jp) saved-next)
+		(signal 'json-readtable-error (list "expected '['")))
+	    (forward-char)
+	    (push ?\] (notmuch-json-term-stack jp))
+	    ;; Expect a value or terminator next
+	    (setf (notmuch-json-next jp) 'expect-value
+		  (notmuch-json-allow-term jp) t)
+	    t)))))
 
 (defun notmuch-json-read (jp)
   "Parse the value at point in JP's buffer.
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 07/10] emacs: Use --use-schema for search
  2012-12-02  2:39 [PATCH 01/10] cli: Framework for structured output versioning Austin Clements
                   ` (4 preceding siblings ...)
  2012-12-02  2:39 ` [PATCH 06/10] emacs: Fix bug in resynchronizing after a JSON parse error Austin Clements
@ 2012-12-02  2:39 ` Austin Clements
  2012-12-08  8:48   ` Mark Walters
  2012-12-02  2:40 ` [PATCH 08/10] emacs: Factor out synchronous notmuch JSON invocations Austin Clements
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 17+ messages in thread
From: Austin Clements @ 2012-12-02  2:39 UTC (permalink / raw)
  To: notmuch

We detect the special exit statuses and use these to produce specific
diagnostic messages.
---
 emacs/notmuch-lib.el |   32 ++++++++++++++++++++++++++++++++
 emacs/notmuch.el     |   17 +++++++++++++----
 2 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 9402456..49b0da6 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -325,6 +325,38 @@ string), a property list of face attributes, or a list of these."
 	(put-text-property pos next 'face (cons face cur))
 	(setq pos next)))))
 
+(defun notmuch-pop-up-error (msg)
+  "Pop up an error buffer displaying MSG."
+
+  (let ((buf (get-buffer-create "*Notmuch errors*")))
+    (with-current-buffer buf
+      (view-mode)
+      (let ((inhibit-read-only t))
+	(erase-buffer)
+	(insert msg)
+	(unless (bolp)
+	  (insert "\n"))
+	(goto-char (point-min))))
+    (pop-to-buffer buf)))
+
+(defun notmuch-version-mismatch-error (exit-status)
+  "Signal a schema version mismatch error.
+
+EXIT-STATUS must be the exit status of the notmuch CLI command,
+and must have the value 20 or 21.  This function will pop up an
+error buffer with a descriptive message and signal an error."
+  (cond ((= exit-status 20)
+	 (notmuch-pop-up-error "Error: Version mismatch.
+Emacs requested an older output format than supported by the notmuch CLI.
+You may need to restart Emacs or upgrade your notmuch Emacs package."))
+	((= exit-status 21)
+	 (notmuch-pop-up-error "Error: Version mismatch.
+Emacs requested a newer output format than supported by the notmuch CLI.
+You may need to restart Emacs or upgrade your notmuch package."))
+	(t
+	 (error "Bad exit status %d" exit-status)))
+  (error "notmuch CLI version mismatch"))
+
 ;; Compatibility functions for versions of emacs before emacs 23.
 ;;
 ;; Both functions here were copied from emacs 23 with the following copyright:
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index f9454d8..e1f28ca 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -644,6 +644,7 @@ of the result."
 	(exit-status (process-exit-status proc))
 	(never-found-target-thread nil))
     (when (memq status '(exit signal))
+      (catch 'return
 	(kill-buffer (process-get proc 'parse-buf))
 	(if (buffer-live-p buffer)
 	    (with-current-buffer buffer
@@ -655,8 +656,16 @@ of the result."
 		      (insert "Incomplete search results (search process was killed).\n"))
 		  (when (eq status 'exit)
 		    (insert "End of search results.")
-		    (unless (= exit-status 0)
-		      (insert (format " (process returned %d)" exit-status)))
+		    (cond ((or (= exit-status 20) (= exit-status 21))
+			   (kill-buffer)
+			   (condition-case err
+			       (notmuch-version-mismatch-error exit-status)
+			     ;; Strange things happen when you signal
+			     ;; an error from a sentinel.
+			     (error (throw 'return nil))))
+			  ((/= exit-status 0)
+			   (insert (format " (process returned %d)"
+					   exit-status))))
 		    (insert "\n")
 		    (if (and atbob
 			     (not (string= notmuch-search-target-thread "found")))
@@ -664,7 +673,7 @@ of the result."
 	      (when (and never-found-target-thread
 		       notmuch-search-target-line)
 		  (goto-char (point-min))
-		  (forward-line (1- notmuch-search-target-line))))))))
+		  (forward-line (1- notmuch-search-target-line)))))))))
 
 (defcustom notmuch-search-line-faces '(("unread" :weight bold)
 				       ("flagged" :foreground "blue"))
@@ -938,7 +947,7 @@ Other optional parameters are used as follows:
 	(let ((proc (start-process
 		     "notmuch-search" buffer
 		     notmuch-command "search"
-		     "--format=json"
+		     "--format=json" "--use-schema=1"
 		     (if oldest-first
 			 "--sort=oldest-first"
 		       "--sort=newest-first")
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 08/10] emacs: Factor out synchronous notmuch JSON invocations
  2012-12-02  2:39 [PATCH 01/10] cli: Framework for structured output versioning Austin Clements
                   ` (5 preceding siblings ...)
  2012-12-02  2:39 ` [PATCH 07/10] emacs: Use --use-schema for search Austin Clements
@ 2012-12-02  2:40 ` Austin Clements
  2012-12-02  2:40 ` [PATCH 09/10] emacs: Improve error handling for notmuch-call-notmuch-json Austin Clements
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: Austin Clements @ 2012-12-02  2:40 UTC (permalink / raw)
  To: notmuch

Previously this code was duplicated between show and reply.  This
factors out synchronously invoking notmuch and parsing the output as
JSON.
---
 emacs/notmuch-lib.el   |   14 ++++++++++++++
 emacs/notmuch-mua.el   |    8 +-------
 emacs/notmuch-query.el |   11 ++---------
 3 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 49b0da6..31c69dc 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -357,6 +357,20 @@ You may need to restart Emacs or upgrade your notmuch package."))
 	 (error "Bad exit status %d" exit-status)))
   (error "notmuch CLI version mismatch"))
 
+(defun notmuch-call-notmuch-json (&rest args)
+  "Invoke `notmuch-command' with `args' and return the parsed JSON output.
+
+The returned output will represent objects using property lists
+and arrays as lists."
+
+  (with-temp-buffer
+    (apply #'call-process notmuch-command nil (list t nil) nil args)
+    (goto-char (point-min))
+    (let ((json-object-type 'plist)
+	  (json-array-type 'list)
+	  (json-false 'nil))
+      (json-read))))
+
 ;; Compatibility functions for versions of emacs before emacs 23.
 ;;
 ;; Both functions here were copied from emacs 23 with the following copyright:
diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index 408b49e..ac2d29e 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -158,13 +158,7 @@ list."
     (setq args (append args (list query-string)))
 
     ;; Get the reply object as JSON, and parse it into an elisp object.
-    (with-temp-buffer
-      (apply 'call-process (append (list notmuch-command nil (list t nil) nil) args))
-      (goto-char (point-min))
-      (let ((json-object-type 'plist)
-	    (json-array-type 'list)
-	    (json-false 'nil))
-	(setq reply (json-read))))
+    (setq reply (apply #'notmuch-call-notmuch-json args))
 
     ;; Extract the original message to simplify the following code.
     (setq original (plist-get reply :original))
diff --git a/emacs/notmuch-query.el b/emacs/notmuch-query.el
index d66baea..e7e3520 100644
--- a/emacs/notmuch-query.el
+++ b/emacs/notmuch-query.el
@@ -29,18 +29,11 @@ A thread is a forest or list of trees. A tree is a two element
 list where the first element is a message, and the second element
 is a possibly empty forest of replies.
 "
-  (let  ((args '("show" "--format=json"))
-	 (json-object-type 'plist)
-	 (json-array-type 'list)
-	 (json-false 'nil))
+  (let ((args '("show" "--format=json")))
     (if notmuch-show-process-crypto
 	(setq args (append args '("--decrypt"))))
     (setq args (append args search-terms))
-    (with-temp-buffer
-      (progn
-	(apply 'call-process (append (list notmuch-command nil (list t nil) nil) args))
-	(goto-char (point-min))
-	(json-read)))))
+    (apply #'notmuch-call-notmuch-json args)))
 
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;; Mapping functions across collections of messages.
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 09/10] emacs: Improve error handling for notmuch-call-notmuch-json
  2012-12-02  2:39 [PATCH 01/10] cli: Framework for structured output versioning Austin Clements
                   ` (6 preceding siblings ...)
  2012-12-02  2:40 ` [PATCH 08/10] emacs: Factor out synchronous notmuch JSON invocations Austin Clements
@ 2012-12-02  2:40 ` Austin Clements
  2012-12-02  2:40 ` [PATCH 10/10] emacs: Use --use-schema for show and reply Austin Clements
  2012-12-03  0:58 ` [PATCH 00/10] CLI output versioning Austin Clements
  9 siblings, 0 replies; 17+ messages in thread
From: Austin Clements @ 2012-12-02  2:40 UTC (permalink / raw)
  To: notmuch

This checks for non-zero exit status and pops up an error buffer
giving the exit status, the stderr, and the stdout.  Schema version
mismatches are handled specially.
---
 emacs/notmuch-lib.el |   40 +++++++++++++++++++++++++++++++++-------
 1 file changed, 33 insertions(+), 7 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 31c69dc..9c6be13 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -361,15 +361,41 @@ You may need to restart Emacs or upgrade your notmuch package."))
   "Invoke `notmuch-command' with `args' and return the parsed JSON output.
 
 The returned output will represent objects using property lists
-and arrays as lists."
+and arrays as lists.  If notmuch exits with a non-zero status,
+this will pop up a buffer containing notmuch's output and signal
+an error."
 
   (with-temp-buffer
-    (apply #'call-process notmuch-command nil (list t nil) nil args)
-    (goto-char (point-min))
-    (let ((json-object-type 'plist)
-	  (json-array-type 'list)
-	  (json-false 'nil))
-      (json-read))))
+    (let ((err-file (make-temp-file "nmerr")))
+      (unwind-protect
+	  (let ((status (apply #'call-process
+			       notmuch-command nil (list t err-file) nil args)))
+	    (cond ((equal status 0)
+		   (goto-char (point-min))
+		   (let ((json-object-type 'plist)
+			 (json-array-type 'list)
+			 (json-false 'nil))
+		     (json-read)))
+		  ((or (equal status 20) (equal status 21))
+		   ;; Special handling of version mismatch errors
+		   (notmuch-version-mismatch-error status))
+		  (t
+		   (goto-char (point-min))
+		   (insert "Error:\n")
+		   (let ((pos (point)))
+		     (insert-file-contents err-file)
+		     (when (= (point) pos)
+		       (insert "(no error output)\n")))
+		   (unless (eobp)
+		     (insert "Output:\n"))
+		   (notmuch-pop-up-error
+		    (concat
+		     (format "CLI error.  %S exited with %s%s.\n"
+			     (cons notmuch-command args)
+			     (if (integerp status) "status " "")
+			     status)
+		     (buffer-string))))))
+	(delete-file err-file)))))
 
 ;; Compatibility functions for versions of emacs before emacs 23.
 ;;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 10/10] emacs: Use --use-schema for show and reply
  2012-12-02  2:39 [PATCH 01/10] cli: Framework for structured output versioning Austin Clements
                   ` (7 preceding siblings ...)
  2012-12-02  2:40 ` [PATCH 09/10] emacs: Improve error handling for notmuch-call-notmuch-json Austin Clements
@ 2012-12-02  2:40 ` Austin Clements
  2012-12-03  0:58 ` [PATCH 00/10] CLI output versioning Austin Clements
  9 siblings, 0 replies; 17+ messages in thread
From: Austin Clements @ 2012-12-02  2:40 UTC (permalink / raw)
  To: notmuch

---
 emacs/notmuch-mua.el   |    2 +-
 emacs/notmuch-query.el |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index ac2d29e..0e1c7a7 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -146,7 +146,7 @@ list."
   (unless (bolp) (insert "\n")))
 
 (defun notmuch-mua-reply (query-string &optional sender reply-all)
-  (let ((args '("reply" "--format=json"))
+  (let ((args '("reply" "--format=json" "--use-schema=1"))
 	reply
 	original)
     (when notmuch-show-process-crypto
diff --git a/emacs/notmuch-query.el b/emacs/notmuch-query.el
index e7e3520..7517aec 100644
--- a/emacs/notmuch-query.el
+++ b/emacs/notmuch-query.el
@@ -29,7 +29,7 @@ A thread is a forest or list of trees. A tree is a two element
 list where the first element is a message, and the second element
 is a possibly empty forest of replies.
 "
-  (let ((args '("show" "--format=json")))
+  (let ((args '("show" "--format=json" "--use-schema=1")))
     (if notmuch-show-process-crypto
 	(setq args (append args '("--decrypt"))))
     (setq args (append args search-terms))
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 00/10] CLI output versioning
  2012-12-02  2:39 [PATCH 01/10] cli: Framework for structured output versioning Austin Clements
                   ` (8 preceding siblings ...)
  2012-12-02  2:40 ` [PATCH 10/10] emacs: Use --use-schema for show and reply Austin Clements
@ 2012-12-03  0:58 ` Austin Clements
  2012-12-08  9:29   ` Mark Walters
  9 siblings, 1 reply; 17+ messages in thread
From: Austin Clements @ 2012-12-03  0:58 UTC (permalink / raw)
  To: notmuch

(Sorry; I forgot to include a cover letter.)

This series is intended to help with our long-standing output format
versioning issue.  While the JSON format is amenable to extension,
there's still a high barrier to extensions because of the need to
support them going forward, and an even higher barrier to modifications
that break backwards compatibility.  Versioning will make the format
more dynamic, enabling us to easily improve and iterate on it.  It will
also address the slew of confusing bugs that people encounter when they
use a mismatched CLI and front-end.

On IRC we've talking about adding version information to the output
format itself.  This series takes a different and, I think, better
approach: callers request a specific output format version on the
command line.  This allows notmuch to remain backwards compatible with
older format versions when it's easy or necessary.  This also doesn't
require shoehorning a version number into the output, which would be
awkward for both the CLI and the consumer.

I called the argument --use-schema, but I'm open to other suggestions.
--use-schema is technically accurate, but perhaps not as self-describing
as something like --schema-version or --format-version (to parallel
--format).

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 06/10] emacs: Fix bug in resynchronizing after a JSON parse error
  2012-12-02  2:39 ` [PATCH 06/10] emacs: Fix bug in resynchronizing after a JSON parse error Austin Clements
@ 2012-12-08  8:32   ` Mark Walters
  0 siblings, 0 replies; 17+ messages in thread
From: Mark Walters @ 2012-12-08  8:32 UTC (permalink / raw)
  To: Austin Clements, notmuch


This patch looks good to me. Should it go in ahead of the rest of the
series as it essentially a bugfix?

Best wishes

Mark

On Sun, 02 Dec 2012, Austin Clements <amdragon@MIT.EDU> wrote:
> Previously, if the input stream consisted only of an error message,
> notmuch-json-begin-compound would signal a (wrong-type-argument
> number-or-marker-p nil) error when reaching the end of the error
> message.  This happened because notmuch-json-scan-to-value would think
> that it reached a value and put the parser into the 'value state.
> Even after notmuch-json-begin-compound signaled the syntax error, the
> parser would remain in this state and when the resynchronization logic
> reached the end of the buffer, the parser would fail because the
> 'value state indicates that characters are available.
>
> This fixes this problem by restoring the parser's previous state if it
> encounters a syntax error.
> ---
>  emacs/notmuch-lib.el |   22 +++++++++++++---------
>  1 file changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
> index 1d0ec17..9402456 100644
> --- a/emacs/notmuch-lib.el
> +++ b/emacs/notmuch-lib.el
> @@ -474,15 +474,19 @@ Entering JSON objects is currently unimplemented."
>    (with-current-buffer (notmuch-json-buffer jp)
>      ;; Disallow terminators
>      (setf (notmuch-json-allow-term jp) nil)
> -    (or (notmuch-json-scan-to-value jp)
> -	(if (/= (char-after) ?\[)
> -	    (signal 'json-readtable-error (list "expected '['"))
> -	  (forward-char)
> -	  (push ?\] (notmuch-json-term-stack jp))
> -	  ;; Expect a value or terminator next
> -	  (setf (notmuch-json-next jp) 'expect-value
> -		(notmuch-json-allow-term jp) t)
> -	  t))))
> +    ;; Save "next" so we can restore it if there's a syntax error
> +    (let ((saved-next (notmuch-json-next jp)))
> +      (or (notmuch-json-scan-to-value jp)
> +	  (if (/= (char-after) ?\[)
> +	      (progn
> +		(setf (notmuch-json-next jp) saved-next)
> +		(signal 'json-readtable-error (list "expected '['")))
> +	    (forward-char)
> +	    (push ?\] (notmuch-json-term-stack jp))
> +	    ;; Expect a value or terminator next
> +	    (setf (notmuch-json-next jp) 'expect-value
> +		  (notmuch-json-allow-term jp) t)
> +	    t)))))
>  
>  (defun notmuch-json-read (jp)
>    "Parse the value at point in JP's buffer.
> -- 
> 1.7.10.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 07/10] emacs: Use --use-schema for search
  2012-12-02  2:39 ` [PATCH 07/10] emacs: Use --use-schema for search Austin Clements
@ 2012-12-08  8:48   ` Mark Walters
  2012-12-13  1:43     ` Austin Clements
  0 siblings, 1 reply; 17+ messages in thread
From: Mark Walters @ 2012-12-08  8:48 UTC (permalink / raw)
  To: Austin Clements, notmuch

On Sun, 02 Dec 2012, Austin Clements <amdragon@MIT.EDU> wrote:
> We detect the special exit statuses and use these to produce specific
> diagnostic messages.
> ---
>  emacs/notmuch-lib.el |   32 ++++++++++++++++++++++++++++++++
>  emacs/notmuch.el     |   17 +++++++++++++----
>  2 files changed, 45 insertions(+), 4 deletions(-)
>
> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
> index 9402456..49b0da6 100644
> --- a/emacs/notmuch-lib.el
> +++ b/emacs/notmuch-lib.el
> @@ -325,6 +325,38 @@ string), a property list of face attributes, or a list of these."
>  	(put-text-property pos next 'face (cons face cur))
>  	(setq pos next)))))
>  
> +(defun notmuch-pop-up-error (msg)
> +  "Pop up an error buffer displaying MSG."
> +
> +  (let ((buf (get-buffer-create "*Notmuch errors*")))
> +    (with-current-buffer buf
> +      (view-mode)
> +      (let ((inhibit-read-only t))
> +	(erase-buffer)
> +	(insert msg)
> +	(unless (bolp)
> +	  (insert "\n"))
> +	(goto-char (point-min))))
> +    (pop-to-buffer buf)))

I am not sure about the erase-buffer in the above: do we definitely want
to remove all previous error information? For version mismatch possibly
we do but in patch 9 it is done for all show command-line error returns.
Incidentally why does show always pop-up an error but search only for
version-mismatches?

Otherwise this looks good to me (but I am not that familiar with lisp
error handling)

Best wishes

Mark






> +(defun notmuch-version-mismatch-error (exit-status)
> +  "Signal a schema version mismatch error.
> +
> +EXIT-STATUS must be the exit status of the notmuch CLI command,
> +and must have the value 20 or 21.  This function will pop up an
> +error buffer with a descriptive message and signal an error."
> +  (cond ((= exit-status 20)
> +	 (notmuch-pop-up-error "Error: Version mismatch.
> +Emacs requested an older output format than supported by the notmuch CLI.
> +You may need to restart Emacs or upgrade your notmuch Emacs package."))
> +	((= exit-status 21)
> +	 (notmuch-pop-up-error "Error: Version mismatch.
> +Emacs requested a newer output format than supported by the notmuch CLI.
> +You may need to restart Emacs or upgrade your notmuch package."))
> +	(t
> +	 (error "Bad exit status %d" exit-status)))
> +  (error "notmuch CLI version mismatch"))
> +
>  ;; Compatibility functions for versions of emacs before emacs 23.
>  ;;
>  ;; Both functions here were copied from emacs 23 with the following copyright:
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index f9454d8..e1f28ca 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -644,6 +644,7 @@ of the result."
>  	(exit-status (process-exit-status proc))
>  	(never-found-target-thread nil))
>      (when (memq status '(exit signal))
> +      (catch 'return
>  	(kill-buffer (process-get proc 'parse-buf))
>  	(if (buffer-live-p buffer)
>  	    (with-current-buffer buffer
> @@ -655,8 +656,16 @@ of the result."
>  		      (insert "Incomplete search results (search process was killed).\n"))
>  		  (when (eq status 'exit)
>  		    (insert "End of search results.")
> -		    (unless (= exit-status 0)
> -		      (insert (format " (process returned %d)" exit-status)))
> +		    (cond ((or (= exit-status 20) (= exit-status 21))
> +			   (kill-buffer)
> +			   (condition-case err
> +			       (notmuch-version-mismatch-error exit-status)
> +			     ;; Strange things happen when you signal
> +			     ;; an error from a sentinel.
> +			     (error (throw 'return nil))))
> +			  ((/= exit-status 0)
> +			   (insert (format " (process returned %d)"
> +					   exit-status))))
>  		    (insert "\n")
>  		    (if (and atbob
>  			     (not (string= notmuch-search-target-thread "found")))
> @@ -664,7 +673,7 @@ of the result."
>  	      (when (and never-found-target-thread
>  		       notmuch-search-target-line)
>  		  (goto-char (point-min))
> -		  (forward-line (1- notmuch-search-target-line))))))))
> +		  (forward-line (1- notmuch-search-target-line)))))))))
>  
>  (defcustom notmuch-search-line-faces '(("unread" :weight bold)
>  				       ("flagged" :foreground "blue"))
> @@ -938,7 +947,7 @@ Other optional parameters are used as follows:
>  	(let ((proc (start-process
>  		     "notmuch-search" buffer
>  		     notmuch-command "search"
> -		     "--format=json"
> +		     "--format=json" "--use-schema=1"
>  		     (if oldest-first
>  			 "--sort=oldest-first"
>  		       "--sort=newest-first")
> -- 
> 1.7.10.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 00/10] CLI output versioning
  2012-12-03  0:58 ` [PATCH 00/10] CLI output versioning Austin Clements
@ 2012-12-08  9:29   ` Mark Walters
  2012-12-13  1:46     ` Austin Clements
  0 siblings, 1 reply; 17+ messages in thread
From: Mark Walters @ 2012-12-08  9:29 UTC (permalink / raw)
  To: Austin Clements, notmuch


Hi

Overall this series looks good. As we discussed on irc I think i would
prefer global NOTMUCH_SCHEMA_MIN as I am a little worried about these
proliferating (eg if someone decides text output also needs versioning
etc) In addition, if we do find the distinction useful it would be easy
to add at a later date.

One tiny comment on the manpage updates: now that you mention two return
values explicitly should the other possibilities be mentioned too or are
they so obvious it is not needed?

Would it be worth having some emacs test for the error handling? (eg set
notmuch-command to something giving some stderr and an error) Inherently
these code paths won't be tested much so I think tests could be
particularly useful.

Best wishes

Mark






On Mon, 03 Dec 2012, Austin Clements <amdragon@MIT.EDU> wrote:
> (Sorry; I forgot to include a cover letter.)
>
> This series is intended to help with our long-standing output format
> versioning issue.  While the JSON format is amenable to extension,
> there's still a high barrier to extensions because of the need to
> support them going forward, and an even higher barrier to modifications
> that break backwards compatibility.  Versioning will make the format
> more dynamic, enabling us to easily improve and iterate on it.  It will
> also address the slew of confusing bugs that people encounter when they
> use a mismatched CLI and front-end.
>
> On IRC we've talking about adding version information to the output
> format itself.  This series takes a different and, I think, better
> approach: callers request a specific output format version on the
> command line.  This allows notmuch to remain backwards compatible with
> older format versions when it's easy or necessary.  This also doesn't
> require shoehorning a version number into the output, which would be
> awkward for both the CLI and the consumer.
>
> I called the argument --use-schema, but I'm open to other suggestions.
> --use-schema is technically accurate, but perhaps not as self-describing
> as something like --schema-version or --format-version (to parallel
> --format).
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 07/10] emacs: Use --use-schema for search
  2012-12-08  8:48   ` Mark Walters
@ 2012-12-13  1:43     ` Austin Clements
  2012-12-13 11:00       ` Mark Walters
  0 siblings, 1 reply; 17+ messages in thread
From: Austin Clements @ 2012-12-13  1:43 UTC (permalink / raw)
  To: Mark Walters; +Cc: notmuch

Quoth Mark Walters on Dec 08 at  8:48 am:
> On Sun, 02 Dec 2012, Austin Clements <amdragon@MIT.EDU> wrote:
> > We detect the special exit statuses and use these to produce specific
> > diagnostic messages.
> > ---
> >  emacs/notmuch-lib.el |   32 ++++++++++++++++++++++++++++++++
> >  emacs/notmuch.el     |   17 +++++++++++++----
> >  2 files changed, 45 insertions(+), 4 deletions(-)
> >
> > diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
> > index 9402456..49b0da6 100644
> > --- a/emacs/notmuch-lib.el
> > +++ b/emacs/notmuch-lib.el
> > @@ -325,6 +325,38 @@ string), a property list of face attributes, or a list of these."
> >  	(put-text-property pos next 'face (cons face cur))
> >  	(setq pos next)))))
> >  
> > +(defun notmuch-pop-up-error (msg)
> > +  "Pop up an error buffer displaying MSG."
> > +
> > +  (let ((buf (get-buffer-create "*Notmuch errors*")))
> > +    (with-current-buffer buf
> > +      (view-mode)
> > +      (let ((inhibit-read-only t))
> > +	(erase-buffer)
> > +	(insert msg)
> > +	(unless (bolp)
> > +	  (insert "\n"))
> > +	(goto-char (point-min))))
> > +    (pop-to-buffer buf)))
> 
> I am not sure about the erase-buffer in the above: do we definitely want
> to remove all previous error information? For version mismatch possibly
> we do but in patch 9 it is done for all show command-line error returns.

Why wouldn't we want to use a cleared buffer in all cases?
notmuch-pop-up-error is only ever used when a command terminates, so
there's no danger of us clearing errors that the user hasn't seen yet.

> Incidentally why does show always pop-up an error but search only for
> version-mismatches?

Historical reasons, I suppose.  I was maintaining the status quo for
search (which already had *some* error handling), but there was no
error-handling status quo for show.  Probably search should be more
vocal when the command fails.

> Otherwise this looks good to me (but I am not that familiar with lisp
> error handling)
> 
> Best wishes
> 
> Mark
> 
> 
> 
> 
> 
> 
> > +(defun notmuch-version-mismatch-error (exit-status)
> > +  "Signal a schema version mismatch error.
> > +
> > +EXIT-STATUS must be the exit status of the notmuch CLI command,
> > +and must have the value 20 or 21.  This function will pop up an
> > +error buffer with a descriptive message and signal an error."
> > +  (cond ((= exit-status 20)
> > +	 (notmuch-pop-up-error "Error: Version mismatch.
> > +Emacs requested an older output format than supported by the notmuch CLI.
> > +You may need to restart Emacs or upgrade your notmuch Emacs package."))
> > +	((= exit-status 21)
> > +	 (notmuch-pop-up-error "Error: Version mismatch.
> > +Emacs requested a newer output format than supported by the notmuch CLI.
> > +You may need to restart Emacs or upgrade your notmuch package."))
> > +	(t
> > +	 (error "Bad exit status %d" exit-status)))
> > +  (error "notmuch CLI version mismatch"))
> > +
> >  ;; Compatibility functions for versions of emacs before emacs 23.
> >  ;;
> >  ;; Both functions here were copied from emacs 23 with the following copyright:
> > diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> > index f9454d8..e1f28ca 100644
> > --- a/emacs/notmuch.el
> > +++ b/emacs/notmuch.el
> > @@ -644,6 +644,7 @@ of the result."
> >  	(exit-status (process-exit-status proc))
> >  	(never-found-target-thread nil))
> >      (when (memq status '(exit signal))
> > +      (catch 'return
> >  	(kill-buffer (process-get proc 'parse-buf))
> >  	(if (buffer-live-p buffer)
> >  	    (with-current-buffer buffer
> > @@ -655,8 +656,16 @@ of the result."
> >  		      (insert "Incomplete search results (search process was killed).\n"))
> >  		  (when (eq status 'exit)
> >  		    (insert "End of search results.")
> > -		    (unless (= exit-status 0)
> > -		      (insert (format " (process returned %d)" exit-status)))
> > +		    (cond ((or (= exit-status 20) (= exit-status 21))
> > +			   (kill-buffer)
> > +			   (condition-case err
> > +			       (notmuch-version-mismatch-error exit-status)
> > +			     ;; Strange things happen when you signal
> > +			     ;; an error from a sentinel.
> > +			     (error (throw 'return nil))))
> > +			  ((/= exit-status 0)
> > +			   (insert (format " (process returned %d)"
> > +					   exit-status))))
> >  		    (insert "\n")
> >  		    (if (and atbob
> >  			     (not (string= notmuch-search-target-thread "found")))
> > @@ -664,7 +673,7 @@ of the result."
> >  	      (when (and never-found-target-thread
> >  		       notmuch-search-target-line)
> >  		  (goto-char (point-min))
> > -		  (forward-line (1- notmuch-search-target-line))))))))
> > +		  (forward-line (1- notmuch-search-target-line)))))))))
> >  
> >  (defcustom notmuch-search-line-faces '(("unread" :weight bold)
> >  				       ("flagged" :foreground "blue"))
> > @@ -938,7 +947,7 @@ Other optional parameters are used as follows:
> >  	(let ((proc (start-process
> >  		     "notmuch-search" buffer
> >  		     notmuch-command "search"
> > -		     "--format=json"
> > +		     "--format=json" "--use-schema=1"
> >  		     (if oldest-first
> >  			 "--sort=oldest-first"
> >  		       "--sort=newest-first")
> >
> > _______________________________________________
> > notmuch mailing list
> > notmuch@notmuchmail.org
> > http://notmuchmail.org/mailman/listinfo/notmuch

-- 
Austin Clements                                      MIT/'06/PhD/CSAIL
amdragon@mit.edu                           http://web.mit.edu/amdragon
       Somewhere in the dream we call reality you will find me,
              searching for the reality we call dreams.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 00/10] CLI output versioning
  2012-12-08  9:29   ` Mark Walters
@ 2012-12-13  1:46     ` Austin Clements
  0 siblings, 0 replies; 17+ messages in thread
From: Austin Clements @ 2012-12-13  1:46 UTC (permalink / raw)
  To: Mark Walters; +Cc: notmuch

Quoth Mark Walters on Dec 08 at  9:29 am:
> 
> Hi
> 
> Overall this series looks good. As we discussed on irc I think i would
> prefer global NOTMUCH_SCHEMA_MIN as I am a little worried about these
> proliferating (eg if someone decides text output also needs versioning
> etc) In addition, if we do find the distinction useful it would be easy
> to add at a later date.

Will do.

> One tiny comment on the manpage updates: now that you mention two return
> values explicitly should the other possibilities be mentioned too or are
> they so obvious it is not needed?

I thought about this, but couldn't figure out what to say for exit
status 1 other than "Unspecified error".  Hence I carefully worded the
man page to say *special* exit status codes.

> Would it be worth having some emacs test for the error handling? (eg set
> notmuch-command to something giving some stderr and an error) Inherently
> these code paths won't be tested much so I think tests could be
> particularly useful.

Good idea.

> Best wishes
> 
> Mark
> 
> 
> 
> 
> 
> 
> On Mon, 03 Dec 2012, Austin Clements <amdragon@MIT.EDU> wrote:
> > (Sorry; I forgot to include a cover letter.)
> >
> > This series is intended to help with our long-standing output format
> > versioning issue.  While the JSON format is amenable to extension,
> > there's still a high barrier to extensions because of the need to
> > support them going forward, and an even higher barrier to modifications
> > that break backwards compatibility.  Versioning will make the format
> > more dynamic, enabling us to easily improve and iterate on it.  It will
> > also address the slew of confusing bugs that people encounter when they
> > use a mismatched CLI and front-end.
> >
> > On IRC we've talking about adding version information to the output
> > format itself.  This series takes a different and, I think, better
> > approach: callers request a specific output format version on the
> > command line.  This allows notmuch to remain backwards compatible with
> > older format versions when it's easy or necessary.  This also doesn't
> > require shoehorning a version number into the output, which would be
> > awkward for both the CLI and the consumer.
> >
> > I called the argument --use-schema, but I'm open to other suggestions.
> > --use-schema is technically accurate, but perhaps not as self-describing
> > as something like --schema-version or --format-version (to parallel
> > --format).
> > _______________________________________________
> > notmuch mailing list
> > notmuch@notmuchmail.org
> > http://notmuchmail.org/mailman/listinfo/notmuch

-- 
Austin Clements                                      MIT/'06/PhD/CSAIL
amdragon@mit.edu                           http://web.mit.edu/amdragon
       Somewhere in the dream we call reality you will find me,
              searching for the reality we call dreams.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 07/10] emacs: Use --use-schema for search
  2012-12-13  1:43     ` Austin Clements
@ 2012-12-13 11:00       ` Mark Walters
  0 siblings, 0 replies; 17+ messages in thread
From: Mark Walters @ 2012-12-13 11:00 UTC (permalink / raw)
  To: Austin Clements, notmuch

On Thu, 13 Dec 2012, Austin Clements <amdragon@MIT.EDU> wrote:
> Quoth Mark Walters on Dec 08 at  8:48 am:
>> On Sun, 02 Dec 2012, Austin Clements <amdragon@MIT.EDU> wrote:
>> > We detect the special exit statuses and use these to produce specific
>> > diagnostic messages.
>> > ---
>> >  emacs/notmuch-lib.el |   32 ++++++++++++++++++++++++++++++++
>> >  emacs/notmuch.el     |   17 +++++++++++++----
>> >  2 files changed, 45 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
>> > index 9402456..49b0da6 100644
>> > --- a/emacs/notmuch-lib.el
>> > +++ b/emacs/notmuch-lib.el
>> > @@ -325,6 +325,38 @@ string), a property list of face attributes, or a list of these."
>> >  	(put-text-property pos next 'face (cons face cur))
>> >  	(setq pos next)))))
>> >  
>> > +(defun notmuch-pop-up-error (msg)
>> > +  "Pop up an error buffer displaying MSG."
>> > +
>> > +  (let ((buf (get-buffer-create "*Notmuch errors*")))
>> > +    (with-current-buffer buf
>> > +      (view-mode)
>> > +      (let ((inhibit-read-only t))
>> > +	(erase-buffer)
>> > +	(insert msg)
>> > +	(unless (bolp)
>> > +	  (insert "\n"))
>> > +	(goto-char (point-min))))
>> > +    (pop-to-buffer buf)))
>> 
>> I am not sure about the erase-buffer in the above: do we definitely want
>> to remove all previous error information? For version mismatch possibly
>> we do but in patch 9 it is done for all show command-line error returns.
>
> Why wouldn't we want to use a cleared buffer in all cases?
> notmuch-pop-up-error is only ever used when a command terminates, so
> there's no danger of us clearing errors that the user hasn't seen yet.

Just to make sure I understand this: whenever an error occurs the error
popup appears so we see the message. Then the next error can safely
erase buffer before showing the new error message?

>> Incidentally why does show always pop-up an error but search only for
>> version-mismatches?
>
> Historical reasons, I suppose.  I was maintaining the status quo for
> search (which already had *some* error handling), but there was no
> error-handling status quo for show.  Probably search should be more
> vocal when the command fails.

Should notmuch-call-notmuch-process also be switched to this framework?
In particular that also puts errors in "*Notmuch errors*" so my earlier
concern may be valid in this case.

Best wishes

Mark
>
>> Otherwise this looks good to me (but I am not that familiar with lisp
>> error handling)
>> 
>> Best wishes
>> 
>> Mark
>> 
>> 
>> 
>> 
>> 
>> 
>> > +(defun notmuch-version-mismatch-error (exit-status)
>> > +  "Signal a schema version mismatch error.
>> > +
>> > +EXIT-STATUS must be the exit status of the notmuch CLI command,
>> > +and must have the value 20 or 21.  This function will pop up an
>> > +error buffer with a descriptive message and signal an error."
>> > +  (cond ((= exit-status 20)
>> > +	 (notmuch-pop-up-error "Error: Version mismatch.
>> > +Emacs requested an older output format than supported by the notmuch CLI.
>> > +You may need to restart Emacs or upgrade your notmuch Emacs package."))
>> > +	((= exit-status 21)
>> > +	 (notmuch-pop-up-error "Error: Version mismatch.
>> > +Emacs requested a newer output format than supported by the notmuch CLI.
>> > +You may need to restart Emacs or upgrade your notmuch package."))
>> > +	(t
>> > +	 (error "Bad exit status %d" exit-status)))
>> > +  (error "notmuch CLI version mismatch"))
>> > +
>> >  ;; Compatibility functions for versions of emacs before emacs 23.
>> >  ;;
>> >  ;; Both functions here were copied from emacs 23 with the following copyright:
>> > diff --git a/emacs/notmuch.el b/emacs/notmuch.el
>> > index f9454d8..e1f28ca 100644
>> > --- a/emacs/notmuch.el
>> > +++ b/emacs/notmuch.el
>> > @@ -644,6 +644,7 @@ of the result."
>> >  	(exit-status (process-exit-status proc))
>> >  	(never-found-target-thread nil))
>> >      (when (memq status '(exit signal))
>> > +      (catch 'return
>> >  	(kill-buffer (process-get proc 'parse-buf))
>> >  	(if (buffer-live-p buffer)
>> >  	    (with-current-buffer buffer
>> > @@ -655,8 +656,16 @@ of the result."
>> >  		      (insert "Incomplete search results (search process was killed).\n"))
>> >  		  (when (eq status 'exit)
>> >  		    (insert "End of search results.")
>> > -		    (unless (= exit-status 0)
>> > -		      (insert (format " (process returned %d)" exit-status)))
>> > +		    (cond ((or (= exit-status 20) (= exit-status 21))
>> > +			   (kill-buffer)
>> > +			   (condition-case err
>> > +			       (notmuch-version-mismatch-error exit-status)
>> > +			     ;; Strange things happen when you signal
>> > +			     ;; an error from a sentinel.
>> > +			     (error (throw 'return nil))))
>> > +			  ((/= exit-status 0)
>> > +			   (insert (format " (process returned %d)"
>> > +					   exit-status))))
>> >  		    (insert "\n")
>> >  		    (if (and atbob
>> >  			     (not (string= notmuch-search-target-thread "found")))
>> > @@ -664,7 +673,7 @@ of the result."
>> >  	      (when (and never-found-target-thread
>> >  		       notmuch-search-target-line)
>> >  		  (goto-char (point-min))
>> > -		  (forward-line (1- notmuch-search-target-line))))))))
>> > +		  (forward-line (1- notmuch-search-target-line)))))))))
>> >  
>> >  (defcustom notmuch-search-line-faces '(("unread" :weight bold)
>> >  				       ("flagged" :foreground "blue"))
>> > @@ -938,7 +947,7 @@ Other optional parameters are used as follows:
>> >  	(let ((proc (start-process
>> >  		     "notmuch-search" buffer
>> >  		     notmuch-command "search"
>> > -		     "--format=json"
>> > +		     "--format=json" "--use-schema=1"
>> >  		     (if oldest-first
>> >  			 "--sort=oldest-first"
>> >  		       "--sort=newest-first")
>> >
>> > _______________________________________________
>> > notmuch mailing list
>> > notmuch@notmuchmail.org
>> > http://notmuchmail.org/mailman/listinfo/notmuch
>
> -- 
> Austin Clements                                      MIT/'06/PhD/CSAIL
> amdragon@mit.edu                           http://web.mit.edu/amdragon
>        Somewhere in the dream we call reality you will find me,
>               searching for the reality we call dreams.

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2012-12-13 11:00 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-12-02  2:39 [PATCH 01/10] cli: Framework for structured output versioning Austin Clements
2012-12-02  2:39 ` [PATCH 02/10] search: Support --use-schema Austin Clements
2012-12-02  2:39 ` [PATCH 03/10] show: " Austin Clements
2012-12-02  2:39 ` [PATCH 04/10] reply: " Austin Clements
2012-12-02  2:39 ` [PATCH 05/10] test: Sanity tests for --use-schema argument Austin Clements
2012-12-02  2:39 ` [PATCH 06/10] emacs: Fix bug in resynchronizing after a JSON parse error Austin Clements
2012-12-08  8:32   ` Mark Walters
2012-12-02  2:39 ` [PATCH 07/10] emacs: Use --use-schema for search Austin Clements
2012-12-08  8:48   ` Mark Walters
2012-12-13  1:43     ` Austin Clements
2012-12-13 11:00       ` Mark Walters
2012-12-02  2:40 ` [PATCH 08/10] emacs: Factor out synchronous notmuch JSON invocations Austin Clements
2012-12-02  2:40 ` [PATCH 09/10] emacs: Improve error handling for notmuch-call-notmuch-json Austin Clements
2012-12-02  2:40 ` [PATCH 10/10] emacs: Use --use-schema for show and reply Austin Clements
2012-12-03  0:58 ` [PATCH 00/10] CLI output versioning Austin Clements
2012-12-08  9:29   ` Mark Walters
2012-12-13  1:46     ` Austin Clements

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).