[PATCH] vim: fix regex after "notmuch show" output change
Jakob writes: > On Sat, 24 Mar 2012 10:58:59 +0200, Tomi Ollila wrote: >> Is this regexp part below good ? >> >> > +... match:\([0-9]*\) excluded:\([[0-9]*\) filename:\(.*\)$', ... >> >> ( --> excluded:\([[0-9]*\) <-- ) > > Yeah, that was the core of the change. With this new field in the > output, the old regex didn't match at all. You can see the fix working by > trying to use the vim plugin as-is and trying to reply to a message or > view a message's ID or any number of other things that fail right now. > After updating this regex, everything starts working again. I meant about the syntax: \([[0-9]*\) i.e. 2 opening [[:s > > Thanks, > Jakob Tomi
[PATCH v2 3/3] cli: refactor "notmuch restore" message tagging into a separate function
Refactor to make tagging code easier to reuse in the future. No functional changes. Signed-off-by: Jani Nikula --- notmuch-restore.c | 148 - 1 files changed, 78 insertions(+), 70 deletions(-) diff --git a/notmuch-restore.c b/notmuch-restore.c index 87d9772..2e965af 100644 --- a/notmuch-restore.c +++ b/notmuch-restore.c @@ -21,6 +21,81 @@ #include "notmuch-client.h" int +tag_message (void *ctx, notmuch_database_t *notmuch, const char *message_id, +char *file_tags, notmuch_bool_t remove_all, +notmuch_bool_t synchronize_flags) +{ +notmuch_status_t status; +notmuch_tags_t *db_tags; +char *db_tags_str; +notmuch_message_t *message = NULL; +const char *tag; +char *next; +int ret = 0; + +status = notmuch_database_find_message (notmuch, message_id, ); +if (status || message == NULL) { + fprintf (stderr, "Warning: Cannot apply tags to %smessage: %s\n", +message ? "" : "missing ", message_id); + if (status) + fprintf (stderr, "%s\n", notmuch_status_to_string(status)); + return 1; +} + +/* In order to detect missing messages, this check/optimization is + * intentionally done *after* first finding the message. */ +if (!remove_all && (file_tags == NULL || *file_tags == '\0')) + goto DONE; + +db_tags_str = NULL; +for (db_tags = notmuch_message_get_tags (message); +notmuch_tags_valid (db_tags); +notmuch_tags_move_to_next (db_tags)) { + tag = notmuch_tags_get (db_tags); + + if (db_tags_str) + db_tags_str = talloc_asprintf_append (db_tags_str, " %s", tag); + else + db_tags_str = talloc_strdup (message, tag); +} + +if (((file_tags == NULL || *file_tags == '\0') && +(db_tags_str == NULL || *db_tags_str == '\0')) || + (file_tags && db_tags_str && strcmp (file_tags, db_tags_str) == 0)) + goto DONE; + +notmuch_message_freeze (message); + +if (remove_all) + notmuch_message_remove_all_tags (message); + +next = file_tags; +while (next) { + tag = strsep (, " "); + if (*tag == '\0') + continue; + status = notmuch_message_add_tag (message, tag); + if (status) { + fprintf (stderr, "Error applying tag %s to message %s:\n", +tag, message_id); + fprintf (stderr, "%s\n", notmuch_status_to_string (status)); + ret = 1; + } +} + +notmuch_message_thaw (message); + +if (synchronize_flags) + notmuch_message_tags_to_maildir_flags (message); + +DONE: +if (message) + notmuch_message_destroy (message); + +return ret; +} + +int notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) { notmuch_config_t *config; @@ -88,11 +163,7 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) while ((line_len = getline (, _size, input)) != -1) { regmatch_t match[3]; - char *message_id, *file_tags, *tag, *next; - notmuch_message_t *message = NULL; - notmuch_status_t status; - notmuch_tags_t *db_tags; - char *db_tags_str; + char *message_id, *file_tags; chomp_newline (line); @@ -109,72 +180,9 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) file_tags = xstrndup (line + match[2].rm_so, match[2].rm_eo - match[2].rm_so); - status = notmuch_database_find_message (notmuch, message_id, ); - if (status || message == NULL) { - fprintf (stderr, "Warning: Cannot apply tags to %smessage: %s\n", -message ? "" : "missing ", message_id); - if (status) - fprintf (stderr, "%s\n", -notmuch_status_to_string(status)); - goto NEXT_LINE; - } - - /* In order to detect missing messages, this check/optimization is -* intentionally done *after* first finding the message. */ - if (accumulate && (file_tags == NULL || *file_tags == '\0')) - { - goto NEXT_LINE; - } - - db_tags_str = NULL; - for (db_tags = notmuch_message_get_tags (message); -notmuch_tags_valid (db_tags); -notmuch_tags_move_to_next (db_tags)) - { - const char *tag = notmuch_tags_get (db_tags); - - if (db_tags_str) - db_tags_str = talloc_asprintf_append (db_tags_str, " %s", tag); - else - db_tags_str = talloc_strdup (message, tag); - } - - if (((file_tags == NULL || *file_tags == '\0') && -(db_tags_str == NULL || *db_tags_str == '\0')) || - (file_tags && db_tags_str && strcmp (file_tags, db_tags_str) == 0)) - { - goto NEXT_LINE; - } - - notmuch_message_freeze (message); - - if (!accumulate) - notmuch_message_remove_all_tags (message);
[PATCH v2 2/3] cli: refactor "notmuch tag" query tagging into a separate function
Refactor to make tagging code easier to reuse in the future. No functional changes. Signed-off-by: Jani Nikula --- notmuch-tag.c | 101 - 1 files changed, 57 insertions(+), 44 deletions(-) diff --git a/notmuch-tag.c b/notmuch-tag.c index c39b235..edbfdec 100644 --- a/notmuch-tag.c +++ b/notmuch-tag.c @@ -106,6 +106,60 @@ _optimize_tag_query (void *ctx, const char *orig_query_string, return query_string; } +static int +tag_query (void *ctx, notmuch_database_t *notmuch, const char *query_string, + tag_operation_t *tag_ops, notmuch_bool_t synchronize_flags) +{ +notmuch_query_t *query; +notmuch_messages_t *messages; +notmuch_message_t *message; +int i; + +/* Optimize the query so it excludes messages that already have + * the specified set of tags. */ +query_string = _optimize_tag_query (ctx, query_string, tag_ops); +if (query_string == NULL) { + fprintf (stderr, "Out of memory.\n"); + return 1; +} + +query = notmuch_query_create (notmuch, query_string); +if (query == NULL) { + fprintf (stderr, "Out of memory.\n"); + return 1; +} + +/* tagging is not interested in any special sort order */ +notmuch_query_set_sort (query, NOTMUCH_SORT_UNSORTED); + +for (messages = notmuch_query_search_messages (query); +notmuch_messages_valid (messages) && !interrupted; +notmuch_messages_move_to_next (messages)) +{ + message = notmuch_messages_get (messages); + + notmuch_message_freeze (message); + + for (i = 0; tag_ops[i].tag; i++) { + if (tag_ops[i].remove) + notmuch_message_remove_tag (message, tag_ops[i].tag); + else + notmuch_message_add_tag (message, tag_ops[i].tag); + } + + notmuch_message_thaw (message); + + if (synchronize_flags) + notmuch_message_tags_to_maildir_flags (message); + + notmuch_message_destroy (message); +} + +notmuch_query_destroy (query); + +return interrupted; +} + int notmuch_tag_command (void *ctx, int argc, char *argv[]) { @@ -114,12 +168,10 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) char *query_string; notmuch_config_t *config; notmuch_database_t *notmuch; -notmuch_query_t *query; -notmuch_messages_t *messages; -notmuch_message_t *message; struct sigaction action; notmuch_bool_t synchronize_flags; int i; +int ret; /* Setup our handler for SIGINT */ memset (, 0, sizeof (struct sigaction)); @@ -166,14 +218,6 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) return 1; } -/* Optimize the query so it excludes messages that already have - * the specified set of tags. */ -query_string = _optimize_tag_query (ctx, query_string, tag_ops); -if (query_string == NULL) { - fprintf (stderr, "Out of memory.\n"); - return 1; -} - config = notmuch_config_open (ctx, NULL, NULL); if (config == NULL) return 1; @@ -185,40 +229,9 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config); -query = notmuch_query_create (notmuch, query_string); -if (query == NULL) { - fprintf (stderr, "Out of memory.\n"); - return 1; -} - -/* tagging is not interested in any special sort order */ -notmuch_query_set_sort (query, NOTMUCH_SORT_UNSORTED); +ret = tag_query (ctx, notmuch, query_string, tag_ops, synchronize_flags); -for (messages = notmuch_query_search_messages (query); -notmuch_messages_valid (messages) && !interrupted; -notmuch_messages_move_to_next (messages)) -{ - message = notmuch_messages_get (messages); - - notmuch_message_freeze (message); - - for (i = 0; tag_ops[i].tag; i++) { - if (tag_ops[i].remove) - notmuch_message_remove_tag (message, tag_ops[i].tag); - else - notmuch_message_add_tag (message, tag_ops[i].tag); - } - - notmuch_message_thaw (message); - - if (synchronize_flags) - notmuch_message_tags_to_maildir_flags (message); - - notmuch_message_destroy (message); -} - -notmuch_query_destroy (query); notmuch_database_close (notmuch); -return interrupted; +return ret; } -- 1.7.5.4
[PATCH v2 1/3] cli: refactor "notmuch tag" data structures for tagging operations
To simplify code, keep all tagging operations in a single array instead of separate add and remove arrays. Apply tag changes in the order specified on the command line, instead of first removing and then adding the tags. This results in a minor functional change: If a tag is both added and removed, the last specified operation is now used. Previously the tag was always added. Signed-off-by: Jani Nikula --- notmuch-tag.c | 79 +--- 1 files changed, 35 insertions(+), 44 deletions(-) diff --git a/notmuch-tag.c b/notmuch-tag.c index 36b9b09..c39b235 100644 --- a/notmuch-tag.c +++ b/notmuch-tag.c @@ -53,10 +53,14 @@ _escape_tag (char *buf, const char *tag) return buf; } +typedef struct { +const char *tag; +notmuch_bool_t remove; +} tag_operation_t; + static char * -_optimize_tag_query (void *ctx, const char *orig_query_string, char *argv[], -int *add_tags, int add_tags_count, -int *remove_tags, int remove_tags_count) +_optimize_tag_query (void *ctx, const char *orig_query_string, +const tag_operation_t *tag_ops) { /* This is subtler than it looks. Xapian ignores the '-' operator * at the beginning both queries and parenthesized groups and, @@ -74,12 +78,9 @@ _optimize_tag_query (void *ctx, const char *orig_query_string, char *argv[], /* 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 < add_tags_count; i++) - if (strlen (argv[add_tags[i]] + 1) > max_tag_len) - max_tag_len = strlen (argv[add_tags[i]] + 1); -for (i = 0; i < remove_tags_count; i++) - if (strlen (argv[remove_tags[i]] + 1) > max_tag_len) - max_tag_len = strlen (argv[remove_tags[i]] + 1); +for (i = 0; tag_ops[i].tag; i++) + if (strlen (tag_ops[i].tag) > max_tag_len) + max_tag_len = strlen (tag_ops[i].tag); escaped = talloc_array(ctx, char, max_tag_len * 2 + 3); if (!escaped) return NULL; @@ -90,16 +91,11 @@ _optimize_tag_query (void *ctx, const char *orig_query_string, char *argv[], else query_string = talloc_asprintf (ctx, "( %s ) and (", orig_query_string); -for (i = 0; i < add_tags_count && query_string; i++) { - query_string = talloc_asprintf_append_buffer ( - query_string, "%snot tag:%s", join, - _escape_tag (escaped, argv[add_tags[i]] + 1)); - join = " or "; -} -for (i = 0; i < remove_tags_count && query_string; i++) { +for (i = 0; tag_ops[i].tag && query_string; i++) { query_string = talloc_asprintf_append_buffer ( - query_string, "%stag:%s", join, - _escape_tag (escaped, argv[remove_tags[i]] + 1)); + query_string, "%s%stag:%s", join, + tag_ops[i].remove ? "" : "not ", + _escape_tag (escaped, tag_ops[i].tag)); join = " or "; } @@ -113,9 +109,8 @@ _optimize_tag_query (void *ctx, const char *orig_query_string, char *argv[], int notmuch_tag_command (void *ctx, int argc, char *argv[]) { -int *add_tags, *remove_tags; -int add_tags_count = 0; -int remove_tags_count = 0; +tag_operation_t *tag_ops; +int tag_ops_count = 0; char *query_string; notmuch_config_t *config; notmuch_database_t *notmuch; @@ -133,35 +128,33 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) action.sa_flags = SA_RESTART; sigaction (SIGINT, , NULL); -add_tags = talloc_size (ctx, argc * sizeof (int)); -if (add_tags == NULL) { - fprintf (stderr, "Out of memory.\n"); - return 1; -} +argc--; argv++; /* skip subcommand argument */ -remove_tags = talloc_size (ctx, argc * sizeof (int)); -if (remove_tags == NULL) { +/* Array of tagging operations (add or remove), terminated with an + * empty element. */ +tag_ops = talloc_array (ctx, tag_operation_t, argc + 1); +if (tag_ops == NULL) { fprintf (stderr, "Out of memory.\n"); return 1; } -argc--; argv++; /* skip subcommand argument */ - for (i = 0; i < argc; i++) { if (strcmp (argv[i], "--") == 0) { i++; break; } - if (argv[i][0] == '+') { - add_tags[add_tags_count++] = i; - } else if (argv[i][0] == '-') { - remove_tags[remove_tags_count++] = i; + if (argv[i][0] == '+' || argv[i][0] == '-') { + tag_ops[tag_ops_count].tag = argv[i] + 1; + tag_ops[tag_ops_count].remove = argv[i][0] == '-'; + tag_ops_count++; } else { break; } } -if (add_tags_count == 0 && remove_tags_count == 0) { +tag_ops[tag_ops_count].tag = NULL; + +if (tag_ops_count == 0) { fprintf (stderr, "Error: 'notmuch tag' requires at least one tag to add or remove.\n"); return 1;
[PATCH v2 0/3] cli: notmuch tag/restore refactoring
v2 of id:"cover.1332604895.git.jani at nikula.org" with the following non-functional changes, addressing David's concerns in mail and IRC: - do not use C99 style struct assignment in patch 1 - for now, keep tag_message() local to notmuch-restore.c in patch 3 BR, Jani. Jani Nikula (3): cli: refactor "notmuch tag" data structures for tagging operations cli: refactor "notmuch tag" query tagging into a separate function cli: refactor "notmuch restore" message tagging into a separate function notmuch-restore.c | 148 +-- notmuch-tag.c | 166 +++-- 2 files changed, 163 insertions(+), 151 deletions(-) -- 1.7.5.4
[BUG/PATCH 2/2] emacs: Fix replying from alternate addresses
Adam Wolfe Gordonwrites: > The bug was that notmuch-mua-mail used `mail-header` to check whether > it was passed a "From" header. The implementation of `mail-header` > must try to compare symbols instead of strings when looking for > headers, as it was returning nil when a From header was present. This > is probably because the mail functions construct headers as alists > with symbols for the header names, while our code uses strings for the > header names. > > Since we don't use `mail-header` anywhere else, and `message-mail` is > perfectly happy to accept string header names, the fix is just to use > `assoc` to look for the From header, so that the strings get compared > properly. I can confirm that everything works as expected with this fix. Best wishes Mark > --- > emacs/notmuch-mua.el |4 ++-- > test/emacs |1 - > 2 files changed, 2 insertions(+), 3 deletions(-) > > diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el > index 6aae3a0..9805d79 100644 > --- a/emacs/notmuch-mua.el > +++ b/emacs/notmuch-mua.el > @@ -187,7 +187,7 @@ OTHER-ARGS are passed through to `message-mail'." >(when (not (string= "" user-agent)) > (push (cons "User-Agent" user-agent) other-headers > > - (unless (mail-header 'From other-headers) > + (unless (assoc "From" other-headers) > (push (cons "From" (concat > (notmuch-user-name) " <" (notmuch-user-primary-email) > ">")) other-headers)) > > @@ -250,7 +250,7 @@ the From: address first." >(interactive "P") >(let ((other-headers >(when (or prompt-for-sender notmuch-always-prompt-for-sender) > -(list (cons 'From (notmuch-mua-prompt-for-sender)) > +(list (cons "From" (notmuch-mua-prompt-for-sender)) > (notmuch-mua-mail nil nil other-headers))) > > (defun notmuch-mua-new-forward-message ( prompt-for-sender) > diff --git a/test/emacs b/test/emacs > index fa5d706..08db1ee 100755 > --- a/test/emacs > +++ b/test/emacs > @@ -275,7 +275,6 @@ EOF > test_expect_equal_file OUTPUT EXPECTED > > test_begin_subtest "Reply from alternate address within emacs" > -test_subtest_known_broken > add_message '[from]="Sender "' \ >[to]=test_suite_other at notmuchmail.org \ >[subject]=notmuch-reply-test \ > -- > 1.7.5.4 > > ___ > notmuch mailing list > notmuch at notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] vim: fix regex after "notmuch show" output change
On Sat, 24 Mar 2012 10:58:59 +0200, Tomi Ollila wrote: > Is this regexp part below good ? > > > +... match:\([0-9]*\) excluded:\([[0-9]*\) filename:\(.*\)$', ... > > ( --> excluded:\([[0-9]*\) <-- ) Yeah, that was the core of the change. With this new field in the output, the old regex didn't match at all. You can see the fix working by trying to use the vim plugin as-is and trying to reply to a message or view a message's ID or any number of other things that fail right now. After updating this regex, everything starts working again. Thanks, Jakob
[PATCH 3/3] cli: refactor "notmuch restore" message tagging into a separate function
On Sat, 24 Mar 2012 18:14:37 +0200, Jani Nikula wrote: > Refactor to make tagging code easier to reuse in the future. No > functional changes. The second two look OK as well. It isn't obvious to me that in the long term we want an API using "file_tags" to pass in a string. It reminds me of the term "stringly typed". Of course, I see why it is this way in an intial refactor, but maybe something to think about before we start propagating such an API. d
mutt-notmuch in notmuch contrib
Hi Zack In the mean time, we at least have a contrib directory, and I think mutt-notmuch would be welcome there. Could you (or somebody) make a reasonable size patch series that adds it to contrib/mutt-notmuch against current master. The patch series should be sent to the upstream mailing list notmuch at notmuchmail.org for review. On the debian side your patch series could also include the necessary changes to make a new binary package. Notmuch itself is already GPL3+ so no hassles there (for once). d
[PATCH 1/3] cli: refactor "notmuch tag" data structures for tagging operations
On Sat, 24 Mar 2012 18:14:35 +0200, Jani Nikula wrote: > To simplify code, keep all tagging operations in a single array > instead of separate add and remove arrays. Apply tag changes in the > order specified on the command line, instead of first removing and > then adding the tags. Like I said on IRC, my only minor reservation with this patch is that the use of C99 struct assignment is maybe not a good tradeoff here, since it makes the code a bit more mysterious (for the great unwashed masses that don't know C99 as well as C89). Still, if you think extra initialization features are worth it, I can live with it. d
[PATCH] emacs: content-type comparison should be case insensitive.
The function notmuch-match-content-type was comparing content types case sensitively. Fix it so it tests case insensitively. This fixes a bug where emacs would not include any body when replying to a message with content-type TEXT/PLAIN. --- emacs/notmuch-lib.el |5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el index c146748..a754de7 100644 --- a/emacs/notmuch-lib.el +++ b/emacs/notmuch-lib.el @@ -185,8 +185,9 @@ the user hasn't set this variable with the old or new value." (st2 (notmuch-split-content-type t2))) (if (or (string= (cadr st1) "*") (string= (cadr st2) "*")) - (string= (car st1) (car st2)) - (string= t1 t2 + ;; Comparison of content types should be case insensitive. + (string= (downcase (car st1)) (downcase (car st2))) + (string= (downcase t1) (downcase t2) (defvar notmuch-multipart/alternative-discouraged '( -- 1.7.9.1
Re: [BUG/PATCH 2/2] emacs: Fix replying from alternate addresses
Adam Wolfe Gordon awg+notm...@xvx.ca writes: The bug was that notmuch-mua-mail used `mail-header` to check whether it was passed a From header. The implementation of `mail-header` must try to compare symbols instead of strings when looking for headers, as it was returning nil when a From header was present. This is probably because the mail functions construct headers as alists with symbols for the header names, while our code uses strings for the header names. Since we don't use `mail-header` anywhere else, and `message-mail` is perfectly happy to accept string header names, the fix is just to use `assoc` to look for the From header, so that the strings get compared properly. I can confirm that everything works as expected with this fix. Best wishes Mark --- emacs/notmuch-mua.el |4 ++-- test/emacs |1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el index 6aae3a0..9805d79 100644 --- a/emacs/notmuch-mua.el +++ b/emacs/notmuch-mua.el @@ -187,7 +187,7 @@ OTHER-ARGS are passed through to `message-mail'. (when (not (string= user-agent)) (push (cons User-Agent user-agent) other-headers - (unless (mail-header 'From other-headers) + (unless (assoc From other-headers) (push (cons From (concat (notmuch-user-name) (notmuch-user-primary-email) )) other-headers)) @@ -250,7 +250,7 @@ the From: address first. (interactive P) (let ((other-headers (when (or prompt-for-sender notmuch-always-prompt-for-sender) -(list (cons 'From (notmuch-mua-prompt-for-sender)) +(list (cons From (notmuch-mua-prompt-for-sender)) (notmuch-mua-mail nil nil other-headers))) (defun notmuch-mua-new-forward-message (optional prompt-for-sender) diff --git a/test/emacs b/test/emacs index fa5d706..08db1ee 100755 --- a/test/emacs +++ b/test/emacs @@ -275,7 +275,6 @@ EOF test_expect_equal_file OUTPUT EXPECTED test_begin_subtest Reply from alternate address within emacs -test_subtest_known_broken add_message '[from]=Sender sen...@example.com' \ [to]=test_suite_ot...@notmuchmail.org \ [subject]=notmuch-reply-test \ -- 1.7.5.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
mutt-notmuch in notmuch contrib
Hi Zack In the mean time, we at least have a contrib directory, and I think mutt-notmuch would be welcome there. Could you (or somebody) make a reasonable size patch series that adds it to contrib/mutt-notmuch against current master. The patch series should be sent to the upstream mailing list notmuch@notmuchmail.org for review. On the debian side your patch series could also include the necessary changes to make a new binary package. Notmuch itself is already GPL3+ so no hassles there (for once). d ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] vim: fix regex after notmuch show output change
On Sat, 24 Mar 2012 10:58:59 +0200, Tomi Ollila tomi.oll...@iki.fi wrote: Is this regexp part below good ? +... match:\([0-9]*\) excluded:\([[0-9]*\) filename:\(.*\)$', ... ( -- excluded:\([[0-9]*\) -- ) Yeah, that was the core of the change. With this new field in the output, the old regex didn't match at all. You can see the fix working by trying to use the vim plugin as-is and trying to reply to a message or view a message's ID or any number of other things that fail right now. After updating this regex, everything starts working again. Thanks, Jakob ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 0/3] cli: notmuch tag/restore refactoring
v2 of id:cover.1332604895.git.j...@nikula.org with the following non-functional changes, addressing David's concerns in mail and IRC: - do not use C99 style struct assignment in patch 1 - for now, keep tag_message() local to notmuch-restore.c in patch 3 BR, Jani. Jani Nikula (3): cli: refactor notmuch tag data structures for tagging operations cli: refactor notmuch tag query tagging into a separate function cli: refactor notmuch restore message tagging into a separate function notmuch-restore.c | 148 +-- notmuch-tag.c | 166 +++-- 2 files changed, 163 insertions(+), 151 deletions(-) -- 1.7.5.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 1/3] cli: refactor notmuch tag data structures for tagging operations
To simplify code, keep all tagging operations in a single array instead of separate add and remove arrays. Apply tag changes in the order specified on the command line, instead of first removing and then adding the tags. This results in a minor functional change: If a tag is both added and removed, the last specified operation is now used. Previously the tag was always added. Signed-off-by: Jani Nikula j...@nikula.org --- notmuch-tag.c | 79 +--- 1 files changed, 35 insertions(+), 44 deletions(-) diff --git a/notmuch-tag.c b/notmuch-tag.c index 36b9b09..c39b235 100644 --- a/notmuch-tag.c +++ b/notmuch-tag.c @@ -53,10 +53,14 @@ _escape_tag (char *buf, const char *tag) return buf; } +typedef struct { +const char *tag; +notmuch_bool_t remove; +} tag_operation_t; + static char * -_optimize_tag_query (void *ctx, const char *orig_query_string, char *argv[], -int *add_tags, int add_tags_count, -int *remove_tags, int remove_tags_count) +_optimize_tag_query (void *ctx, const char *orig_query_string, +const tag_operation_t *tag_ops) { /* This is subtler than it looks. Xapian ignores the '-' operator * at the beginning both queries and parenthesized groups and, @@ -74,12 +78,9 @@ _optimize_tag_query (void *ctx, const char *orig_query_string, char *argv[], /* 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 add_tags_count; i++) - if (strlen (argv[add_tags[i]] + 1) max_tag_len) - max_tag_len = strlen (argv[add_tags[i]] + 1); -for (i = 0; i remove_tags_count; i++) - if (strlen (argv[remove_tags[i]] + 1) max_tag_len) - max_tag_len = strlen (argv[remove_tags[i]] + 1); +for (i = 0; tag_ops[i].tag; i++) + if (strlen (tag_ops[i].tag) max_tag_len) + max_tag_len = strlen (tag_ops[i].tag); escaped = talloc_array(ctx, char, max_tag_len * 2 + 3); if (!escaped) return NULL; @@ -90,16 +91,11 @@ _optimize_tag_query (void *ctx, const char *orig_query_string, char *argv[], else query_string = talloc_asprintf (ctx, ( %s ) and (, orig_query_string); -for (i = 0; i add_tags_count query_string; i++) { - query_string = talloc_asprintf_append_buffer ( - query_string, %snot tag:%s, join, - _escape_tag (escaped, argv[add_tags[i]] + 1)); - join = or ; -} -for (i = 0; i remove_tags_count query_string; i++) { +for (i = 0; tag_ops[i].tag query_string; i++) { query_string = talloc_asprintf_append_buffer ( - query_string, %stag:%s, join, - _escape_tag (escaped, argv[remove_tags[i]] + 1)); + query_string, %s%stag:%s, join, + tag_ops[i].remove ? : not , + _escape_tag (escaped, tag_ops[i].tag)); join = or ; } @@ -113,9 +109,8 @@ _optimize_tag_query (void *ctx, const char *orig_query_string, char *argv[], int notmuch_tag_command (void *ctx, int argc, char *argv[]) { -int *add_tags, *remove_tags; -int add_tags_count = 0; -int remove_tags_count = 0; +tag_operation_t *tag_ops; +int tag_ops_count = 0; char *query_string; notmuch_config_t *config; notmuch_database_t *notmuch; @@ -133,35 +128,33 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) action.sa_flags = SA_RESTART; sigaction (SIGINT, action, NULL); -add_tags = talloc_size (ctx, argc * sizeof (int)); -if (add_tags == NULL) { - fprintf (stderr, Out of memory.\n); - return 1; -} +argc--; argv++; /* skip subcommand argument */ -remove_tags = talloc_size (ctx, argc * sizeof (int)); -if (remove_tags == NULL) { +/* Array of tagging operations (add or remove), terminated with an + * empty element. */ +tag_ops = talloc_array (ctx, tag_operation_t, argc + 1); +if (tag_ops == NULL) { fprintf (stderr, Out of memory.\n); return 1; } -argc--; argv++; /* skip subcommand argument */ - for (i = 0; i argc; i++) { if (strcmp (argv[i], --) == 0) { i++; break; } - if (argv[i][0] == '+') { - add_tags[add_tags_count++] = i; - } else if (argv[i][0] == '-') { - remove_tags[remove_tags_count++] = i; + if (argv[i][0] == '+' || argv[i][0] == '-') { + tag_ops[tag_ops_count].tag = argv[i] + 1; + tag_ops[tag_ops_count].remove = argv[i][0] == '-'; + tag_ops_count++; } else { break; } } -if (add_tags_count == 0 remove_tags_count == 0) { +tag_ops[tag_ops_count].tag = NULL; + +if (tag_ops_count == 0) { fprintf (stderr, Error: 'notmuch tag' requires at least one tag to add or remove.\n); return 1; } @@ -175,9
[PATCH v2 2/3] cli: refactor notmuch tag query tagging into a separate function
Refactor to make tagging code easier to reuse in the future. No functional changes. Signed-off-by: Jani Nikula j...@nikula.org --- notmuch-tag.c | 101 - 1 files changed, 57 insertions(+), 44 deletions(-) diff --git a/notmuch-tag.c b/notmuch-tag.c index c39b235..edbfdec 100644 --- a/notmuch-tag.c +++ b/notmuch-tag.c @@ -106,6 +106,60 @@ _optimize_tag_query (void *ctx, const char *orig_query_string, return query_string; } +static int +tag_query (void *ctx, notmuch_database_t *notmuch, const char *query_string, + tag_operation_t *tag_ops, notmuch_bool_t synchronize_flags) +{ +notmuch_query_t *query; +notmuch_messages_t *messages; +notmuch_message_t *message; +int i; + +/* Optimize the query so it excludes messages that already have + * the specified set of tags. */ +query_string = _optimize_tag_query (ctx, query_string, tag_ops); +if (query_string == NULL) { + fprintf (stderr, Out of memory.\n); + return 1; +} + +query = notmuch_query_create (notmuch, query_string); +if (query == NULL) { + fprintf (stderr, Out of memory.\n); + return 1; +} + +/* tagging is not interested in any special sort order */ +notmuch_query_set_sort (query, NOTMUCH_SORT_UNSORTED); + +for (messages = notmuch_query_search_messages (query); +notmuch_messages_valid (messages) !interrupted; +notmuch_messages_move_to_next (messages)) +{ + message = notmuch_messages_get (messages); + + notmuch_message_freeze (message); + + for (i = 0; tag_ops[i].tag; i++) { + if (tag_ops[i].remove) + notmuch_message_remove_tag (message, tag_ops[i].tag); + else + notmuch_message_add_tag (message, tag_ops[i].tag); + } + + notmuch_message_thaw (message); + + if (synchronize_flags) + notmuch_message_tags_to_maildir_flags (message); + + notmuch_message_destroy (message); +} + +notmuch_query_destroy (query); + +return interrupted; +} + int notmuch_tag_command (void *ctx, int argc, char *argv[]) { @@ -114,12 +168,10 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) char *query_string; notmuch_config_t *config; notmuch_database_t *notmuch; -notmuch_query_t *query; -notmuch_messages_t *messages; -notmuch_message_t *message; struct sigaction action; notmuch_bool_t synchronize_flags; int i; +int ret; /* Setup our handler for SIGINT */ memset (action, 0, sizeof (struct sigaction)); @@ -166,14 +218,6 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) return 1; } -/* Optimize the query so it excludes messages that already have - * the specified set of tags. */ -query_string = _optimize_tag_query (ctx, query_string, tag_ops); -if (query_string == NULL) { - fprintf (stderr, Out of memory.\n); - return 1; -} - config = notmuch_config_open (ctx, NULL, NULL); if (config == NULL) return 1; @@ -185,40 +229,9 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config); -query = notmuch_query_create (notmuch, query_string); -if (query == NULL) { - fprintf (stderr, Out of memory.\n); - return 1; -} - -/* tagging is not interested in any special sort order */ -notmuch_query_set_sort (query, NOTMUCH_SORT_UNSORTED); +ret = tag_query (ctx, notmuch, query_string, tag_ops, synchronize_flags); -for (messages = notmuch_query_search_messages (query); -notmuch_messages_valid (messages) !interrupted; -notmuch_messages_move_to_next (messages)) -{ - message = notmuch_messages_get (messages); - - notmuch_message_freeze (message); - - for (i = 0; tag_ops[i].tag; i++) { - if (tag_ops[i].remove) - notmuch_message_remove_tag (message, tag_ops[i].tag); - else - notmuch_message_add_tag (message, tag_ops[i].tag); - } - - notmuch_message_thaw (message); - - if (synchronize_flags) - notmuch_message_tags_to_maildir_flags (message); - - notmuch_message_destroy (message); -} - -notmuch_query_destroy (query); notmuch_database_close (notmuch); -return interrupted; +return ret; } -- 1.7.5.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 3/3] cli: refactor notmuch restore message tagging into a separate function
Refactor to make tagging code easier to reuse in the future. No functional changes. Signed-off-by: Jani Nikula j...@nikula.org --- notmuch-restore.c | 148 - 1 files changed, 78 insertions(+), 70 deletions(-) diff --git a/notmuch-restore.c b/notmuch-restore.c index 87d9772..2e965af 100644 --- a/notmuch-restore.c +++ b/notmuch-restore.c @@ -21,6 +21,81 @@ #include notmuch-client.h int +tag_message (void *ctx, notmuch_database_t *notmuch, const char *message_id, +char *file_tags, notmuch_bool_t remove_all, +notmuch_bool_t synchronize_flags) +{ +notmuch_status_t status; +notmuch_tags_t *db_tags; +char *db_tags_str; +notmuch_message_t *message = NULL; +const char *tag; +char *next; +int ret = 0; + +status = notmuch_database_find_message (notmuch, message_id, message); +if (status || message == NULL) { + fprintf (stderr, Warning: Cannot apply tags to %smessage: %s\n, +message ? : missing , message_id); + if (status) + fprintf (stderr, %s\n, notmuch_status_to_string(status)); + return 1; +} + +/* In order to detect missing messages, this check/optimization is + * intentionally done *after* first finding the message. */ +if (!remove_all (file_tags == NULL || *file_tags == '\0')) + goto DONE; + +db_tags_str = NULL; +for (db_tags = notmuch_message_get_tags (message); +notmuch_tags_valid (db_tags); +notmuch_tags_move_to_next (db_tags)) { + tag = notmuch_tags_get (db_tags); + + if (db_tags_str) + db_tags_str = talloc_asprintf_append (db_tags_str, %s, tag); + else + db_tags_str = talloc_strdup (message, tag); +} + +if (((file_tags == NULL || *file_tags == '\0') +(db_tags_str == NULL || *db_tags_str == '\0')) || + (file_tags db_tags_str strcmp (file_tags, db_tags_str) == 0)) + goto DONE; + +notmuch_message_freeze (message); + +if (remove_all) + notmuch_message_remove_all_tags (message); + +next = file_tags; +while (next) { + tag = strsep (next, ); + if (*tag == '\0') + continue; + status = notmuch_message_add_tag (message, tag); + if (status) { + fprintf (stderr, Error applying tag %s to message %s:\n, +tag, message_id); + fprintf (stderr, %s\n, notmuch_status_to_string (status)); + ret = 1; + } +} + +notmuch_message_thaw (message); + +if (synchronize_flags) + notmuch_message_tags_to_maildir_flags (message); + +DONE: +if (message) + notmuch_message_destroy (message); + +return ret; +} + +int notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) { notmuch_config_t *config; @@ -88,11 +163,7 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) while ((line_len = getline (line, line_size, input)) != -1) { regmatch_t match[3]; - char *message_id, *file_tags, *tag, *next; - notmuch_message_t *message = NULL; - notmuch_status_t status; - notmuch_tags_t *db_tags; - char *db_tags_str; + char *message_id, *file_tags; chomp_newline (line); @@ -109,72 +180,9 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) file_tags = xstrndup (line + match[2].rm_so, match[2].rm_eo - match[2].rm_so); - status = notmuch_database_find_message (notmuch, message_id, message); - if (status || message == NULL) { - fprintf (stderr, Warning: Cannot apply tags to %smessage: %s\n, -message ? : missing , message_id); - if (status) - fprintf (stderr, %s\n, -notmuch_status_to_string(status)); - goto NEXT_LINE; - } - - /* In order to detect missing messages, this check/optimization is -* intentionally done *after* first finding the message. */ - if (accumulate (file_tags == NULL || *file_tags == '\0')) - { - goto NEXT_LINE; - } - - db_tags_str = NULL; - for (db_tags = notmuch_message_get_tags (message); -notmuch_tags_valid (db_tags); -notmuch_tags_move_to_next (db_tags)) - { - const char *tag = notmuch_tags_get (db_tags); - - if (db_tags_str) - db_tags_str = talloc_asprintf_append (db_tags_str, %s, tag); - else - db_tags_str = talloc_strdup (message, tag); - } - - if (((file_tags == NULL || *file_tags == '\0') -(db_tags_str == NULL || *db_tags_str == '\0')) || - (file_tags db_tags_str strcmp (file_tags, db_tags_str) == 0)) - { - goto NEXT_LINE; - } - - notmuch_message_freeze (message); - - if (!accumulate) - notmuch_message_remove_all_tags
Re: [PATCH] vim: fix regex after notmuch show output change
Jakob ja...@pipefour.org writes: On Sat, 24 Mar 2012 10:58:59 +0200, Tomi Ollila tomi.oll...@iki.fi wrote: Is this regexp part below good ? +... match:\([0-9]*\) excluded:\([[0-9]*\) filename:\(.*\)$', ... ( -- excluded:\([[0-9]*\) -- ) Yeah, that was the core of the change. With this new field in the output, the old regex didn't match at all. You can see the fix working by trying to use the vim plugin as-is and trying to reply to a message or view a message's ID or any number of other things that fail right now. After updating this regex, everything starts working again. I meant about the syntax: \([[0-9]*\) i.e. 2 opening [[:s Thanks, Jakob Tomi ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v2 2/3] cli: refactor notmuch tag query tagging into a separate function
Jani Nikula j...@nikula.org writes: Refactor to make tagging code easier to reuse in the future. No functional changes. Signed-off-by: Jani Nikula j...@nikula.org --- tag_query() is pretty generic name for a function which (also) does tagging operations (adds and removes tags based on tag_ops). If, however, tag_opts != NULL (as is needs to be) but tag_opts[0] == NULL (now tag_query() would not do any tagging operations, but optimize_tag_query would mangle query string ( '*' to '()' and 'any_other' to '( any_other ) and ()' ) I.e. IMO the function name should be more spesific the fact that this function needs tag changing data in tag_ops array should be documented. --- Minor thing in patch #1: + tag_ops[tag_ops_count].remove = argv[i][0] == '-'; would be more clearer as: + tag_ops[tag_ops_count].remove = (argv[i][0] == '-'); Everything else LGTM. Tomi notmuch-tag.c | 101 - 1 files changed, 57 insertions(+), 44 deletions(-) diff --git a/notmuch-tag.c b/notmuch-tag.c index c39b235..edbfdec 100644 --- a/notmuch-tag.c +++ b/notmuch-tag.c @@ -106,6 +106,60 @@ _optimize_tag_query (void *ctx, const char *orig_query_string, return query_string; } +static int +tag_query (void *ctx, notmuch_database_t *notmuch, const char *query_string, +tag_operation_t *tag_ops, notmuch_bool_t synchronize_flags) +{ +notmuch_query_t *query; +notmuch_messages_t *messages; +notmuch_message_t *message; +int i; + +/* Optimize the query so it excludes messages that already have + * the specified set of tags. */ +query_string = _optimize_tag_query (ctx, query_string, tag_ops); +if (query_string == NULL) { + fprintf (stderr, Out of memory.\n); + return 1; +} + +query = notmuch_query_create (notmuch, query_string); +if (query == NULL) { + fprintf (stderr, Out of memory.\n); + return 1; +} + +/* tagging is not interested in any special sort order */ +notmuch_query_set_sort (query, NOTMUCH_SORT_UNSORTED); + +for (messages = notmuch_query_search_messages (query); + notmuch_messages_valid (messages) !interrupted; + notmuch_messages_move_to_next (messages)) +{ + message = notmuch_messages_get (messages); + + notmuch_message_freeze (message); + + for (i = 0; tag_ops[i].tag; i++) { + if (tag_ops[i].remove) + notmuch_message_remove_tag (message, tag_ops[i].tag); + else + notmuch_message_add_tag (message, tag_ops[i].tag); + } + + notmuch_message_thaw (message); + + if (synchronize_flags) + notmuch_message_tags_to_maildir_flags (message); + + notmuch_message_destroy (message); +} + +notmuch_query_destroy (query); + +return interrupted; +} + int notmuch_tag_command (void *ctx, int argc, char *argv[]) { @@ -114,12 +168,10 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) char *query_string; notmuch_config_t *config; notmuch_database_t *notmuch; -notmuch_query_t *query; -notmuch_messages_t *messages; -notmuch_message_t *message; struct sigaction action; notmuch_bool_t synchronize_flags; int i; +int ret; /* Setup our handler for SIGINT */ memset (action, 0, sizeof (struct sigaction)); @@ -166,14 +218,6 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) return 1; } -/* Optimize the query so it excludes messages that already have - * the specified set of tags. */ -query_string = _optimize_tag_query (ctx, query_string, tag_ops); -if (query_string == NULL) { - fprintf (stderr, Out of memory.\n); - return 1; -} - config = notmuch_config_open (ctx, NULL, NULL); if (config == NULL) return 1; @@ -185,40 +229,9 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config); -query = notmuch_query_create (notmuch, query_string); -if (query == NULL) { - fprintf (stderr, Out of memory.\n); - return 1; -} - -/* tagging is not interested in any special sort order */ -notmuch_query_set_sort (query, NOTMUCH_SORT_UNSORTED); +ret = tag_query (ctx, notmuch, query_string, tag_ops, synchronize_flags); -for (messages = notmuch_query_search_messages (query); - notmuch_messages_valid (messages) !interrupted; - notmuch_messages_move_to_next (messages)) -{ - message = notmuch_messages_get (messages); - - notmuch_message_freeze (message); - - for (i = 0; tag_ops[i].tag; i++) { - if (tag_ops[i].remove) - notmuch_message_remove_tag (message, tag_ops[i].tag); - else - notmuch_message_add_tag (message, tag_ops[i].tag); - } - -
Re: [PATCH v2 2/3] cli: refactor notmuch tag query tagging into a separate function
On Sun, 25 Mar 2012 23:45:39 +0300, Tomi Ollila tomi.oll...@iki.fi wrote: Jani Nikula j...@nikula.org writes: Refactor to make tagging code easier to reuse in the future. No functional changes. Signed-off-by: Jani Nikula j...@nikula.org --- tag_query() is pretty generic name for a function which (also) does tagging operations (adds and removes tags based on tag_ops). Mmmh, if you think of tag as a verb, it fairly accurately describes what the function does: tag (messages matching this) query (according given arguments). I don't mind changing, but can't come up with anything better right now either. notmuch_{search,query}_change_tags()? Meh? If, however, tag_opts != NULL (as is needs to be) but tag_opts[0] == NULL (now tag_query() would not do any tagging operations, but optimize_tag_query would mangle query string ( '*' to '()' and 'any_other' to '( any_other ) and ()' ) I'm not sure I follow you. AFAICT the behaviour does not change from what it was before, apart from the tag addition and removal being mixed together. I.e. IMO the function name should be more spesific the fact that this function needs tag changing data in tag_ops array should be documented. Documentation good. :) Minor thing in patch #1: +tag_ops[tag_ops_count].remove = argv[i][0] == '-'; would be more clearer as: +tag_ops[tag_ops_count].remove = (argv[i][0] == '-'); I usually don't add braces where they aren't needed, but can do here. Everything else LGTM. Thanks for the review, Jani. Tomi notmuch-tag.c | 101 - 1 files changed, 57 insertions(+), 44 deletions(-) diff --git a/notmuch-tag.c b/notmuch-tag.c index c39b235..edbfdec 100644 --- a/notmuch-tag.c +++ b/notmuch-tag.c @@ -106,6 +106,60 @@ _optimize_tag_query (void *ctx, const char *orig_query_string, return query_string; } +static int +tag_query (void *ctx, notmuch_database_t *notmuch, const char *query_string, + tag_operation_t *tag_ops, notmuch_bool_t synchronize_flags) +{ +notmuch_query_t *query; +notmuch_messages_t *messages; +notmuch_message_t *message; +int i; + +/* Optimize the query so it excludes messages that already have + * the specified set of tags. */ +query_string = _optimize_tag_query (ctx, query_string, tag_ops); +if (query_string == NULL) { + fprintf (stderr, Out of memory.\n); + return 1; +} + +query = notmuch_query_create (notmuch, query_string); +if (query == NULL) { + fprintf (stderr, Out of memory.\n); + return 1; +} + +/* tagging is not interested in any special sort order */ +notmuch_query_set_sort (query, NOTMUCH_SORT_UNSORTED); + +for (messages = notmuch_query_search_messages (query); +notmuch_messages_valid (messages) !interrupted; +notmuch_messages_move_to_next (messages)) +{ + message = notmuch_messages_get (messages); + + notmuch_message_freeze (message); + + for (i = 0; tag_ops[i].tag; i++) { + if (tag_ops[i].remove) + notmuch_message_remove_tag (message, tag_ops[i].tag); + else + notmuch_message_add_tag (message, tag_ops[i].tag); + } + + notmuch_message_thaw (message); + + if (synchronize_flags) + notmuch_message_tags_to_maildir_flags (message); + + notmuch_message_destroy (message); +} + +notmuch_query_destroy (query); + +return interrupted; +} + int notmuch_tag_command (void *ctx, int argc, char *argv[]) { @@ -114,12 +168,10 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) char *query_string; notmuch_config_t *config; notmuch_database_t *notmuch; -notmuch_query_t *query; -notmuch_messages_t *messages; -notmuch_message_t *message; struct sigaction action; notmuch_bool_t synchronize_flags; int i; +int ret; /* Setup our handler for SIGINT */ memset (action, 0, sizeof (struct sigaction)); @@ -166,14 +218,6 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) return 1; } -/* Optimize the query so it excludes messages that already have - * the specified set of tags. */ -query_string = _optimize_tag_query (ctx, query_string, tag_ops); -if (query_string == NULL) { - fprintf (stderr, Out of memory.\n); - return 1; -} - config = notmuch_config_open (ctx, NULL, NULL); if (config == NULL) return 1; @@ -185,40 +229,9 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config); -query = notmuch_query_create (notmuch, query_string); -if (query == NULL) { - fprintf (stderr, Out of memory.\n); - return 1; -} - -/* tagging is not
Re: [PATCH v2 2/3] cli: refactor notmuch tag query tagging into a separate function
Jani Nikula j...@nikula.org writes: On Sun, 25 Mar 2012 23:45:39 +0300, Tomi Ollila tomi.oll...@iki.fi wrote: Jani Nikula j...@nikula.org writes: Refactor to make tagging code easier to reuse in the future. No functional changes. Signed-off-by: Jani Nikula j...@nikula.org --- tag_query() is pretty generic name for a function which (also) does tagging operations (adds and removes tags based on tag_ops). Mmmh, if you think of tag as a verb, it fairly accurately describes what the function does: tag (messages matching this) query (according given arguments). I don't mind changing, but can't come up with anything better right now either. notmuch_{search,query}_change_tags()? Meh? Hmmm, yes... -- tag...query... ok, I can live with that if no-one else get same impression I got first... If, however, tag_opts != NULL (as is needs to be) but tag_opts[0] == NULL (now tag_query() would not do any tagging operations, but optimize_tag_query would mangle query string ( '*' to '()' and 'any_other' to '( any_other ) and ()' ) I'm not sure I follow you. AFAICT the behaviour does not change from what it was before, apart from the tag addition and removal being mixed together. The thing is that notmuch_tag_command () check that there is at least one tagging operation to be done, but tag_query () doesn't do such checking... I.e. IMO the function name should be more spesific the fact that this function needs tag changing data in tag_ops array should be documented. Documentation good. :) ... therefore the requirement that there needs to tagging information in tag_ops is in place. Minor thing in patch #1: + tag_ops[tag_ops_count].remove = argv[i][0] == '-'; would be more clearer as: + tag_ops[tag_ops_count].remove = (argv[i][0] == '-'); I usually don't add braces where they aren't needed, but can do here. Assigment is much lower in precedence than equality comparison -- but as this is so seldom-used construct, humans read (with understanding) the code faster with this added clarity. Everything else LGTM. Thanks for the review, Jani. Tomi Tomi notmuch-tag.c | 101 - 1 files changed, 57 insertions(+), 44 deletions(-) diff --git a/notmuch-tag.c b/notmuch-tag.c index c39b235..edbfdec 100644 --- a/notmuch-tag.c +++ b/notmuch-tag.c @@ -106,6 +106,60 @@ _optimize_tag_query (void *ctx, const char *orig_query_string, return query_string; } +static int +tag_query (void *ctx, notmuch_database_t *notmuch, const char *query_string, + tag_operation_t *tag_ops, notmuch_bool_t synchronize_flags) +{ +notmuch_query_t *query; +notmuch_messages_t *messages; +notmuch_message_t *message; +int i; + +/* Optimize the query so it excludes messages that already have + * the specified set of tags. */ +query_string = _optimize_tag_query (ctx, query_string, tag_ops); +if (query_string == NULL) { + fprintf (stderr, Out of memory.\n); + return 1; +} + +query = notmuch_query_create (notmuch, query_string); +if (query == NULL) { + fprintf (stderr, Out of memory.\n); + return 1; +} + +/* tagging is not interested in any special sort order */ +notmuch_query_set_sort (query, NOTMUCH_SORT_UNSORTED); + +for (messages = notmuch_query_search_messages (query); + notmuch_messages_valid (messages) !interrupted; + notmuch_messages_move_to_next (messages)) +{ + message = notmuch_messages_get (messages); + + notmuch_message_freeze (message); + + for (i = 0; tag_ops[i].tag; i++) { + if (tag_ops[i].remove) + notmuch_message_remove_tag (message, tag_ops[i].tag); + else + notmuch_message_add_tag (message, tag_ops[i].tag); + } + + notmuch_message_thaw (message); + + if (synchronize_flags) + notmuch_message_tags_to_maildir_flags (message); + + notmuch_message_destroy (message); +} + +notmuch_query_destroy (query); + +return interrupted; +} + int notmuch_tag_command (void *ctx, int argc, char *argv[]) { @@ -114,12 +168,10 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) char *query_string; notmuch_config_t *config; notmuch_database_t *notmuch; -notmuch_query_t *query; -notmuch_messages_t *messages; -notmuch_message_t *message; struct sigaction action; notmuch_bool_t synchronize_flags; int i; +int ret; /* Setup our handler for SIGINT */ memset (action, 0, sizeof (struct sigaction)); @@ -166,14 +218,6 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) return 1; } -/* Optimize the query so it excludes messages that already have - * the specified set of tags. */ -query_string = _optimize_tag_query (ctx, query_string,