Re: Xapian-quoting based batch-tagging.
On Tue, 25 Dec 2012, da...@tethera.net wrote: > This is an alternative version of > > id:1356313183-9266-1-git-send-email-da...@tethera.net > > batch tagging patches rebased on top of > > id:1356415076-5692-1-git-send-email-amdra...@mit.edu > > This mainly consisted of removing > > [Patch v9 04/17] notmuch-tag: factor out double quoting routine > (superceded by one of Austin's patches) > > [Patch v9 05/17] util/string-util: add a new string tokenized function > [Patch v9 06/17] unhex_and_quote: new function to quote hex-decoded > queries > [Patch v9 07/17] notmuch-restore: move query handling for batch > (uneeded if query is passed verbatim to xapian) > > I also removed two tests, since they are about how we handle > quoting: > > [Patch v9 13/17] test/tagging: add test for compound queries with batch > tagging > [Patch v9 17/17] test/tagging: add test for handling of parenthesized > tag queries. > > A few small fixes were needed to the tests, and a fair amount of > changes to the notmuch-tag man page. Hi I am unclear about how this is going to deal with queries containing newlines. For dump/restore I think this is not a problem (as Austin and others have said), but for batch tagging I think it could be; for example the query could be for a tag containing a newline. Best wishes Mark > > Diffstat (against Austin's series) is as follows > > man/man1/notmuch-tag.1 | 99 - > notmuch-tag.c | 169 > tag-util.c | 87 +-- > tag-util.h | 15 > test/tagging | 195 ++ > 5 files changed, 480 insertions(+), 85 deletions(-) > > > ___ > 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: Xapian-quoting based batch-tagging.
Mark Walters writes: > I am unclear about how this is going to deal with queries containing > newlines. For dump/restore I think this is not a problem (as Austin and > others have said), but for batch tagging I think it could be; for > example the query could be for a tag containing a newline. Yes, that's true, this patch series does not support queries with tags with embedded newlines. They can still be removed (and added) via either batch tagging or the command line. We could just live with this, or - The current syntax allows for detecting options at the start of the line; perhaps a future fix would be to have the batch tagging and command line tagging accept an optionally hex encoded query, something like: --hex +found%20it -- tag:%22stupid%0Atag%22 - Alternatively, we could add hex decoding on top of xapian quoting for queries. One UI downside is that people have to remember that % are special. +found%25it -- tag:lost%25it Another is that quoting is still (surprisingly) necessary for encoded spaces +found%20it -- tag:"lost%20it" Introducing yet another escape format, e.g. "\n" would require more code, and not really much benefit afaict versus re-using hex-encoding. Offhand I don't see how to avoid this without some level of query pre-processing a-la id:1356313183-9266-1-git-send-email-da...@tethera.net d ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 09/11] notmuch-tag.1: tidy synopsis formatting, reference
da...@tethera.net writes: > From: David Bremner > > Consistently use [...]; one less space. Use singular > --- pushed this one patch. d ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 01/11] parse_tag_line: use enum for return value.
da...@tethera.net writes: > From: David Bremner > > This is essentially cosmetic, since success=0 is promised by > the comments in tag-utils.h. Pushed this one patch. Both of the two I pushed from this series are really nothing to do with batch tagging, and were just cluttering things up. d ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v2 1/5] util: Factor out boolean term quoting routine
Austin Clements writes: > +/* Construct a boolean term query with the specified prefix (e.g., > + * "id") and search term, quoting term as necessary. > + * > + * Output is into buf; it may be talloc_realloced. > + * Return: 0 on success, non-zero on memory allocation failure. > + */ > +int make_boolean_term (void *talloc_ctx, const char *prefix, const char > *term, > +char **buf, size_t *len); > + I think the quoting should be described a bit more precisely here, especially since you are going to refer to it in the comment above parse_boolean_term. d ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v2 2/5] util: Function to parse boolean term queries
Austin Clements writes: > + char *out = talloc_strdup (ctx, pos + 1); > + int closed = 0; > + /* Find the closing quote and un-double doubled internal > + * quotes. */ > + for (pos = *term_out = out; *pos; ) { Since you strdup anyway, wouldn't it be easier to understand if pos read from the input string and out wrote to term_out? Something like (untested) index db01b4b..e4157d0 100644 --- a/util/string-util.c +++ b/util/string-util.c @@ -112,11 +112,12 @@ parse_boolean_term (void *ctx, const char *str, /* Implement de-quoting compatible with make_boolean_term. */ if (*pos == '"') { - char *out = talloc_strdup (ctx, pos + 1); + char *out; int closed = 0; + *term_out= talloc_strdup (ctx, pos + 1); /* Find the closing quote and un-double doubled internal * quotes. */ - for (pos = *term_out = out; *pos; ) { + for (out = *term_out; *pos; ) { if (*pos == '"') { ++pos; Perhaps the two talloc_strdups can even be unified, but I wouldn't worry too much about that. ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v2 0/5] Use Xapian query syntax for batch-tag dump/restore
Austin Clements writes: > This obsoletes > > id:1356415076-5692-1-git-send-email-amdra...@mit.edu > > In addition to incorporating all of David's suggestions, this reworks > the boolean term parsing so it only handles the subset of quoting > syntax used by make_boolean_term (which also happens to be all that we > described in the man page for the format). The diff from v1 is below. Implementation wise this series now looks OK to me; I think the dequote routine could be slightly clearer, but it's a bit subjective at this point. IMHO the real question is if we like (or can live with) the resulting UI for batch tagging. Mark ( id:87ehid5h64@qmul.ac.uk ) brings up the question of whether tags with newlines should be somehow supported in a batch-tagging context. The other UI issue I'm aware of is the fact that tags with spaces needed to be quoted in two different ways for tag operations and for queries. A relatively smooth upgrade path would be to support Xapian quoting for tag operations in the future. d ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] emacs: show: make id links respect window
Mark Walters writes: >> I can imagine that people would want/like the "open in other window" >> effect of the current code, even if the reason is a bug. > > That's definitely possible. I generally expect a mouse click to select > the window I click and this feels counter intuitive. I think that some > people might like an option "open this link in a new window" but I would > guess that would like that whether they clicked or pressed RET on the > button. I don't care much either way myself, but before we change notmuch-show behaviour (effectively) to accomodate notmuch-pick, I'd like a bit more feedback from other people. d ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] notmuch.c: run uncrustify
From: David Bremner In anticipation of doing some updates to this code, it simplifies life if the code is "uncrustify clean" to start with --- notmuch.c | 23 +++ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/notmuch.c b/notmuch.c index 9516dfb..ee2892e 100644 --- a/notmuch.c +++ b/notmuch.c @@ -22,7 +22,7 @@ #include "notmuch-client.h" -typedef int (*command_function_t) (void *ctx, int argc, char *argv[]); +typedef int (*command_function_t)(void *ctx, int argc, char *argv[]); typedef struct command { const char *name; @@ -39,8 +39,8 @@ typedef struct alias { } alias_t; alias_t aliases[] = { -{ "part", { "show", "--format=raw"}}, -{ "search-tags", {"search", "--output=tags", "*"}} +{ "part", { "show", "--format=raw" } }, +{ "search-tags", { "search", "--output=tags", "*" } } }; static int @@ -66,7 +66,7 @@ static command_t commands[] = { "[options...] [...]", "Construct a reply template for a set of messages." }, { "tag", notmuch_tag_command, - "+|- [...] [--] [...]" , + "+|- [...] [--] [...]", "Add/remove tags for all messages matching the search terms." }, { "dump", notmuch_dump_command, "[] [--] []", @@ -107,8 +107,8 @@ usage (FILE *out) fprintf (out, "\n"); fprintf (out, -"Use \"notmuch help \" for more details on each command\n" -"and \"notmuch help search-terms\" for the common search-terms syntax.\n\n"); +"Use \"notmuch help \" for more details on each command\n" +"and \"notmuch help search-terms\" for the common search-terms syntax.\n\n"); } void @@ -281,15 +281,14 @@ main (int argc, char *argv[]) return notmuch_help_command (NULL, argc - 1, &argv[1]); if (strcmp (argv[1], "--version") == 0) { - printf ("notmuch " STRINGIFY(NOTMUCH_VERSION) "\n"); + printf ("notmuch " STRINGIFY (NOTMUCH_VERSION) "\n"); return 0; } for (i = 0; i < ARRAY_SIZE (aliases); i++) { alias = &aliases[i]; - if (strcmp (argv[1], alias->name) == 0) - { + if (strcmp (argv[1], alias->name) == 0) { int substitutions; argv_local = talloc_size (local, sizeof (char *) * @@ -304,14 +303,14 @@ main (int argc, char *argv[]) for (j = 0; j < MAX_ALIAS_SUBSTITUTIONS; j++) { if (alias->substitutions[j] == NULL) break; - argv_local[j+1] = alias->substitutions[j]; + argv_local[j + 1] = alias->substitutions[j]; } substitutions = j; /* And copy all original arguments (skipping the argument * that matched the alias of course. */ for (j = 2; j < (unsigned) argc; j++) { - argv_local[substitutions+j-1] = argv[j]; + argv_local[substitutions + j - 1] = argv[j]; } argc += substitutions - 1; @@ -323,7 +322,7 @@ main (int argc, char *argv[]) command = &commands[i]; if (strcmp (argv[1], command->name) == 0) - return (command->function) (local, argc - 1, &argv[1]); + return (command->function)(local, argc - 1, &argv[1]); } fprintf (stderr, "Error: Unknown command '%s' (see \"notmuch help\")\n", -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/2] test/dump-restore: new tests for empty files and leading comments/whitespace.
From: David Bremner Three of these are marked broken; the third is a regression test, since it passes by virtue of batch-tag being the default input format. --- test/dump-restore | 42 ++ 1 file changed, 42 insertions(+) diff --git a/test/dump-restore b/test/dump-restore index 6a989b6..c2ddb92 100755 --- a/test/dump-restore +++ b/test/dump-restore @@ -136,6 +136,48 @@ notmuch dump --format=batch-tag > BACKUP notmuch tag +"$tag1" +"$tag2" +"$tag3" -inbox -unread "*" +# initial segment of file used for several tests below. +cat < comments-and-blanks +# this is a comment + +# next line has leading whitespace + + +EOF + +test_begin_subtest 'restoring empty file is not an error' +test_subtest_known_broken +notmuch restore < /dev/null 2>OUTPUT.$test_count +cp /dev/null EXPECTED +test_expect_equal_file EXPECTED OUTPUT.$test_count + +test_begin_subtest 'file of comments and blank lines is not an error' +test_subtest_known_broken +notmuch restore --input=comments-and-blanks +ret_val=$? +test_expect_equal "$ret_val" "0" + +cp comments-and-blanks leading-comments-blanks-batch-tag +echo "+some_tag -- id:yun1vjwegii@aiko.keithp.com" \ +>> leading-comments-blanks-batch-tag + +test_begin_subtest 'detect format=batch-tag with leading comments and blanks' +notmuch restore --input=leading-comments-blanks-batch-tag +notmuch search --output=tags id:yun1vjwegii@aiko.keithp.com > OUTPUT.$test_count +echo "some_tag" > EXPECTED +test_expect_equal_file EXPECTED OUTPUT.$test_count + +cp comments-and-blanks leading-comments-blanks-sup +echo "yun1vjwegii@aiko.keithp.com (another_tag)" \ +>> leading-comments-blanks-sup + +test_begin_subtest 'detect format=sup with leading comments and blanks' +test_subtest_known_broken +notmuch restore --input=leading-comments-blanks-sup +notmuch search --output=tags id:yun1vjwegii@aiko.keithp.com > OUTPUT.$test_count +echo "another_tag" > EXPECTED +test_expect_equal_file EXPECTED OUTPUT.$test_count + test_begin_subtest 'format=batch-tag, round trip with strange tags' notmuch dump --format=batch-tag > EXPECTED.$test_count notmuch dump --format=batch-tag | notmuch restore --format=batch-tag -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 2/2] notmuch-restore: handle empty input file, leading blank lines and comments.
From: David Bremner This patch corrects several undesirable behaviours: 1) Empty files were not detected, leading to buffer read overrun. 2) An initial blank line cause restore to silently abort 3) Initial comment line caused format detection to fail --- notmuch-restore.c | 17 - test/dump-restore |3 --- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/notmuch-restore.c b/notmuch-restore.c index 9ed9b51..c93f1ac 100644 --- a/notmuch-restore.c +++ b/notmuch-restore.c @@ -180,11 +180,6 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) argv[opt_index]); return 1; } -char *p; - -line_len = getline (&line, &line_size, input); -if (line_len == 0) - return 0; tag_ops = tag_op_list_create (ctx); if (tag_ops == NULL) { @@ -192,6 +187,18 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) return 1; } +do { + line_len = getline (&line, &line_size, input); + + /* empty input file not considered an error */ + if (line_len < 0) + return 0; + +} while ((line_len == 0) || +(line[0] == '#') || +(strspn (line, " \t\n") == strlen (line))); + +char *p; for (p = line; (input_format == DUMP_FORMAT_AUTO) && *p; p++) { if (*p == '(') input_format = DUMP_FORMAT_SUP; diff --git a/test/dump-restore b/test/dump-restore index c2ddb92..ae30cd1 100755 --- a/test/dump-restore +++ b/test/dump-restore @@ -146,13 +146,11 @@ cat < comments-and-blanks EOF test_begin_subtest 'restoring empty file is not an error' -test_subtest_known_broken notmuch restore < /dev/null 2>OUTPUT.$test_count cp /dev/null EXPECTED test_expect_equal_file EXPECTED OUTPUT.$test_count test_begin_subtest 'file of comments and blank lines is not an error' -test_subtest_known_broken notmuch restore --input=comments-and-blanks ret_val=$? test_expect_equal "$ret_val" "0" @@ -172,7 +170,6 @@ echo "yun1vjwegii@aiko.keithp.com (another_tag)" \ >> leading-comments-blanks-sup test_begin_subtest 'detect format=sup with leading comments and blanks' -test_subtest_known_broken notmuch restore --input=leading-comments-blanks-sup notmuch search --output=tags id:yun1vjwegii@aiko.keithp.com > OUTPUT.$test_count echo "another_tag" > EXPECTED -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
v2 talloc leak report
Obsoletes id:1355714648-23144-1-git-send-email-da...@tethera.netz [Patch v2 1/4] CLI: add talloc leak report, controlled by an - comments and formatting [Patch v2 2/4] util: add talloc-extra.[ch] - rename, fix dangerous memcpy [Patch v2 3/4] notmuch-restore: use debug version of talloc_strndup no change, except new names [Patch v2 4/4] perf-test: initial support for talloc leak report in New patch, use talloc reports in memory tests. ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[Patch v2 2/4] util: add talloc-extra.[ch]
From: David Bremner These are intended to be simple wrappers to provide slightly better debugging information than what talloc currently provides natively. --- notmuch-client.h|2 +- util/Makefile.local |2 +- util/talloc-extra.c | 14 ++ util/talloc-extra.h | 18 ++ 4 files changed, 34 insertions(+), 2 deletions(-) create mode 100644 util/talloc-extra.c create mode 100644 util/talloc-extra.h diff --git a/notmuch-client.h b/notmuch-client.h index d7b352e..5f28836 100644 --- a/notmuch-client.h +++ b/notmuch-client.h @@ -58,7 +58,7 @@ typedef GMimeCipherContext notmuch_crypto_context_t; #include #include -#include +#include "talloc-extra.h" #define unused(x) x __attribute__ ((unused)) diff --git a/util/Makefile.local b/util/Makefile.local index a11e35b..29c0ce6 100644 --- a/util/Makefile.local +++ b/util/Makefile.local @@ -4,7 +4,7 @@ dir := util extra_cflags += -I$(srcdir)/$(dir) libutil_c_srcs := $(dir)/xutil.c $(dir)/error_util.c $(dir)/hex-escape.c \ - $(dir)/string-util.c + $(dir)/string-util.c $(dir)/talloc-extra.c libutil_modules := $(libutil_c_srcs:.c=.o) diff --git a/util/talloc-extra.c b/util/talloc-extra.c new file mode 100644 index 000..4a5f9c0 --- /dev/null +++ b/util/talloc-extra.c @@ -0,0 +1,14 @@ +#include +#include "talloc-extra.h" + +char * +talloc_strndup_named_const (void *ctx, const char *str, +size_t len, const char *name) +{ +char *ptr = talloc_strndup (ctx, str, len); + +if (ptr) + talloc_set_name_const(ptr, name); + +return ptr; +} diff --git a/util/talloc-extra.h b/util/talloc-extra.h new file mode 100644 index 000..5b8ca28 --- /dev/null +++ b/util/talloc-extra.h @@ -0,0 +1,18 @@ +#ifndef _XTALLOC_H +#define _XTALLOC_H + +#include + +/* Like talloc_strndup, but take an extra parameter for the internal talloc + * name (for debugging) */ + +char * +talloc_strndup_named_const (void *ctx, const char *str, + size_t len, const char *name); + +/* use the __location__ macro from talloc.h to name a string according to its + * source location */ + +#define talloc_strndup_debug(ctx, str, len) talloc_strndup_named_const (ctx, str, len, __location__) + +#endif -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[Patch v2 3/4] notmuch-restore: use debug version of talloc_strndup
From: David Bremner This gives line numbers for better debugging. --- notmuch-restore.c |9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/notmuch-restore.c b/notmuch-restore.c index c93f1ac..6111977 100644 --- a/notmuch-restore.c +++ b/notmuch-restore.c @@ -88,10 +88,11 @@ parse_sup_line (void *ctx, char *line, return 1; } -*query_str = talloc_strndup (ctx, line + match[1].rm_so, -match[1].rm_eo - match[1].rm_so); -file_tags = talloc_strndup (ctx, line + match[2].rm_so, - match[2].rm_eo - match[2].rm_so); +*query_str = talloc_strndup_debug (ctx, line + match[1].rm_so, + match[1].rm_eo - match[1].rm_so); + +file_tags = talloc_strndup_debug (ctx, line + match[2].rm_so, + match[2].rm_eo - match[2].rm_so); char *tok = file_tags; size_t tok_len = 0; -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[Patch v2 4/4] perf-test: initial support for talloc leak report in memory tests
From: David Bremner As with the valgrind logs, we print a (very) brief summary and leave the log for inspection. --- performance-test/perf-test-lib.sh |5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/performance-test/perf-test-lib.sh b/performance-test/perf-test-lib.sh index 10d05e0..9ee7661 100644 --- a/performance-test/perf-test-lib.sh +++ b/performance-test/perf-test-lib.sh @@ -126,13 +126,16 @@ memory_run () test_count=$(($test_count+1)) log_file=$log_dir/$test_count.log +talloc_log=$log_dir/$test_count.talloc printf "[ %d ]\t%s\n" $test_count "$1" -valgrind --leak-check=full --log-file="$log_file" $2 +NOTMUCH_TALLOC_REPORT="$talloc_log" valgrind --leak-check=full --log-file="$log_file" $2 awk '/LEAK SUMMARY/,/suppressed/ { sub(/^==[0-9]*==/," "); print }' "$log_file" echo +sed -n -e 's/.*[(]total *\([^)]*\)[)]/talloced at exit: \1/p' $talloc_log +echo } memory_done () -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[Patch v2 1/4] CLI: add talloc leak report, controlled by an environment variable.
From: David Bremner The argument handling in notmuch.c seems due for an overhaul, but until then use an environment variable to specify a location to write the talloc leak report to. This is only enabled for the (interesting) case where some notmuch subcommand is invoked. --- notmuch.c | 24 ++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/notmuch.c b/notmuch.c index ee2892e..6b7aae8 100644 --- a/notmuch.c +++ b/notmuch.c @@ -321,8 +321,28 @@ main (int argc, char *argv[]) for (i = 0; i < ARRAY_SIZE (commands); i++) { command = &commands[i]; - if (strcmp (argv[1], command->name) == 0) - return (command->function)(local, argc - 1, &argv[1]); + if (strcmp (argv[1], command->name) == 0) { + int ret; + char *talloc_report; + + ret = (command->function)(local, argc - 1, &argv[1]); + + /* in the future support for this environment variable may +* be supplemented or replaced by command line arguments +* --leak-report and/or --leak-report-full */ + + talloc_report = getenv ("NOTMUCH_TALLOC_REPORT"); + + /* this relies on the previous call to +* talloc_enable_null_tracking */ + + if (talloc_report && strcmp (talloc_report, "") != 0) { + FILE *report = fopen (talloc_report, "w"); + talloc_report_full (NULL, report); + } + + return ret; + } } fprintf (stderr, "Error: Unknown command '%s' (see \"notmuch help\")\n", -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] emacs: tweak error buffer handling
On Tue, 25 Dec 2012, Tomi Ollila wrote: > On Sat, Dec 22 2012, Mark Walters wrote: > >> view-mode-enter changed between emacs 23 and emacs 24: the current >> code makes the error buffer disappear in emacs 24 on quitting it (ie >> pressing q) but this just kills the buffer without closing the split >> window in emacs 23. >> >> This patch makes the error buffer window disappear in emacs 23 >> too. Since the view-mode-enter function changed we have to test for >> version and do the correct thing in each case. >> --- >> >> This seems to work but I have only tested on 23.4 and 24.2 > > I run emacs 23.1.1 to get the documentation of view-mode-enter > there. So, this patch instructs to delete WINDOW when exiting > view mode... > > Documentation of pop-to-buffer says: > > "Select buffer BUFFER-OR-NAME in some window, preferably a different one." > > What if pop-up-windows's value is nil -- the content of current window > is replaced with this view stuff -- and when exiting view mode, the > window will be deleted ? What happens with emacs 24 in this case ? Hi You are quite right there are problems here under emacs 23: if you already have a split window when the error occurs in one part the error is displayed in the other window and then on exit that (previously existing) window is closed. What do people think should happen on an error? I, personally, don't like taking over an existing window, and Jamie liked some of the errors (eg non-fatal `locked database' tagging errors) to be shown in the mini-buffer. I also think it is going to be difficult to get this right: emacs 23 and 24 are different and there are also some user configuration variable that affect what happens. Best wishes Mark > > Tomi > >> Best wishes >> >> Mark >> >> >> >> emacs/notmuch-lib.el |8 +--- >> 1 files changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el >> index 77a591d..0407f8a 100644 >> --- a/emacs/notmuch-lib.el >> +++ b/emacs/notmuch-lib.el >> @@ -324,15 +324,17 @@ the user dismisses it." >> >>(let ((buf (get-buffer-create "*Notmuch errors*"))) >> (with-current-buffer buf >> - (view-mode-enter nil #'kill-buffer) >> + (pop-to-buffer buf) >> + (view-mode-enter (when (< emacs-major-version 24) >> + (cons (selected-window) (cons nil t))) >> + #'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))) >> + (insert "\n")) >> >> (defun notmuch-check-async-exit-status (proc msg) >>"If PROC exited abnormally, pop up an error buffer and signal an error. >> -- >> 1.7.9.1 >> >> ___ >> 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 v2 0/2] Add flush/reopen methods to notmuch_database_t
Michael Forney writes: > On Thu, 18 Oct 2012 00:52:20 +0300, Adrien Bustany wrote: >> The code of the patches in unchanged, but the formatting issues are now >> hopefully fixed. > > I would like to bump this patch set. I also need these features from > libnotmuch. Currently there is no way to recover from Xapian errors, > which is pretty limiting. > > If it is the flush/commit that is the issue, I would be happy to make an > updated patch set. Hi Michael; Right, we should use the current name of 'commit'. We also need some way to check the xapian version in the configure script, and to make sure the right name is supported. Jani refers to id:1350487737-32058-2-git-send-email-bgamari.f...@gmail.com farther up the thread as having some relevant code/ideas. I think it's reasonable to require Xapian version at least 1.2.0, so that might simplify the configuration check. The other thing I wondered is if the error handling in these routines is as good as it could be. I guess it isn't worse than what is there, but especially since you are explicitly wanting to handle error conditions, it would be good to think if we can do something better than adding yet another printf to the library. d ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Xapian-quoting based batch-tagging.
On Tue, 25 Dec 2012, david at tethera.net wrote: > This is an alternative version of > > id:1356313183-9266-1-git-send-email-david at tethera.net > > batch tagging patches rebased on top of > > id:1356415076-5692-1-git-send-email-amdragon at mit.edu > > This mainly consisted of removing > > [Patch v9 04/17] notmuch-tag: factor out double quoting routine > (superceded by one of Austin's patches) > > [Patch v9 05/17] util/string-util: add a new string tokenized function > [Patch v9 06/17] unhex_and_quote: new function to quote hex-decoded > queries > [Patch v9 07/17] notmuch-restore: move query handling for batch > (uneeded if query is passed verbatim to xapian) > > I also removed two tests, since they are about how we handle > quoting: > > [Patch v9 13/17] test/tagging: add test for compound queries with batch > tagging > [Patch v9 17/17] test/tagging: add test for handling of parenthesized > tag queries. > > A few small fixes were needed to the tests, and a fair amount of > changes to the notmuch-tag man page. Hi I am unclear about how this is going to deal with queries containing newlines. For dump/restore I think this is not a problem (as Austin and others have said), but for batch tagging I think it could be; for example the query could be for a tag containing a newline. Best wishes Mark > > Diffstat (against Austin's series) is as follows > > man/man1/notmuch-tag.1 | 99 - > notmuch-tag.c | 169 > tag-util.c | 87 +-- > tag-util.h | 15 > test/tagging | 195 ++ > 5 files changed, 480 insertions(+), 85 deletions(-) > > > ___ > notmuch mailing list > notmuch at notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch
Xapian-quoting based batch-tagging.
Mark Walters writes: > I am unclear about how this is going to deal with queries containing > newlines. For dump/restore I think this is not a problem (as Austin and > others have said), but for batch tagging I think it could be; for > example the query could be for a tag containing a newline. Yes, that's true, this patch series does not support queries with tags with embedded newlines. They can still be removed (and added) via either batch tagging or the command line. We could just live with this, or - The current syntax allows for detecting options at the start of the line; perhaps a future fix would be to have the batch tagging and command line tagging accept an optionally hex encoded query, something like: --hex +found%20it -- tag:%22stupid%0Atag%22 - Alternatively, we could add hex decoding on top of xapian quoting for queries. One UI downside is that people have to remember that % are special. +found%25it -- tag:lost%25it Another is that quoting is still (surprisingly) necessary for encoded spaces +found%20it -- tag:"lost%20it" Introducing yet another escape format, e.g. "\n" would require more code, and not really much benefit afaict versus re-using hex-encoding. Offhand I don't see how to avoid this without some level of query pre-processing a-la id:1356313183-9266-1-git-send-email-david at tethera.net d
[PATCH 09/11] notmuch-tag.1: tidy synopsis formatting, reference
david at tethera.net writes: > From: David Bremner > > Consistently use [...]; one less space. Use singular > --- pushed this one patch. d
[PATCH 01/11] parse_tag_line: use enum for return value.
david at tethera.net writes: > From: David Bremner > > This is essentially cosmetic, since success=0 is promised by > the comments in tag-utils.h. Pushed this one patch. Both of the two I pushed from this series are really nothing to do with batch tagging, and were just cluttering things up. d
[PATCH v2 1/5] util: Factor out boolean term quoting routine
Austin Clements writes: > +/* Construct a boolean term query with the specified prefix (e.g., > + * "id") and search term, quoting term as necessary. > + * > + * Output is into buf; it may be talloc_realloced. > + * Return: 0 on success, non-zero on memory allocation failure. > + */ > +int make_boolean_term (void *talloc_ctx, const char *prefix, const char > *term, > +char **buf, size_t *len); > + I think the quoting should be described a bit more precisely here, especially since you are going to refer to it in the comment above parse_boolean_term. d
[PATCH v2 2/5] util: Function to parse boolean term queries
Austin Clements writes: > + char *out = talloc_strdup (ctx, pos + 1); > + int closed = 0; > + /* Find the closing quote and un-double doubled internal > + * quotes. */ > + for (pos = *term_out = out; *pos; ) { Since you strdup anyway, wouldn't it be easier to understand if pos read from the input string and out wrote to term_out? Something like (untested) index db01b4b..e4157d0 100644 --- a/util/string-util.c +++ b/util/string-util.c @@ -112,11 +112,12 @@ parse_boolean_term (void *ctx, const char *str, /* Implement de-quoting compatible with make_boolean_term. */ if (*pos == '"') { - char *out = talloc_strdup (ctx, pos + 1); + char *out; int closed = 0; + *term_out= talloc_strdup (ctx, pos + 1); /* Find the closing quote and un-double doubled internal * quotes. */ - for (pos = *term_out = out; *pos; ) { + for (out = *term_out; *pos; ) { if (*pos == '"') { ++pos; Perhaps the two talloc_strdups can even be unified, but I wouldn't worry too much about that.
[PATCH v2 0/5] Use Xapian query syntax for batch-tag dump/restore
Austin Clements writes: > This obsoletes > > id:1356415076-5692-1-git-send-email-amdragon at mit.edu > > In addition to incorporating all of David's suggestions, this reworks > the boolean term parsing so it only handles the subset of quoting > syntax used by make_boolean_term (which also happens to be all that we > described in the man page for the format). The diff from v1 is below. Implementation wise this series now looks OK to me; I think the dequote routine could be slightly clearer, but it's a bit subjective at this point. IMHO the real question is if we like (or can live with) the resulting UI for batch tagging. Mark ( id:87ehid5h64.fsf at qmul.ac.uk ) brings up the question of whether tags with newlines should be somehow supported in a batch-tagging context. The other UI issue I'm aware of is the fact that tags with spaces needed to be quoted in two different ways for tag operations and for queries. A relatively smooth upgrade path would be to support Xapian quoting for tag operations in the future. d
[PATCH] emacs: show: make id links respect window
Mark Walters writes: >> I can imagine that people would want/like the "open in other window" >> effect of the current code, even if the reason is a bug. > > That's definitely possible. I generally expect a mouse click to select > the window I click and this feels counter intuitive. I think that some > people might like an option "open this link in a new window" but I would > guess that would like that whether they clicked or pressed RET on the > button. I don't care much either way myself, but before we change notmuch-show behaviour (effectively) to accomodate notmuch-pick, I'd like a bit more feedback from other people. d
[PATCH] notmuch.c: run uncrustify
From: David Bremner In anticipation of doing some updates to this code, it simplifies life if the code is "uncrustify clean" to start with --- notmuch.c | 23 +++ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/notmuch.c b/notmuch.c index 9516dfb..ee2892e 100644 --- a/notmuch.c +++ b/notmuch.c @@ -22,7 +22,7 @@ #include "notmuch-client.h" -typedef int (*command_function_t) (void *ctx, int argc, char *argv[]); +typedef int (*command_function_t)(void *ctx, int argc, char *argv[]); typedef struct command { const char *name; @@ -39,8 +39,8 @@ typedef struct alias { } alias_t; alias_t aliases[] = { -{ "part", { "show", "--format=raw"}}, -{ "search-tags", {"search", "--output=tags", "*"}} +{ "part", { "show", "--format=raw" } }, +{ "search-tags", { "search", "--output=tags", "*" } } }; static int @@ -66,7 +66,7 @@ static command_t commands[] = { "[options...] [...]", "Construct a reply template for a set of messages." }, { "tag", notmuch_tag_command, - "+|- [...] [--] [...]" , + "+|- [...] [--] [...]", "Add/remove tags for all messages matching the search terms." }, { "dump", notmuch_dump_command, "[] [--] []", @@ -107,8 +107,8 @@ usage (FILE *out) fprintf (out, "\n"); fprintf (out, -"Use \"notmuch help \" for more details on each command\n" -"and \"notmuch help search-terms\" for the common search-terms syntax.\n\n"); +"Use \"notmuch help \" for more details on each command\n" +"and \"notmuch help search-terms\" for the common search-terms syntax.\n\n"); } void @@ -281,15 +281,14 @@ main (int argc, char *argv[]) return notmuch_help_command (NULL, argc - 1, &argv[1]); if (strcmp (argv[1], "--version") == 0) { - printf ("notmuch " STRINGIFY(NOTMUCH_VERSION) "\n"); + printf ("notmuch " STRINGIFY (NOTMUCH_VERSION) "\n"); return 0; } for (i = 0; i < ARRAY_SIZE (aliases); i++) { alias = &aliases[i]; - if (strcmp (argv[1], alias->name) == 0) - { + if (strcmp (argv[1], alias->name) == 0) { int substitutions; argv_local = talloc_size (local, sizeof (char *) * @@ -304,14 +303,14 @@ main (int argc, char *argv[]) for (j = 0; j < MAX_ALIAS_SUBSTITUTIONS; j++) { if (alias->substitutions[j] == NULL) break; - argv_local[j+1] = alias->substitutions[j]; + argv_local[j + 1] = alias->substitutions[j]; } substitutions = j; /* And copy all original arguments (skipping the argument * that matched the alias of course. */ for (j = 2; j < (unsigned) argc; j++) { - argv_local[substitutions+j-1] = argv[j]; + argv_local[substitutions + j - 1] = argv[j]; } argc += substitutions - 1; @@ -323,7 +322,7 @@ main (int argc, char *argv[]) command = &commands[i]; if (strcmp (argv[1], command->name) == 0) - return (command->function) (local, argc - 1, &argv[1]); + return (command->function)(local, argc - 1, &argv[1]); } fprintf (stderr, "Error: Unknown command '%s' (see \"notmuch help\")\n", -- 1.7.10.4
[PATCH 1/2] test/dump-restore: new tests for empty files and leading comments/whitespace.
From: David Bremner Three of these are marked broken; the third is a regression test, since it passes by virtue of batch-tag being the default input format. --- test/dump-restore | 42 ++ 1 file changed, 42 insertions(+) diff --git a/test/dump-restore b/test/dump-restore index 6a989b6..c2ddb92 100755 --- a/test/dump-restore +++ b/test/dump-restore @@ -136,6 +136,48 @@ notmuch dump --format=batch-tag > BACKUP notmuch tag +"$tag1" +"$tag2" +"$tag3" -inbox -unread "*" +# initial segment of file used for several tests below. +cat < comments-and-blanks +# this is a comment + +# next line has leading whitespace + + +EOF + +test_begin_subtest 'restoring empty file is not an error' +test_subtest_known_broken +notmuch restore < /dev/null 2>OUTPUT.$test_count +cp /dev/null EXPECTED +test_expect_equal_file EXPECTED OUTPUT.$test_count + +test_begin_subtest 'file of comments and blank lines is not an error' +test_subtest_known_broken +notmuch restore --input=comments-and-blanks +ret_val=$? +test_expect_equal "$ret_val" "0" + +cp comments-and-blanks leading-comments-blanks-batch-tag +echo "+some_tag -- id:yun1vjwegii.fsf at aiko.keithp.com" \ +>> leading-comments-blanks-batch-tag + +test_begin_subtest 'detect format=batch-tag with leading comments and blanks' +notmuch restore --input=leading-comments-blanks-batch-tag +notmuch search --output=tags id:yun1vjwegii.fsf at aiko.keithp.com > OUTPUT.$test_count +echo "some_tag" > EXPECTED +test_expect_equal_file EXPECTED OUTPUT.$test_count + +cp comments-and-blanks leading-comments-blanks-sup +echo "yun1vjwegii.fsf at aiko.keithp.com (another_tag)" \ +>> leading-comments-blanks-sup + +test_begin_subtest 'detect format=sup with leading comments and blanks' +test_subtest_known_broken +notmuch restore --input=leading-comments-blanks-sup +notmuch search --output=tags id:yun1vjwegii.fsf at aiko.keithp.com > OUTPUT.$test_count +echo "another_tag" > EXPECTED +test_expect_equal_file EXPECTED OUTPUT.$test_count + test_begin_subtest 'format=batch-tag, round trip with strange tags' notmuch dump --format=batch-tag > EXPECTED.$test_count notmuch dump --format=batch-tag | notmuch restore --format=batch-tag -- 1.7.10.4
[PATCH 2/2] notmuch-restore: handle empty input file, leading blank lines and comments.
From: David Bremner This patch corrects several undesirable behaviours: 1) Empty files were not detected, leading to buffer read overrun. 2) An initial blank line cause restore to silently abort 3) Initial comment line caused format detection to fail --- notmuch-restore.c | 17 - test/dump-restore |3 --- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/notmuch-restore.c b/notmuch-restore.c index 9ed9b51..c93f1ac 100644 --- a/notmuch-restore.c +++ b/notmuch-restore.c @@ -180,11 +180,6 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) argv[opt_index]); return 1; } -char *p; - -line_len = getline (&line, &line_size, input); -if (line_len == 0) - return 0; tag_ops = tag_op_list_create (ctx); if (tag_ops == NULL) { @@ -192,6 +187,18 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) return 1; } +do { + line_len = getline (&line, &line_size, input); + + /* empty input file not considered an error */ + if (line_len < 0) + return 0; + +} while ((line_len == 0) || +(line[0] == '#') || +(strspn (line, " \t\n") == strlen (line))); + +char *p; for (p = line; (input_format == DUMP_FORMAT_AUTO) && *p; p++) { if (*p == '(') input_format = DUMP_FORMAT_SUP; diff --git a/test/dump-restore b/test/dump-restore index c2ddb92..ae30cd1 100755 --- a/test/dump-restore +++ b/test/dump-restore @@ -146,13 +146,11 @@ cat < comments-and-blanks EOF test_begin_subtest 'restoring empty file is not an error' -test_subtest_known_broken notmuch restore < /dev/null 2>OUTPUT.$test_count cp /dev/null EXPECTED test_expect_equal_file EXPECTED OUTPUT.$test_count test_begin_subtest 'file of comments and blank lines is not an error' -test_subtest_known_broken notmuch restore --input=comments-and-blanks ret_val=$? test_expect_equal "$ret_val" "0" @@ -172,7 +170,6 @@ echo "yun1vjwegii.fsf at aiko.keithp.com (another_tag)" \ >> leading-comments-blanks-sup test_begin_subtest 'detect format=sup with leading comments and blanks' -test_subtest_known_broken notmuch restore --input=leading-comments-blanks-sup notmuch search --output=tags id:yun1vjwegii.fsf at aiko.keithp.com > OUTPUT.$test_count echo "another_tag" > EXPECTED -- 1.7.10.4
v2 talloc leak report
Obsoletes id:1355714648-23144-1-git-send-email-david at tethera.netZ [Patch v2 1/4] CLI: add talloc leak report, controlled by an - comments and formatting [Patch v2 2/4] util: add talloc-extra.[ch] - rename, fix dangerous memcpy [Patch v2 3/4] notmuch-restore: use debug version of talloc_strndup no change, except new names [Patch v2 4/4] perf-test: initial support for talloc leak report in New patch, use talloc reports in memory tests.
[Patch v2 2/4] util: add talloc-extra.[ch]
From: David Bremner These are intended to be simple wrappers to provide slightly better debugging information than what talloc currently provides natively. --- notmuch-client.h|2 +- util/Makefile.local |2 +- util/talloc-extra.c | 14 ++ util/talloc-extra.h | 18 ++ 4 files changed, 34 insertions(+), 2 deletions(-) create mode 100644 util/talloc-extra.c create mode 100644 util/talloc-extra.h diff --git a/notmuch-client.h b/notmuch-client.h index d7b352e..5f28836 100644 --- a/notmuch-client.h +++ b/notmuch-client.h @@ -58,7 +58,7 @@ typedef GMimeCipherContext notmuch_crypto_context_t; #include #include -#include +#include "talloc-extra.h" #define unused(x) x __attribute__ ((unused)) diff --git a/util/Makefile.local b/util/Makefile.local index a11e35b..29c0ce6 100644 --- a/util/Makefile.local +++ b/util/Makefile.local @@ -4,7 +4,7 @@ dir := util extra_cflags += -I$(srcdir)/$(dir) libutil_c_srcs := $(dir)/xutil.c $(dir)/error_util.c $(dir)/hex-escape.c \ - $(dir)/string-util.c + $(dir)/string-util.c $(dir)/talloc-extra.c libutil_modules := $(libutil_c_srcs:.c=.o) diff --git a/util/talloc-extra.c b/util/talloc-extra.c new file mode 100644 index 000..4a5f9c0 --- /dev/null +++ b/util/talloc-extra.c @@ -0,0 +1,14 @@ +#include +#include "talloc-extra.h" + +char * +talloc_strndup_named_const (void *ctx, const char *str, +size_t len, const char *name) +{ +char *ptr = talloc_strndup (ctx, str, len); + +if (ptr) + talloc_set_name_const(ptr, name); + +return ptr; +} diff --git a/util/talloc-extra.h b/util/talloc-extra.h new file mode 100644 index 000..5b8ca28 --- /dev/null +++ b/util/talloc-extra.h @@ -0,0 +1,18 @@ +#ifndef _XTALLOC_H +#define _XTALLOC_H + +#include + +/* Like talloc_strndup, but take an extra parameter for the internal talloc + * name (for debugging) */ + +char * +talloc_strndup_named_const (void *ctx, const char *str, + size_t len, const char *name); + +/* use the __location__ macro from talloc.h to name a string according to its + * source location */ + +#define talloc_strndup_debug(ctx, str, len) talloc_strndup_named_const (ctx, str, len, __location__) + +#endif -- 1.7.10.4
[Patch v2 3/4] notmuch-restore: use debug version of talloc_strndup
From: David Bremner This gives line numbers for better debugging. --- notmuch-restore.c |9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/notmuch-restore.c b/notmuch-restore.c index c93f1ac..6111977 100644 --- a/notmuch-restore.c +++ b/notmuch-restore.c @@ -88,10 +88,11 @@ parse_sup_line (void *ctx, char *line, return 1; } -*query_str = talloc_strndup (ctx, line + match[1].rm_so, -match[1].rm_eo - match[1].rm_so); -file_tags = talloc_strndup (ctx, line + match[2].rm_so, - match[2].rm_eo - match[2].rm_so); +*query_str = talloc_strndup_debug (ctx, line + match[1].rm_so, + match[1].rm_eo - match[1].rm_so); + +file_tags = talloc_strndup_debug (ctx, line + match[2].rm_so, + match[2].rm_eo - match[2].rm_so); char *tok = file_tags; size_t tok_len = 0; -- 1.7.10.4
[Patch v2 4/4] perf-test: initial support for talloc leak report in memory tests
From: David Bremner As with the valgrind logs, we print a (very) brief summary and leave the log for inspection. --- performance-test/perf-test-lib.sh |5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/performance-test/perf-test-lib.sh b/performance-test/perf-test-lib.sh index 10d05e0..9ee7661 100644 --- a/performance-test/perf-test-lib.sh +++ b/performance-test/perf-test-lib.sh @@ -126,13 +126,16 @@ memory_run () test_count=$(($test_count+1)) log_file=$log_dir/$test_count.log +talloc_log=$log_dir/$test_count.talloc printf "[ %d ]\t%s\n" $test_count "$1" -valgrind --leak-check=full --log-file="$log_file" $2 +NOTMUCH_TALLOC_REPORT="$talloc_log" valgrind --leak-check=full --log-file="$log_file" $2 awk '/LEAK SUMMARY/,/suppressed/ { sub(/^==[0-9]*==/," "); print }' "$log_file" echo +sed -n -e 's/.*[(]total *\([^)]*\)[)]/talloced at exit: \1/p' $talloc_log +echo } memory_done () -- 1.7.10.4
[Patch v2 1/4] CLI: add talloc leak report, controlled by an environment variable.
From: David Bremner The argument handling in notmuch.c seems due for an overhaul, but until then use an environment variable to specify a location to write the talloc leak report to. This is only enabled for the (interesting) case where some notmuch subcommand is invoked. --- notmuch.c | 24 ++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/notmuch.c b/notmuch.c index ee2892e..6b7aae8 100644 --- a/notmuch.c +++ b/notmuch.c @@ -321,8 +321,28 @@ main (int argc, char *argv[]) for (i = 0; i < ARRAY_SIZE (commands); i++) { command = &commands[i]; - if (strcmp (argv[1], command->name) == 0) - return (command->function)(local, argc - 1, &argv[1]); + if (strcmp (argv[1], command->name) == 0) { + int ret; + char *talloc_report; + + ret = (command->function)(local, argc - 1, &argv[1]); + + /* in the future support for this environment variable may +* be supplemented or replaced by command line arguments +* --leak-report and/or --leak-report-full */ + + talloc_report = getenv ("NOTMUCH_TALLOC_REPORT"); + + /* this relies on the previous call to +* talloc_enable_null_tracking */ + + if (talloc_report && strcmp (talloc_report, "") != 0) { + FILE *report = fopen (talloc_report, "w"); + talloc_report_full (NULL, report); + } + + return ret; + } } fprintf (stderr, "Error: Unknown command '%s' (see \"notmuch help\")\n", -- 1.7.10.4
[PATCH] emacs: tweak error buffer handling
On Tue, 25 Dec 2012, Tomi Ollila wrote: > On Sat, Dec 22 2012, Mark Walters wrote: > >> view-mode-enter changed between emacs 23 and emacs 24: the current >> code makes the error buffer disappear in emacs 24 on quitting it (ie >> pressing q) but this just kills the buffer without closing the split >> window in emacs 23. >> >> This patch makes the error buffer window disappear in emacs 23 >> too. Since the view-mode-enter function changed we have to test for >> version and do the correct thing in each case. >> --- >> >> This seems to work but I have only tested on 23.4 and 24.2 > > I run emacs 23.1.1 to get the documentation of view-mode-enter > there. So, this patch instructs to delete WINDOW when exiting > view mode... > > Documentation of pop-to-buffer says: > > "Select buffer BUFFER-OR-NAME in some window, preferably a different one." > > What if pop-up-windows's value is nil -- the content of current window > is replaced with this view stuff -- and when exiting view mode, the > window will be deleted ? What happens with emacs 24 in this case ? Hi You are quite right there are problems here under emacs 23: if you already have a split window when the error occurs in one part the error is displayed in the other window and then on exit that (previously existing) window is closed. What do people think should happen on an error? I, personally, don't like taking over an existing window, and Jamie liked some of the errors (eg non-fatal `locked database' tagging errors) to be shown in the mini-buffer. I also think it is going to be difficult to get this right: emacs 23 and 24 are different and there are also some user configuration variable that affect what happens. Best wishes Mark > > Tomi > >> Best wishes >> >> Mark >> >> >> >> emacs/notmuch-lib.el |8 +--- >> 1 files changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el >> index 77a591d..0407f8a 100644 >> --- a/emacs/notmuch-lib.el >> +++ b/emacs/notmuch-lib.el >> @@ -324,15 +324,17 @@ the user dismisses it." >> >>(let ((buf (get-buffer-create "*Notmuch errors*"))) >> (with-current-buffer buf >> - (view-mode-enter nil #'kill-buffer) >> + (pop-to-buffer buf) >> + (view-mode-enter (when (< emacs-major-version 24) >> + (cons (selected-window) (cons nil t))) >> + #'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))) >> + (insert "\n")) >> >> (defun notmuch-check-async-exit-status (proc msg) >>"If PROC exited abnormally, pop up an error buffer and signal an error. >> -- >> 1.7.9.1 >> >> ___ >> notmuch mailing list >> notmuch at notmuchmail.org >> http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 0/2] Add flush/reopen methods to notmuch_database_t
Michael Forney writes: > On Thu, 18 Oct 2012 00:52:20 +0300, Adrien Bustany > wrote: >> The code of the patches in unchanged, but the formatting issues are now >> hopefully fixed. > > I would like to bump this patch set. I also need these features from > libnotmuch. Currently there is no way to recover from Xapian errors, > which is pretty limiting. > > If it is the flush/commit that is the issue, I would be happy to make an > updated patch set. Hi Michael; Right, we should use the current name of 'commit'. We also need some way to check the xapian version in the configure script, and to make sure the right name is supported. Jani refers to id:1350487737-32058-2-git-send-email-bgamari.foss at gmail.com farther up the thread as having some relevant code/ideas. I think it's reasonable to require Xapian version at least 1.2.0, so that might simplify the configuration check. The other thing I wondered is if the error handling in these routines is as good as it could be. I guess it isn't worse than what is there, but especially since you are explicitly wanting to handle error conditions, it would be good to think if we can do something better than adding yet another printf to the library. d