From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by olra.theworths.org (Postfix) with ESMTP id 99958431FAF for ; Sun, 2 Dec 2012 05:29:53 -0800 (PST) X-Virus-Scanned: Debian amavisd-new at olra.theworths.org X-Spam-Flag: NO X-Spam-Score: -0.7 X-Spam-Level: X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5 tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled Received: from olra.theworths.org ([127.0.0.1]) by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 1GhUazsQcveL for ; Sun, 2 Dec 2012 05:29:49 -0800 (PST) Received: from mail-lb0-f181.google.com (mail-lb0-f181.google.com [209.85.217.181]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) (No client certificate requested) by olra.theworths.org (Postfix) with ESMTPS id AF574431FAE for ; Sun, 2 Dec 2012 05:29:48 -0800 (PST) Received: by mail-lb0-f181.google.com with SMTP id ge1so1763467lbb.26 for ; Sun, 02 Dec 2012 05:29:47 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=from:to:cc:subject:in-reply-to:references:user-agent:date :message-id:mime-version:content-type:x-gm-message-state; bh=4pG3dsHLro2a6t1zGdq7KCFVB5k2FuL7Aixh8+mW97k=; b=iOG8OULvDABOB7FJf7Wzj4oBmL8rG323NkNMRj+etCZsvLo9H+4gjNIptLqD25o34k nBmw7naYTezYcET5v5HTojICOYoTbE01b7mHW3c2rN/OzcU1DpdwMMQSEdOZD5Gn/VJq +16FGNtf3QBlUE44eAsx2m0jTWJRL66vXr9Olk+pn+m95Dug3OnMYScjSsbvJxNimj9x G//hiLAcxNZxUNSkg5plYVT7RiamSkJpGSpJWseQfgqGMoOkWU/bAv3dXtMm2914uYuf bzlgoyR4xVWoX+GVUKRykl6rYYhUb1HdQQCmAYZhlHLXjHLygYFnUrDm2vNQg+0ACfp5 WYNg== Received: by 10.152.114.65 with SMTP id je1mr6695753lab.33.1354454987058; Sun, 02 Dec 2012 05:29:47 -0800 (PST) Received: from localhost (dsl-hkibrasgw4-fe51df00-27.dhcp.inet.fi. [80.223.81.27]) by mx.google.com with ESMTPS id b4sm4150390lbi.0.2012.12.02.05.29.45 (version=SSLv3 cipher=OTHER); Sun, 02 Dec 2012 05:29:46 -0800 (PST) From: Jani Nikula To: david@tethera.net, notmuch@notmuchmail.org Subject: Re: [Patch v2 13/17] notmuch-restore: add support for input format 'batch-tag' In-Reply-To: <1353792017-31459-14-git-send-email-david@tethera.net> References: <1353792017-31459-1-git-send-email-david@tethera.net> <1353792017-31459-14-git-send-email-david@tethera.net> User-Agent: Notmuch/0.14+124~g3b17402 (http://notmuchmail.org) Emacs/23.4.1 (i686-pc-linux-gnu) Date: Sun, 02 Dec 2012 15:29:43 +0200 Message-ID: <87zk1wd1ko.fsf@nikula.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Gm-Message-State: ALoCoQnkuhDlQ10YCjDeEjMG8q7DRMmvK6smTHhMB09E9uuzdYSSICajMJXHt3UXjYtQLy0EcQ+N Cc: David Bremner X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 02 Dec 2012 13:29:53 -0000 On Sat, 24 Nov 2012, david@tethera.net wrote: > From: David Bremner > > This is the same as the batch input for notmuch tag, except by default > it removes all tags before modifying a given message id and only "id:" > is supported. > --- > notmuch-restore.c | 199 +++++++++++++++++++++++++++++++++-------------------- > 1 file changed, 125 insertions(+), 74 deletions(-) > > diff --git a/notmuch-restore.c b/notmuch-restore.c > index f03dcac..22fcd2d 100644 > --- a/notmuch-restore.c > +++ b/notmuch-restore.c > @@ -19,18 +19,22 @@ > */ > > #include "notmuch-client.h" > +#include "dump-restore-private.h" > +#include "tag-util.h" > +#include "string-util.h" > + > +static volatile sig_atomic_t interrupted; > +static regex_t regex; > > static int > -tag_message (notmuch_database_t *notmuch, const char *message_id, > - char *file_tags, notmuch_bool_t remove_all, > - notmuch_bool_t synchronize_flags) > +tag_message (unused (void *ctx), > + notmuch_database_t *notmuch, > + const char *message_id, > + tag_op_list_t *tag_ops, > + tag_op_flag_t 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); > @@ -44,55 +48,63 @@ tag_message (notmuch_database_t *notmuch, const char *message_id, > > /* 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 ( (flags & TAG_FLAG_REMOVE_ALL) || (tag_op_list_size (tag_ops))) Extra space between ('s, and no need to wrap tag_op_list_size call in braces. > + tag_op_list_apply (message, tag_ops, flags); > > - 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)) This is a necessary optimization you're throwing away, but I'll get back to this after checking the last patch in the series. > - goto DONE; > + if (message) > + notmuch_message_destroy (message); message != NULL always, you can remove the if. > > - notmuch_message_freeze (message); > + return ret; > +} > > - if (remove_all) > - notmuch_message_remove_all_tags (message); > +static int > +parse_sup_line (void *ctx, char *line, > + char **query_str, tag_op_list_t *tag_ops) > +{ > > - 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; > - } > + regmatch_t match[3]; > + char *file_tags; > + int rerr; > + > + tag_op_list_reset (tag_ops); > + > + chomp_newline (line); > + > + /* Silently ignore blank lines */ > + if (line[0] == '\0') { > + return 1; > + } > + > + rerr = xregexec (®ex, line, 3, match, 0); > + if (rerr == REG_NOMATCH) { > + fprintf (stderr, "Warning: Ignoring invalid input line: %s\n", > + line); > + return 1; > } > > - notmuch_message_thaw (message); > + *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); > > - if (synchronize_flags) > - notmuch_message_tags_to_maildir_flags (message); > + char *tok = file_tags; > + size_t tok_len = 0; > > - DONE: > - if (message) > - notmuch_message_destroy (message); > + tag_op_list_reset (tag_ops); > + > + while ((tok = strtok_len (tok + tok_len, " ", &tok_len)) != NULL) { > + > + if (*(tok + tok_len) != '\0') { > + *(tok + tok_len) = '\0'; > + tok_len++; > + } > + > + if (tag_op_list_append (ctx, tag_ops, tok, FALSE)) > + return -1; > + } > + > + return 0; > > - return ret; > } > > int > @@ -100,16 +112,19 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) > { > notmuch_config_t *config; > notmuch_database_t *notmuch; > - notmuch_bool_t synchronize_flags; > notmuch_bool_t accumulate = FALSE; > + tag_op_flag_t flags = 0; > + tag_op_list_t *tag_ops; > + > char *input_file_name = NULL; > FILE *input = stdin; > char *line = NULL; > size_t line_size; > ssize_t line_len; > - regex_t regex; > - int rerr; > + > + int ret = 0; > int opt_index; > + int input_format = DUMP_FORMAT_AUTO; > > config = notmuch_config_open (ctx, NULL, NULL); > if (config == NULL) > @@ -119,9 +134,15 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) > NOTMUCH_DATABASE_MODE_READ_WRITE, ¬much)) > return 1; > > - synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config); > + if (notmuch_config_get_maildir_synchronize_flags (config)) > + flags |= TAG_FLAG_MAILDIR_SYNC; > > notmuch_opt_desc_t options[] = { > + { NOTMUCH_OPT_KEYWORD, &input_format, "format", 'f', > + (notmuch_keyword_t []){ { "auto", DUMP_FORMAT_AUTO }, > + { "batch-tag", DUMP_FORMAT_BATCH_TAG }, > + { "sup", DUMP_FORMAT_SUP }, > + { 0, 0 } } }, > { NOTMUCH_OPT_STRING, &input_file_name, "input", 'i', 0 }, > { NOTMUCH_OPT_BOOLEAN, &accumulate, "accumulate", 'a', 0 }, > { 0, 0, 0, 0, 0 } > @@ -134,6 +155,9 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) > return 1; > } > > + if (! accumulate) > + flags |= TAG_FLAG_REMOVE_ALL; > + > if (input_file_name) { > input = fopen (input_file_name, "r"); > if (input == NULL) { > @@ -154,35 +178,61 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) > * non-space characters for the message-id, then one or more > * spaces, then a list of space-separated tags as a sequence of > * characters within literal '(' and ')'. */ > - if ( xregcomp (®ex, > - "^([^ ]+) \\(([^)]*)\\)$", > - REG_EXTENDED) ) > - INTERNAL_ERROR ("compile time constant regex failed."); > + char *p; > > - while ((line_len = getline (&line, &line_size, input)) != -1) { > - regmatch_t match[3]; > - char *message_id, *file_tags; > + line_len = getline (&line, &line_size, input); > + if (line_len == 0) > + return 0; > > - chomp_newline (line); > + for (p = line; *p; p++) { > + if (*p == '(') > + input_format = DUMP_FORMAT_SUP; > + } > > - rerr = xregexec (®ex, line, 3, match, 0); > - if (rerr == REG_NOMATCH) { > - fprintf (stderr, "Warning: Ignoring invalid input line: %s\n", > - line); > - continue; > + if (input_format == DUMP_FORMAT_AUTO) > + input_format = DUMP_FORMAT_BATCH_TAG; > + > + if (input_format == DUMP_FORMAT_SUP) > + if ( xregcomp (®ex, > + "^([^ ]+) \\(([^)]*)\\)$", > + REG_EXTENDED) ) > + INTERNAL_ERROR ("compile time constant regex failed."); > + > + tag_ops = tag_op_list_create (ctx); > + if (tag_ops == NULL) { > + fprintf (stderr, "Out of memory.\n"); > + return 1; > + } Tag op list creation could be moved earlier to keep the parsing stuff closer together. > + > + do { > + char *query_string; > + > + if (input_format == DUMP_FORMAT_SUP) { > + ret = parse_sup_line (ctx, line, &query_string, tag_ops); > + } else { > + ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS, > + &query_string, tag_ops); Extra spaces after each = above. > + > + if (ret == 0) { > + if ( strncmp ("id:", query_string, 3) != 0) { > + fprintf (stderr, "Unsupported query: %s\n", query_string); > + continue; > + } There should probably be a comment somewhere here saying that everything after id: is taken to be the message id, i.e. a query of "id:foo AND tag:bar" will not do what you want. An option is to add a TAG_FLAG to accept only message id as the query in parse_tag_line. It could trim trailing whitespace (if it doesn't already) and look for spaces in between individual search terms *before* hex decoding the query string, and fail if it finds any, and then check for id: prefix after hex decode. > + /* delete id: from front of string; tag_message expects a > + * raw message-id */ > + query_string = query_string + 3; > + } > } > > - message_id = xstrndup (line + match[1].rm_so, > - match[1].rm_eo - match[1].rm_so); > - file_tags = xstrndup (line + match[2].rm_so, > - match[2].rm_eo - match[2].rm_so); > + if (ret > 0) > + continue; > > - tag_message (notmuch, message_id, file_tags, ! accumulate, > - synchronize_flags); > + if (ret < 0 || tag_message (ctx, notmuch, query_string, > + tag_ops, flags)) > + break; > + > + } while ((line_len = getline (&line, &line_size, input)) != -1); > > - free (message_id); > - free (file_tags); > - } > > regfree (®ex); Only do this for sup format. BR, Jani. > > @@ -190,8 +240,9 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) > free (line); > > notmuch_database_destroy (notmuch); > + > if (input != stdin) > fclose (input); > > - return 0; > + return ret; > } > -- > 1.7.10.4 > > _______________________________________________ > notmuch mailing list > notmuch@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch