Re: [PATCH 0/7] Structed output versioning support
LGTM +1 Best wishes Mark On Sun, 16 Dec 2012, Austin Clements wrote: > This series obsoletes [0] and must be applied on top of the Emacs CLI > error handling series [1]. This is much simpler than the original > series because it no longer includes the Emacs CLI error handling. > This also switches to a CLI-wide minimum version instead of a > per-command version, prints a warning if an old but still supported > version is requested to ease client maintenance, and renames > --use-schema to --format-version (which parallels --format better and > simplifies error text). > > [0] id:1354416002-3557-1-git-send-email-amdra...@mit.edu > [1] id:1355601860-30121-1-git-send-email-amdra...@mit.edu ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[Patch v7 05/14] quote_and_decode_query: new function to quote hex-decoded queries
Jani Nikula writes: > strtok_len() will eat all the leading delimiters at each call, and will > not return a zero-length token if you have multiple consecutive > delimiters. Which means you may end up losing stuff here. Right, I think for ':' it does matter, but it should be fixable with a a little loop to copy ':'s to the query string after the (possibly quoted) token.
[PATCH 7/7] emacs: Use --format-version for search, show, and reply
---
emacs/notmuch-mua.el |2 +-
emacs/notmuch-query.el |2 +-
emacs/notmuch.el |6 +-
test/emacs |2 +-
test/emacs-show|2 +-
5 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index ac2d29e..24eebff 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" "--format-version=1"))
reply
original)
(when notmuch-show-process-crypto
diff --git a/emacs/notmuch-query.el b/emacs/notmuch-query.el
index e7e3520..6e9f406 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" "--format-version=1")))
(if notmuch-show-process-crypto
(setq args (append args '("--decrypt"
(setq args (append args search-terms))
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index b0fd387..63387a2 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -650,6 +650,10 @@ of the result."
(insert "Incomplete search results (search process was
killed).\n"))
(when (eq status 'exit)
(insert "End of search results.\n")
+ ;; For version mismatch, there's no point in
+ ;; showing the search buffer
+ (when (or (= exit-status 20) (= exit-status 21))
+ (kill-buffer))
(condition-case nil
(notmuch-check-async-exit-status proc msg)
;; Suppress the error signal since strange
@@ -935,7 +939,7 @@ Other optional parameters are used as follows:
(let ((proc (start-process
"notmuch-search" buffer
notmuch-command "search"
-"--format=json"
+"--format=json" "--format-version=1"
(if oldest-first
"--sort=oldest-first"
"--sort=newest-first")
diff --git a/test/emacs b/test/emacs
index 5067d67..6b18968 100755
--- a/test/emacs
+++ b/test/emacs
@@ -873,7 +873,7 @@ This is output
Error: Unexpected output from notmuch search:
This is an error
End of search results.
-Error invoking notmuch. $PWD/notmuch_fail search --format=json
--sort=newest-first tag:inbox exited with status 1."
+Error invoking notmuch. $PWD/notmuch_fail search --format=json
--format-version=1 --sort=newest-first tag:inbox exited with status 1."
test_done
diff --git a/test/emacs-show b/test/emacs-show
index 40fab61..ebf530b 100755
--- a/test/emacs-show
+++ b/test/emacs-show
@@ -178,7 +178,7 @@ test_emacs "(let ((notmuch-command \"$PWD/notmuch_fail\"))
(with-current-buffer \"*Notmuch errors*\"
(test-output \"ERROR\")))"
test_expect_equal "$(cat OUTPUT ERROR)" "\
-Error invoking notmuch. $PWD/notmuch_fail show --format=json --exclude=false
' * ' exited with status 1.
+Error invoking notmuch. $PWD/notmuch_fail show --format=json
--format-version=1 --exclude=false ' * ' exited with status 1.
Error:
This is an error
Output:
--
1.7.10.4
[PATCH 6/7] emacs: Special handling for version mismatch errors
Since Emacs has more semantic information, we suppress the generic format version error from the CLI and give a more informative error. --- emacs/notmuch-lib.el | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el index 8f84087..9ec30ea 100644 --- a/emacs/notmuch-lib.el +++ b/emacs/notmuch-lib.el @@ -360,9 +360,18 @@ giving the output of command. ERR-FILE, if provided, is the name of a file containing the error output of command. OUTPUT and the contents of ERR-FILE will be included in the error message." - ;; This is implemented as a cond to make it easy to expand. (cond ((eq exit-status 0) t) + ((eq 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.") +(error "notmuch CLI version mismatch")) + ((eq 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.") +(error "notmuch CLI version mismatch")) (t (notmuch-pop-up-error (concat -- 1.7.10.4
[PATCH 5/7] test: Sanity tests for the --format-version argument
---
test/json |6 ++
1 file changed, 6 insertions(+)
diff --git a/test/json b/test/json
index bfafd55..8a01117 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 "Format version: too low" \
+"notmuch search --format-version=0 \\*"
+
+test_expect_code 21 "Format version: too high" \
+"notmuch search --format-version=999 \\*"
+
test_done
--
1.7.10.4
[PATCH 4/7] reply: Support --format-version
---
man/man1/notmuch-reply.1 | 21 +
notmuch-reply.c |3 +++
2 files changed, 24 insertions(+)
diff --git a/man/man1/notmuch-reply.1 b/man/man1/notmuch-reply.1
index fa04c9e..9fa1956 100644
--- a/man/man1/notmuch-reply.1
+++ b/man/man1/notmuch-reply.1
@@ -57,6 +57,16 @@ to create a reply message intelligently.
Only produces In\-Reply\-To, References, To, Cc, and Bcc headers.
.RE
.RE
+
+.RS
+.TP 4
+.BR \-\-format-version=N
+
+Use the specified structured output format 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 )
@@ -99,6 +109,17 @@ formats do not.
.RE
.RE
+.SH EXIT STATUS
+
+This command supports the following special exit status codes
+
+.TP
+.B 20
+The requested format version is too old.
+.TP
+.B 21
+The requested format version is too new.
+
.SH SEE ALSO
\fBnotmuch\fR(1), \fBnotmuch-config\fR(1), \fBnotmuch-count\fR(1),
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 720749d..22c58ff 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -733,6 +733,7 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
{ "sexp", FORMAT_SEXP },
{ "headers-only", FORMAT_HEADERS_ONLY },
{ 0, 0 } } },
+ { NOTMUCH_OPT_INT, ¬much_format_version, "format-version", 0, 0 },
{ NOTMUCH_OPT_KEYWORD, &reply_all, "reply-to", 'r',
(notmuch_keyword_t []){ { "all", TRUE },
{ "sender", FALSE },
@@ -759,6 +760,8 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
reply_format_func = notmuch_reply_format_default;
}
+notmuch_exit_if_unsupported_format ();
+
config = notmuch_config_open (ctx, NULL, NULL);
if (config == NULL)
return 1;
--
1.7.10.4
[PATCH 3/7] show: Support --format-version
---
man/man1/notmuch-show.1 | 20
notmuch-show.c |3 +++
2 files changed, 23 insertions(+)
diff --git a/man/man1/notmuch-show.1 b/man/man1/notmuch-show.1
index bd41c48..44cec46 100644
--- a/man/man1/notmuch-show.1
+++ b/man/man1/notmuch-show.1
@@ -128,6 +128,15 @@ message.
.RS 4
.TP 4
+.BR \-\-format-version=N
+
+Use the specified structured output format 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
@@ -201,6 +210,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 format version is too old.
+.TP
+.B 21
+The requested format version is too new.
+
.SH SEE ALSO
\fBnotmuch\fR(1), \fBnotmuch-config\fR(1), \fBnotmuch-count\fR(1),
diff --git a/notmuch-show.c b/notmuch-show.c
index a83fef9..39faf21 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -1066,6 +1066,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, ¬much_format_version, "format-version", 0, 0 },
{ NOTMUCH_OPT_KEYWORD, &exclude, "exclude", 'x',
(notmuch_keyword_t []){ { "true", EXCLUDE_TRUE },
{ "false", EXCLUDE_FALSE },
@@ -1128,6 +1129,8 @@ notmuch_show_command (void *ctx, unused (int argc),
unused (char *argv[]))
break;
}
+notmuch_exit_if_unsupported_format ();
+
/* Default is entire-thread = FALSE except for format=json and
* format=sexp. */
if (entire_thread == ENTIRE_THREAD_DEFAULT) {
--
1.7.10.4
[PATCH 2/7] search: Support --format-version
---
man/man1/notmuch-search.1 | 20
notmuch-search.c |3 +++
2 files changed, 23 insertions(+)
diff --git a/man/man1/notmuch-search.1 b/man/man1/notmuch-search.1
index 0aff348..5c771fa 100644
--- a/man/man1/notmuch-search.1
+++ b/man/man1/notmuch-search.1
@@ -32,6 +32,15 @@ Presents the results in either JSON, S-Expressions or
plain-text (default).
.RS 4
.TP 4
+.BR \-\-format-version=N
+
+Use the specified structured output format 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
@@ -126,6 +135,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 format version is too old.
+.TP
+.B 21
+The requested format version is too new.
+
.SH SEE ALSO
\fBnotmuch\fR(1), \fBnotmuch-config\fR(1), \fBnotmuch-count\fR(1),
diff --git a/notmuch-search.c b/notmuch-search.c
index 6218622..7704915 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -318,6 +318,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
{ "sexp", NOTMUCH_FORMAT_SEXP },
{ "text", NOTMUCH_FORMAT_TEXT },
{ 0, 0 } } },
+ { NOTMUCH_OPT_INT, ¬much_format_version, "format-version", 0, 0 },
{ NOTMUCH_OPT_KEYWORD, &output, "output", 'o',
(notmuch_keyword_t []){ { "summary", OUTPUT_SUMMARY },
{ "threads", OUTPUT_THREADS },
@@ -356,6 +357,8 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
INTERNAL_ERROR("no output format selected");
}
+notmuch_exit_if_unsupported_format ();
+
config = notmuch_config_open (ctx, NULL, NULL);
if (config == NULL)
return 1;
--
1.7.10.4
[PATCH 1/7] cli: Framework for structured output versioning
Currently there is a period of pain whenever we make
backward-incompatible changes to the structured output format, which
discourages not only backward-incompatible improvements to the format,
but also backwards-compatible additions that may not be "perfect". In
the end, these problems limit experimentation and innovation.
This series of patches introduces a way for CLI callers to request a
specific format 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 format
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.
---
devel/schemata |2 ++
notmuch-client.h | 45 +
notmuch.c| 32
3 files changed, 79 insertions(+)
diff --git a/devel/schemata b/devel/schemata
index d1ab983..292f287 100644
--- a/devel/schemata
+++ b/devel/schemata
@@ -14,6 +14,8 @@ are interleaved. Keys are printed as keywords (symbols
preceded by a
colon), e.g. (:id "123" :time 54321 :from "foobar"). Null is printed as
nil, true as t and false as nil.
+This is version 1 of the structured output format.
+
Common non-terminals
diff --git a/notmuch-client.h b/notmuch-client.h
index 1c336dc..d7b352e 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -117,6 +117,51 @@ chomp_newline (char *str)
str[strlen(str)-1] = '\0';
}
+/* Exit status code indicating the requested format version is too old
+ * (support for that version has been dropped). CLI code should use
+ * notmuch_exit_if_unsupported_format rather than directly exiting
+ * with this code.
+ */
+#define NOTMUCH_EXIT_FORMAT_TOO_OLD 20
+/* Exit status code indicating the requested format version is newer
+ * than the version supported by the CLI. CLI code should use
+ * notmuch_exit_if_unsupported_format rather than directly exiting
+ * with this code.
+ */
+#define NOTMUCH_EXIT_FORMAT_TOO_NEW 21
+
+/* The current structured output format version. Requests for format
+ * 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 (required) map fields can be added without increasing
+ * this.
+ */
+#define NOTMUCH_FORMAT_CUR 1
+/* The minimum supported structured output format version. Requests
+ * for format versions below this will return an error. */
+#define NOTMUCH_FORMAT_MIN 1
+
+/* The output format version requested by the caller on the command
+ * line. If no format version is requested, this will be set to
+ * NOTMUCH_FORMAT_CUR. Even though the command-line option is
+ * per-command, this is global because commands can share structured
+ * output code.
+ */
+extern int notmuch_format_version;
+
+/* Commands that support structured output should support the
+ * following argument
+ * { NOTMUCH_OPT_INT, ¬much_format_version, "format-version", 0, 0 }
+ * and should invoke notmuch_exit_if_unsupported_format to check the
+ * requested version. If notmuch_format_version is outside the
+ * supported range, this will print a detailed diagnostic message for
+ * the user and exit with NOTMUCH_EXIT_FORMAT_TOO_{OLD,NEW} to inform
+ * the invoking program of the problem.
+ */
+void
+notmuch_exit_if_unsupported_format (void);
+
notmuch_crypto_context_t *
notmuch_crypto_get_context (notmuch_crypto_t *crypto, const char *protocol);
diff --git a/notmuch.c b/notmuch.c
index 4ff66e3..9516dfb 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -82,6 +82,8 @@ static command_t commands[] = {
"This message, or more detailed help for the named command." }
};
+int notmuch_format_version;
+
static void
usage (FILE *out)
{
@@ -109,6 +111,33 @@ usage (FILE *out)
"and \"notmuch help search-terms\" for the common search-terms
syntax.\n\n");
}
+void
+notmuch_exit_if_unsupported_format (void)
+{
+if (notmuch_format_version > NOTMUCH_FORMAT_CUR) {
+ fprintf (stderr, "\
+A caller requested output format version %d, but the installed notmuch\n\
+CLI only supports up to format version %d. You may need to upgrade your\n\
+notmuch CLI.\n",
+notmuch_format_version, NOTMUCH_FORMAT_CUR);
+ exit (NOTMUCH_EXIT_FORMAT_TOO_NEW);
+} else if (notmuch_format_version < NOTMUCH_FORMAT_MIN) {
+ fprintf (stderr, "\
+A caller requested output format version %d, which is no longer supported\n\
+by the notmuch CLI (it requires at least version %d). You may need to\n\
+upgrade your notmuch front-end.\n",
+
[PATCH 0/7] Structed output versioning support
This series obsoletes [0] and must be applied on top of the Emacs CLI error handling series [1]. This is much simpler than the original series because it no longer includes the Emacs CLI error handling. This also switches to a CLI-wide minimum version instead of a per-command version, prints a warning if an old but still supported version is requested to ease client maintenance, and renames --use-schema to --format-version (which parallels --format better and simplifies error text). [0] id:1354416002-3557-1-git-send-email-amdragon at mit.edu [1] id:1355601860-30121-1-git-send-email-amdragon at mit.edu
[PATCH v5 0/4] indicate length of omitted body content
On Sat, 15 Dec 2012 08:45:33 +, Mark Walters wrote: > > This version looks good to me with one very minor comments: > > Perhaps the name format-omitted-part-meta should include sprinter (eg > format-omitted-part-meta-sprinter)? Seems so. I'll roll up another series if that's necessary. Peter
[PATCH] cli: add support for batch tagging operations to "notmuch tag"
From: Jani Nikula
Add support for batch tagging operations through stdin to "notmuch
tag". This can be enabled with the new --batch command line option to
"notmuch tag". The input must consist of lines of the format:
+|- [...] [--] [...]
Each line is interpreted similarly to "notmuch tag" command line
arguments. The delimiter is one or more spaces ' '. Any characters in
and MAY be hex encoded with %NN where NN is the
hexadecimal value of the character. Any ' ' and '%' characters in
and MUST be hex encoded (using %20 and %25,
respectively). Any characters that are not part of or
MUST NOT be hex encoded.
Leading and trailing space ' ' is ignored. Empty lines and lines
beginning with '#' are ignored.
Signed-off-by: Jani Nikula
Hacked-like-crazy-by: David Bremner
---
I missed somehow the change catch non-zero return from tag_query in tag_file.
This version is slightly cleaned up.
notmuch-tag.c | 91 -
1 file changed, 83 insertions(+), 8 deletions(-)
diff --git a/notmuch-tag.c b/notmuch-tag.c
index 13f2268..44b5bb4 100644
--- a/notmuch-tag.c
+++ b/notmuch-tag.c
@@ -130,6 +130,46 @@ tag_query (void *ctx, notmuch_database_t *notmuch, const
char *query_string,
return interrupted;
}
+static int
+tag_file (void *ctx, notmuch_database_t *notmuch, tag_op_flag_t flags,
+ FILE *input)
+{
+char *line = NULL;
+char *query_string = NULL;
+size_t line_size = 0;
+ssize_t line_len;
+int ret = 0;
+tag_op_list_t *tag_ops;
+
+tag_ops = tag_op_list_create (ctx);
+if (tag_ops == NULL) {
+ fprintf (stderr, "Out of memory.\n");
+ return 1;
+}
+
+while ((line_len = getline (&line, &line_size, input)) != -1 &&
+ ! interrupted) {
+
+ ret = parse_tag_line (ctx, line, TAG_FLAG_NONE,
+ &query_string, tag_ops);
+
+ if (ret > 0)
+ continue;
+
+ if (ret < 0)
+ break;
+
+ ret = tag_query (ctx, notmuch, query_string, tag_ops, flags);
+ if (ret)
+ break;
+}
+
+if (line)
+ free (line);
+
+return ret;
+}
+
int
notmuch_tag_command (void *ctx, int argc, char *argv[])
{
@@ -139,6 +179,10 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
notmuch_database_t *notmuch;
struct sigaction action;
tag_op_flag_t tag_flags = TAG_FLAG_NONE;
+notmuch_bool_t batch = FALSE;
+FILE *input = stdin;
+char *input_file_name = NULL;
+int opt_index;
int ret = 0;
/* Setup our handler for SIGINT */
@@ -148,15 +192,43 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
action.sa_flags = SA_RESTART;
sigaction (SIGINT, &action, NULL);
-tag_ops = tag_op_list_create (ctx);
-if (tag_ops == NULL) {
- fprintf (stderr, "Out of memory.\n");
+notmuch_opt_desc_t options[] = {
+ { NOTMUCH_OPT_BOOLEAN, &batch, "batch", 0, 0 },
+ { NOTMUCH_OPT_STRING, &input_file_name, "input", 'i', 0 },
+ { 0, 0, 0, 0, 0 }
+};
+
+opt_index = parse_arguments (argc, argv, options, 1);
+if (opt_index < 0)
return 1;
+
+if (input_file_name) {
+ batch = TRUE;
+ input = fopen (input_file_name, "r");
+ if (input == NULL) {
+ fprintf (stderr, "Error opening %s for reading: %s\n",
+input_file_name, strerror (errno));
+ return 1;
+ }
}
-if (parse_tag_command_line (ctx, argc - 1, argv + 1,
- &query_string, tag_ops))
- return 1;
+if (batch) {
+ if (opt_index != argc) {
+ fprintf (stderr, "Can't specify both cmdline and stdin!\n");
+ return 1;
+ }
+} else {
+
+ tag_ops = tag_op_list_create (ctx);
+ if (tag_ops == NULL) {
+ fprintf (stderr, "Out of memory.\n");
+ return 1;
+ }
+
+ if (parse_tag_command_line (ctx, argc - opt_index, argv + opt_index,
+ &query_string, tag_ops))
+ return 1;
+}
config = notmuch_config_open (ctx, NULL, NULL);
if (config == NULL)
@@ -169,9 +241,12 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
if (notmuch_config_get_maildir_synchronize_flags (config))
tag_flags |= TAG_FLAG_MAILDIR_SYNC;
-ret = tag_query (ctx, notmuch, query_string, tag_ops, tag_flags);
+if (batch)
+ ret = tag_file (ctx, notmuch, tag_flags, input);
+else
+ ret = tag_query (ctx, notmuch, query_string, tag_ops, tag_flags);
notmuch_database_destroy (notmuch);
-return ret;
+return ret || interrupted;
}
--
1.7.10.4
Re: [PATCH 0/7] Structed output versioning support
On Sun, Dec 16 2012, Austin Clements wrote: > This series obsoletes [0] and must be applied on top of the Emacs CLI > error handling series [1]. This is much simpler than the original > series because it no longer includes the Emacs CLI error handling. > This also switches to a CLI-wide minimum version instead of a > per-command version, prints a warning if an old but still supported > version is requested to ease client maintenance, and renames > --use-schema to --format-version (which parallels --format better and > simplifies error text). LGTM. > [0] id:1354416002-3557-1-git-send-email-amdra...@mit.edu > [1] id:1355601860-30121-1-git-send-email-amdra...@mit.edu Tomi ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [Patch v7 05/14] quote_and_decode_query: new function to quote hex-decoded queries
Jani Nikula writes: > strtok_len() will eat all the leading delimiters at each call, and will > not return a zero-length token if you have multiple consecutive > delimiters. Which means you may end up losing stuff here. Right, I think for ':' it does matter, but it should be fixable with a a little loop to copy ':'s to the query string after the (possibly quoted) token. ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 3/7] show: Support --format-version
---
man/man1/notmuch-show.1 | 20
notmuch-show.c |3 +++
2 files changed, 23 insertions(+)
diff --git a/man/man1/notmuch-show.1 b/man/man1/notmuch-show.1
index bd41c48..44cec46 100644
--- a/man/man1/notmuch-show.1
+++ b/man/man1/notmuch-show.1
@@ -128,6 +128,15 @@ message.
.RS 4
.TP 4
+.BR \-\-format-version=N
+
+Use the specified structured output format 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
@@ -201,6 +210,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 format version is too old.
+.TP
+.B 21
+The requested format version is too new.
+
.SH SEE ALSO
\fBnotmuch\fR(1), \fBnotmuch-config\fR(1), \fBnotmuch-count\fR(1),
diff --git a/notmuch-show.c b/notmuch-show.c
index a83fef9..39faf21 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -1066,6 +1066,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, ¬much_format_version, "format-version", 0, 0 },
{ NOTMUCH_OPT_KEYWORD, &exclude, "exclude", 'x',
(notmuch_keyword_t []){ { "true", EXCLUDE_TRUE },
{ "false", EXCLUDE_FALSE },
@@ -1128,6 +1129,8 @@ notmuch_show_command (void *ctx, unused (int argc),
unused (char *argv[]))
break;
}
+notmuch_exit_if_unsupported_format ();
+
/* Default is entire-thread = FALSE except for format=json and
* format=sexp. */
if (entire_thread == ENTIRE_THREAD_DEFAULT) {
--
1.7.10.4
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 6/7] emacs: Special handling for version mismatch errors
Since Emacs has more semantic information, we suppress the generic format version error from the CLI and give a more informative error. --- emacs/notmuch-lib.el | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el index 8f84087..9ec30ea 100644 --- a/emacs/notmuch-lib.el +++ b/emacs/notmuch-lib.el @@ -360,9 +360,18 @@ giving the output of command. ERR-FILE, if provided, is the name of a file containing the error output of command. OUTPUT and the contents of ERR-FILE will be included in the error message." - ;; This is implemented as a cond to make it easy to expand. (cond ((eq exit-status 0) t) + ((eq 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.") +(error "notmuch CLI version mismatch")) + ((eq 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.") +(error "notmuch CLI version mismatch")) (t (notmuch-pop-up-error (concat -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 5/7] test: Sanity tests for the --format-version argument
---
test/json |6 ++
1 file changed, 6 insertions(+)
diff --git a/test/json b/test/json
index bfafd55..8a01117 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 "Format version: too low" \
+"notmuch search --format-version=0 \\*"
+
+test_expect_code 21 "Format version: too high" \
+"notmuch search --format-version=999 \\*"
+
test_done
--
1.7.10.4
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 7/7] emacs: Use --format-version for search, show, and reply
---
emacs/notmuch-mua.el |2 +-
emacs/notmuch-query.el |2 +-
emacs/notmuch.el |6 +-
test/emacs |2 +-
test/emacs-show|2 +-
5 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index ac2d29e..24eebff 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" "--format-version=1"))
reply
original)
(when notmuch-show-process-crypto
diff --git a/emacs/notmuch-query.el b/emacs/notmuch-query.el
index e7e3520..6e9f406 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" "--format-version=1")))
(if notmuch-show-process-crypto
(setq args (append args '("--decrypt"
(setq args (append args search-terms))
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index b0fd387..63387a2 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -650,6 +650,10 @@ of the result."
(insert "Incomplete search results (search process was
killed).\n"))
(when (eq status 'exit)
(insert "End of search results.\n")
+ ;; For version mismatch, there's no point in
+ ;; showing the search buffer
+ (when (or (= exit-status 20) (= exit-status 21))
+ (kill-buffer))
(condition-case nil
(notmuch-check-async-exit-status proc msg)
;; Suppress the error signal since strange
@@ -935,7 +939,7 @@ Other optional parameters are used as follows:
(let ((proc (start-process
"notmuch-search" buffer
notmuch-command "search"
-"--format=json"
+"--format=json" "--format-version=1"
(if oldest-first
"--sort=oldest-first"
"--sort=newest-first")
diff --git a/test/emacs b/test/emacs
index 5067d67..6b18968 100755
--- a/test/emacs
+++ b/test/emacs
@@ -873,7 +873,7 @@ This is output
Error: Unexpected output from notmuch search:
This is an error
End of search results.
-Error invoking notmuch. $PWD/notmuch_fail search --format=json
--sort=newest-first tag:inbox exited with status 1."
+Error invoking notmuch. $PWD/notmuch_fail search --format=json
--format-version=1 --sort=newest-first tag:inbox exited with status 1."
test_done
diff --git a/test/emacs-show b/test/emacs-show
index 40fab61..ebf530b 100755
--- a/test/emacs-show
+++ b/test/emacs-show
@@ -178,7 +178,7 @@ test_emacs "(let ((notmuch-command \"$PWD/notmuch_fail\"))
(with-current-buffer \"*Notmuch errors*\"
(test-output \"ERROR\")))"
test_expect_equal "$(cat OUTPUT ERROR)" "\
-Error invoking notmuch. $PWD/notmuch_fail show --format=json --exclude=false
' * ' exited with status 1.
+Error invoking notmuch. $PWD/notmuch_fail show --format=json
--format-version=1 --exclude=false ' * ' exited with status 1.
Error:
This is an error
Output:
--
1.7.10.4
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 4/7] reply: Support --format-version
---
man/man1/notmuch-reply.1 | 21 +
notmuch-reply.c |3 +++
2 files changed, 24 insertions(+)
diff --git a/man/man1/notmuch-reply.1 b/man/man1/notmuch-reply.1
index fa04c9e..9fa1956 100644
--- a/man/man1/notmuch-reply.1
+++ b/man/man1/notmuch-reply.1
@@ -57,6 +57,16 @@ to create a reply message intelligently.
Only produces In\-Reply\-To, References, To, Cc, and Bcc headers.
.RE
.RE
+
+.RS
+.TP 4
+.BR \-\-format-version=N
+
+Use the specified structured output format 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 )
@@ -99,6 +109,17 @@ formats do not.
.RE
.RE
+.SH EXIT STATUS
+
+This command supports the following special exit status codes
+
+.TP
+.B 20
+The requested format version is too old.
+.TP
+.B 21
+The requested format version is too new.
+
.SH SEE ALSO
\fBnotmuch\fR(1), \fBnotmuch-config\fR(1), \fBnotmuch-count\fR(1),
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 720749d..22c58ff 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -733,6 +733,7 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
{ "sexp", FORMAT_SEXP },
{ "headers-only", FORMAT_HEADERS_ONLY },
{ 0, 0 } } },
+ { NOTMUCH_OPT_INT, ¬much_format_version, "format-version", 0, 0 },
{ NOTMUCH_OPT_KEYWORD, &reply_all, "reply-to", 'r',
(notmuch_keyword_t []){ { "all", TRUE },
{ "sender", FALSE },
@@ -759,6 +760,8 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
reply_format_func = notmuch_reply_format_default;
}
+notmuch_exit_if_unsupported_format ();
+
config = notmuch_config_open (ctx, NULL, NULL);
if (config == NULL)
return 1;
--
1.7.10.4
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/7] cli: Framework for structured output versioning
Currently there is a period of pain whenever we make
backward-incompatible changes to the structured output format, which
discourages not only backward-incompatible improvements to the format,
but also backwards-compatible additions that may not be "perfect". In
the end, these problems limit experimentation and innovation.
This series of patches introduces a way for CLI callers to request a
specific format 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 format
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.
---
devel/schemata |2 ++
notmuch-client.h | 45 +
notmuch.c| 32
3 files changed, 79 insertions(+)
diff --git a/devel/schemata b/devel/schemata
index d1ab983..292f287 100644
--- a/devel/schemata
+++ b/devel/schemata
@@ -14,6 +14,8 @@ are interleaved. Keys are printed as keywords (symbols
preceded by a
colon), e.g. (:id "123" :time 54321 :from "foobar"). Null is printed as
nil, true as t and false as nil.
+This is version 1 of the structured output format.
+
Common non-terminals
diff --git a/notmuch-client.h b/notmuch-client.h
index 1c336dc..d7b352e 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -117,6 +117,51 @@ chomp_newline (char *str)
str[strlen(str)-1] = '\0';
}
+/* Exit status code indicating the requested format version is too old
+ * (support for that version has been dropped). CLI code should use
+ * notmuch_exit_if_unsupported_format rather than directly exiting
+ * with this code.
+ */
+#define NOTMUCH_EXIT_FORMAT_TOO_OLD 20
+/* Exit status code indicating the requested format version is newer
+ * than the version supported by the CLI. CLI code should use
+ * notmuch_exit_if_unsupported_format rather than directly exiting
+ * with this code.
+ */
+#define NOTMUCH_EXIT_FORMAT_TOO_NEW 21
+
+/* The current structured output format version. Requests for format
+ * 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 (required) map fields can be added without increasing
+ * this.
+ */
+#define NOTMUCH_FORMAT_CUR 1
+/* The minimum supported structured output format version. Requests
+ * for format versions below this will return an error. */
+#define NOTMUCH_FORMAT_MIN 1
+
+/* The output format version requested by the caller on the command
+ * line. If no format version is requested, this will be set to
+ * NOTMUCH_FORMAT_CUR. Even though the command-line option is
+ * per-command, this is global because commands can share structured
+ * output code.
+ */
+extern int notmuch_format_version;
+
+/* Commands that support structured output should support the
+ * following argument
+ * { NOTMUCH_OPT_INT, ¬much_format_version, "format-version", 0, 0 }
+ * and should invoke notmuch_exit_if_unsupported_format to check the
+ * requested version. If notmuch_format_version is outside the
+ * supported range, this will print a detailed diagnostic message for
+ * the user and exit with NOTMUCH_EXIT_FORMAT_TOO_{OLD,NEW} to inform
+ * the invoking program of the problem.
+ */
+void
+notmuch_exit_if_unsupported_format (void);
+
notmuch_crypto_context_t *
notmuch_crypto_get_context (notmuch_crypto_t *crypto, const char *protocol);
diff --git a/notmuch.c b/notmuch.c
index 4ff66e3..9516dfb 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -82,6 +82,8 @@ static command_t commands[] = {
"This message, or more detailed help for the named command." }
};
+int notmuch_format_version;
+
static void
usage (FILE *out)
{
@@ -109,6 +111,33 @@ usage (FILE *out)
"and \"notmuch help search-terms\" for the common search-terms
syntax.\n\n");
}
+void
+notmuch_exit_if_unsupported_format (void)
+{
+if (notmuch_format_version > NOTMUCH_FORMAT_CUR) {
+ fprintf (stderr, "\
+A caller requested output format version %d, but the installed notmuch\n\
+CLI only supports up to format version %d. You may need to upgrade your\n\
+notmuch CLI.\n",
+notmuch_format_version, NOTMUCH_FORMAT_CUR);
+ exit (NOTMUCH_EXIT_FORMAT_TOO_NEW);
+} else if (notmuch_format_version < NOTMUCH_FORMAT_MIN) {
+ fprintf (stderr, "\
+A caller requested output format version %d, which is no longer supported\n\
+by the notmuch CLI (it requires at least version %d). You may need to\n\
+upgrade your notmuch front-end.\n",
+
[PATCH 0/7] Structed output versioning support
This series obsoletes [0] and must be applied on top of the Emacs CLI error handling series [1]. This is much simpler than the original series because it no longer includes the Emacs CLI error handling. This also switches to a CLI-wide minimum version instead of a per-command version, prints a warning if an old but still supported version is requested to ease client maintenance, and renames --use-schema to --format-version (which parallels --format better and simplifies error text). [0] id:1354416002-3557-1-git-send-email-amdra...@mit.edu [1] id:1355601860-30121-1-git-send-email-amdra...@mit.edu ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 2/7] search: Support --format-version
---
man/man1/notmuch-search.1 | 20
notmuch-search.c |3 +++
2 files changed, 23 insertions(+)
diff --git a/man/man1/notmuch-search.1 b/man/man1/notmuch-search.1
index 0aff348..5c771fa 100644
--- a/man/man1/notmuch-search.1
+++ b/man/man1/notmuch-search.1
@@ -32,6 +32,15 @@ Presents the results in either JSON, S-Expressions or
plain-text (default).
.RS 4
.TP 4
+.BR \-\-format-version=N
+
+Use the specified structured output format 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
@@ -126,6 +135,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 format version is too old.
+.TP
+.B 21
+The requested format version is too new.
+
.SH SEE ALSO
\fBnotmuch\fR(1), \fBnotmuch-config\fR(1), \fBnotmuch-count\fR(1),
diff --git a/notmuch-search.c b/notmuch-search.c
index 6218622..7704915 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -318,6 +318,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
{ "sexp", NOTMUCH_FORMAT_SEXP },
{ "text", NOTMUCH_FORMAT_TEXT },
{ 0, 0 } } },
+ { NOTMUCH_OPT_INT, ¬much_format_version, "format-version", 0, 0 },
{ NOTMUCH_OPT_KEYWORD, &output, "output", 'o',
(notmuch_keyword_t []){ { "summary", OUTPUT_SUMMARY },
{ "threads", OUTPUT_THREADS },
@@ -356,6 +357,8 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
INTERNAL_ERROR("no output format selected");
}
+notmuch_exit_if_unsupported_format ();
+
config = notmuch_config_open (ctx, NULL, NULL);
if (config == NULL)
return 1;
--
1.7.10.4
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v2 0/7] Improve Emacs CLI error handling
On Sat, Dec 15 2012, Austin Clements wrote: > This obsoletes id:1355548513-10085-1-git-send-email-amdra...@mit.edu > and fixes the things Mark and Tomi commented on. The interdiff is > below. Ok, new error messages appear at the end. Good. +1 Tomi > > diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el > index cf61635..8f84087 100644 > --- a/emacs/notmuch-lib.el > +++ b/emacs/notmuch-lib.el > @@ -326,10 +326,12 @@ the user dismisses it." > (with-current-buffer buf >(view-mode-enter nil #'kill-buffer) >(let ((inhibit-read-only t)) > + (goto-char (point-max)) > + (unless (bobp) > + (insert "\n")) > (insert msg) > (unless (bolp) > - (insert "\n")) > - (goto-char (point-min > + (insert "\n" > (pop-to-buffer buf))) > ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v5 0/4] indicate length of omitted body content
On Sat, Dec 15 2012, David Bremner wrote: > Peter Wang writes: > >> On Sat, 15 Dec 2012 08:45:33 +, Mark Walters > gmail.com> wrote: >>> >>> This version looks good to me with one very minor comments: >>> >>> Perhaps the name format-omitted-part-meta should include sprinter (eg >>> format-omitted-part-meta-sprinter)? >> >> Seems so. I'll roll up another series if that's necessary. > > Maybe just post the one amended patch as a reply to the patch it's > amending? The 'format_omitted_part_meta' in in context of 2 patches; Are you planning to do the change sin one patch (3/4) only (or making 5/4 which changes naming) ? And... The patch series looks good to me. > > d Tomi
v3 performance tests improvements
On Sat, Dec 15 2012, david at tethera.net wrote: > This obsoletes the series > > id:1354762908-5788-1-git-send-email-david at tethera.net > > I addition to fixing some small things that Austin noticed, this > series adds a convenience function "time_start" that unpacks the mail, > prints the header, and recovers a database cache if present. This > should reduce the amount of boilerplate at the beginning of tests. This patch set looks tolerable to me :-D Tomi
[PATCH 1/7] emacs: Centralize notmuch command error handling
On Sat, Dec 15 2012, Austin Clements wrote: > This provides library functions for unified handling of errors from > the notmuch CLI. Follow-up patches will convert some scattered error > handling to use this and add error handling where we currently ignore > errors. > --- The series looks pretty good, some hardcoded patchs Mark noticed and it looks to me (after viewing id:8738z7hd6x.fsf at qmul.ac.uk that err -> nil could be done. Would (goto-char (point-min)) be needed before (insert msg) in this patch -- what If user has moved cursor while viewing old error messages but not pressed q (dismissed) on the buffer. Also, what about (unless (eobp) (insert "\n")) to add empty line between error messages ? Tomi > emacs/notmuch-lib.el | 53 > ++ > 1 file changed, 53 insertions(+) > > diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el > index 9c4ee71..851098f 100644 > --- a/emacs/notmuch-lib.el > +++ b/emacs/notmuch-lib.el > @@ -316,6 +316,59 @@ 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. > + > +This will accumulate error messages in the errors buffer until > +the user dismisses it." > + > + (let ((buf (get-buffer-create "*Notmuch errors*"))) > +(with-current-buffer buf > + (view-mode-enter nil #'kill-buffer) > + (let ((inhibit-read-only t)) > + (insert msg) > + (unless (bolp) > + (insert "\n")) > + (goto-char (point-min > +(pop-to-buffer buf))) > + > +(defun notmuch-check-exit-status (exit-status command &optional output > err-file) > + "If EXIT-STATUS is non-zero, pop up an error buffer and signal an error. > + > +If EXIT-STATUS is non-zero, pop up a notmuch error buffer > +describing the error and signal an Elisp error. EXIT-STATUS must > +be a number indicating the exit status code of a process or a > +string describing the signal that terminated the process (such as > +returned by `call-process'). COMMAND must be a list giving the > +command and its arguments. OUTPUT, if provided, is a string > +giving the output of command. ERR-FILE, if provided, is the name > +of a file containing the error output of command. OUTPUT and the > +contents of ERR-FILE will be included in the error message." > + > + ;; This is implemented as a cond to make it easy to expand. > + (cond > + ((eq exit-status 0) t) > + (t > +(notmuch-pop-up-error > + (concat > + (format "Error invoking notmuch. %s exited with %s%s.\n" > + (mapconcat #'identity command " ") > + ;; Signal strings look like "Terminated", hence the > + ;; colon. > + (if (integerp exit-status) "status " "signal: ") > + exit-status) > + (when err-file > + (concat "Error:\n" > + (with-temp-buffer > + (insert-file-contents err-file) > + (if (eobp) > + "(no error output)\n" > + (buffer-string) > + (when (and output (not (equal output ""))) > + (format "Output:\n%s" output > +;; Mimic `process-lines' > +(error "%s exited with status %s" (car command) exit-status > + > ;; Compatibility functions for versions of emacs before emacs 23. > ;; > ;; Both functions here were copied from emacs 23 with the following > copyright: > -- > 1.7.10.4 > > ___ > notmuch mailing list > notmuch at notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch
[Patch v7 04/14] notmuch-tag: factor out double quoting routine
On Fri, 14 Dec 2012, david at tethera.net wrote:
> From: David Bremner
>
> This could live in tag-util as well, but it is really nothing specific
> to tags (although the conventions are arguable specific to Xapian).
>
> The API is changed from "caller-allocates" to "readline-like". The scan for
> max tag length is pushed down into the double quoting routine.
> ---
> notmuch-tag.c | 50 --
> util/string-util.c | 34 ++
> util/string-util.h |8
> 3 files changed, 58 insertions(+), 34 deletions(-)
>
> diff --git a/notmuch-tag.c b/notmuch-tag.c
> index 0965ee7..13f2268 100644
> --- a/notmuch-tag.c
> +++ b/notmuch-tag.c
> @@ -20,6 +20,7 @@
>
> #include "notmuch-client.h"
> #include "tag-util.h"
> +#include "string-util.h"
>
> static volatile sig_atomic_t interrupted;
>
> @@ -37,25 +38,6 @@ handle_sigint (unused (int sig))
> }
>
> static char *
> -_escape_tag (char *buf, const char *tag)
> -{
> -const char *in = tag;
> -char *out = buf;
> -
> -/* Boolean terms surrounded by double quotes can contain any
> - * character. Double quotes are quoted by doubling them. */
> -*out++ = '"';
> -while (*in) {
> - if (*in == '"')
> - *out++ = '"';
> - *out++ = *in++;
> -}
> -*out++ = '"';
> -*out = 0;
> -return buf;
> -}
> -
> -static char *
> _optimize_tag_query (void *ctx, const char *orig_query_string,
>const tag_op_list_t *list)
> {
> @@ -67,44 +49,44 @@ _optimize_tag_query (void *ctx, const char
> *orig_query_string,
> * parenthesize and the exclusion part of the query must not use
> * the '-' operator (though the NOT operator is fine). */
>
> -char *escaped, *query_string;
> +char *escaped = NULL;
> +size_t escaped_len = 0;
> +char *query_string;
> const char *join = "";
> size_t i;
> -unsigned int max_tag_len = 0;
>
> /* Don't optimize if there are no tag changes. */
> if (tag_op_list_size (list) == 0)
> return talloc_strdup (ctx, orig_query_string);
>
> -/* Allocate a buffer for escaping tags. This is large enough to
> - * hold a fully escaped tag with every character doubled plus
> - * enclosing quotes and a NUL. */
> -for (i = 0; i < tag_op_list_size (list); i++)
> - if (strlen (tag_op_list_tag (list, i)) > max_tag_len)
> - max_tag_len = strlen (tag_op_list_tag (list, i));
> -
> -escaped = talloc_array (ctx, char, max_tag_len * 2 + 3);
> -if (! escaped)
> - return NULL;
> -
> /* Build the new query string */
> if (strcmp (orig_query_string, "*") == 0)
> query_string = talloc_strdup (ctx, "(");
> else
> query_string = talloc_asprintf (ctx, "( %s ) and (", orig_query_string);
>
> +
> +/* Boolean terms surrounded by double quotes can contain any
> + * character. Double quotes are quoted by doubling them. */
> +
> for (i = 0; i < tag_op_list_size (list) && query_string; i++) {
> + double_quote_str (ctx,
> + tag_op_list_tag (list, i),
> + &escaped, &escaped_len);
> +
> query_string = talloc_asprintf_append_buffer (
> query_string, "%s%stag:%s", join,
> tag_op_list_isremove (list, i) ? "" : "not ",
> - _escape_tag (escaped, tag_op_list_tag (list, i)));
> + escaped);
> join = " or ";
> }
>
> if (query_string)
> query_string = talloc_strdup_append_buffer (query_string, ")");
>
> -talloc_free (escaped);
> +if (escaped)
> + talloc_free (escaped);
> +
> return query_string;
> }
>
> diff --git a/util/string-util.c b/util/string-util.c
> index 44f8cd3..ea7c25b 100644
> --- a/util/string-util.c
> +++ b/util/string-util.c
> @@ -20,6 +20,7 @@
>
>
> #include "string-util.h"
> +#include "talloc.h"
>
> char *
> strtok_len (char *s, const char *delim, size_t *len)
> @@ -32,3 +33,36 @@ strtok_len (char *s, const char *delim, size_t *len)
>
> return *len ? s : NULL;
> }
> +
> +
> +int
> +double_quote_str (void *ctx, const char *str,
> + char **buf, size_t *len)
> +{
> +const char *in;
> +char *out;
> +size_t needed = 3;
> +
> +for (in = str; *in; in++)
> + needed += (*in == '"') ? 2 : 1;
> +
> +if (needed > *len)
> + *buf = talloc_realloc (ctx, *buf, char, 2*needed);
> +
> +if (! *buf)
> + return 1;
> +
> +out = *buf;
> +
> +*out++ = '"';
> +in = str;
> +while (*in) {
> + if (*in == '"')
> + *out++ = '"';
> + *out++ = *in++;
> +}
> +*out++ = '"';
> +*out = 0;
Just a triviality: isn't '\0' preferred?
Best wishes
Mark
> +
> +return 0;
> +}
> diff --git a/util/string-util.h b/util/string-util.h
> index ac7676c..b593bc7 100644
> --- a/util/string-util.h
> +++ b/util/string-util.h
> @@ -19,4 +19,12 @@
>
> char *strtok_len (char *s, const char *delim
[PATCH] emacs: Fix bug in resynchronizing after a JSON parse error
On Fri, Dec 14 2012, Austin Clements 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. > --- I couldn't understand this (setf... even reading the docstring (and no time to experiment) anyway the code change is pretty trivial and i cannot see what it could break. I believe the code works as expected, so +1 Tomi > > This patch was already okayed by Mark Walters [0] in the context of a > larger series. Since it's independent of that larger series, I'm > re-sending it separately. > > [0] id:87hanxgczt.fsf at qmul.ac.uk > > 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 9c4ee71..fb6d3e7 100644 > --- a/emacs/notmuch-lib.el > +++ b/emacs/notmuch-lib.el > @@ -465,15 +465,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 at notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch
[Patch v7 06/14] notmuch-restore: move query handling for batch restore to parser
On Fri, 14 Dec 2012, david at tethera.net wrote:
> From: David Bremner
>
> We are able to detect more errors by looking at the string before it
> is hex-decoded. We also need this to avoid the query quoting for more
> general queries (to be written) that will mess up raw message-ids.
> ---
> notmuch-restore.c | 18 +-
> tag-util.c| 26 --
> tag-util.h|5 -
> test/dump-restore |5 ++---
> 4 files changed, 27 insertions(+), 27 deletions(-)
>
> diff --git a/notmuch-restore.c b/notmuch-restore.c
> index 40596a8..112f2f3 100644
> --- a/notmuch-restore.c
> +++ b/notmuch-restore.c
> @@ -208,24 +208,8 @@ notmuch_restore_command (unused (void *ctx), int argc,
> char *argv[])
> if (input_format == DUMP_FORMAT_SUP) {
> ret = parse_sup_line (ctx, line, &query_string, tag_ops);
> } else {
> - ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS,
> + ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS |
> TAG_FLAG_ID_ONLY,
> &query_string, tag_ops);
> -
> - if (ret == 0) {
> - if (strncmp ("id:", query_string, 3) != 0) {
> - fprintf (stderr, "Warning: unsupported query: %s\n",
> query_string);
> - continue;
> - }
> - /* delete id: from front of string; tag_message
> - * expects a raw message-id.
> - *
> - * XXX: Note that query string id:foo and bar will be
> - * interpreted as a message id "foo and bar". This
> - * should eventually be fixed to give a better error
> - * message.
> - */
> - query_string = query_string + 3;
> - }
> }
>
> if (ret > 0)
> diff --git a/tag-util.c b/tag-util.c
> index e1181f8..8fea76c 100644
> --- a/tag-util.c
> +++ b/tag-util.c
> @@ -201,14 +201,28 @@ parse_tag_line (void *ctx, char *line,
> }
>
> /* tok now points to the query string */
> -if (hex_decode_inplace (tok) != HEX_SUCCESS) {
> - ret = line_error (TAG_PARSE_INVALID, line_for_error,
> - "hex decoding of query %s failed", tok);
> - goto DONE;
> +if (flags & TAG_FLAG_ID_ONLY) {
> + /* this is under the assumption that any whitespace in the
> + * message-id must be hex-encoded. The check is probably not
> + * perfect for exotic unicode whitespace; as fallback the
> + * search for strange message-ids will fail */
> + if ((strncmp ("id:", tok, 3) != 0) ||
> + (strcspn (tok, " \t") < strlen (tok))) {
> + ret = line_error (TAG_PARSE_INVALID, line_for_error,
> + "query '%s' is not 'id:'", tok);
> + goto DONE;
> + }
> + if (hex_decode_inplace (tok) != HEX_SUCCESS) {
> + ret = line_error (TAG_PARSE_INVALID, line_for_error,
> + "hex decoding of query %s failed", tok);
> + goto DONE;
> + }
> + /* skip 'id:' */
> + *query_string = tok + 3;
This looks like it doesn't double_quote the query_string in this (the
TAG_FLAG_ID_ONLY) case. Is that deliberate?
Best wishes
Mark
> +} else {
> + ret = quote_and_decode_query (ctx, tok, line_for_error, query_string);
> }
>
> -*query_string = tok;
> -
>DONE:
> talloc_free (line_for_error);
> return ret;
> diff --git a/tag-util.h b/tag-util.h
> index 2889736..7674051 100644
> --- a/tag-util.h
> +++ b/tag-util.h
> @@ -26,7 +26,10 @@ typedef enum {
> /* Accept strange tags that might be user error;
> * intended for use by notmuch-restore.
> */
> -TAG_FLAG_BE_GENEROUS = (1 << 3)
> +TAG_FLAG_BE_GENEROUS = (1 << 3),
> +
> +/* Query consists of a single id:$message-id */
> +TAG_FLAG_ID_ONLY = (1 << 4)
>
> } tag_op_flag_t;
>
> diff --git a/test/dump-restore b/test/dump-restore
> index 6a989b6..eb7933a 100755
> --- a/test/dump-restore
> +++ b/test/dump-restore
> @@ -199,19 +199,18 @@ a
> # the next non-comment line should report an an empty tag error for
> # batch tagging, but not for restore
> + +e -- id:20091117232137.GA7669 at griffis1.net
> -# highlight the sketchy id parsing; this should be last
> +g -- id:foo and bar
> EOF
>
> cat < EXPECTED
> -Warning: unsupported query: a
> +Warning: query 'a' is not 'id:' [a]
> Warning: no query string [+0]
> Warning: no query string [+a +b]
> Warning: missing query string [+a +b ]
> Warning: no query string after -- [+c +d --]
> Warning: hex decoding of tag %zz failed [+%zz -- id:whatever]
> Warning: hex decoding of query id:%yy failed [+e +f id:%yy]
> -Warning: cannot apply tags to missing message: foo and bar
> +Warning: query 'id:foo and bar' is not 'id:' [+g -- id:foo and
> bar]
> EOF
>
> test_expect_equal_file EXPECTED OUTPUT
> --
> 1.7.10.4
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http:
[Patch v7 05/14] quote_and_decode_query: new function to quote hex-decoded queries
On Fri, 14 Dec 2012, david at tethera.net wrote:
> From: David Bremner
>
> The query is split into tokens, with ' ' and ':' as delimiters. Any
> token containing some hex-escaped character is quoted according to
> Xapian rules. This maps id:foo%20%22bar to id:"foo ""bar".
> This intentionally does not quote prefixes, so they still work as prefixes.
> ---
> tag-util.c | 50 ++
> 1 file changed, 50 insertions(+)
>
> diff --git a/tag-util.c b/tag-util.c
> index f89669a..e1181f8 100644
> --- a/tag-util.c
> +++ b/tag-util.c
> @@ -56,6 +56,56 @@ illegal_tag (const char *tag, notmuch_bool_t remove)
> return NULL;
> }
>
> +static tag_parse_status_t
> +quote_and_decode_query (void *ctx, char *encoded, const char *line_for_error,
> + char **query_string)
> +{
Would decode_and_quote_query be a better name given the order these two
happen? Also a comment describing the function would be nice.
> +char *tok = encoded;
> +size_t tok_len = 0;
> +char *buf = NULL;
> +size_t buf_len = 0;
> +tag_parse_status_t ret = TAG_PARSE_SUCCESS;
> +
> +*query_string = talloc_strdup (ctx, "");
> +
> +while (*query_string &&
> +(tok = strtok_len (tok + tok_len, ": ", &tok_len)) != NULL) {
> + char delim = tok[tok_len];
> +
> + *(tok + tok_len++) = '\0';
These two look a little odd: I would prefer either array or pointer in
both cases.
> +
> + if (strcspn (tok, "%") < tok_len - 1) {
> + /* something to decode */
> + if (hex_decode_inplace (tok) != HEX_SUCCESS) {
> + ret = line_error (TAG_PARSE_INVALID, line_for_error,
> + "hex decoding of token '%s' failed", tok);
> + goto DONE;
> + }
> +
> + if (double_quote_str (ctx, tok, &buf, &buf_len)) {
> + ret = line_error (TAG_PARSE_OUT_OF_MEMORY,
> + line_for_error, "aborting");
> + goto DONE;
> + }
> + *query_string = talloc_asprintf_append_buffer (
> + *query_string, "%s%c", buf, delim);
> +
> + } else {
> + /* This is not just an optimization, but used to preserve
> + * prefixes like id:, which cannot be quoted.
> + */
> + *query_string = talloc_asprintf_append_buffer (
> + *query_string, "%s%c", tok, delim);
> + }
What happens if a message id (for example) contains a ':'? Is a query of
the form
id:stuff"encoded_stuff"
acceptable? (As far as I can see from the man page ':' does not need to
be in hex.)
Best wishes
Mark
> +
> +}
> +
> + DONE:
> +if (ret != TAG_PARSE_SUCCESS && *query_string)
> + talloc_free (*query_string);
> +return ret;
> +}
> +
> tag_parse_status_t
> parse_tag_line (void *ctx, char *line,
> tag_op_flag_t flags,
> --
> 1.7.10.4
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v2 0/7] Improve Emacs CLI error handling
This series looks good to me. Just one possible thought: would it be worth time-stamping the errors (mostly in case the user does not quit the error buffer). But +1 anyway Best wishes Mark On Sat, 15 Dec 2012, Austin Clements wrote: > This obsoletes id:1355548513-10085-1-git-send-email-amdra...@mit.edu > and fixes the things Mark and Tomi commented on. The interdiff is > below. > > diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el > index cf61635..8f84087 100644 > --- a/emacs/notmuch-lib.el > +++ b/emacs/notmuch-lib.el > @@ -326,10 +326,12 @@ the user dismisses it." > (with-current-buffer buf >(view-mode-enter nil #'kill-buffer) >(let ((inhibit-read-only t)) > + (goto-char (point-max)) > + (unless (bobp) > + (insert "\n")) > (insert msg) > (unless (bolp) > - (insert "\n")) > - (goto-char (point-min > + (insert "\n" > (pop-to-buffer buf))) > > (defun notmuch-check-async-exit-status (proc msg) > diff --git a/emacs/notmuch.el b/emacs/notmuch.el > index c20de13..b0fd387 100644 > --- a/emacs/notmuch.el > +++ b/emacs/notmuch.el > @@ -536,8 +536,9 @@ If BARE is set then do not prefix with \"thread:\"" > (defun notmuch-call-notmuch-process (&rest args) >"Synchronously invoke \"notmuch\" with the given list of arguments. > > -Output from the process will be presented to the user as an error > -and will also appear in a buffer named \"*Notmuch errors*\"." > +If notmuch exits with a non-zero status, output from the process > +will appear in a buffer named \"*Notmuch errors*\" and an error > +will be signaled." >(with-temp-buffer > (let ((status (apply #'call-process notmuch-command nil t nil args))) >(notmuch-check-exit-status status (cons notmuch-command args) > @@ -649,7 +650,7 @@ of the result." > (insert "Incomplete search results (search process was > killed).\n")) > (when (eq status 'exit) > (insert "End of search results.\n") > - (condition-case err > + (condition-case nil > (notmuch-check-async-exit-status proc msg) > ;; Suppress the error signal since strange > ;; things happen if a sentinel signals. > diff --git a/test/emacs b/test/emacs > index 88b062c..5067d67 100755 > --- a/test/emacs > +++ b/test/emacs > @@ -873,7 +873,7 @@ This is output > Error: Unexpected output from notmuch search: > This is an error > End of search results. > -Error invoking notmuch. /tmp/nmtest/tmp.emacs/notmuch_fail search > --format=json --sort=newest-first tag:inbox exited with status 1." > +Error invoking notmuch. $PWD/notmuch_fail search --format=json > --sort=newest-first tag:inbox exited with status 1." > > > test_done > diff --git a/test/emacs-show b/test/emacs-show > index c67c6a4..40fab61 100755 > --- a/test/emacs-show > +++ b/test/emacs-show > @@ -178,7 +178,7 @@ test_emacs "(let ((notmuch-command \"$PWD/notmuch_fail\")) > (with-current-buffer \"*Notmuch errors*\" > (test-output \"ERROR\")))" > test_expect_equal "$(cat OUTPUT ERROR)" "\ > -Error invoking notmuch. /tmp/nmtest/tmp.emacs-show/notmuch_fail show > --format=json --exclude=false ' * ' exited with status 1. > +Error invoking notmuch. $PWD/notmuch_fail show --format=json > --exclude=false ' * ' exited with status 1. > Error: > This is an error > Output: ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] contrib: pick: archive message updated
On Sat, Dec 08 2012, Mark Walters wrote: > Update pick's archive message to respect notmuch-archive-tags. Also > split archive message into an archiving part and a separate > "then-next" part, to move more inline with show. Update the keybinding > so default behaviour is unchanged. > --- LGTM. Tomi > > Notmuch pick had fallen behind show so update. > > Best wishes > > Mark > > > contrib/notmuch-pick/notmuch-pick.el | 21 + > 1 files changed, 17 insertions(+), 4 deletions(-) > > diff --git a/contrib/notmuch-pick/notmuch-pick.el > b/contrib/notmuch-pick/notmuch-pick.el > index 755cbbc..36587a6 100644 > --- a/contrib/notmuch-pick/notmuch-pick.el > +++ b/contrib/notmuch-pick/notmuch-pick.el > @@ -173,7 +173,7 @@ > (define-key map "q" 'notmuch-pick-quit) > (define-key map "x" 'notmuch-pick-quit) > (define-key map "?" 'notmuch-help) > -(define-key map "a" 'notmuch-pick-archive-message) > +(define-key map "a" 'notmuch-pick-archive-message-then-next) > (define-key map "=" 'notmuch-pick-refresh-view) > (define-key map "s" 'notmuch-search) > (define-key map "z" 'notmuch-pick) > @@ -393,10 +393,23 @@ Does NOT change the database." >(kill-buffer notmuch-pick-message-buffer)) > t)) > > -(defun notmuch-pick-archive-message () > +(defun notmuch-pick-archive-message (&optional unarchive) > + "Archive the current message. > + > +Archive the current message by applying the tag changes in > +`notmuch-archive-tags' to it (remove the \"inbox\" tag by > +default). If a prefix argument is given, the message will be > +\"unarchived\", i.e. the tag changes in `notmuch-archive-tags' > +will be reversed." > + (interactive "P") > + (when notmuch-archive-tags > +(apply 'notmuch-pick-tag > +(notmuch-tag-change-list notmuch-archive-tags unarchive > + > +(defun notmuch-pick-archive-message-then-next (&optional unarchive) >"Archive the current message and move to next matching message." > - (interactive) > - (notmuch-pick-tag "-inbox") > + (interactive "P") > + (notmuch-pick-archive-message unarchive) >(notmuch-pick-next-matching-message)) > > (defun notmuch-pick-next-message () > -- > 1.7.9.1 > > ___ > notmuch mailing list > notmuch at notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] cli: add support for batch tagging operations to "notmuch tag"
From: Jani Nikula
Add support for batch tagging operations through stdin to "notmuch
tag". This can be enabled with the new --batch command line option to
"notmuch tag". The input must consist of lines of the format:
+|- [...] [--] [...]
Each line is interpreted similarly to "notmuch tag" command line
arguments. The delimiter is one or more spaces ' '. Any characters in
and MAY be hex encoded with %NN where NN is the
hexadecimal value of the character. Any ' ' and '%' characters in
and MUST be hex encoded (using %20 and %25,
respectively). Any characters that are not part of or
MUST NOT be hex encoded.
Leading and trailing space ' ' is ignored. Empty lines and lines
beginning with '#' are ignored.
Signed-off-by: Jani Nikula
Hacked-like-crazy-by: David Bremner
---
I missed somehow the change catch non-zero return from tag_query in tag_file.
This version is slightly cleaned up.
notmuch-tag.c | 91 -
1 file changed, 83 insertions(+), 8 deletions(-)
diff --git a/notmuch-tag.c b/notmuch-tag.c
index 13f2268..44b5bb4 100644
--- a/notmuch-tag.c
+++ b/notmuch-tag.c
@@ -130,6 +130,46 @@ tag_query (void *ctx, notmuch_database_t *notmuch, const
char *query_string,
return interrupted;
}
+static int
+tag_file (void *ctx, notmuch_database_t *notmuch, tag_op_flag_t flags,
+ FILE *input)
+{
+char *line = NULL;
+char *query_string = NULL;
+size_t line_size = 0;
+ssize_t line_len;
+int ret = 0;
+tag_op_list_t *tag_ops;
+
+tag_ops = tag_op_list_create (ctx);
+if (tag_ops == NULL) {
+ fprintf (stderr, "Out of memory.\n");
+ return 1;
+}
+
+while ((line_len = getline (&line, &line_size, input)) != -1 &&
+ ! interrupted) {
+
+ ret = parse_tag_line (ctx, line, TAG_FLAG_NONE,
+ &query_string, tag_ops);
+
+ if (ret > 0)
+ continue;
+
+ if (ret < 0)
+ break;
+
+ ret = tag_query (ctx, notmuch, query_string, tag_ops, flags);
+ if (ret)
+ break;
+}
+
+if (line)
+ free (line);
+
+return ret;
+}
+
int
notmuch_tag_command (void *ctx, int argc, char *argv[])
{
@@ -139,6 +179,10 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
notmuch_database_t *notmuch;
struct sigaction action;
tag_op_flag_t tag_flags = TAG_FLAG_NONE;
+notmuch_bool_t batch = FALSE;
+FILE *input = stdin;
+char *input_file_name = NULL;
+int opt_index;
int ret = 0;
/* Setup our handler for SIGINT */
@@ -148,15 +192,43 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
action.sa_flags = SA_RESTART;
sigaction (SIGINT, &action, NULL);
-tag_ops = tag_op_list_create (ctx);
-if (tag_ops == NULL) {
- fprintf (stderr, "Out of memory.\n");
+notmuch_opt_desc_t options[] = {
+ { NOTMUCH_OPT_BOOLEAN, &batch, "batch", 0, 0 },
+ { NOTMUCH_OPT_STRING, &input_file_name, "input", 'i', 0 },
+ { 0, 0, 0, 0, 0 }
+};
+
+opt_index = parse_arguments (argc, argv, options, 1);
+if (opt_index < 0)
return 1;
+
+if (input_file_name) {
+ batch = TRUE;
+ input = fopen (input_file_name, "r");
+ if (input == NULL) {
+ fprintf (stderr, "Error opening %s for reading: %s\n",
+input_file_name, strerror (errno));
+ return 1;
+ }
}
-if (parse_tag_command_line (ctx, argc - 1, argv + 1,
- &query_string, tag_ops))
- return 1;
+if (batch) {
+ if (opt_index != argc) {
+ fprintf (stderr, "Can't specify both cmdline and stdin!\n");
+ return 1;
+ }
+} else {
+
+ tag_ops = tag_op_list_create (ctx);
+ if (tag_ops == NULL) {
+ fprintf (stderr, "Out of memory.\n");
+ return 1;
+ }
+
+ if (parse_tag_command_line (ctx, argc - opt_index, argv + opt_index,
+ &query_string, tag_ops))
+ return 1;
+}
config = notmuch_config_open (ctx, NULL, NULL);
if (config == NULL)
@@ -169,9 +241,12 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
if (notmuch_config_get_maildir_synchronize_flags (config))
tag_flags |= TAG_FLAG_MAILDIR_SYNC;
-ret = tag_query (ctx, notmuch, query_string, tag_ops, tag_flags);
+if (batch)
+ ret = tag_file (ctx, notmuch, tag_flags, input);
+else
+ ret = tag_query (ctx, notmuch, query_string, tag_ops, tag_flags);
notmuch_database_destroy (notmuch);
-return ret;
+return ret || interrupted;
}
--
1.7.10.4
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] fixup for hex encoding desription in notmuch-tag.1
From: David Bremner --- and here is the updated description. man/man1/notmuch-tag.1 | 19 ++- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/man/man1/notmuch-tag.1 b/man/man1/notmuch-tag.1 index ac2c8d7..a203f53 100644 --- a/man/man1/notmuch-tag.1 +++ b/man/man1/notmuch-tag.1 @@ -67,13 +67,22 @@ The input must consist of lines of the format: Each line is interpreted similarly to .B notmuch tag command line arguments. The delimiter is one or more spaces ' '. Any -characters in and +characters in +.RI < tag > +and +.RI < search-term > .B may -be hex encoded with %NN where NN is the hexadecimal value of the -character. Any ' ' and '%' characters in and +be hex-encoded with %NN where NN is the hexadecimal value of the +character (more precisely with %NN[%MM...] where NN, MM, etc... are +the hexadecimal values of the bytes in the UTF-8 encoding of +the character). Any characters in and not +matching the regex +.B [A-Za-z0-9@=.,_+-] .B must -be hex encoded (using %20 and %25, respectively). Any characters that -are not part of or +be hex-encoded. Any characters that are not part of +.RI < tag > +or +.RI < search-term > .B must not be hex encoded. -- 1.7.10.4
[PATCH v5b] show: indicate length, encoding of omitted body content
If a leaf part's body content is omitted, return the encoded length and
transfer encoding in --format=json output. This information may be used
by the consumer, e.g. to decide whether to download a large attachment
over a slow link.
Returning the _encoded_ content length is more efficient than returning
the _decoded_ content length. Returning the transfer encoding allows
the consumer to estimate the decoded content length.
---
devel/schemata | 9 -
notmuch-show.c | 16 ++--
2 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/devel/schemata b/devel/schemata
index d1ab983..7d167be 100644
--- a/devel/schemata
+++ b/devel/schemata
@@ -75,7 +75,14 @@ part = {
# A leaf part's body content is optional, but may be included if
# it can be correctly encoded as a string. Consumers should use
# this in preference to fetching the part content separately.
-content?: string
+content?: string,
+# If a leaf part's body content is not included, the length of
+# the encoded content (in bytes) may be given instead.
+content-length?: int,
+# If a leaf part's body content is not included, its transfer encoding
+# may be given. Using this and the encoded content length, it is
+# possible for the consumer to estimate the decoded content length.
+content-transfer-encoding?: string
}
# The headers of a message or part (format_headers_sprinter with reply = FALSE)
diff --git a/notmuch-show.c b/notmuch-show.c
index 6a9278c..2d59865 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -600,14 +600,26 @@ format_part_text (const void *ctx, sprinter_t *sp,
mime_node_t *node,
}
static void
-format_omitted_part_meta_sprinter (sprinter_t *sp, GMimeObject *meta)
+format_omitted_part_meta_sprinter (sprinter_t *sp, GMimeObject *meta,
GMimePart *part)
{
const char *content_charset = g_mime_object_get_content_type_parameter
(meta, "charset");
+const char *cte = g_mime_object_get_header (meta,
"content-transfer-encoding");
+GMimeDataWrapper *wrapper = g_mime_part_get_content_object (part);
+GMimeStream *stream = g_mime_data_wrapper_get_stream (wrapper);
+ssize_t content_length = g_mime_stream_length (stream);
if (content_charset != NULL) {
sp->map_key (sp, "content-charset");
sp->string (sp, content_charset);
}
+if (cte != NULL) {
+ sp->map_key (sp, "content-transfer-encoding");
+ sp->string (sp, cte);
+}
+if (content_length >= 0) {
+ sp->map_key (sp, "content-length");
+ sp->integer (sp, content_length);
+}
}
void
@@ -699,7 +711,7 @@ format_part_sprinter (const void *ctx, sprinter_t *sp,
mime_node_t *node,
sp->string_len (sp, (char *) part_content->data, part_content->len);
g_object_unref (stream_memory);
} else {
- format_omitted_part_meta_sprinter (sp, meta);
+ format_omitted_part_meta_sprinter (sp, meta, GMIME_PART
(node->part));
}
} else if (GMIME_IS_MULTIPART (node->part)) {
sp->map_key (sp, "content");
--
1.7.12.1
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v5b] show: indicate charset for all omitted parts
Write a "charset" field for all omitted parts for which it is applicable,
not only text/html parts. Factor out the code to a separate function.
It will be extended with more fields next.
---
notmuch-show.c | 24
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/notmuch-show.c b/notmuch-show.c
index a83fef9..6a9278c 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -599,6 +599,17 @@ format_part_text (const void *ctx, sprinter_t *sp,
mime_node_t *node,
return NOTMUCH_STATUS_SUCCESS;
}
+static void
+format_omitted_part_meta_sprinter (sprinter_t *sp, GMimeObject *meta)
+{
+const char *content_charset = g_mime_object_get_content_type_parameter
(meta, "charset");
+
+if (content_charset != NULL) {
+ sp->map_key (sp, "content-charset");
+ sp->string (sp, content_charset);
+}
+}
+
void
format_part_sprinter (const void *ctx, sprinter_t *sp, mime_node_t *node,
notmuch_bool_t first, notmuch_bool_t output_body)
@@ -677,14 +688,9 @@ format_part_sprinter (const void *ctx, sprinter_t *sp,
mime_node_t *node,
* makes charset decoding the responsibility on the caller, we
* report the charset for text/html parts.
*/
- if (g_mime_content_type_is_type (content_type, "text", "html")) {
- const char *content_charset =
g_mime_object_get_content_type_parameter (meta, "charset");
-
- if (content_charset != NULL) {
- sp->map_key (sp, "content-charset");
- sp->string (sp, content_charset);
- }
- } else if (g_mime_content_type_is_type (content_type, "text", "*")) {
+ if (g_mime_content_type_is_type (content_type, "text", "*") &&
+ ! g_mime_content_type_is_type (content_type, "text", "html"))
+ {
GMimeStream *stream_memory = g_mime_stream_mem_new ();
GByteArray *part_content;
show_text_part_content (node->part, stream_memory, 0);
@@ -692,6 +698,8 @@ format_part_sprinter (const void *ctx, sprinter_t *sp,
mime_node_t *node,
sp->map_key (sp, "content");
sp->string_len (sp, (char *) part_content->data, part_content->len);
g_object_unref (stream_memory);
+ } else {
+ format_omitted_part_meta_sprinter (sp, meta);
}
} else if (GMIME_IS_MULTIPART (node->part)) {
sp->map_key (sp, "content");
--
1.7.12.1
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: [Patch v7 05/14] quote_and_decode_query: new function to quote hex-decoded queries
On Fri, 14 Dec 2012, da...@tethera.net wrote:
> From: David Bremner
>
> The query is split into tokens, with ' ' and ':' as delimiters. Any
> token containing some hex-escaped character is quoted according to
> Xapian rules. This maps id:foo%20%22bar to id:"foo ""bar".
> This intentionally does not quote prefixes, so they still work as prefixes.
> ---
> tag-util.c | 50 ++
> 1 file changed, 50 insertions(+)
>
> diff --git a/tag-util.c b/tag-util.c
> index f89669a..e1181f8 100644
> --- a/tag-util.c
> +++ b/tag-util.c
> @@ -56,6 +56,56 @@ illegal_tag (const char *tag, notmuch_bool_t remove)
> return NULL;
> }
>
> +static tag_parse_status_t
> +quote_and_decode_query (void *ctx, char *encoded, const char *line_for_error,
> + char **query_string)
> +{
> +char *tok = encoded;
> +size_t tok_len = 0;
> +char *buf = NULL;
> +size_t buf_len = 0;
> +tag_parse_status_t ret = TAG_PARSE_SUCCESS;
> +
> +*query_string = talloc_strdup (ctx, "");
> +
> +while (*query_string &&
> +(tok = strtok_len (tok + tok_len, ": ", &tok_len)) != NULL) {
strtok_len() will eat all the leading delimiters at each call, and will
not return a zero-length token if you have multiple consecutive
delimiters. Which means you may end up losing stuff here. Whether that
matters or not I'm too tired to tell...
BR,
Jani.
> + char delim = tok[tok_len];
> +
> + *(tok + tok_len++) = '\0';
> +
> + if (strcspn (tok, "%") < tok_len - 1) {
> + /* something to decode */
> + if (hex_decode_inplace (tok) != HEX_SUCCESS) {
> + ret = line_error (TAG_PARSE_INVALID, line_for_error,
> + "hex decoding of token '%s' failed", tok);
> + goto DONE;
> + }
> +
> + if (double_quote_str (ctx, tok, &buf, &buf_len)) {
> + ret = line_error (TAG_PARSE_OUT_OF_MEMORY,
> + line_for_error, "aborting");
> + goto DONE;
> + }
> + *query_string = talloc_asprintf_append_buffer (
> + *query_string, "%s%c", buf, delim);
> +
> + } else {
> + /* This is not just an optimization, but used to preserve
> + * prefixes like id:, which cannot be quoted.
> + */
> + *query_string = talloc_asprintf_append_buffer (
> + *query_string, "%s%c", tok, delim);
> + }
> +
> +}
> +
> + DONE:
> +if (ret != TAG_PARSE_SUCCESS && *query_string)
> + talloc_free (*query_string);
> +return ret;
> +}
> +
> tag_parse_status_t
> parse_tag_line (void *ctx, char *line,
> tag_op_flag_t flags,
> --
> 1.7.10.4
>
> ___
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] fixup: clarify TAG_FLAG_ID_ONLY comments and name
From: David Bremner
---
After some chatter on IRC, Mark and I converged to the following
notmuch-restore.c |2 +-
tag-util.c|2 +-
tag-util.h|6 --
3 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/notmuch-restore.c b/notmuch-restore.c
index 112f2f3..1b66e76 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -208,7 +208,7 @@ notmuch_restore_command (unused (void *ctx), int argc, char
*argv[])
if (input_format == DUMP_FORMAT_SUP) {
ret = parse_sup_line (ctx, line, &query_string, tag_ops);
} else {
- ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS |
TAG_FLAG_ID_ONLY,
+ ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS |
TAG_FLAG_ID_DIRECT,
&query_string, tag_ops);
}
diff --git a/tag-util.c b/tag-util.c
index 8fea76c..37bffd5 100644
--- a/tag-util.c
+++ b/tag-util.c
@@ -201,7 +201,7 @@ parse_tag_line (void *ctx, char *line,
}
/* tok now points to the query string */
-if (flags & TAG_FLAG_ID_ONLY) {
+if (flags & TAG_FLAG_ID_DIRECT) {
/* this is under the assumption that any whitespace in the
* message-id must be hex-encoded. The check is probably not
* perfect for exotic unicode whitespace; as fallback the
diff --git a/tag-util.h b/tag-util.h
index 7674051..eec00cf 100644
--- a/tag-util.h
+++ b/tag-util.h
@@ -28,8 +28,10 @@ typedef enum {
*/
TAG_FLAG_BE_GENEROUS = (1 << 3),
-/* Query consists of a single id:$message-id */
-TAG_FLAG_ID_ONLY = (1 << 4)
+/* Directly look up messages by hex-decoded message-id, rather
+ * than parsing a general query. The query MUST be of the form
+ * id:$message-id. */
+TAG_FLAG_ID_DIRECT = (1 << 4)
} tag_op_flag_t;
--
1.7.10.4
Re: [Patch v7 07/14] cli: add support for batch tagging operations to "notmuch tag"
On Fri, 14 Dec 2012, da...@tethera.net wrote:
> From: Jani Nikula
>
> Add support for batch tagging operations through stdin to "notmuch
> tag". This can be enabled with the new --batch command line option to
> "notmuch tag". The input must consist of lines of the format:
>
> +|- [...] [--] [...]
>
> Each line is interpreted similarly to "notmuch tag" command line
> arguments. The delimiter is one or more spaces ' '. Any characters in
> and MAY be hex encoded with %NN where NN is the
> hexadecimal value of the character. Any ' ' and '%' characters in
> and MUST be hex encoded (using %20 and %25,
> respectively). Any characters that are not part of or
> MUST NOT be hex encoded.
>
> Leading and trailing space ' ' is ignored. Empty lines and lines
> beginning with '#' are ignored.
>
> Signed-off-by: Jani Nikula
>
> Hacked-like-crazy-by: David Bremner
> ---
> notmuch-tag.c | 88
> +++--
> 1 file changed, 80 insertions(+), 8 deletions(-)
>
> diff --git a/notmuch-tag.c b/notmuch-tag.c
> index 13f2268..a81d911 100644
> --- a/notmuch-tag.c
> +++ b/notmuch-tag.c
> @@ -130,6 +130,43 @@ tag_query (void *ctx, notmuch_database_t *notmuch, const
> char *query_string,
> return interrupted;
> }
>
> +static int
> +tag_file (void *ctx, notmuch_database_t *notmuch, tag_op_flag_t flags,
> + FILE *input)
> +{
> +char *line = NULL;
> +char *query_string = NULL;
> +size_t line_size = 0;
> +ssize_t line_len;
> +int ret = 0;
> +tag_op_list_t *tag_ops;
> +
> +tag_ops = tag_op_list_create (ctx);
> +if (tag_ops == NULL) {
> + fprintf (stderr, "Out of memory.\n");
> + return 1;
> +}
> +
> +while ((line_len = getline (&line, &line_size, input)) != -1 &&
> +! interrupted) {
> +
> + ret = parse_tag_line (ctx, line, TAG_FLAG_NONE,
> + &query_string, tag_ops);
> +
> + if (ret > 0)
> + continue;
> +
> + if (ret < 0 || tag_query (ctx, notmuch, query_string,
> + tag_ops, flags))
> + break;
Hey, your changelog in the cover letter says you fixed the tag_query
return value propagation, but it's not here?
BR,
Jani.
> +}
> +
> +if (line)
> + free (line);
> +
> +return ret;
> +}
> +
> int
> notmuch_tag_command (void *ctx, int argc, char *argv[])
> {
> @@ -139,6 +176,10 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
> notmuch_database_t *notmuch;
> struct sigaction action;
> tag_op_flag_t tag_flags = TAG_FLAG_NONE;
> +notmuch_bool_t batch = FALSE;
> +FILE *input = stdin;
> +char *input_file_name = NULL;
> +int opt_index;
> int ret = 0;
>
> /* Setup our handler for SIGINT */
> @@ -148,15 +189,43 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
> action.sa_flags = SA_RESTART;
> sigaction (SIGINT, &action, NULL);
>
> -tag_ops = tag_op_list_create (ctx);
> -if (tag_ops == NULL) {
> - fprintf (stderr, "Out of memory.\n");
> +notmuch_opt_desc_t options[] = {
> + { NOTMUCH_OPT_BOOLEAN, &batch, "batch", 0, 0 },
> + { NOTMUCH_OPT_STRING, &input_file_name, "input", 'i', 0 },
> + { 0, 0, 0, 0, 0 }
> +};
> +
> +opt_index = parse_arguments (argc, argv, options, 1);
> +if (opt_index < 0)
> return 1;
> +
> +if (input_file_name) {
> + batch = TRUE;
> + input = fopen (input_file_name, "r");
> + if (input == NULL) {
> + fprintf (stderr, "Error opening %s for reading: %s\n",
> + input_file_name, strerror (errno));
> + return 1;
> + }
> }
>
> -if (parse_tag_command_line (ctx, argc - 1, argv + 1,
> - &query_string, tag_ops))
> - return 1;
> +if (batch) {
> + if (opt_index != argc) {
> + fprintf (stderr, "Can't specify both cmdline and stdin!\n");
> + return 1;
> + }
> +} else {
> +
> + tag_ops = tag_op_list_create (ctx);
> + if (tag_ops == NULL) {
> + fprintf (stderr, "Out of memory.\n");
> + return 1;
> + }
> +
> + if (parse_tag_command_line (ctx, argc - opt_index, argv + opt_index,
> + &query_string, tag_ops))
> + return 1;
> +}
>
> config = notmuch_config_open (ctx, NULL, NULL);
> if (config == NULL)
> @@ -169,9 +238,12 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
> if (notmuch_config_get_maildir_synchronize_flags (config))
> tag_flags |= TAG_FLAG_MAILDIR_SYNC;
>
> -ret = tag_query (ctx, notmuch, query_string, tag_ops, tag_flags);
> +if (batch)
> + ret = tag_file (ctx, notmuch, tag_flags, input);
> +else
> + ret = tag_query (ctx, notmuch, query_string, tag_ops, tag_flags);
>
> notmuch_database_destroy (notmuch);
>
> -return ret;
> +return ret || interrupted;
> }
> --
> 1.7.10.4
___
notmu
Re: [PATCH] fixup for hex encoding desription in notmuch-tag.1
On Sat, 15 Dec 2012, da...@tethera.net wrote: > From: David Bremner > > --- > and here is the updated description. > > man/man1/notmuch-tag.1 | 19 ++- > 1 file changed, 14 insertions(+), 5 deletions(-) > > diff --git a/man/man1/notmuch-tag.1 b/man/man1/notmuch-tag.1 > index ac2c8d7..a203f53 100644 > --- a/man/man1/notmuch-tag.1 > +++ b/man/man1/notmuch-tag.1 > @@ -67,13 +67,22 @@ The input must consist of lines of the format: > Each line is interpreted similarly to > .B notmuch tag > command line arguments. The delimiter is one or more spaces ' '. Any > -characters in and > +characters in > +.RI < tag > > +and > +.RI < search-term > > .B may > -be hex encoded with %NN where NN is the hexadecimal value of the > -character. Any ' ' and '%' characters in and > +be hex-encoded with %NN where NN is the hexadecimal value of the > +character (more precisely with %NN[%MM...] where NN, MM, etc... are > +the hexadecimal values of the bytes in the UTF-8 encoding of > +the character). Any characters in and not > +matching the regex > +.B [A-Za-z0-9@=.,_+-] And this actually means you can't have "id:foo" there, as : needs to be encoded. Not user friendly at all. If it should really mean : is okay un-encoded as prefix delimiter but not otherwise... it gets a bit messy to document. BR, Jani. > .B must > -be hex encoded (using %20 and %25, respectively). Any characters that > -are not part of or > +be hex-encoded. Any characters that are not part of > +.RI < tag > > +or > +.RI < search-term > > .B must not > be hex encoded. > > -- > 1.7.10.4 > > ___ > notmuch mailing list > notmuch@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [Patch v7 06/14] notmuch-restore: move query handling for batch restore to parser
On Fri, 14 Dec 2012, da...@tethera.net wrote:
> From: David Bremner
>
> We are able to detect more errors by looking at the string before it
> is hex-decoded. We also need this to avoid the query quoting for more
> general queries (to be written) that will mess up raw message-ids.
> ---
> notmuch-restore.c | 18 +-
> tag-util.c| 26 --
> tag-util.h|5 -
> test/dump-restore |5 ++---
> 4 files changed, 27 insertions(+), 27 deletions(-)
>
> diff --git a/notmuch-restore.c b/notmuch-restore.c
> index 40596a8..112f2f3 100644
> --- a/notmuch-restore.c
> +++ b/notmuch-restore.c
> @@ -208,24 +208,8 @@ notmuch_restore_command (unused (void *ctx), int argc,
> char *argv[])
> if (input_format == DUMP_FORMAT_SUP) {
> ret = parse_sup_line (ctx, line, &query_string, tag_ops);
> } else {
> - ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS,
> + ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS |
> TAG_FLAG_ID_ONLY,
> &query_string, tag_ops);
I realize that we've screwed up a bit here already in the restore
series. parse_sup_line() allocates query_string for each input line, but
doesn't free it during parsing. parse_tag_line() sets query_string to
point to the query string within line. And now it gets worse when
query_string is allocated vs. set to point to another buffer depending
on TAG_FLAG_ID_ONLY, so it's not an easy interface to use.
> -
> - if (ret == 0) {
> - if (strncmp ("id:", query_string, 3) != 0) {
> - fprintf (stderr, "Warning: unsupported query: %s\n",
> query_string);
> - continue;
> - }
> - /* delete id: from front of string; tag_message
> - * expects a raw message-id.
> - *
> - * XXX: Note that query string id:foo and bar will be
> - * interpreted as a message id "foo and bar". This
> - * should eventually be fixed to give a better error
> - * message.
> - */
> - query_string = query_string + 3;
> - }
> }
>
> if (ret > 0)
> diff --git a/tag-util.c b/tag-util.c
> index e1181f8..8fea76c 100644
> --- a/tag-util.c
> +++ b/tag-util.c
> @@ -201,14 +201,28 @@ parse_tag_line (void *ctx, char *line,
> }
>
> /* tok now points to the query string */
> -if (hex_decode_inplace (tok) != HEX_SUCCESS) {
> - ret = line_error (TAG_PARSE_INVALID, line_for_error,
> - "hex decoding of query %s failed", tok);
> - goto DONE;
> +if (flags & TAG_FLAG_ID_ONLY) {
> + /* this is under the assumption that any whitespace in the
> + * message-id must be hex-encoded. The check is probably not
> + * perfect for exotic unicode whitespace; as fallback the
> + * search for strange message-ids will fail */
> + if ((strncmp ("id:", tok, 3) != 0) ||
The current wording is, "Any characters in and MAY
be hex encoded with %NN...", but the above no longer matches that, as
"id:" must not be encoded. In the interest of keeping the documentation
concise, I think you should move the above check after
hex_decode_inplace(), but keep the below check here. That should do it,
right?
> + (strcspn (tok, " \t") < strlen (tok))) {
> + ret = line_error (TAG_PARSE_INVALID, line_for_error,
> + "query '%s' is not 'id:'", tok);
> + goto DONE;
> + }
> + if (hex_decode_inplace (tok) != HEX_SUCCESS) {
> + ret = line_error (TAG_PARSE_INVALID, line_for_error,
> + "hex decoding of query %s failed", tok);
> + goto DONE;
> + }
> + /* skip 'id:' */
> + *query_string = tok + 3;
> +} else {
> + ret = quote_and_decode_query (ctx, tok, line_for_error, query_string);
> }
It's not pretty that query_string gets allocated or not depending on a
flag that doesn't seem to have anything to do with that. I don't have a
good suggestion now, though, and I'd like to keep the restore path like
it is, without allocation.
>
> -*query_string = tok;
> -
>DONE:
> talloc_free (line_for_error);
> return ret;
> diff --git a/tag-util.h b/tag-util.h
> index 2889736..7674051 100644
> --- a/tag-util.h
> +++ b/tag-util.h
> @@ -26,7 +26,10 @@ typedef enum {
> /* Accept strange tags that might be user error;
> * intended for use by notmuch-restore.
> */
> -TAG_FLAG_BE_GENEROUS = (1 << 3)
> +TAG_FLAG_BE_GENEROUS = (1 << 3),
> +
> +/* Query consists of a single id:$message-id */
> +TAG_FLAG_ID_ONLY = (1 << 4)
>
> } tag_op_flag_t;
>
> diff --git a/test/dump-restore b/test/dump-restore
> index 6a989b6..eb7933a 100755
> --- a/test/dump-restore
> +++ b/test/dump-restore
> @@ -199,19 +199,18 @@ a
> # the next non-comment line should report an an empty tag error for
> # batch tagging, but not for r
[PATCH v2 7/7] test: Test search's handling of subprocess errors
--- test/emacs | 23 +++ 1 file changed, 23 insertions(+) diff --git a/test/emacs b/test/emacs index 5403930..5067d67 100755 --- a/test/emacs +++ b/test/emacs @@ -853,4 +853,27 @@ test_expect_success "Rendering HTML mail with images" \ 'cat OUTPUT && grep -q smiley OUTPUT' +test_begin_subtest "Search handles subprocess errors" +cat > notmuch_fail <&2 +exit 1 +EOF +chmod a+x notmuch_fail +test_emacs "(let ((notmuch-command \"$PWD/notmuch_fail\")) + (notmuch-search \"tag:inbox\") + (notmuch-test-wait) + (test-output) + (with-current-buffer \"*Notmuch errors*\" + (test-output \"ERROR\")))" +test_expect_equal "$(cat OUTPUT ERROR)" "\ +Error: Unexpected output from notmuch search: +This is output +Error: Unexpected output from notmuch search: +This is an error +End of search results. +Error invoking notmuch. $PWD/notmuch_fail search --format=json --sort=newest-first tag:inbox exited with status 1." + + test_done -- 1.7.10.4
[PATCH v2 6/7] emacs: Use unified error handling in search
This slightly changes the output of an existing test since we now
report non-zero exits with a pop-up buffer instead of at the end of
the search results.
---
emacs/notmuch-lib.el | 13 +
emacs/notmuch.el | 13 -
test/emacs-large-search-buffer |2 +-
3 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 7f7022a..8f84087 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -334,6 +334,19 @@ the user dismisses it."
(insert "\n"
(pop-to-buffer buf)))
+(defun notmuch-check-async-exit-status (proc msg)
+ "If PROC exited abnormally, pop up an error buffer and signal an error.
+
+This is a wrapper around `notmuch-check-exit-status' for
+asynchronous process sentinels. PROC and MSG must be the
+arguments passed to the sentinel."
+ (let ((exit-status
+(case (process-status proc)
+ ((exit) (process-exit-status proc))
+ ((signal) msg
+(when exit-status
+ (notmuch-check-exit-status exit-status (process-command proc)
+
(defun notmuch-check-exit-status (exit-status command &optional output
err-file)
"If EXIT-STATUS is non-zero, pop up an error buffer and signal an error.
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 9da8df4..b0fd387 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -638,6 +638,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
@@ -648,17 +649,19 @@ of the result."
(if (eq status 'signal)
(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)))
- (insert "\n")
+ (insert "End of search results.\n")
+ (condition-case nil
+ (notmuch-check-async-exit-status proc msg)
+ ;; Suppress the error signal since strange
+ ;; things happen if a sentinel signals.
+ (error (throw 'return nil)))
(if (and atbob
(not (string= notmuch-search-target-thread
"found")))
(set 'never-found-target-thread t)
(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"))
diff --git a/test/emacs-large-search-buffer b/test/emacs-large-search-buffer
index 678328d..9dcbef5 100755
--- a/test/emacs-large-search-buffer
+++ b/test/emacs-large-search-buffer
@@ -36,7 +36,7 @@ test_emacs '(notmuch-search "--this-option-does-not-exist")
cat
[PATCH v2 5/7] test: Test show's handling of subprocess errors
--- test/emacs-show | 22 ++ 1 file changed, 22 insertions(+) diff --git a/test/emacs-show b/test/emacs-show index b670abf..40fab61 100755 --- a/test/emacs-show +++ b/test/emacs-show @@ -163,4 +163,26 @@ EOF test_expect_equal_file OUTPUT EXPECTED +test_begin_subtest "Show handles subprocess errors" +cat > notmuch_fail <&2 +exit 1 +EOF +chmod a+x notmuch_fail +test_emacs "(let ((notmuch-command \"$PWD/notmuch_fail\")) + (ignore-errors (notmuch-show \"*\")) + (notmuch-test-wait) + (test-output) + (with-current-buffer \"*Notmuch errors*\" + (test-output \"ERROR\")))" +test_expect_equal "$(cat OUTPUT ERROR)" "\ +Error invoking notmuch. $PWD/notmuch_fail show --format=json --exclude=false ' * ' exited with status 1. +Error: +This is an error +Output: +This is output" + + test_done -- 1.7.10.4
[PATCH v2 4/7] emacs: Improve error handling for notmuch-call-notmuch-json
This checks for non-zero exit status from JSON CLI calls and pops up an error buffer with stderr and stdout. A consequence of this is that show and reply now handle errors, rather than ignoring them. --- emacs/notmuch-lib.el | 22 +++--- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el index f534770..7f7022a 100644 --- a/emacs/notmuch-lib.el +++ b/emacs/notmuch-lib.el @@ -375,15 +375,23 @@ contents of ERR-FILE will be included in the error message." "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))) + (notmuch-check-exit-status status (cons notmuch-command args) + (buffer-string) err-file) + (goto-char (point-min)) + (let ((json-object-type 'plist) + (json-array-type 'list) + (json-false 'nil)) + (json-read))) + (delete-file err-file) ;; Compatibility functions for versions of emacs before emacs 23. ;; -- 1.7.10.4
[PATCH v2 3/7] emacs: Factor out synchronous notmuch JSON invocations
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 f757ce7..f534770 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -371,6 +371,20 @@ contents of ERR-FILE will be included in the error
message."
;; Mimic `process-lines'
(error "%s exited with status %s" (car command) exit-status
+(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
[PATCH v2 2/7] emacs: Use unified error handling in notmuch-call-notmuch-process
This makes notmuch-call-notmuch-process use the unified CLI error handling, which basically refines the error handling this function already did. --- emacs/notmuch.el | 20 +++- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/emacs/notmuch.el b/emacs/notmuch.el index f9454d8..9da8df4 100644 --- a/emacs/notmuch.el +++ b/emacs/notmuch.el @@ -536,19 +536,13 @@ If BARE is set then do not prefix with \"thread:\"" (defun notmuch-call-notmuch-process (&rest args) "Synchronously invoke \"notmuch\" with the given list of arguments. -Output from the process will be presented to the user as an error -and will also appear in a buffer named \"*Notmuch errors*\"." - (let ((error-buffer (get-buffer-create "*Notmuch errors*"))) -(with-current-buffer error-buffer - (erase-buffer)) -(if (eq (apply 'call-process notmuch-command nil error-buffer nil args) 0) - (point) - (progn - (with-current-buffer error-buffer - (let ((beg (point-min)) - (end (- (point-max) 1))) - (error (buffer-substring beg end)) - )) +If notmuch exits with a non-zero status, output from the process +will appear in a buffer named \"*Notmuch errors*\" and an error +will be signaled." + (with-temp-buffer +(let ((status (apply #'call-process notmuch-command nil t nil args))) + (notmuch-check-exit-status status (cons notmuch-command args) +(buffer-string) (defun notmuch-search-set-tags (tags &optional pos) (let ((new-result (plist-put (notmuch-search-get-result pos) :tags tags))) -- 1.7.10.4
[PATCH v2 1/7] emacs: Centralize notmuch command error handling
This provides library functions for unified handling of errors from the notmuch CLI. Follow-up patches will convert some scattered error handling to use this and add error handling where we currently ignore errors. --- emacs/notmuch-lib.el | 55 ++ 1 file changed, 55 insertions(+) diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el index 9c4ee71..f757ce7 100644 --- a/emacs/notmuch-lib.el +++ b/emacs/notmuch-lib.el @@ -316,6 +316,61 @@ 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. + +This will accumulate error messages in the errors buffer until +the user dismisses it." + + (let ((buf (get-buffer-create "*Notmuch errors*"))) +(with-current-buffer buf + (view-mode-enter nil #'kill-buffer) + (let ((inhibit-read-only t)) + (goto-char (point-max)) + (unless (bobp) + (insert "\n")) + (insert msg) + (unless (bolp) + (insert "\n" +(pop-to-buffer buf))) + +(defun notmuch-check-exit-status (exit-status command &optional output err-file) + "If EXIT-STATUS is non-zero, pop up an error buffer and signal an error. + +If EXIT-STATUS is non-zero, pop up a notmuch error buffer +describing the error and signal an Elisp error. EXIT-STATUS must +be a number indicating the exit status code of a process or a +string describing the signal that terminated the process (such as +returned by `call-process'). COMMAND must be a list giving the +command and its arguments. OUTPUT, if provided, is a string +giving the output of command. ERR-FILE, if provided, is the name +of a file containing the error output of command. OUTPUT and the +contents of ERR-FILE will be included in the error message." + + ;; This is implemented as a cond to make it easy to expand. + (cond + ((eq exit-status 0) t) + (t +(notmuch-pop-up-error + (concat + (format "Error invoking notmuch. %s exited with %s%s.\n" + (mapconcat #'identity command " ") + ;; Signal strings look like "Terminated", hence the + ;; colon. + (if (integerp exit-status) "status " "signal: ") + exit-status) + (when err-file + (concat "Error:\n" + (with-temp-buffer + (insert-file-contents err-file) + (if (eobp) + "(no error output)\n" + (buffer-string) + (when (and output (not (equal output ""))) + (format "Output:\n%s" output +;; Mimic `process-lines' +(error "%s exited with status %s" (car command) exit-status + ;; Compatibility functions for versions of emacs before emacs 23. ;; ;; Both functions here were copied from emacs 23 with the following copyright: -- 1.7.10.4
[PATCH v2 0/7] Improve Emacs CLI error handling
This obsoletes id:1355548513-10085-1-git-send-email-amdragon at mit.edu and fixes the things Mark and Tomi commented on. The interdiff is below. diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el index cf61635..8f84087 100644 --- a/emacs/notmuch-lib.el +++ b/emacs/notmuch-lib.el @@ -326,10 +326,12 @@ the user dismisses it." (with-current-buffer buf (view-mode-enter nil #'kill-buffer) (let ((inhibit-read-only t)) + (goto-char (point-max)) + (unless (bobp) + (insert "\n")) (insert msg) (unless (bolp) - (insert "\n")) - (goto-char (point-min + (insert "\n" (pop-to-buffer buf))) (defun notmuch-check-async-exit-status (proc msg) diff --git a/emacs/notmuch.el b/emacs/notmuch.el index c20de13..b0fd387 100644 --- a/emacs/notmuch.el +++ b/emacs/notmuch.el @@ -536,8 +536,9 @@ If BARE is set then do not prefix with \"thread:\"" (defun notmuch-call-notmuch-process (&rest args) "Synchronously invoke \"notmuch\" with the given list of arguments. -Output from the process will be presented to the user as an error -and will also appear in a buffer named \"*Notmuch errors*\"." +If notmuch exits with a non-zero status, output from the process +will appear in a buffer named \"*Notmuch errors*\" and an error +will be signaled." (with-temp-buffer (let ((status (apply #'call-process notmuch-command nil t nil args))) (notmuch-check-exit-status status (cons notmuch-command args) @@ -649,7 +650,7 @@ of the result." (insert "Incomplete search results (search process was killed).\n")) (when (eq status 'exit) (insert "End of search results.\n") - (condition-case err + (condition-case nil (notmuch-check-async-exit-status proc msg) ;; Suppress the error signal since strange ;; things happen if a sentinel signals. diff --git a/test/emacs b/test/emacs index 88b062c..5067d67 100755 --- a/test/emacs +++ b/test/emacs @@ -873,7 +873,7 @@ This is output Error: Unexpected output from notmuch search: This is an error End of search results. -Error invoking notmuch. /tmp/nmtest/tmp.emacs/notmuch_fail search --format=json --sort=newest-first tag:inbox exited with status 1." +Error invoking notmuch. $PWD/notmuch_fail search --format=json --sort=newest-first tag:inbox exited with status 1." test_done diff --git a/test/emacs-show b/test/emacs-show index c67c6a4..40fab61 100755 --- a/test/emacs-show +++ b/test/emacs-show @@ -178,7 +178,7 @@ test_emacs "(let ((notmuch-command \"$PWD/notmuch_fail\")) (with-current-buffer \"*Notmuch errors*\" (test-output \"ERROR\")))" test_expect_equal "$(cat OUTPUT ERROR)" "\ -Error invoking notmuch. /tmp/nmtest/tmp.emacs-show/notmuch_fail show --format=json --exclude=false ' * ' exited with status 1. +Error invoking notmuch. $PWD/notmuch_fail show --format=json --exclude=false ' * ' exited with status 1. Error: This is an error Output:
[Patch v7 05/14] quote_and_decode_query: new function to quote hex-decoded queries
Mark Walters writes: > What happens if a message id (for example) contains a ':'? Is a query of > the form > > id:stuff"encoded_stuff" > > acceptable? (As far as I can see from the man page ':' does not need to > be in hex.) The updated version of the notmuch-dump man page does say that : will be hex encoded, so I think the fix here is to update the notmuch tag man page.
[PATCH 6/7] emacs: Use unified error handling in search
On Sat, 15 Dec 2012, Mark Walters wrote:
> On Sat, 15 Dec 2012, Austin Clements wrote:
>> This slightly changes the output of an existing test since we now
>> report non-zero exits with a pop-up buffer instead of at the end of
>> the search results.
>> ---
>> emacs/notmuch-lib.el | 13 +
>> emacs/notmuch.el | 13 -
>> test/emacs-large-search-buffer |2 +-
>> 3 files changed, 22 insertions(+), 6 deletions(-)
>>
>> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
>> index 9222de8..cf61635 100644
>> --- a/emacs/notmuch-lib.el
>> +++ b/emacs/notmuch-lib.el
>> @@ -332,6 +332,19 @@ the user dismisses it."
>> (goto-char (point-min
>> (pop-to-buffer buf)))
>>
>> +(defun notmuch-check-async-exit-status (proc msg)
>> + "If PROC exited abnormally, pop up an error buffer and signal an error.
>> +
>> +This is a wrapper around `notmuch-check-exit-status' for
>> +asynchronous process sentinels. PROC and MSG must be the
>> +arguments passed to the sentinel."
>> + (let ((exit-status
>> + (case (process-status proc)
>> + ((exit) (process-exit-status proc))
>> + ((signal) msg
>> +(when exit-status
>> + (notmuch-check-exit-status exit-status (process-command proc)
>> +
>> (defun notmuch-check-exit-status (exit-status command &optional output
>> err-file)
>>"If EXIT-STATUS is non-zero, pop up an error buffer and signal an error.
>>
>> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
>> index e25b54e..c20de13 100644
>> --- a/emacs/notmuch.el
>> +++ b/emacs/notmuch.el
>> @@ -637,6 +637,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
>> @@ -647,17 +648,19 @@ of the result."
>>(if (eq status 'signal)
>>(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)))
>> -(insert "\n")
>> +(insert "End of search results.\n")
>> +(condition-case err
>> +(notmuch-check-async-exit-status proc msg)
>> + ;; Suppress the error signal since strange
>> + ;; things happen if a sentinel signals.
>> + (error (throw 'return nil)))
>
> A total triviality (mostly just to check that I am understanding these
> parts): is the "err" after the condition-case used/needed?
It's not used or needed. Syntactically you need something there, of
course, so I changed it to nil as Tomi suggested, which causes
condition-case to simply not bind the error value to anything.
> Best wishes
>
> Mark
>> (if (and atbob
>> (not (string= notmuch-search-target-thread
>> "found")))
>> (set 'never-found-target-thread t)
>>(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"))
>> diff --git a/test/emacs-large-search-buffer b/test/emacs-large-search-buffer
>> index 678328d..9dcbef5 100755
>> --- a/test/emacs-large-search-buffer
>> +++ b/test/emacs-large-search-buffer
>> @@ -36,7 +36,7 @@ test_emacs '(notmuch-search "--this-option-does-not-exist")
>> cat > Error: Unexpected output from notmuch search:
>> Unrecognized option: --this-option-does-not-exist
>> -End of search results. (process returned 1)
>> +End of search results.
>> EOF
>> test_expect_equal_file OUTPUT EXPECTED
>>
>> --
>> 1.7.10.4
[PATCH 2/7] emacs: Use unified error handling in notmuch-call-notmuch-process
On Sat, 15 Dec 2012, Mark Walters wrote: > On Sat, 15 Dec 2012, Austin Clements wrote: >> This makes notmuch-call-notmuch-process use the unified CLI error >> handling, which basically refines the error handling this function >> already did. >> --- >> emacs/notmuch.el | 15 --- >> 1 file changed, 4 insertions(+), 11 deletions(-) >> >> diff --git a/emacs/notmuch.el b/emacs/notmuch.el >> index f9454d8..e25b54e 100644 >> --- a/emacs/notmuch.el >> +++ b/emacs/notmuch.el >> @@ -538,17 +538,10 @@ If BARE is set then do not prefix with \"thread:\"" >> >> Output from the process will be presented to the user as an error >> and will also appear in a buffer named \"*Notmuch errors*\"." > > This comment looks like it is not true (but wasn't true before > either). I think output will only be presented to the user if notmuch > exits with a non-zero status? Fixed. >> - (let ((error-buffer (get-buffer-create "*Notmuch errors*"))) >> -(with-current-buffer error-buffer >> -(erase-buffer)) >> -(if (eq (apply 'call-process notmuch-command nil error-buffer nil args) >> 0) >> -(point) >> - (progn >> -(with-current-buffer error-buffer >> - (let ((beg (point-min)) >> -(end (- (point-max) 1))) >> -(error (buffer-substring beg end)) >> -)) >> + (with-temp-buffer >> +(let ((status (apply #'call-process notmuch-command nil t nil args))) >> + (notmuch-check-exit-status status (cons notmuch-command args) >> + (buffer-string) >> > > Would it be worth separating stderr and stdout here? (Quite plausibly it > isn't.) Actually I think it's better to not separate stdout and stderr when we can avoid it, since stdout may give context for what's on stderr. When we're trying to interpret stdout it's important to separate it, but that's not the case here. > Best wishes > > Mark > > >> (defun notmuch-search-set-tags (tags &optional pos) >>(let ((new-result (plist-put (notmuch-search-get-result pos) :tags tags))) >> -- >> 1.7.10.4
[PATCH 1/7] emacs: Centralize notmuch command error handling
On Sat, 15 Dec 2012, Tomi Ollila wrote: > On Sat, Dec 15 2012, Austin Clements wrote: > >> This provides library functions for unified handling of errors from >> the notmuch CLI. Follow-up patches will convert some scattered error >> handling to use this and add error handling where we currently ignore >> errors. >> --- > > The series looks pretty good, some hardcoded patchs Mark noticed > and it looks to me (after viewing id:8738z7hd6x.fsf at qmul.ac.uk > that err -> nil could be done. Fixed. > Would (goto-char (point-min)) be needed before (insert msg) in > this patch -- what If user has moved cursor while viewing old > error messages but not pressed q (dismissed) on the buffer. Good idea. Actually, I had intended to *append* the error to the buffer so it would be in chronological order. I tweaked things a bit so messages should always be appended and point will be left at the end of the error log. > Also, what about (unless (eobp) (insert "\n")) to add empty > line between error messages ? Done. > Tomi
[PATCH] contrib: pick: archive message updated
Mark Walters writes: > Update pick's archive message to respect notmuch-archive-tags. Also > split archive message into an archiving part and a separate > "then-next" part, to move more inline with show. Update the keybinding > so default behaviour is unchanged. > --- pushed, d
[PATCH] emacs: Fix bug in resynchronizing after a JSON parse error
Austin Clements writes: > 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. pushed. d
v3 performance tests improvements
Tomi Ollila writes: > > This patch set looks tolerable to me :-D > > Tomi With such a strong endorsement, I had to push the series right away. d
Re: [Patch v7 04/14] notmuch-tag: factor out double quoting routine
On Fri, 14 Dec 2012, da...@tethera.net wrote:
> From: David Bremner
>
> This could live in tag-util as well, but it is really nothing specific
> to tags (although the conventions are arguable specific to Xapian).
>
> The API is changed from "caller-allocates" to "readline-like". The scan for
> max tag length is pushed down into the double quoting routine.
> ---
> notmuch-tag.c | 50 --
> util/string-util.c | 34 ++
> util/string-util.h |8
> 3 files changed, 58 insertions(+), 34 deletions(-)
>
> diff --git a/notmuch-tag.c b/notmuch-tag.c
> index 0965ee7..13f2268 100644
> --- a/notmuch-tag.c
> +++ b/notmuch-tag.c
> @@ -20,6 +20,7 @@
>
> #include "notmuch-client.h"
> #include "tag-util.h"
> +#include "string-util.h"
>
> static volatile sig_atomic_t interrupted;
>
> @@ -37,25 +38,6 @@ handle_sigint (unused (int sig))
> }
>
> static char *
> -_escape_tag (char *buf, const char *tag)
> -{
> -const char *in = tag;
> -char *out = buf;
> -
> -/* Boolean terms surrounded by double quotes can contain any
> - * character. Double quotes are quoted by doubling them. */
> -*out++ = '"';
> -while (*in) {
> - if (*in == '"')
> - *out++ = '"';
> - *out++ = *in++;
> -}
> -*out++ = '"';
> -*out = 0;
> -return buf;
> -}
> -
> -static char *
> _optimize_tag_query (void *ctx, const char *orig_query_string,
>const tag_op_list_t *list)
> {
> @@ -67,44 +49,44 @@ _optimize_tag_query (void *ctx, const char
> *orig_query_string,
> * parenthesize and the exclusion part of the query must not use
> * the '-' operator (though the NOT operator is fine). */
>
> -char *escaped, *query_string;
> +char *escaped = NULL;
> +size_t escaped_len = 0;
> +char *query_string;
> const char *join = "";
> size_t i;
> -unsigned int max_tag_len = 0;
>
> /* Don't optimize if there are no tag changes. */
> if (tag_op_list_size (list) == 0)
> return talloc_strdup (ctx, orig_query_string);
>
> -/* Allocate a buffer for escaping tags. This is large enough to
> - * hold a fully escaped tag with every character doubled plus
> - * enclosing quotes and a NUL. */
> -for (i = 0; i < tag_op_list_size (list); i++)
> - if (strlen (tag_op_list_tag (list, i)) > max_tag_len)
> - max_tag_len = strlen (tag_op_list_tag (list, i));
> -
> -escaped = talloc_array (ctx, char, max_tag_len * 2 + 3);
> -if (! escaped)
> - return NULL;
> -
> /* Build the new query string */
> if (strcmp (orig_query_string, "*") == 0)
> query_string = talloc_strdup (ctx, "(");
> else
> query_string = talloc_asprintf (ctx, "( %s ) and (", orig_query_string);
>
> +
> +/* Boolean terms surrounded by double quotes can contain any
> + * character. Double quotes are quoted by doubling them. */
> +
> for (i = 0; i < tag_op_list_size (list) && query_string; i++) {
> + double_quote_str (ctx,
> + tag_op_list_tag (list, i),
> + &escaped, &escaped_len);
Check return value?
> +
> query_string = talloc_asprintf_append_buffer (
> query_string, "%s%stag:%s", join,
> tag_op_list_isremove (list, i) ? "" : "not ",
> - _escape_tag (escaped, tag_op_list_tag (list, i)));
> + escaped);
> join = " or ";
> }
>
> if (query_string)
> query_string = talloc_strdup_append_buffer (query_string, ")");
>
> -talloc_free (escaped);
> +if (escaped)
> + talloc_free (escaped);
> +
> return query_string;
> }
>
> diff --git a/util/string-util.c b/util/string-util.c
> index 44f8cd3..ea7c25b 100644
> --- a/util/string-util.c
> +++ b/util/string-util.c
> @@ -20,6 +20,7 @@
>
>
> #include "string-util.h"
> +#include "talloc.h"
>
> char *
> strtok_len (char *s, const char *delim, size_t *len)
> @@ -32,3 +33,36 @@ strtok_len (char *s, const char *delim, size_t *len)
>
> return *len ? s : NULL;
> }
> +
> +
> +int
> +double_quote_str (void *ctx, const char *str,
> + char **buf, size_t *len)
> +{
> +const char *in;
> +char *out;
> +size_t needed = 3;
> +
> +for (in = str; *in; in++)
> + needed += (*in == '"') ? 2 : 1;
> +
> +if (needed > *len)
> + *buf = talloc_realloc (ctx, *buf, char, 2*needed);
You fail to set *len to 2*needed, leading to doing realloc every time.
Also, I think you should follow the getline pattern like you did in
hex_encode: if *buf == NULL, the input value of *len is ignored.
BR,
Jani.
> +
> +if (! *buf)
> + return 1;
> +
> +out = *buf;
> +
> +*out++ = '"';
> +in = str;
> +while (*in) {
> + if (*in == '"')
> + *out++ = '"';
> + *out++ = *in++;
> +}
> +*out++ = '"';
> +*out = 0;
> +
> +return 0;
> +}
> diff --git a/util/string-util.h b/ut
[Patch v7 06/14] notmuch-restore: move query handling for batch restore to parser
Mark Walters writes:
>> +if (hex_decode_inplace (tok) != HEX_SUCCESS) {
>> +ret = line_error (TAG_PARSE_INVALID, line_for_error,
>> + "hex decoding of query %s failed", tok);
>> +goto DONE;
>> +}
>> +/* skip 'id:' */
>> +*query_string = tok + 3;
>
> This looks like it doesn't double_quote the query_string in this (the
> TAG_FLAG_ID_ONLY) case. Is that deliberate?
Yes, that's what I meant by
,
| We also need this to avoid the query quoting for more
| general queries (to be written) that will mess up raw message-ids.
`
perhaps it deserves a comment in the code.
[PATCH v5 4/4] test: conform to content length, encoding fields
Update tests to expect content-length and content-transfer-encoding
fields in show --format=json output, for leaf parts with omitted body
content.
---
test/crypto| 30 +-
test/json | 4 +++-
test/multipart | 24 +---
test/sexp | 4 +++-
4 files changed, 40 insertions(+), 22 deletions(-)
diff --git a/test/crypto b/test/crypto
index 5dd14c4..aa96ec2 100755
--- a/test/crypto
+++ b/test/crypto
@@ -61,7 +61,8 @@ expected='[[[{"id": "X",
"content-type": "text/plain",
"content": "This is a test signed message.\n"},
{"id": 3,
- "content-type": "application/pgp-signature"}]}]},
+ "content-type": "application/pgp-signature",
+ "content-length": 315}]}]},
['
test_expect_equal_json \
"$output" \
@@ -95,7 +96,8 @@ expected='[[[{"id": "X",
"content-type": "text/plain",
"content": "This is a test signed message.\n"},
{"id": 3,
- "content-type": "application/pgp-signature"}]}]},
+ "content-type": "application/pgp-signature",
+ "content-length": 315}]}]},
['
test_expect_equal_json \
"$output" \
@@ -127,7 +129,8 @@ expected='[[[{"id": "X",
"content-type": "text/plain",
"content": "This is a test signed message.\n"},
{"id": 3,
- "content-type": "application/pgp-signature"}]}]},
+ "content-type": "application/pgp-signature",
+ "content-length": 315}]}]},
['
test_expect_equal_json \
"$output" \
@@ -196,7 +199,8 @@ expected='[[[{"id": "X",
"sigstatus": [],
"content-type": "multipart/encrypted",
"content": [{"id": 2,
- "content-type": "application/pgp-encrypted"},
+ "content-type": "application/pgp-encrypted",
+ "content-length": 11},
{"id": 3,
"content-type": "multipart/mixed",
"content": [{"id": 4,
@@ -204,6 +208,8 @@ expected='[[[{"id": "X",
"content": "This is a test encrypted message.\n"},
{"id": 5,
"content-type": "application/octet-stream",
+ "content-length": 28,
+ "content-transfer-encoding": "base64",
"filename": "TESTATTACHMENT"}]}]}]},
['
test_expect_equal_json \
@@ -231,9 +237,11 @@ test_expect_equal_file OUTPUT TESTATTACHMENT
test_begin_subtest "decryption failure with missing key"
mv "${GNUPGHOME}"{,.bak}
+# The length of the encrypted attachment varies so must be normalized.
output=$(notmuch show --format=json --decrypt subject:"test encrypted message
001" \
| notmuch_json_show_sanitize \
-| sed -e 's|"created": [1234567890]*|"created": 946728000|')
+| sed -e 's|"created": [1234567890]*|"created": 946728000|' \
+| sed -e 's|"content-length": 6[1234567890]*|"content-length": 652|')
expected='[[[{"id": "X",
"match": true,
"excluded": false,
@@ -249,9 +257,11 @@ expected='[[[{"id": "X",
"encstatus": [{"status": "bad"}],
"content-type": "multipart/encrypted",
"content": [{"id": 2,
- "content-type": "application/pgp-encrypted"},
+ "content-type": "application/pgp-encrypted",
+ "content-length": 11},
{"id": 3,
- "content-type": "application/octet-stream"}]}]},
+ "content-type": "application/octet-stream",
+ "content-length": 652}]}]},
['
test_expect_equal_json \
"$output" \
@@ -287,7 +297,8 @@ expected='[[[{"id": "X",
"userid": " Notmuch Test Suite (INSECURE!)"}],
"content-type": "multipart/encrypted",
"content": [{"id": 2,
- "content-type": "application/pgp-encrypted"},
+ "content-type": "application/pgp-encrypted",
+ "content-length": 11},
{"id": 3,
"content-type": "text/plain",
"content": "This is another test encrypted message.\n"}]}]},
@@ -342,7 +353,8 @@ expected='[[[{"id": "X",
"content-type": "text/plain",
"content": "This is a test signed message.\n"},
{"id": 3,
- "content-type": "application/pgp-signature"}]}]},
+ "content-type": "application/pgp-signature",
+ "content-length": 315}]}]},
['
test_expect_equal_json \
"$output" \
diff --git a/test/json b/test/json
index bfafd55..ff7fbe1 100755
--- a/test/json
+++ b/test/json
@@ -45,7 +45,9 @@ emacs_deliver_message \
(insert \"Message-ID: <$id>\n\")"
output=$(notmuch show --format=json "id:$id")
filename=$(notmuch search --output=files "id:$id")
-test_expect_equal_json "$output" "[[[{\"id\": \"$id\", \"match\": true,
\"excluded\": false, \"filename\": \"$filename\", \"timestamp\": 946728000,
\"date_relative\": \"2000-01-01\", \"tags\": [\"inbox\"], \"headers\":
{\"Subject\": \"$subject\", \"From\": \"Notmuch Test Suite \", \"To\": \"test_suite at notmuchmail.org\", \"Date\": \"Sat,
01 Jan 2000 12:00:00 +\"}, \"body\": [{\"id\": 1, \"content-type\":
\"multipart/mixed\", \"content\": [{\"id\": 2, \"content-type\":
\"text/plain\", \"content\": \"This is a test message with inline attachment
with a filename\"}, {\"id\": 3, \"content-type\": \"application/octet-stream\",
\"filename\": \"README\"}]}]}, ["
+# Get length of README after base64-encoding, minus additional newline.
+attachment_length=$(( $(base64 $TEST_DIRECTORY/README | wc -c) - 1 ))
+test_expect_equal_json "$output" "[[[{\"id
[PATCH v5 3/4] show: indicate length, encoding of omitted body content
If a leaf part's body content is omitted, return the encoded length and
transfer encoding in --format=json output. This information may be used
by the consumer, e.g. to decide whether to download a large attachment
over a slow link.
Returning the _encoded_ content length is more efficient than returning
the _decoded_ content length. Returning the transfer encoding allows
the consumer to estimate the decoded content length.
---
devel/schemata | 9 -
notmuch-show.c | 16 ++--
2 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/devel/schemata b/devel/schemata
index d1ab983..7d167be 100644
--- a/devel/schemata
+++ b/devel/schemata
@@ -75,7 +75,14 @@ part = {
# A leaf part's body content is optional, but may be included if
# it can be correctly encoded as a string. Consumers should use
# this in preference to fetching the part content separately.
-content?: string
+content?: string,
+# If a leaf part's body content is not included, the length of
+# the encoded content (in bytes) may be given instead.
+content-length?: int,
+# If a leaf part's body content is not included, its transfer encoding
+# may be given. Using this and the encoded content length, it is
+# possible for the consumer to estimate the decoded content length.
+content-transfer-encoding?: string
}
# The headers of a message or part (format_headers_sprinter with reply = FALSE)
diff --git a/notmuch-show.c b/notmuch-show.c
index a781a49..a8a348d 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -600,14 +600,26 @@ format_part_text (const void *ctx, sprinter_t *sp,
mime_node_t *node,
}
static void
-format_omitted_part_meta (sprinter_t *sp, GMimeObject *meta)
+format_omitted_part_meta (sprinter_t *sp, GMimeObject *meta, GMimePart *part)
{
const char *content_charset = g_mime_object_get_content_type_parameter
(meta, "charset");
+const char *cte = g_mime_object_get_header (meta,
"content-transfer-encoding");
+GMimeDataWrapper *wrapper = g_mime_part_get_content_object (part);
+GMimeStream *stream = g_mime_data_wrapper_get_stream (wrapper);
+ssize_t content_length = g_mime_stream_length (stream);
if (content_charset != NULL) {
sp->map_key (sp, "content-charset");
sp->string (sp, content_charset);
}
+if (cte != NULL) {
+ sp->map_key (sp, "content-transfer-encoding");
+ sp->string (sp, cte);
+}
+if (content_length >= 0) {
+ sp->map_key (sp, "content-length");
+ sp->integer (sp, content_length);
+}
}
void
@@ -699,7 +711,7 @@ format_part_sprinter (const void *ctx, sprinter_t *sp,
mime_node_t *node,
sp->string_len (sp, (char *) part_content->data, part_content->len);
g_object_unref (stream_memory);
} else {
- format_omitted_part_meta (sp, meta);
+ format_omitted_part_meta (sp, meta, GMIME_PART (node->part));
}
} else if (GMIME_IS_MULTIPART (node->part)) {
sp->map_key (sp, "content");
--
1.7.12.1
[PATCH v5 2/4] show: indicate charset for all omitted parts
Write a "charset" field for all omitted parts for which it is applicable,
not only text/html parts. Factor out the code to a separate function.
It will be extended with more fields next.
---
notmuch-show.c | 24
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/notmuch-show.c b/notmuch-show.c
index a83fef9..a781a49 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -599,6 +599,17 @@ format_part_text (const void *ctx, sprinter_t *sp,
mime_node_t *node,
return NOTMUCH_STATUS_SUCCESS;
}
+static void
+format_omitted_part_meta (sprinter_t *sp, GMimeObject *meta)
+{
+const char *content_charset = g_mime_object_get_content_type_parameter
(meta, "charset");
+
+if (content_charset != NULL) {
+ sp->map_key (sp, "content-charset");
+ sp->string (sp, content_charset);
+}
+}
+
void
format_part_sprinter (const void *ctx, sprinter_t *sp, mime_node_t *node,
notmuch_bool_t first, notmuch_bool_t output_body)
@@ -677,14 +688,9 @@ format_part_sprinter (const void *ctx, sprinter_t *sp,
mime_node_t *node,
* makes charset decoding the responsibility on the caller, we
* report the charset for text/html parts.
*/
- if (g_mime_content_type_is_type (content_type, "text", "html")) {
- const char *content_charset =
g_mime_object_get_content_type_parameter (meta, "charset");
-
- if (content_charset != NULL) {
- sp->map_key (sp, "content-charset");
- sp->string (sp, content_charset);
- }
- } else if (g_mime_content_type_is_type (content_type, "text", "*")) {
+ if (g_mime_content_type_is_type (content_type, "text", "*") &&
+ ! g_mime_content_type_is_type (content_type, "text", "html"))
+ {
GMimeStream *stream_memory = g_mime_stream_mem_new ();
GByteArray *part_content;
show_text_part_content (node->part, stream_memory, 0);
@@ -692,6 +698,8 @@ format_part_sprinter (const void *ctx, sprinter_t *sp,
mime_node_t *node,
sp->map_key (sp, "content");
sp->string_len (sp, (char *) part_content->data, part_content->len);
g_object_unref (stream_memory);
+ } else {
+ format_omitted_part_meta (sp, meta);
}
} else if (GMIME_IS_MULTIPART (node->part)) {
sp->map_key (sp, "content");
--
1.7.12.1
[PATCH v5 1/4] test: normalize only message filenames in show json
notmuch_json_show_sanitize replaced "filename" field values even in part structures, where the value is predictable. Make it only normalize the filename value if it is an absolute path (begins with slash), which is true of the Maildir filenames that were intended to be normalized away. --- test/multipart | 2 +- test/test-lib.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/multipart b/test/multipart index 0527f84..344ed81 100755 --- a/test/multipart +++ b/test/multipart @@ -630,7 +630,7 @@ cat
[PATCH v5 0/4] indicate length of omitted body content
This obsoletes 1355057796-19260-1-git-send-email-markwalters1009 at gmail.com Peter Wang (4): test: normalize only message filenames in show json show: indicate charset for all omitted parts show: indicate length, encoding of omitted body content test: conform to content length, encoding fields devel/schemata | 9 - notmuch-show.c | 36 test/crypto | 30 +- test/json| 4 +++- test/multipart | 26 ++ test/sexp| 4 +++- test/test-lib.sh | 2 +- 7 files changed, 78 insertions(+), 33 deletions(-) -- 1.7.12.1
[PATCH v3 0/5] Use invisibility to toggle display of all parts including multipart
Quoth Mark Walters on Dec 15 at 8:15 am: > This is version 3 of this set: version 2 is at > id:1355389924-3718-1-git-send-email-markwalters1009 at gmail.com > > This fixes the things Austin pointed out in the previous version. > > Diff from v2 is below the diffstat. > > Best wishes > > Mark LGTM.
[PATCH] fixup for hex encoding desription in notmuch-tag.1
From: David Bremner --- and here is the updated description. man/man1/notmuch-tag.1 | 19 ++- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/man/man1/notmuch-tag.1 b/man/man1/notmuch-tag.1 index ac2c8d7..a203f53 100644 --- a/man/man1/notmuch-tag.1 +++ b/man/man1/notmuch-tag.1 @@ -67,13 +67,22 @@ The input must consist of lines of the format: Each line is interpreted similarly to .B notmuch tag command line arguments. The delimiter is one or more spaces ' '. Any -characters in and +characters in +.RI < tag > +and +.RI < search-term > .B may -be hex encoded with %NN where NN is the hexadecimal value of the -character. Any ' ' and '%' characters in and +be hex-encoded with %NN where NN is the hexadecimal value of the +character (more precisely with %NN[%MM...] where NN, MM, etc... are +the hexadecimal values of the bytes in the UTF-8 encoding of +the character). Any characters in and not +matching the regex +.B [A-Za-z0-9@=.,_+-] .B must -be hex encoded (using %20 and %25, respectively). Any characters that -are not part of or +be hex-encoded. Any characters that are not part of +.RI < tag > +or +.RI < search-term > .B must not be hex encoded. -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 4/7] emacs: Improve error handling for notmuch-call-notmuch-json
This checks for non-zero exit status from JSON CLI calls and pops up an error buffer with stderr and stdout. A consequence of this is that show and reply now handle errors, rather than ignoring them. --- emacs/notmuch-lib.el | 22 +++--- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el index f534770..7f7022a 100644 --- a/emacs/notmuch-lib.el +++ b/emacs/notmuch-lib.el @@ -375,15 +375,23 @@ contents of ERR-FILE will be included in the error message." "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))) + (notmuch-check-exit-status status (cons notmuch-command args) + (buffer-string) err-file) + (goto-char (point-min)) + (let ((json-object-type 'plist) + (json-array-type 'list) + (json-false 'nil)) + (json-read))) + (delete-file err-file) ;; Compatibility functions for versions of emacs before emacs 23. ;; -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 3/7] emacs: Factor out synchronous notmuch JSON invocations
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 f757ce7..f534770 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -371,6 +371,20 @@ contents of ERR-FILE will be included in the error
message."
;; Mimic `process-lines'
(error "%s exited with status %s" (car command) exit-status
+(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
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 6/7] emacs: Use unified error handling in search
This slightly changes the output of an existing test since we now
report non-zero exits with a pop-up buffer instead of at the end of
the search results.
---
emacs/notmuch-lib.el | 13 +
emacs/notmuch.el | 13 -
test/emacs-large-search-buffer |2 +-
3 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 7f7022a..8f84087 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -334,6 +334,19 @@ the user dismisses it."
(insert "\n"
(pop-to-buffer buf)))
+(defun notmuch-check-async-exit-status (proc msg)
+ "If PROC exited abnormally, pop up an error buffer and signal an error.
+
+This is a wrapper around `notmuch-check-exit-status' for
+asynchronous process sentinels. PROC and MSG must be the
+arguments passed to the sentinel."
+ (let ((exit-status
+(case (process-status proc)
+ ((exit) (process-exit-status proc))
+ ((signal) msg
+(when exit-status
+ (notmuch-check-exit-status exit-status (process-command proc)
+
(defun notmuch-check-exit-status (exit-status command &optional output
err-file)
"If EXIT-STATUS is non-zero, pop up an error buffer and signal an error.
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 9da8df4..b0fd387 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -638,6 +638,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
@@ -648,17 +649,19 @@ of the result."
(if (eq status 'signal)
(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)))
- (insert "\n")
+ (insert "End of search results.\n")
+ (condition-case nil
+ (notmuch-check-async-exit-status proc msg)
+ ;; Suppress the error signal since strange
+ ;; things happen if a sentinel signals.
+ (error (throw 'return nil)))
(if (and atbob
(not (string= notmuch-search-target-thread
"found")))
(set 'never-found-target-thread t)
(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"))
diff --git a/test/emacs-large-search-buffer b/test/emacs-large-search-buffer
index 678328d..9dcbef5 100755
--- a/test/emacs-large-search-buffer
+++ b/test/emacs-large-search-buffer
@@ -36,7 +36,7 @@ test_emacs '(notmuch-search "--this-option-does-not-exist")
cat
[PATCH v2 0/7] Improve Emacs CLI error handling
This obsoletes id:1355548513-10085-1-git-send-email-amdra...@mit.edu and fixes the things Mark and Tomi commented on. The interdiff is below. diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el index cf61635..8f84087 100644 --- a/emacs/notmuch-lib.el +++ b/emacs/notmuch-lib.el @@ -326,10 +326,12 @@ the user dismisses it." (with-current-buffer buf (view-mode-enter nil #'kill-buffer) (let ((inhibit-read-only t)) + (goto-char (point-max)) + (unless (bobp) + (insert "\n")) (insert msg) (unless (bolp) - (insert "\n")) - (goto-char (point-min + (insert "\n" (pop-to-buffer buf))) (defun notmuch-check-async-exit-status (proc msg) diff --git a/emacs/notmuch.el b/emacs/notmuch.el index c20de13..b0fd387 100644 --- a/emacs/notmuch.el +++ b/emacs/notmuch.el @@ -536,8 +536,9 @@ If BARE is set then do not prefix with \"thread:\"" (defun notmuch-call-notmuch-process (&rest args) "Synchronously invoke \"notmuch\" with the given list of arguments. -Output from the process will be presented to the user as an error -and will also appear in a buffer named \"*Notmuch errors*\"." +If notmuch exits with a non-zero status, output from the process +will appear in a buffer named \"*Notmuch errors*\" and an error +will be signaled." (with-temp-buffer (let ((status (apply #'call-process notmuch-command nil t nil args))) (notmuch-check-exit-status status (cons notmuch-command args) @@ -649,7 +650,7 @@ of the result." (insert "Incomplete search results (search process was killed).\n")) (when (eq status 'exit) (insert "End of search results.\n") - (condition-case err + (condition-case nil (notmuch-check-async-exit-status proc msg) ;; Suppress the error signal since strange ;; things happen if a sentinel signals. diff --git a/test/emacs b/test/emacs index 88b062c..5067d67 100755 --- a/test/emacs +++ b/test/emacs @@ -873,7 +873,7 @@ This is output Error: Unexpected output from notmuch search: This is an error End of search results. -Error invoking notmuch. /tmp/nmtest/tmp.emacs/notmuch_fail search --format=json --sort=newest-first tag:inbox exited with status 1." +Error invoking notmuch. $PWD/notmuch_fail search --format=json --sort=newest-first tag:inbox exited with status 1." test_done diff --git a/test/emacs-show b/test/emacs-show index c67c6a4..40fab61 100755 --- a/test/emacs-show +++ b/test/emacs-show @@ -178,7 +178,7 @@ test_emacs "(let ((notmuch-command \"$PWD/notmuch_fail\")) (with-current-buffer \"*Notmuch errors*\" (test-output \"ERROR\")))" test_expect_equal "$(cat OUTPUT ERROR)" "\ -Error invoking notmuch. /tmp/nmtest/tmp.emacs-show/notmuch_fail show --format=json --exclude=false ' * ' exited with status 1. +Error invoking notmuch. $PWD/notmuch_fail show --format=json --exclude=false ' * ' exited with status 1. Error: This is an error Output: ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 5/7] test: Test show's handling of subprocess errors
--- test/emacs-show | 22 ++ 1 file changed, 22 insertions(+) diff --git a/test/emacs-show b/test/emacs-show index b670abf..40fab61 100755 --- a/test/emacs-show +++ b/test/emacs-show @@ -163,4 +163,26 @@ EOF test_expect_equal_file OUTPUT EXPECTED +test_begin_subtest "Show handles subprocess errors" +cat > notmuch_fail <&2 +exit 1 +EOF +chmod a+x notmuch_fail +test_emacs "(let ((notmuch-command \"$PWD/notmuch_fail\")) + (ignore-errors (notmuch-show \"*\")) + (notmuch-test-wait) + (test-output) + (with-current-buffer \"*Notmuch errors*\" + (test-output \"ERROR\")))" +test_expect_equal "$(cat OUTPUT ERROR)" "\ +Error invoking notmuch. $PWD/notmuch_fail show --format=json --exclude=false ' * ' exited with status 1. +Error: +This is an error +Output: +This is output" + + test_done -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 7/7] test: Test search's handling of subprocess errors
--- test/emacs | 23 +++ 1 file changed, 23 insertions(+) diff --git a/test/emacs b/test/emacs index 5403930..5067d67 100755 --- a/test/emacs +++ b/test/emacs @@ -853,4 +853,27 @@ test_expect_success "Rendering HTML mail with images" \ 'cat OUTPUT && grep -q smiley OUTPUT' +test_begin_subtest "Search handles subprocess errors" +cat > notmuch_fail <&2 +exit 1 +EOF +chmod a+x notmuch_fail +test_emacs "(let ((notmuch-command \"$PWD/notmuch_fail\")) + (notmuch-search \"tag:inbox\") + (notmuch-test-wait) + (test-output) + (with-current-buffer \"*Notmuch errors*\" + (test-output \"ERROR\")))" +test_expect_equal "$(cat OUTPUT ERROR)" "\ +Error: Unexpected output from notmuch search: +This is output +Error: Unexpected output from notmuch search: +This is an error +End of search results. +Error invoking notmuch. $PWD/notmuch_fail search --format=json --sort=newest-first tag:inbox exited with status 1." + + test_done -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 2/7] emacs: Use unified error handling in notmuch-call-notmuch-process
This makes notmuch-call-notmuch-process use the unified CLI error handling, which basically refines the error handling this function already did. --- emacs/notmuch.el | 20 +++- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/emacs/notmuch.el b/emacs/notmuch.el index f9454d8..9da8df4 100644 --- a/emacs/notmuch.el +++ b/emacs/notmuch.el @@ -536,19 +536,13 @@ If BARE is set then do not prefix with \"thread:\"" (defun notmuch-call-notmuch-process (&rest args) "Synchronously invoke \"notmuch\" with the given list of arguments. -Output from the process will be presented to the user as an error -and will also appear in a buffer named \"*Notmuch errors*\"." - (let ((error-buffer (get-buffer-create "*Notmuch errors*"))) -(with-current-buffer error-buffer - (erase-buffer)) -(if (eq (apply 'call-process notmuch-command nil error-buffer nil args) 0) - (point) - (progn - (with-current-buffer error-buffer - (let ((beg (point-min)) - (end (- (point-max) 1))) - (error (buffer-substring beg end)) - )) +If notmuch exits with a non-zero status, output from the process +will appear in a buffer named \"*Notmuch errors*\" and an error +will be signaled." + (with-temp-buffer +(let ((status (apply #'call-process notmuch-command nil t nil args))) + (notmuch-check-exit-status status (cons notmuch-command args) +(buffer-string) (defun notmuch-search-set-tags (tags &optional pos) (let ((new-result (plist-put (notmuch-search-get-result pos) :tags tags))) -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 1/7] emacs: Centralize notmuch command error handling
This provides library functions for unified handling of errors from the notmuch CLI. Follow-up patches will convert some scattered error handling to use this and add error handling where we currently ignore errors. --- emacs/notmuch-lib.el | 55 ++ 1 file changed, 55 insertions(+) diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el index 9c4ee71..f757ce7 100644 --- a/emacs/notmuch-lib.el +++ b/emacs/notmuch-lib.el @@ -316,6 +316,61 @@ 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. + +This will accumulate error messages in the errors buffer until +the user dismisses it." + + (let ((buf (get-buffer-create "*Notmuch errors*"))) +(with-current-buffer buf + (view-mode-enter nil #'kill-buffer) + (let ((inhibit-read-only t)) + (goto-char (point-max)) + (unless (bobp) + (insert "\n")) + (insert msg) + (unless (bolp) + (insert "\n" +(pop-to-buffer buf))) + +(defun notmuch-check-exit-status (exit-status command &optional output err-file) + "If EXIT-STATUS is non-zero, pop up an error buffer and signal an error. + +If EXIT-STATUS is non-zero, pop up a notmuch error buffer +describing the error and signal an Elisp error. EXIT-STATUS must +be a number indicating the exit status code of a process or a +string describing the signal that terminated the process (such as +returned by `call-process'). COMMAND must be a list giving the +command and its arguments. OUTPUT, if provided, is a string +giving the output of command. ERR-FILE, if provided, is the name +of a file containing the error output of command. OUTPUT and the +contents of ERR-FILE will be included in the error message." + + ;; This is implemented as a cond to make it easy to expand. + (cond + ((eq exit-status 0) t) + (t +(notmuch-pop-up-error + (concat + (format "Error invoking notmuch. %s exited with %s%s.\n" + (mapconcat #'identity command " ") + ;; Signal strings look like "Terminated", hence the + ;; colon. + (if (integerp exit-status) "status " "signal: ") + exit-status) + (when err-file + (concat "Error:\n" + (with-temp-buffer + (insert-file-contents err-file) + (if (eobp) + "(no error output)\n" + (buffer-string) + (when (and output (not (equal output ""))) + (format "Output:\n%s" output +;; Mimic `process-lines' +(error "%s exited with status %s" (car command) exit-status + ;; Compatibility functions for versions of emacs before emacs 23. ;; ;; Both functions here were copied from emacs 23 with the following copyright: -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 6/7] emacs: Use unified error handling in search
On Sat, 15 Dec 2012, Mark Walters wrote:
> On Sat, 15 Dec 2012, Austin Clements wrote:
>> This slightly changes the output of an existing test since we now
>> report non-zero exits with a pop-up buffer instead of at the end of
>> the search results.
>> ---
>> emacs/notmuch-lib.el | 13 +
>> emacs/notmuch.el | 13 -
>> test/emacs-large-search-buffer |2 +-
>> 3 files changed, 22 insertions(+), 6 deletions(-)
>>
>> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
>> index 9222de8..cf61635 100644
>> --- a/emacs/notmuch-lib.el
>> +++ b/emacs/notmuch-lib.el
>> @@ -332,6 +332,19 @@ the user dismisses it."
>> (goto-char (point-min
>> (pop-to-buffer buf)))
>>
>> +(defun notmuch-check-async-exit-status (proc msg)
>> + "If PROC exited abnormally, pop up an error buffer and signal an error.
>> +
>> +This is a wrapper around `notmuch-check-exit-status' for
>> +asynchronous process sentinels. PROC and MSG must be the
>> +arguments passed to the sentinel."
>> + (let ((exit-status
>> + (case (process-status proc)
>> + ((exit) (process-exit-status proc))
>> + ((signal) msg
>> +(when exit-status
>> + (notmuch-check-exit-status exit-status (process-command proc)
>> +
>> (defun notmuch-check-exit-status (exit-status command &optional output
>> err-file)
>>"If EXIT-STATUS is non-zero, pop up an error buffer and signal an error.
>>
>> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
>> index e25b54e..c20de13 100644
>> --- a/emacs/notmuch.el
>> +++ b/emacs/notmuch.el
>> @@ -637,6 +637,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
>> @@ -647,17 +648,19 @@ of the result."
>>(if (eq status 'signal)
>>(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)))
>> -(insert "\n")
>> +(insert "End of search results.\n")
>> +(condition-case err
>> +(notmuch-check-async-exit-status proc msg)
>> + ;; Suppress the error signal since strange
>> + ;; things happen if a sentinel signals.
>> + (error (throw 'return nil)))
>
> A total triviality (mostly just to check that I am understanding these
> parts): is the "err" after the condition-case used/needed?
It's not used or needed. Syntactically you need something there, of
course, so I changed it to nil as Tomi suggested, which causes
condition-case to simply not bind the error value to anything.
> Best wishes
>
> Mark
>> (if (and atbob
>> (not (string= notmuch-search-target-thread
>> "found")))
>> (set 'never-found-target-thread t)
>>(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"))
>> diff --git a/test/emacs-large-search-buffer b/test/emacs-large-search-buffer
>> index 678328d..9dcbef5 100755
>> --- a/test/emacs-large-search-buffer
>> +++ b/test/emacs-large-search-buffer
>> @@ -36,7 +36,7 @@ test_emacs '(notmuch-search "--this-option-does-not-exist")
>> cat > Error: Unexpected output from notmuch search:
>> Unrecognized option: --this-option-does-not-exist
>> -End of search results. (process returned 1)
>> +End of search results.
>> EOF
>> test_expect_equal_file OUTPUT EXPECTED
>>
>> --
>> 1.7.10.4
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 2/7] emacs: Use unified error handling in notmuch-call-notmuch-process
On Sat, 15 Dec 2012, Mark Walters wrote: > On Sat, 15 Dec 2012, Austin Clements wrote: >> This makes notmuch-call-notmuch-process use the unified CLI error >> handling, which basically refines the error handling this function >> already did. >> --- >> emacs/notmuch.el | 15 --- >> 1 file changed, 4 insertions(+), 11 deletions(-) >> >> diff --git a/emacs/notmuch.el b/emacs/notmuch.el >> index f9454d8..e25b54e 100644 >> --- a/emacs/notmuch.el >> +++ b/emacs/notmuch.el >> @@ -538,17 +538,10 @@ If BARE is set then do not prefix with \"thread:\"" >> >> Output from the process will be presented to the user as an error >> and will also appear in a buffer named \"*Notmuch errors*\"." > > This comment looks like it is not true (but wasn't true before > either). I think output will only be presented to the user if notmuch > exits with a non-zero status? Fixed. >> - (let ((error-buffer (get-buffer-create "*Notmuch errors*"))) >> -(with-current-buffer error-buffer >> -(erase-buffer)) >> -(if (eq (apply 'call-process notmuch-command nil error-buffer nil args) >> 0) >> -(point) >> - (progn >> -(with-current-buffer error-buffer >> - (let ((beg (point-min)) >> -(end (- (point-max) 1))) >> -(error (buffer-substring beg end)) >> -)) >> + (with-temp-buffer >> +(let ((status (apply #'call-process notmuch-command nil t nil args))) >> + (notmuch-check-exit-status status (cons notmuch-command args) >> + (buffer-string) >> > > Would it be worth separating stderr and stdout here? (Quite plausibly it > isn't.) Actually I think it's better to not separate stdout and stderr when we can avoid it, since stdout may give context for what's on stderr. When we're trying to interpret stdout it's important to separate it, but that's not the case here. > Best wishes > > Mark > > >> (defun notmuch-search-set-tags (tags &optional pos) >>(let ((new-result (plist-put (notmuch-search-get-result pos) :tags tags))) >> -- >> 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 1/7] emacs: Centralize notmuch command error handling
On Sat, 15 Dec 2012, Tomi Ollila wrote: > On Sat, Dec 15 2012, Austin Clements wrote: > >> This provides library functions for unified handling of errors from >> the notmuch CLI. Follow-up patches will convert some scattered error >> handling to use this and add error handling where we currently ignore >> errors. >> --- > > The series looks pretty good, some hardcoded patchs Mark noticed > and it looks to me (after viewing id:8738z7hd6x@qmul.ac.uk > that err -> nil could be done. Fixed. > Would (goto-char (point-min)) be needed before (insert msg) in > this patch -- what If user has moved cursor while viewing old > error messages but not pressed q (dismissed) on the buffer. Good idea. Actually, I had intended to *append* the error to the buffer so it would be in chronological order. I tweaked things a bit so messages should always be appended and point will be left at the end of the error log. > Also, what about (unless (eobp) (insert "\n")) to add empty > line between error messages ? Done. > Tomi ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] fixup: clarify TAG_FLAG_ID_ONLY comments and name
From: David Bremner
---
After some chatter on IRC, Mark and I converged to the following
notmuch-restore.c |2 +-
tag-util.c|2 +-
tag-util.h|6 --
3 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/notmuch-restore.c b/notmuch-restore.c
index 112f2f3..1b66e76 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -208,7 +208,7 @@ notmuch_restore_command (unused (void *ctx), int argc, char
*argv[])
if (input_format == DUMP_FORMAT_SUP) {
ret = parse_sup_line (ctx, line, &query_string, tag_ops);
} else {
- ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS |
TAG_FLAG_ID_ONLY,
+ ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS |
TAG_FLAG_ID_DIRECT,
&query_string, tag_ops);
}
diff --git a/tag-util.c b/tag-util.c
index 8fea76c..37bffd5 100644
--- a/tag-util.c
+++ b/tag-util.c
@@ -201,7 +201,7 @@ parse_tag_line (void *ctx, char *line,
}
/* tok now points to the query string */
-if (flags & TAG_FLAG_ID_ONLY) {
+if (flags & TAG_FLAG_ID_DIRECT) {
/* this is under the assumption that any whitespace in the
* message-id must be hex-encoded. The check is probably not
* perfect for exotic unicode whitespace; as fallback the
diff --git a/tag-util.h b/tag-util.h
index 7674051..eec00cf 100644
--- a/tag-util.h
+++ b/tag-util.h
@@ -28,8 +28,10 @@ typedef enum {
*/
TAG_FLAG_BE_GENEROUS = (1 << 3),
-/* Query consists of a single id:$message-id */
-TAG_FLAG_ID_ONLY = (1 << 4)
+/* Directly look up messages by hex-decoded message-id, rather
+ * than parsing a general query. The query MUST be of the form
+ * id:$message-id. */
+TAG_FLAG_ID_DIRECT = (1 << 4)
} tag_op_flag_t;
--
1.7.10.4
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
handling mail sent to a subscribed list
> If understand correctly, your concern is with the second copy of a > message with the same message-id not showing up in your inbox? If so, this is > more or less a feature (although in the case where the duplicated > message id's are because of malice or stupidity on the sender's part, > and not duplicated messages, it is also a known bug). > > On the command line you can try "notmuch search --output=files id:foo" > where id:foo is copied via "c i" in the emacs interface. > > Or maybe I misunderstand your problem completely. Thanks for the reply. Yes I think you summed it up; I figured that it was behaving properly. In short it's that I have the email that I sent sitting wherever it was Fcc'ed and then shortly thereafter I receive from the email list a message with the same ID. Since the Fcc'ed one was already added to the database, the one that I received from the list just sits there. But I guess others using mailing lists have encountered this too, so I was asking how they handle it since I haven't figured it out yet. I'll give that command a try to see if I can put it to good use somehow. Regards, -brandon
Re: [PATCH v3 0/5] Use invisibility to toggle display of all parts including multipart
Quoth Mark Walters on Dec 15 at 8:15 am: > This is version 3 of this set: version 2 is at > id:1355389924-3718-1-git-send-email-markwalters1...@gmail.com > > This fixes the things Austin pointed out in the previous version. > > Diff from v2 is below the diffstat. > > Best wishes > > Mark LGTM. ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [Patch v7 05/14] quote_and_decode_query: new function to quote hex-decoded queries
Mark Walters writes: > What happens if a message id (for example) contains a ':'? Is a query of > the form > > id:stuff"encoded_stuff" > > acceptable? (As far as I can see from the man page ':' does not need to > be in hex.) The updated version of the notmuch-dump man page does say that : will be hex encoded, so I think the fix here is to update the notmuch tag man page. ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
notmuch for christmas, again
There are some pick patches I would quite like to get in: (note all patches below only touch contrib/notmuch-pick/notmuch-pick.el) id:1354970494-18050-1-git-send-email-markwalters1009 at gmail.com bugfix to use archive tags rather than always "-inbox" id:1354970674-18136-1-git-send-email-markwalters1009 at gmail.com bugfix so that running pick or search from split pane closes the split id:1355085019-18534-1-git-send-email-markwalters1009 at gmail.com tweak so that message in message pane is not indented then there are some possibles: id:1355391109-4150-1-git-send-email-markwalters1009 at gmail.com means that if the focus gets into the message pane then quitting from it closes the pane. This seems to work well and fixes a genuine issue but there might be better ways to fix it. (*) id:1354970914-18342-1-git-send-email-markwalters1009 at gmail.com 2 patches to add some thread based operations to pick some tweaks so that various extra commands are available: eg stashing message ids, viewing attachments etc. This is not intrusive but since most of the patches above tweak key bindings the contexts tend to overlap. (*) The patches marked * redefine a couple of functions in notmuch-show.el so theoretically could cause breakage of vanilla notmuch functionality (but only if pick is loaded). Best wishes Mark On Sat, 15 Dec 2012, David Bremner wrote: > As a nascent tradition (see id:87obvaypwc.fsf at zancas.localnet), I'd like > to have a feature freeze on December 25, and release notmuch 0.15 around > January 1, 2013. > > As usual there are a few last minute requests > > - I'd like to get the two memory leak fixes in > id:1355234087-6886-1-git-send-email-david at tethera.net > id:1355196820-29734-1-git-send-email-david at tethera.net > > - Austin requested the scheme versioning patches; I think he will send a > followup to id:1354416002-3557-1-git-send-email-amdragon at mit.edu soon. > > Generally speaking anything that happens to get reviewed and polished > before the feature freeze is fine > > - I know Damien has been working pretty hard on > id:1355404167-31750-1-git-send-email-damien.cassou at gmail.com and I > have the impression that it is either ready or very close. > > - It would be nice to get the batch tagging stuff in > (id:1355492062-7546-1-git-send-email-david at tethera.net) but it can > also wait for the next release if it isn't ready. > > Anything else people want to push for (and presumably review)? > > d > > ___ > notmuch mailing list > notmuch at notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch
gmail importer script
Quoting Jason A. Donenfeld (2012-12-15 08:22:46) > On Thu, Dec 13, 2012 at 5:48 PM, Patrick Totzke > wrote: > > > notmuch.errors.FileNotEmailError > > > Delete the file it dies on and try again. Well, thats not the point.. the script shouldn't die like this. I think it's be better if the script caught that exception, deleted the file and continued.. Thanks, /p -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 198 bytes Desc: signature URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20121215/a8b6075b/attachment.pgp>
Re: [PATCH] contrib: pick: archive message updated
Mark Walters writes: > Update pick's archive message to respect notmuch-archive-tags. Also > split archive message into an archiving part and a separate > "then-next" part, to move more inline with show. Update the keybinding > so default behaviour is unchanged. > --- pushed, d ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] emacs: Fix bug in resynchronizing after a JSON parse error
Austin Clements writes: > 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. pushed. d ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: v3 performance tests improvements
Tomi Ollila writes: > > This patch set looks tolerable to me :-D > > Tomi With such a strong endorsement, I had to push the series right away. d ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [Patch v7 06/14] notmuch-restore: move query handling for batch restore to parser
Mark Walters writes:
>> +if (hex_decode_inplace (tok) != HEX_SUCCESS) {
>> +ret = line_error (TAG_PARSE_INVALID, line_for_error,
>> + "hex decoding of query %s failed", tok);
>> +goto DONE;
>> +}
>> +/* skip 'id:' */
>> +*query_string = tok + 3;
>
> This looks like it doesn't double_quote the query_string in this (the
> TAG_FLAG_ID_ONLY) case. Is that deliberate?
Yes, that's what I meant by
,
| We also need this to avoid the query quoting for more
| general queries (to be written) that will mess up raw message-ids.
`
perhaps it deserves a comment in the code.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: [Patch v7 04/14] notmuch-tag: factor out double quoting routine
On Fri, 14 Dec 2012, da...@tethera.net wrote:
> From: David Bremner
>
> This could live in tag-util as well, but it is really nothing specific
> to tags (although the conventions are arguable specific to Xapian).
>
> The API is changed from "caller-allocates" to "readline-like". The scan for
> max tag length is pushed down into the double quoting routine.
> ---
> notmuch-tag.c | 50 --
> util/string-util.c | 34 ++
> util/string-util.h |8
> 3 files changed, 58 insertions(+), 34 deletions(-)
>
> diff --git a/notmuch-tag.c b/notmuch-tag.c
> index 0965ee7..13f2268 100644
> --- a/notmuch-tag.c
> +++ b/notmuch-tag.c
> @@ -20,6 +20,7 @@
>
> #include "notmuch-client.h"
> #include "tag-util.h"
> +#include "string-util.h"
>
> static volatile sig_atomic_t interrupted;
>
> @@ -37,25 +38,6 @@ handle_sigint (unused (int sig))
> }
>
> static char *
> -_escape_tag (char *buf, const char *tag)
> -{
> -const char *in = tag;
> -char *out = buf;
> -
> -/* Boolean terms surrounded by double quotes can contain any
> - * character. Double quotes are quoted by doubling them. */
> -*out++ = '"';
> -while (*in) {
> - if (*in == '"')
> - *out++ = '"';
> - *out++ = *in++;
> -}
> -*out++ = '"';
> -*out = 0;
> -return buf;
> -}
> -
> -static char *
> _optimize_tag_query (void *ctx, const char *orig_query_string,
>const tag_op_list_t *list)
> {
> @@ -67,44 +49,44 @@ _optimize_tag_query (void *ctx, const char
> *orig_query_string,
> * parenthesize and the exclusion part of the query must not use
> * the '-' operator (though the NOT operator is fine). */
>
> -char *escaped, *query_string;
> +char *escaped = NULL;
> +size_t escaped_len = 0;
> +char *query_string;
> const char *join = "";
> size_t i;
> -unsigned int max_tag_len = 0;
>
> /* Don't optimize if there are no tag changes. */
> if (tag_op_list_size (list) == 0)
> return talloc_strdup (ctx, orig_query_string);
>
> -/* Allocate a buffer for escaping tags. This is large enough to
> - * hold a fully escaped tag with every character doubled plus
> - * enclosing quotes and a NUL. */
> -for (i = 0; i < tag_op_list_size (list); i++)
> - if (strlen (tag_op_list_tag (list, i)) > max_tag_len)
> - max_tag_len = strlen (tag_op_list_tag (list, i));
> -
> -escaped = talloc_array (ctx, char, max_tag_len * 2 + 3);
> -if (! escaped)
> - return NULL;
> -
> /* Build the new query string */
> if (strcmp (orig_query_string, "*") == 0)
> query_string = talloc_strdup (ctx, "(");
> else
> query_string = talloc_asprintf (ctx, "( %s ) and (", orig_query_string);
>
> +
> +/* Boolean terms surrounded by double quotes can contain any
> + * character. Double quotes are quoted by doubling them. */
> +
> for (i = 0; i < tag_op_list_size (list) && query_string; i++) {
> + double_quote_str (ctx,
> + tag_op_list_tag (list, i),
> + &escaped, &escaped_len);
> +
> query_string = talloc_asprintf_append_buffer (
> query_string, "%s%stag:%s", join,
> tag_op_list_isremove (list, i) ? "" : "not ",
> - _escape_tag (escaped, tag_op_list_tag (list, i)));
> + escaped);
> join = " or ";
> }
>
> if (query_string)
> query_string = talloc_strdup_append_buffer (query_string, ")");
>
> -talloc_free (escaped);
> +if (escaped)
> + talloc_free (escaped);
> +
> return query_string;
> }
>
> diff --git a/util/string-util.c b/util/string-util.c
> index 44f8cd3..ea7c25b 100644
> --- a/util/string-util.c
> +++ b/util/string-util.c
> @@ -20,6 +20,7 @@
>
>
> #include "string-util.h"
> +#include "talloc.h"
>
> char *
> strtok_len (char *s, const char *delim, size_t *len)
> @@ -32,3 +33,36 @@ strtok_len (char *s, const char *delim, size_t *len)
>
> return *len ? s : NULL;
> }
> +
> +
> +int
> +double_quote_str (void *ctx, const char *str,
> + char **buf, size_t *len)
> +{
> +const char *in;
> +char *out;
> +size_t needed = 3;
> +
> +for (in = str; *in; in++)
> + needed += (*in == '"') ? 2 : 1;
> +
> +if (needed > *len)
> + *buf = talloc_realloc (ctx, *buf, char, 2*needed);
> +
> +if (! *buf)
> + return 1;
> +
> +out = *buf;
> +
> +*out++ = '"';
> +in = str;
> +while (*in) {
> + if (*in == '"')
> + *out++ = '"';
> + *out++ = *in++;
> +}
> +*out++ = '"';
> +*out = 0;
Just a triviality: isn't '\0' preferred?
Best wishes
Mark
> +
> +return 0;
> +}
> diff --git a/util/string-util.h b/util/string-util.h
> index ac7676c..b593bc7 100644
> --- a/util/string-util.h
> +++ b/util/string-util.h
> @@ -19,4 +19,12 @@
>
> char *strtok_len (char *s, const char *delim, s
Re: [Patch v7 06/14] notmuch-restore: move query handling for batch restore to parser
On Fri, 14 Dec 2012, da...@tethera.net wrote:
> From: David Bremner
>
> We are able to detect more errors by looking at the string before it
> is hex-decoded. We also need this to avoid the query quoting for more
> general queries (to be written) that will mess up raw message-ids.
> ---
> notmuch-restore.c | 18 +-
> tag-util.c| 26 --
> tag-util.h|5 -
> test/dump-restore |5 ++---
> 4 files changed, 27 insertions(+), 27 deletions(-)
>
> diff --git a/notmuch-restore.c b/notmuch-restore.c
> index 40596a8..112f2f3 100644
> --- a/notmuch-restore.c
> +++ b/notmuch-restore.c
> @@ -208,24 +208,8 @@ notmuch_restore_command (unused (void *ctx), int argc,
> char *argv[])
> if (input_format == DUMP_FORMAT_SUP) {
> ret = parse_sup_line (ctx, line, &query_string, tag_ops);
> } else {
> - ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS,
> + ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS |
> TAG_FLAG_ID_ONLY,
> &query_string, tag_ops);
> -
> - if (ret == 0) {
> - if (strncmp ("id:", query_string, 3) != 0) {
> - fprintf (stderr, "Warning: unsupported query: %s\n",
> query_string);
> - continue;
> - }
> - /* delete id: from front of string; tag_message
> - * expects a raw message-id.
> - *
> - * XXX: Note that query string id:foo and bar will be
> - * interpreted as a message id "foo and bar". This
> - * should eventually be fixed to give a better error
> - * message.
> - */
> - query_string = query_string + 3;
> - }
> }
>
> if (ret > 0)
> diff --git a/tag-util.c b/tag-util.c
> index e1181f8..8fea76c 100644
> --- a/tag-util.c
> +++ b/tag-util.c
> @@ -201,14 +201,28 @@ parse_tag_line (void *ctx, char *line,
> }
>
> /* tok now points to the query string */
> -if (hex_decode_inplace (tok) != HEX_SUCCESS) {
> - ret = line_error (TAG_PARSE_INVALID, line_for_error,
> - "hex decoding of query %s failed", tok);
> - goto DONE;
> +if (flags & TAG_FLAG_ID_ONLY) {
> + /* this is under the assumption that any whitespace in the
> + * message-id must be hex-encoded. The check is probably not
> + * perfect for exotic unicode whitespace; as fallback the
> + * search for strange message-ids will fail */
> + if ((strncmp ("id:", tok, 3) != 0) ||
> + (strcspn (tok, " \t") < strlen (tok))) {
> + ret = line_error (TAG_PARSE_INVALID, line_for_error,
> + "query '%s' is not 'id:'", tok);
> + goto DONE;
> + }
> + if (hex_decode_inplace (tok) != HEX_SUCCESS) {
> + ret = line_error (TAG_PARSE_INVALID, line_for_error,
> + "hex decoding of query %s failed", tok);
> + goto DONE;
> + }
> + /* skip 'id:' */
> + *query_string = tok + 3;
This looks like it doesn't double_quote the query_string in this (the
TAG_FLAG_ID_ONLY) case. Is that deliberate?
Best wishes
Mark
> +} else {
> + ret = quote_and_decode_query (ctx, tok, line_for_error, query_string);
> }
>
> -*query_string = tok;
> -
>DONE:
> talloc_free (line_for_error);
> return ret;
> diff --git a/tag-util.h b/tag-util.h
> index 2889736..7674051 100644
> --- a/tag-util.h
> +++ b/tag-util.h
> @@ -26,7 +26,10 @@ typedef enum {
> /* Accept strange tags that might be user error;
> * intended for use by notmuch-restore.
> */
> -TAG_FLAG_BE_GENEROUS = (1 << 3)
> +TAG_FLAG_BE_GENEROUS = (1 << 3),
> +
> +/* Query consists of a single id:$message-id */
> +TAG_FLAG_ID_ONLY = (1 << 4)
>
> } tag_op_flag_t;
>
> diff --git a/test/dump-restore b/test/dump-restore
> index 6a989b6..eb7933a 100755
> --- a/test/dump-restore
> +++ b/test/dump-restore
> @@ -199,19 +199,18 @@ a
> # the next non-comment line should report an an empty tag error for
> # batch tagging, but not for restore
> + +e -- id:20091117232137.ga7...@griffis1.net
> -# highlight the sketchy id parsing; this should be last
> +g -- id:foo and bar
> EOF
>
> cat < EXPECTED
> -Warning: unsupported query: a
> +Warning: query 'a' is not 'id:' [a]
> Warning: no query string [+0]
> Warning: no query string [+a +b]
> Warning: missing query string [+a +b ]
> Warning: no query string after -- [+c +d --]
> Warning: hex decoding of tag %zz failed [+%zz -- id:whatever]
> Warning: hex decoding of query id:%yy failed [+e +f id:%yy]
> -Warning: cannot apply tags to missing message: foo and bar
> +Warning: query 'id:foo and bar' is not 'id:' [+g -- id:foo and
> bar]
> EOF
>
> test_expect_equal_file EXPECTED OUTPUT
> --
> 1.7.10.4
>
> ___
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuch
Re: [Patch v7 05/14] quote_and_decode_query: new function to quote hex-decoded queries
On Fri, 14 Dec 2012, da...@tethera.net wrote:
> From: David Bremner
>
> The query is split into tokens, with ' ' and ':' as delimiters. Any
> token containing some hex-escaped character is quoted according to
> Xapian rules. This maps id:foo%20%22bar to id:"foo ""bar".
> This intentionally does not quote prefixes, so they still work as prefixes.
> ---
> tag-util.c | 50 ++
> 1 file changed, 50 insertions(+)
>
> diff --git a/tag-util.c b/tag-util.c
> index f89669a..e1181f8 100644
> --- a/tag-util.c
> +++ b/tag-util.c
> @@ -56,6 +56,56 @@ illegal_tag (const char *tag, notmuch_bool_t remove)
> return NULL;
> }
>
> +static tag_parse_status_t
> +quote_and_decode_query (void *ctx, char *encoded, const char *line_for_error,
> + char **query_string)
> +{
Would decode_and_quote_query be a better name given the order these two
happen? Also a comment describing the function would be nice.
> +char *tok = encoded;
> +size_t tok_len = 0;
> +char *buf = NULL;
> +size_t buf_len = 0;
> +tag_parse_status_t ret = TAG_PARSE_SUCCESS;
> +
> +*query_string = talloc_strdup (ctx, "");
> +
> +while (*query_string &&
> +(tok = strtok_len (tok + tok_len, ": ", &tok_len)) != NULL) {
> + char delim = tok[tok_len];
> +
> + *(tok + tok_len++) = '\0';
These two look a little odd: I would prefer either array or pointer in
both cases.
> +
> + if (strcspn (tok, "%") < tok_len - 1) {
> + /* something to decode */
> + if (hex_decode_inplace (tok) != HEX_SUCCESS) {
> + ret = line_error (TAG_PARSE_INVALID, line_for_error,
> + "hex decoding of token '%s' failed", tok);
> + goto DONE;
> + }
> +
> + if (double_quote_str (ctx, tok, &buf, &buf_len)) {
> + ret = line_error (TAG_PARSE_OUT_OF_MEMORY,
> + line_for_error, "aborting");
> + goto DONE;
> + }
> + *query_string = talloc_asprintf_append_buffer (
> + *query_string, "%s%c", buf, delim);
> +
> + } else {
> + /* This is not just an optimization, but used to preserve
> + * prefixes like id:, which cannot be quoted.
> + */
> + *query_string = talloc_asprintf_append_buffer (
> + *query_string, "%s%c", tok, delim);
> + }
What happens if a message id (for example) contains a ':'? Is a query of
the form
id:stuff"encoded_stuff"
acceptable? (As far as I can see from the man page ':' does not need to
be in hex.)
Best wishes
Mark
> +
> +}
> +
> + DONE:
> +if (ret != TAG_PARSE_SUCCESS && *query_string)
> + talloc_free (*query_string);
> +return ret;
> +}
> +
> tag_parse_status_t
> parse_tag_line (void *ctx, char *line,
> tag_op_flag_t flags,
> --
> 1.7.10.4
>
> ___
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 7/7] test: Test search's handling of subprocess errors
On Sat, 15 Dec 2012, Austin Clements wrote: > --- > test/emacs | 23 +++ > 1 file changed, 23 insertions(+) > > diff --git a/test/emacs b/test/emacs > index 5403930..88b062c 100755 > --- a/test/emacs > +++ b/test/emacs > @@ -853,4 +853,27 @@ test_expect_success "Rendering HTML mail with images" \ > 'cat OUTPUT && grep -q smiley OUTPUT' > > > +test_begin_subtest "Search handles subprocess errors" > +cat > notmuch_fail < +#!/bin/sh > +echo This is output > +echo This is an error >&2 > +exit 1 > +EOF > +chmod a+x notmuch_fail > +test_emacs "(let ((notmuch-command \"$PWD/notmuch_fail\")) > +(notmuch-search \"tag:inbox\") > +(notmuch-test-wait) > +(test-output) > +(with-current-buffer \"*Notmuch errors*\" > + (test-output \"ERROR\")))" > +test_expect_equal "$(cat OUTPUT ERROR)" "\ > +Error: Unexpected output from notmuch search: > +This is output > +Error: Unexpected output from notmuch search: > +This is an error > +End of search results. > +Error invoking notmuch. /tmp/nmtest/tmp.emacs/notmuch_fail search > --format=json --sort=newest-first tag:inbox exited with status 1." The filename above is hardcoded so fails for me, and the same for patch 5/7 (modulo this the tests would both pass) Best wishes Mark > + > + > test_done > -- > 1.7.10.4
[PATCH 6/7] emacs: Use unified error handling in search
On Sat, 15 Dec 2012, Austin Clements wrote:
> This slightly changes the output of an existing test since we now
> report non-zero exits with a pop-up buffer instead of at the end of
> the search results.
> ---
> emacs/notmuch-lib.el | 13 +
> emacs/notmuch.el | 13 -
> test/emacs-large-search-buffer |2 +-
> 3 files changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
> index 9222de8..cf61635 100644
> --- a/emacs/notmuch-lib.el
> +++ b/emacs/notmuch-lib.el
> @@ -332,6 +332,19 @@ the user dismisses it."
> (goto-char (point-min
> (pop-to-buffer buf)))
>
> +(defun notmuch-check-async-exit-status (proc msg)
> + "If PROC exited abnormally, pop up an error buffer and signal an error.
> +
> +This is a wrapper around `notmuch-check-exit-status' for
> +asynchronous process sentinels. PROC and MSG must be the
> +arguments passed to the sentinel."
> + (let ((exit-status
> + (case (process-status proc)
> +((exit) (process-exit-status proc))
> +((signal) msg
> +(when exit-status
> + (notmuch-check-exit-status exit-status (process-command proc)
> +
> (defun notmuch-check-exit-status (exit-status command &optional output
> err-file)
>"If EXIT-STATUS is non-zero, pop up an error buffer and signal an error.
>
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index e25b54e..c20de13 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -637,6 +637,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
> @@ -647,17 +648,19 @@ of the result."
> (if (eq status 'signal)
> (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)))
> - (insert "\n")
> + (insert "End of search results.\n")
> + (condition-case err
> + (notmuch-check-async-exit-status proc msg)
> + ;; Suppress the error signal since strange
> + ;; things happen if a sentinel signals.
> + (error (throw 'return nil)))
A total triviality (mostly just to check that I am understanding these
parts): is the "err" after the condition-case used/needed?
Best wishes
Mark
> (if (and atbob
>(not (string= notmuch-search-target-thread
> "found")))
> (set 'never-found-target-thread t)
> (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"))
> diff --git a/test/emacs-large-search-buffer b/test/emacs-large-search-buffer
> index 678328d..9dcbef5 100755
> --- a/test/emacs-large-search-buffer
> +++ b/test/emacs-large-search-buffer
> @@ -36,7 +36,7 @@ test_emacs '(notmuch-search "--this-option-does-not-exist")
> cat Error: Unexpected output from notmuch search:
> Unrecognized option: --this-option-does-not-exist
> -End of search results. (process returned 1)
> +End of search results.
> EOF
> test_expect_equal_file OUTPUT EXPECTED
>
> --
> 1.7.10.4
[PATCH 2/7] emacs: Use unified error handling in notmuch-call-notmuch-process
On Sat, 15 Dec 2012, Austin Clements wrote: > This makes notmuch-call-notmuch-process use the unified CLI error > handling, which basically refines the error handling this function > already did. > --- > emacs/notmuch.el | 15 --- > 1 file changed, 4 insertions(+), 11 deletions(-) > > diff --git a/emacs/notmuch.el b/emacs/notmuch.el > index f9454d8..e25b54e 100644 > --- a/emacs/notmuch.el > +++ b/emacs/notmuch.el > @@ -538,17 +538,10 @@ If BARE is set then do not prefix with \"thread:\"" > > Output from the process will be presented to the user as an error > and will also appear in a buffer named \"*Notmuch errors*\"." This comment looks like it is not true (but wasn't true before either). I think output will only be presented to the user if notmuch exits with a non-zero status? > - (let ((error-buffer (get-buffer-create "*Notmuch errors*"))) > -(with-current-buffer error-buffer > - (erase-buffer)) > -(if (eq (apply 'call-process notmuch-command nil error-buffer nil args) > 0) > - (point) > - (progn > - (with-current-buffer error-buffer > - (let ((beg (point-min)) > - (end (- (point-max) 1))) > - (error (buffer-substring beg end)) > - )) > + (with-temp-buffer > +(let ((status (apply #'call-process notmuch-command nil t nil args))) > + (notmuch-check-exit-status status (cons notmuch-command args) > + (buffer-string) > Would it be worth separating stderr and stdout here? (Quite plausibly it isn't.) Best wishes Mark > (defun notmuch-search-set-tags (tags &optional pos) >(let ((new-result (plist-put (notmuch-search-get-result pos) :tags tags))) > -- > 1.7.10.4
notmuch python bindings corrupt db index (was: gmail importer script)
On Sat, Dec 15, 2012 at 7:18 AM, Austin Clements wrote: > > need to at least os.fsync before the os.link. Fixed, thanks for the suggestion. http://git.zx2c4.com/gmail-notmuch/commit/?id=3f9646058bfd91d7d0e2eda035521f97de92eabc -- next part -- An HTML attachment was scrubbed... URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20121215/559ddabd/attachment.html>
gmail importer script
On Thu, Dec 13, 2012 at 5:48 PM, Patrick Totzke wrote: > > notmuch.errors.FileNotEmailError Delete the file it dies on and try again. -- next part -- An HTML attachment was scrubbed... URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20121215/7ac4ba22/attachment.html>
Re: [PATCH v5 0/4] indicate length of omitted body content
On Sat, Dec 15 2012, David Bremner wrote: > Peter Wang writes: > >> On Sat, 15 Dec 2012 08:45:33 +, Mark Walters >> wrote: >>> >>> This version looks good to me with one very minor comments: >>> >>> Perhaps the name format-omitted-part-meta should include sprinter (eg >>> format-omitted-part-meta-sprinter)? >> >> Seems so. I'll roll up another series if that's necessary. > > Maybe just post the one amended patch as a reply to the patch it's > amending? The 'format_omitted_part_meta' in in context of 2 patches; Are you planning to do the change sin one patch (3/4) only (or making 5/4 which changes naming) ? And... The patch series looks good to me. > > d Tomi ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v5 0/4] indicate length of omitted body content
Peter Wang writes: > On Sat, 15 Dec 2012 08:45:33 +, Mark Walters gmail.com> wrote: >> >> This version looks good to me with one very minor comments: >> >> Perhaps the name format-omitted-part-meta should include sprinter (eg >> format-omitted-part-meta-sprinter)? > > Seems so. I'll roll up another series if that's necessary. Maybe just post the one amended patch as a reply to the patch it's amending? d
Re: v3 performance tests improvements
On Sat, Dec 15 2012, da...@tethera.net wrote: > This obsoletes the series > > id:1354762908-5788-1-git-send-email-da...@tethera.net > > I addition to fixing some small things that Austin noticed, this > series adds a convenience function "time_start" that unpacks the mail, > prints the header, and recovers a database cache if present. This > should reduce the amount of boilerplate at the beginning of tests. This patch set looks tolerable to me :-D Tomi ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v5 0/4] indicate length of omitted body content
This version looks good to me with one very minor comments: Perhaps the name format-omitted-part-meta should include sprinter (eg format-omitted-part-meta-sprinter)? Best wishes Mark On Sat, 15 Dec 2012, Peter Wang wrote: > This obsoletes 1355057796-19260-1-git-send-email-markwalters1009 at gmail.com > > Peter Wang (4): > test: normalize only message filenames in show json > show: indicate charset for all omitted parts > show: indicate length, encoding of omitted body content > test: conform to content length, encoding fields > > devel/schemata | 9 - > notmuch-show.c | 36 > test/crypto | 30 +- > test/json| 4 +++- > test/multipart | 26 ++ > test/sexp| 4 +++- > test/test-lib.sh | 2 +- > 7 files changed, 78 insertions(+), 33 deletions(-) > > -- > 1.7.12.1 > > ___ > notmuch mailing list > notmuch at notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch
[Patch v3 11/11] perf-test: use nmbug tags in dump-restore tests
From: David Bremner This makes the tag set a bit less trivial. Note that if you use the small corpus, this is not so interesting (and is also a bit noisy) since the messages will not be found. In the future this could be checked for. Conflicts: performance-test/01-dump-restore --- performance-test/01-dump-restore |1 + 1 file changed, 1 insertion(+) diff --git a/performance-test/01-dump-restore b/performance-test/01-dump-restore index 0ee3a28..b2ff940 100755 --- a/performance-test/01-dump-restore +++ b/performance-test/01-dump-restore @@ -6,6 +6,7 @@ test_description='dump and restore' time_start +time_run 'load nmbug tags' 'notmuch restore --accumulate < corpus.tags/nmbug.sup-dump' time_run 'dump *' 'notmuch dump > tags.out' time_run 'restore *' 'notmuch restore < tags.out' -- 1.7.10.4
[Patch v3 10/11] perf-test: split basic into 00-new, 01-dump-restore, and 02-tag
From: David Bremner We use the new "time_start" function to restore the database from cache if possible. --- performance-test/00-new| 19 +++ performance-test/01-dump-restore | 12 performance-test/02-tag| 14 ++ performance-test/README|8 ++-- performance-test/basic | 20 performance-test/notmuch-perf-test |4 +++- 6 files changed, 54 insertions(+), 23 deletions(-) create mode 100755 performance-test/00-new create mode 100755 performance-test/01-dump-restore create mode 100755 performance-test/02-tag delete mode 100755 performance-test/basic diff --git a/performance-test/00-new b/performance-test/00-new new file mode 100755 index 000..6f0b50c --- /dev/null +++ b/performance-test/00-new @@ -0,0 +1,19 @@ +#!/bin/bash + +test_description='notmuch new' + +. ./perf-test-lib.sh + +uncache_database + +time_start + +time_run 'initial notmuch new' 'notmuch new' + +cache_database + +for i in $(seq 2 6); do +time_run "notmuch new #$i" 'notmuch new' +done + +time_done diff --git a/performance-test/01-dump-restore b/performance-test/01-dump-restore new file mode 100755 index 000..0ee3a28 --- /dev/null +++ b/performance-test/01-dump-restore @@ -0,0 +1,12 @@ +#!/bin/bash + +test_description='dump and restore' + +. ./perf-test-lib.sh + +time_start + +time_run 'dump *' 'notmuch dump > tags.out' +time_run 'restore *' 'notmuch restore < tags.out' + +time_done diff --git a/performance-test/02-tag b/performance-test/02-tag new file mode 100755 index 000..78ceccc --- /dev/null +++ b/performance-test/02-tag @@ -0,0 +1,14 @@ +#!/bin/bash + +test_description='tagging' + +. ./perf-test-lib.sh + +time_start + +time_run 'tag * +new_tag' "notmuch tag +new_tag '*'" +time_run 'tag * +existing_tag' "notmuch tag +new_tag '*'" +time_run 'tag * -existing_tag' "notmuch tag -new_tag '*'" +time_run 'tag * -missing_tag' "notmuch tag -new_tag '*'" + +time_done diff --git a/performance-test/README b/performance-test/README index d36612d..1481660 100644 --- a/performance-test/README +++ b/performance-test/README @@ -51,8 +51,8 @@ Each test script supports the following arguments Writing tests - -Have a look at "basic" for an example. Sourcing "perf-test-lib.sh" is -mandatory. Utility functions include +Have a look at "01-dump-restore" for an example. Sourcing +"perf-test-lib.sh" is mandatory. Utility functions include - 'add_email_corpus' unpacks a set of messages and adds them to the database. - 'cache_database': makes a snapshot of the current database @@ -62,3 +62,7 @@ mandatory. Utility functions include cannot find a cache of the appropriate corpus. - 'time_done' does the cleanup; comment it out or pass --debug to the script to leave the temporary files around. + +Scripts are run in the order specified in notmuch-perf-test. In the +future this order might be chosen automatically so please follow the +convention of starting the name with two digits to specify the order. diff --git a/performance-test/basic b/performance-test/basic deleted file mode 100755 index 41a7ff1..000 --- a/performance-test/basic +++ /dev/null @@ -1,20 +0,0 @@ -#!/bin/bash - -. ./perf-test-lib.sh - -uncache_database - -add_email_corpus - -print_header - -time_run 'initial notmuch new' 'notmuch new' - -cache_database - -time_run 'second notmuch new' 'notmuch new' -time_run 'dump *' 'notmuch dump > tags.out' -time_run 'restore *' 'notmuch restore < tags.out' -time_run 'tag * +new_tag' "notmuch tag +new_tag '*'" - -time_done diff --git a/performance-test/notmuch-perf-test b/performance-test/notmuch-perf-test index 1bea345..fc39d8a 100755 --- a/performance-test/notmuch-perf-test +++ b/performance-test/notmuch-perf-test @@ -17,7 +17,9 @@ fi cd $(dirname "$0") TESTS=" - basic + 00-new + 01-dump-restore + 02-tag " for test in $TESTS; do -- 1.7.10.4
