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 1A5F9431FAF for ; Mon, 10 Dec 2012 13:51:50 -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 LimJP3IJywas for ; Mon, 10 Dec 2012 13:51: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 B2D88431FAE for ; Mon, 10 Dec 2012 13:51:48 -0800 (PST) Received: by mail-lb0-f181.google.com with SMTP id ge1so2531934lbb.26 for ; Mon, 10 Dec 2012 13:51: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=f/OUC3lmB+BLaW8GFoWa5HBczTKKvNvwg0gySxJunZI=; b=Y4yoZOR8Y+m3E2aWRbRgIZFVj87OzXJMpQB2FMDo0mk0Ak+J+qQgo1Rfand3TzkQ/i tN+Jod93/MWHutEKygxnWc/RJyFqqdRwLpzynJy7zSLpGAsGm84fZqAjz3F3P2eknu7z 8zuj00MBqMRc4cbLeem+AyJJneSOF5uMfyMr02A+r9fX33rowW/+cZziD3FXEaaknqHT KgOQ0a2m+sSILbuu05uadEI8nOr2SOSGCo6PeyURfZOHVvLmjObDeSs2H4MHMzTlXOkT BfktRPzMFq1VS3XxwpZwR3rUdDmXEFC9WBOiujeA3TlzbP4fz8SqcKfu93P6bajViazu 3+9w== Received: by 10.152.108.42 with SMTP id hh10mr15113564lab.4.1355176307103; Mon, 10 Dec 2012 13:51:47 -0800 (PST) Received: from localhost (dsl-hkibrasgw4-fe51df00-27.dhcp.inet.fi. [80.223.81.27]) by mx.google.com with ESMTPS id gr12sm8400940lab.3.2012.12.10.13.51.45 (version=SSLv3 cipher=OTHER); Mon, 10 Dec 2012 13:51:46 -0800 (PST) From: Jani Nikula To: david@tethera.net, notmuch@notmuchmail.org Subject: Re: [Patch v6 2/6] notmuch-tag.c: convert to use tag-utils In-Reply-To: <1355096008-4544-3-git-send-email-david@tethera.net> References: <1355096008-4544-1-git-send-email-david@tethera.net> <1355096008-4544-3-git-send-email-david@tethera.net> User-Agent: Notmuch/0.14+138~g7041c56 (http://notmuchmail.org) Emacs/23.4.1 (i686-pc-linux-gnu) Date: Mon, 10 Dec 2012 23:51:44 +0200 Message-ID: <876249a83z.fsf@nikula.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Gm-Message-State: ALoCoQkM8H3R9FPmyJBWB8sdXMtLKBBwo82Ok0pLR6q91VT5glClMDnvxwwExclWrkRDKqU5A/o6 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: Mon, 10 Dec 2012 21:51:50 -0000 On Mon, 10 Dec 2012, david@tethera.net wrote: > From: David Bremner > > Command line parsing is factored out into a function > parse_tag_command_line in tag-utils.c. > > There is some duplicated code eliminated in tag_query, and a bunch of > translation from using the bare tag_op structs to using that tag-utils > API. > --- > notmuch-tag.c | 104 +++++++++++++-------------------------------------------- > tag-util.c | 48 ++++++++++++++++++++++++++ > tag-util.h | 16 +++++++++ > 3 files changed, 87 insertions(+), 81 deletions(-) > > diff --git a/notmuch-tag.c b/notmuch-tag.c > index 88d559b..b5cd578 100644 > --- a/notmuch-tag.c > +++ b/notmuch-tag.c > @@ -19,6 +19,7 @@ > */ > > #include "notmuch-client.h" > +#include "tag-util.h" > > static volatile sig_atomic_t interrupted; > > @@ -54,14 +55,9 @@ _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, > - const tag_operation_t *tag_ops) > + const tag_op_list_t *list) > { > /* This is subtler than it looks. Xapian ignores the '-' operator > * at the beginning both queries and parenthesized groups and, > @@ -73,19 +69,20 @@ _optimize_tag_query (void *ctx, const char *orig_query_string, > > char *escaped, *query_string; > const char *join = ""; > - int i; > + size_t i; > unsigned int max_tag_len = 0; > > /* Don't optimize if there are no tag changes. */ > - if (tag_ops[0].tag == NULL) > + if (tag_op_list_size (list) == 0) > return talloc_strdup (ctx, orig_query_string); > > /* Allocate a buffer for escaping tags. This is large enough to > * hold a fully escaped tag with every character doubled plus > * enclosing quotes and a NUL. */ > - for (i = 0; tag_ops[i].tag; i++) > - if (strlen (tag_ops[i].tag) > max_tag_len) > - max_tag_len = strlen (tag_ops[i].tag); > + for (i = 0; i < tag_op_list_size (list); i++) > + if (strlen (tag_op_list_tag (list, i)) > max_tag_len) > + max_tag_len = strlen (tag_op_list_tag (list, i)); > + > escaped = talloc_array (ctx, char, max_tag_len * 2 + 3); > if (! escaped) > return NULL; > @@ -96,11 +93,11 @@ _optimize_tag_query (void *ctx, const char *orig_query_string, > else > query_string = talloc_asprintf (ctx, "( %s ) and (", orig_query_string); > > - for (i = 0; tag_ops[i].tag && query_string; i++) { > + for (i = 0; i < tag_op_list_size (list) && query_string; i++) { > query_string = talloc_asprintf_append_buffer ( > query_string, "%s%stag:%s", join, > - tag_ops[i].remove ? "" : "not ", > - _escape_tag (escaped, tag_ops[i].tag)); > + tag_op_list_isremove (list, i) ? "" : "not ", > + _escape_tag (escaped, tag_op_list_tag (list, i))); > join = " or "; > } > > @@ -116,12 +113,11 @@ _optimize_tag_query (void *ctx, const char *orig_query_string, > * element. */ The comment "array of tagging operations terminated with an empty element" needs revision too. BR, Jani. > static int > tag_query (void *ctx, notmuch_database_t *notmuch, const char *query_string, > - tag_operation_t *tag_ops, notmuch_bool_t synchronize_flags) > + tag_op_list_t *tag_ops, tag_op_flag_t 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. */ > @@ -144,21 +140,7 @@ tag_query (void *ctx, notmuch_database_t *notmuch, const char *query_string, > 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); > - > + tag_op_list_apply (message, tag_ops, flags | TAG_FLAG_PRE_OPTIMIZED); > notmuch_message_destroy (message); > } > > @@ -170,15 +152,13 @@ tag_query (void *ctx, notmuch_database_t *notmuch, const char *query_string, > int > notmuch_tag_command (void *ctx, int argc, char *argv[]) > { > - tag_operation_t *tag_ops; > - int tag_ops_count = 0; > - char *query_string; > + tag_op_list_t *tag_ops = NULL; > + char *query_string = NULL; > notmuch_config_t *config; > notmuch_database_t *notmuch; > struct sigaction action; > - notmuch_bool_t synchronize_flags; > - int i; > - int ret; > + tag_op_flag_t tag_flags = TAG_FLAG_NONE; > + int ret = 0; > > /* Setup our handler for SIGINT */ > memset (&action, 0, sizeof (struct sigaction)); > @@ -187,54 +167,15 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) > action.sa_flags = SA_RESTART; > sigaction (SIGINT, &action, NULL); > > - argc--; argv++; /* skip subcommand argument */ > - > - /* Array of tagging operations (add or remove), terminated with an > - * empty element. */ > - tag_ops = talloc_array (ctx, tag_operation_t, argc + 1); > + tag_ops = tag_op_list_create (ctx); > if (tag_ops == NULL) { > fprintf (stderr, "Out of memory.\n"); > return 1; > } > > - for (i = 0; i < argc; i++) { > - if (strcmp (argv[i], "--") == 0) { > - i++; > - break; > - } > - if (argv[i][0] == '+' || argv[i][0] == '-') { > - if (argv[i][0] == '+' && argv[i][1] == '\0') { > - fprintf (stderr, "Error: tag names cannot be empty.\n"); > - return 1; > - } > - if (argv[i][0] == '+' && argv[i][1] == '-') { > - /* This disallows adding the non-removable tag "-" and > - * enables notmuch tag to take long options in the > - * future. */ > - fprintf (stderr, "Error: tag names must not start with '-'.\n"); > - return 1; > - } > - tag_ops[tag_ops_count].tag = argv[i] + 1; > - tag_ops[tag_ops_count].remove = (argv[i][0] == '-'); > - tag_ops_count++; > - } else { > - break; > - } > - } > - > - 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; > - } > - > - query_string = query_string_from_args (ctx, argc - i, &argv[i]); > - > - if (*query_string == '\0') { > - fprintf (stderr, "Error: notmuch tag requires at least one search term.\n"); > + if (parse_tag_command_line (ctx, argc - 1, argv + 1, > + 0, &query_string, tag_ops)) > return 1; > - } > > config = notmuch_config_open (ctx, NULL, NULL); > if (config == NULL) > @@ -244,9 +185,10 @@ notmuch_tag_command (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)) > + tag_flags |= TAG_FLAG_MAILDIR_SYNC; > > - ret = tag_query (ctx, notmuch, query_string, tag_ops, synchronize_flags); > + ret = tag_query (ctx, notmuch, query_string, tag_ops, tag_flags); > > notmuch_database_destroy (notmuch); > > diff --git a/tag-util.c b/tag-util.c > index c071ea8..6e3c069 100644 > --- a/tag-util.c > +++ b/tag-util.c > @@ -162,6 +162,54 @@ parse_tag_line (void *ctx, char *line, > return ret; > } > > +int > +parse_tag_command_line (void *ctx, int argc, char **argv, > + unused(tag_op_flag_t flags), > + char **query_str, tag_op_list_t *tag_ops){ > + > + int i; > + > + tag_op_list_reset(tag_ops); > + > + for (i = 0; i < argc; i++) { > + if (strcmp (argv[i], "--") == 0) { > + i++; > + break; > + } > + > + if (argv[i][0] == '+' || argv[i][0] == '-') { > + notmuch_bool_t is_remove = argv[i][0] == '-'; > + const char *msg; > + > + msg = illegal_tag(argv[i]+1, is_remove); > + if (msg) { > + fprintf (stderr, "Error: %s", msg); > + return 1; > + } > + > + tag_op_list_append (ctx, tag_ops, > + argv[i] + 1, (argv[i][0] == '-')); > + } else { > + break; > + } > + } > + > + if (tag_op_list_size (tag_ops) == 0) { > + fprintf (stderr, "Error: 'notmuch tag' requires at least one tag to add or remove.\n"); > + return 1; > + } > + > + *query_str = query_string_from_args (ctx, argc - i, &argv[i]); > + > + if (**query_str == '\0') { > + fprintf (stderr, "Error: notmuch tag requires at least one search term.\n"); > + return 1; > + } > + > + return 0; > +} > + > + > static inline void > message_error (notmuch_message_t *message, > notmuch_status_t status, > diff --git a/tag-util.h b/tag-util.h > index 99b0fa0..4956725 100644 > --- a/tag-util.h > +++ b/tag-util.h > @@ -72,6 +72,22 @@ parse_tag_line (void *ctx, char *line, > tag_op_flag_t flags, > char **query_str, tag_op_list_t *ops); > > + > + > +/* Parse a command line of the following format: > + * > + * +|- [...] [--] > + * > + * Output Parameters: > + * ops contains a list of tag operations > + * query_str the search terms. > + */ > + > +tag_parse_status_t > +parse_tag_command_line (void *ctx, int argc, char **argv, > + tag_op_flag_t flags, > + char **query_str, tag_op_list_t *ops); > + > /* > * Create an empty list of tag operations > * > -- > 1.7.10.4 > > _______________________________________________ > notmuch mailing list > notmuch@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch