1f/bcaa43a293174579030c8618bb76b785f16c51

   1 Return-Path: <jani@nikula.org>\r
   2 X-Original-To: notmuch@notmuchmail.org\r
   3 Delivered-To: notmuch@notmuchmail.org\r
   4 Received: from localhost (localhost [127.0.0.1])\r
   5         by olra.theworths.org (Postfix) with ESMTP id 7BA17431FAF\r
   6         for <notmuch@notmuchmail.org>; Mon,  7 Jan 2013 10:52:19 -0800 (PST)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: -0.7\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5\r
  12         tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\r
  13 Received: from olra.theworths.org ([127.0.0.1])\r
  14         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  15         with ESMTP id vLDuLkK6BanN for <notmuch@notmuchmail.org>;\r
  16         Mon,  7 Jan 2013 10:52:15 -0800 (PST)\r
  17 Received: from mail-la0-f46.google.com (mail-la0-f46.google.com\r
  18         [209.85.215.46]) (using TLSv1 with cipher RC4-SHA (128/128 bits))\r
  19         (No client certificate requested)\r
  20         by olra.theworths.org (Postfix) with ESMTPS id 40024431FAE\r
  21         for <notmuch@notmuchmail.org>; Mon,  7 Jan 2013 10:52:15 -0800 (PST)\r
  22 Received: by mail-la0-f46.google.com with SMTP id fq13so16849149lab.19\r
  23         for <notmuch@notmuchmail.org>; Mon, 07 Jan 2013 10:52:13 -0800 (PST)\r
  24 X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;\r
  25         d=google.com; s=20120113;\r
  26         h=x-received:from:to:cc:subject:in-reply-to:references:user-agent\r
  27         :date:message-id:mime-version:content-type:x-gm-message-state;\r
  28         bh=nif7xF3g8c7npyjQq6hF9GlhdSnoOj5Mk04ABq56vI0=;\r
  29         b=bqwieIp9a/i7gfX6ETevz4vgi2l048GEHGiZI73roF+B0eRKBTOhlBGEKoaw0yOpFs\r
  30         1whT2B4J8iwXSgwoyDGHsOJUtZp63fheMHMpCi5tFSe/XkKuAA/0WxV5m0FeML1ssEGy\r
  31         EL44CqSxxJtzQZ4bb7vInH7j5ujuB14MpTARqdR3u6Hk5K/doLK2YPVtjWXgzDihGghC\r
  32         xUpCawa+0oMJXGCHQX9E3IN3m/OMfF19FniVm1czf8DQETvH0BDS0jYyYV5++2+K10vU\r
  33         KSb2SXE1PZpKUhIosG0AgZGHfz+90xDuW9dQW1mY9oAqR9RVRzHtCPKWUKSYFz4Gqw/d\r
  34         shFQ==\r
  35 X-Received: by 10.112.98.71 with SMTP id eg7mr25409594lbb.133.1357584732362;\r
  36         Mon, 07 Jan 2013 10:52:12 -0800 (PST)\r
  37 Received: from localhost (dsl-hkibrasgw4-50df51-27.dhcp.inet.fi.\r
  38         [80.223.81.27]) by mx.google.com with ESMTPS id\r
  39         hc20sm23570663lab.11.2013.01.07.10.52.09\r
  40         (version=SSLv3 cipher=OTHER); Mon, 07 Jan 2013 10:52:11 -0800 (PST)\r
  41 From: Jani Nikula <jani@nikula.org>\r
  42 To: david@tethera.net, notmuch@notmuchmail.org\r
  43 Subject: Re: [Patch v2 2/9] notmuch-tag.c: convert to use tag-utils\r
  44 In-Reply-To: <1357528614-6413-3-git-send-email-david@tethera.net>\r
  45 References: <1357528614-6413-1-git-send-email-david@tethera.net>\r
  46         <1357528614-6413-3-git-send-email-david@tethera.net>\r
  47 User-Agent: Notmuch/0.14+211~gc8d6546 (http://notmuchmail.org) Emacs/24.2.1\r
  48         (x86_64-pc-linux-gnu)\r
  49 Date: Mon, 07 Jan 2013 20:52:02 +0200\r
  50 Message-ID: <87obh0esh9.fsf@nikula.org>\r
  51 MIME-Version: 1.0\r
  52 Content-Type: text/plain\r
  53 X-Gm-Message-State:\r
  54  ALoCoQk3jsG1G3PwgTRGXNP9oqBwH6t6PCG7j84Jj8hMYdQ/rKU0xyQ68ATK8RAb9WzcfurR9pbZ\r
  55 Cc: David Bremner <bremner@debian.org>\r
  56 X-BeenThere: notmuch@notmuchmail.org\r
  57 X-Mailman-Version: 2.1.13\r
  58 Precedence: list\r
  59 List-Id: "Use and development of the notmuch mail system."\r
  60         <notmuch.notmuchmail.org>\r
  61 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  62         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  63 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  64 List-Post: <mailto:notmuch@notmuchmail.org>\r
  65 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  66 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  67         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  68 X-List-Received-Date: Mon, 07 Jan 2013 18:52:19 -0000\r
  69 \r
  70 On Mon, 07 Jan 2013, david@tethera.net wrote:\r
  71 > From: David Bremner <bremner@debian.org>\r
  72 >\r
  73 > Command line parsing is factored out into a function\r
  74 > parse_tag_command_line in tag-utils.c.\r
  75 >\r
  76 > There is some duplicated code eliminated in tag_query, and a bunch of\r
  77 > translation from using the bare tag_op structs to using that tag-utils\r
  78 > API.\r
  79 > ---\r
  80 >  notmuch-tag.c |   99 ++++++++++++---------------------------------------------\r
  81 >  tag-util.c    |   51 +++++++++++++++++++++++++++--\r
  82 >  tag-util.h    |   15 +++++++++\r
  83 >  3 files changed, 84 insertions(+), 81 deletions(-)\r
  84 >\r
  85 > diff --git a/notmuch-tag.c b/notmuch-tag.c\r
  86 > index fc9d43a..8129912 100644\r
  87 > --- a/notmuch-tag.c\r
  88 > +++ b/notmuch-tag.c\r
  89 > @@ -19,6 +19,7 @@\r
  90 >   */\r
  91 >  \r
  92 >  #include "notmuch-client.h"\r
  93 > +#include "tag-util.h"\r
  94 >  #include "string-util.h"\r
  95 >  \r
  96 >  static volatile sig_atomic_t interrupted;\r
  97 > @@ -36,14 +37,10 @@ handle_sigint (unused (int sig))\r
  98 >      interrupted = 1;\r
  99 >  }\r
 100 >  \r
 101 > -typedef struct {\r
 102 > -    const char *tag;\r
 103 > -    notmuch_bool_t remove;\r
 104 > -} tag_operation_t;\r
 105 >  \r
 106 >  static char *\r
 107 >  _optimize_tag_query (void *ctx, const char *orig_query_string,\r
 108 > -                  const tag_operation_t *tag_ops)\r
 109 > +                  const tag_op_list_t *list)\r
 110 >  {\r
 111 >      /* This is subtler than it looks.  Xapian ignores the '-' operator\r
 112 >       * at the beginning both queries and parenthesized groups and,\r
 113 > @@ -60,7 +57,7 @@ _optimize_tag_query (void *ctx, const char *orig_query_string,\r
 114 >      size_t i;\r
 115 >  \r
 116 >      /* Don't optimize if there are no tag changes. */\r
 117 > -    if (tag_ops[0].tag == NULL)\r
 118 > +    if (tag_op_list_size (list) == 0)\r
 119 >       return talloc_strdup (ctx, orig_query_string);\r
 120 >  \r
 121 >      /* Build the new query string */\r
 122 > @@ -69,17 +66,17 @@ _optimize_tag_query (void *ctx, const char *orig_query_string,\r
 123 >      else\r
 124 >       query_string = talloc_asprintf (ctx, "( %s ) and (", orig_query_string);\r
 125 >  \r
 126 > -    for (i = 0; tag_ops[i].tag && query_string; i++) {\r
 127 > +    for (i = 0; i < tag_op_list_size (list) && query_string; i++) {\r
 128 >       /* XXX in case of OOM, query_string will be deallocated when\r
 129 >        * ctx is, which might be at shutdown */\r
 130 >       if (make_boolean_term (ctx,\r
 131 > -                            "tag", tag_ops[i].tag,\r
 132 > +                            "tag", tag_op_list_tag (list, i),\r
 133 >                              &escaped, &escaped_len))\r
 134 >           return NULL;\r
 135 >  \r
 136 >       query_string = talloc_asprintf_append_buffer (\r
 137 >           query_string, "%s%s%s", join,\r
 138 > -         tag_ops[i].remove ? "" : "not ",\r
 139 > +         tag_op_list_isremove (list, i) ? "" : "not ",\r
 140 >           escaped);\r
 141 >       join = " or ";\r
 142 >      }\r
 143 > @@ -91,17 +88,15 @@ _optimize_tag_query (void *ctx, const char *orig_query_string,\r
 144 >      return query_string;\r
 145 >  }\r
 146 >  \r
 147 > -/* Tag messages matching 'query_string' according to 'tag_ops', which\r
 148 > - * must be an array of tagging operations terminated with an empty\r
 149 > - * element. */\r
 150 > +/* Tag messages matching 'query_string' according to 'tag_ops'\r
 151 > + */\r
 152 >  static int\r
 153 >  tag_query (void *ctx, notmuch_database_t *notmuch, const char *query_string,\r
 154 > -        tag_operation_t *tag_ops, notmuch_bool_t synchronize_flags)\r
 155 > +        tag_op_list_t *tag_ops, tag_op_flag_t flags)\r
 156 >  {\r
 157 >      notmuch_query_t *query;\r
 158 >      notmuch_messages_t *messages;\r
 159 >      notmuch_message_t *message;\r
 160 > -    int i;\r
 161 >  \r
 162 >      /* Optimize the query so it excludes messages that already have\r
 163 >       * the specified set of tags. */\r
 164 > @@ -124,21 +119,7 @@ tag_query (void *ctx, notmuch_database_t *notmuch, const char *query_string,\r
 165 >        notmuch_messages_valid (messages) && ! interrupted;\r
 166 >        notmuch_messages_move_to_next (messages)) {\r
 167 >       message = notmuch_messages_get (messages);\r
 168 > -\r
 169 > -     notmuch_message_freeze (message);\r
 170 > -\r
 171 > -     for (i = 0; tag_ops[i].tag; i++) {\r
 172 > -         if (tag_ops[i].remove)\r
 173 > -             notmuch_message_remove_tag (message, tag_ops[i].tag);\r
 174 > -         else\r
 175 > -             notmuch_message_add_tag (message, tag_ops[i].tag);\r
 176 > -     }\r
 177 > -\r
 178 > -     notmuch_message_thaw (message);\r
 179 > -\r
 180 > -     if (synchronize_flags)\r
 181 > -         notmuch_message_tags_to_maildir_flags (message);\r
 182 > -\r
 183 > +     tag_op_list_apply (message, tag_ops, flags | TAG_FLAG_PRE_OPTIMIZED);\r
 184 \r
 185 I think we probably want to check for errors and bail out on them\r
 186 here. It's a functional change, so it belongs in a follow-up patch. We\r
 187 haven't done it before, but it'll be more important with batch tagging.\r
 188 \r
 189 BR,\r
 190 Jani.\r
 191 \r
 192 \r
 193 >       notmuch_message_destroy (message);\r
 194 >      }\r
 195 >  \r
 196 > @@ -150,15 +131,13 @@ tag_query (void *ctx, notmuch_database_t *notmuch, const char *query_string,\r
 197 >  int\r
 198 >  notmuch_tag_command (void *ctx, int argc, char *argv[])\r
 199 >  {\r
 200 > -    tag_operation_t *tag_ops;\r
 201 > -    int tag_ops_count = 0;\r
 202 > -    char *query_string;\r
 203 > +    tag_op_list_t *tag_ops = NULL;\r
 204 > +    char *query_string = NULL;\r
 205 >      notmuch_config_t *config;\r
 206 >      notmuch_database_t *notmuch;\r
 207 >      struct sigaction action;\r
 208 > -    notmuch_bool_t synchronize_flags;\r
 209 > -    int i;\r
 210 > -    int ret;\r
 211 > +    tag_op_flag_t tag_flags = TAG_FLAG_NONE;\r
 212 > +    int ret = 0;\r
 213 >  \r
 214 >      /* Setup our handler for SIGINT */\r
 215 >      memset (&action, 0, sizeof (struct sigaction));\r
 216 > @@ -167,54 +146,15 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])\r
 217 >      action.sa_flags = SA_RESTART;\r
 218 >      sigaction (SIGINT, &action, NULL);\r
 219 >  \r
 220 > -    argc--; argv++; /* skip subcommand argument */\r
 221 > -\r
 222 > -    /* Array of tagging operations (add or remove), terminated with an\r
 223 > -     * empty element. */\r
 224 > -    tag_ops = talloc_array (ctx, tag_operation_t, argc + 1);\r
 225 > +    tag_ops = tag_op_list_create (ctx);\r
 226 >      if (tag_ops == NULL) {\r
 227 >       fprintf (stderr, "Out of memory.\n");\r
 228 >       return 1;\r
 229 >      }\r
 230 >  \r
 231 > -    for (i = 0; i < argc; i++) {\r
 232 > -     if (strcmp (argv[i], "--") == 0) {\r
 233 > -         i++;\r
 234 > -         break;\r
 235 > -     }\r
 236 > -     if (argv[i][0] == '+' || argv[i][0] == '-') {\r
 237 > -         if (argv[i][0] == '+' && argv[i][1] == '\0') {\r
 238 > -             fprintf (stderr, "Error: tag names cannot be empty.\n");\r
 239 > -             return 1;\r
 240 > -         }\r
 241 > -         if (argv[i][0] == '+' && argv[i][1] == '-') {\r
 242 > -             /* This disallows adding the non-removable tag "-" and\r
 243 > -              * enables notmuch tag to take long options in the\r
 244 > -              * future. */\r
 245 > -             fprintf (stderr, "Error: tag names must not start with '-'.\n");\r
 246 > -             return 1;\r
 247 > -         }\r
 248 > -         tag_ops[tag_ops_count].tag = argv[i] + 1;\r
 249 > -         tag_ops[tag_ops_count].remove = (argv[i][0] == '-');\r
 250 > -         tag_ops_count++;\r
 251 > -     } else {\r
 252 > -         break;\r
 253 > -     }\r
 254 > -    }\r
 255 > -\r
 256 > -    tag_ops[tag_ops_count].tag = NULL;\r
 257 > -\r
 258 > -    if (tag_ops_count == 0) {\r
 259 > -     fprintf (stderr, "Error: 'notmuch tag' requires at least one tag to add or remove.\n");\r
 260 > -     return 1;\r
 261 > -    }\r
 262 > -\r
 263 > -    query_string = query_string_from_args (ctx, argc - i, &argv[i]);\r
 264 > -\r
 265 > -    if (*query_string == '\0') {\r
 266 > -     fprintf (stderr, "Error: notmuch tag requires at least one search term.\n");\r
 267 > +    if (parse_tag_command_line (ctx, argc - 1, argv + 1,\r
 268 > +                             &query_string, tag_ops))\r
 269 >       return 1;\r
 270 > -    }\r
 271 >  \r
 272 >      config = notmuch_config_open (ctx, NULL, NULL);\r
 273 >      if (config == NULL)\r
 274 > @@ -224,9 +164,10 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])\r
 275 >                              NOTMUCH_DATABASE_MODE_READ_WRITE, &notmuch))\r
 276 >       return 1;\r
 277 >  \r
 278 > -    synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config);\r
 279 > +    if (notmuch_config_get_maildir_synchronize_flags (config))\r
 280 > +     tag_flags |= TAG_FLAG_MAILDIR_SYNC;\r
 281 >  \r
 282 > -    ret = tag_query (ctx, notmuch, query_string, tag_ops, synchronize_flags);\r
 283 > +    ret = tag_query (ctx, notmuch, query_string, tag_ops, tag_flags);\r
 284 >  \r
 285 >      notmuch_database_destroy (notmuch);\r
 286 >  \r
 287 > diff --git a/tag-util.c b/tag-util.c\r
 288 > index 0a4fe78..701d329 100644\r
 289 > --- a/tag-util.c\r
 290 > +++ b/tag-util.c\r
 291 > @@ -45,8 +45,9 @@ illegal_tag (const char *tag, notmuch_bool_t remove)\r
 292 >      if (*tag == '\0' && ! remove)\r
 293 >       return "empty tag forbidden";\r
 294 >  \r
 295 > -    /* This disallows adding the non-removable tag "-" and\r
 296 > -     * enables notmuch tag to take long options more easily.\r
 297 > +    /* This disallows adding tags starting with "-", in particular the\r
 298 > +     * non-removable tag "-" and enables notmuch tag to take long\r
 299 > +     * options more easily.\r
 300 >       */\r
 301 >  \r
 302 >      if (*tag == '-' && ! remove)\r
 303 > @@ -157,6 +158,52 @@ parse_tag_line (void *ctx, char *line,\r
 304 >      return ret;\r
 305 >  }\r
 306 >  \r
 307 > +tag_parse_status_t\r
 308 > +parse_tag_command_line (void *ctx, int argc, char **argv,\r
 309 > +                     char **query_str, tag_op_list_t *tag_ops)\r
 310 > +{\r
 311 > +\r
 312 > +    int i;\r
 313 > +\r
 314 > +    tag_op_list_reset (tag_ops);\r
 315 > +\r
 316 > +    for (i = 0; i < argc; i++) {\r
 317 > +     if (strcmp (argv[i], "--") == 0) {\r
 318 > +         i++;\r
 319 > +         break;\r
 320 > +     }\r
 321 > +\r
 322 > +     if (argv[i][0] != '+' && argv[i][0] != '-')\r
 323 > +         break;\r
 324 > +\r
 325 > +     notmuch_bool_t is_remove = argv[i][0] == '-';\r
 326 > +     const char *msg;\r
 327 > +\r
 328 > +     msg = illegal_tag (argv[i] + 1, is_remove);\r
 329 > +     if (msg) {\r
 330 > +         fprintf (stderr, "Error: %s", msg);\r
 331 > +         return TAG_PARSE_INVALID;\r
 332 > +     }\r
 333 > +\r
 334 > +     tag_op_list_append (tag_ops, argv[i] + 1, is_remove);\r
 335 > +    }\r
 336 > +\r
 337 > +    if (tag_op_list_size (tag_ops) == 0) {\r
 338 > +     fprintf (stderr, "Error: 'notmuch tag' requires at least one tag to add or remove.\n");\r
 339 > +     return TAG_PARSE_INVALID;\r
 340 > +    }\r
 341 > +\r
 342 > +    *query_str = query_string_from_args (ctx, argc - i, &argv[i]);\r
 343 > +\r
 344 > +    if (*query_str == NULL || **query_str == '\0') {\r
 345 > +     fprintf (stderr, "Error: notmuch tag requires at least one search term.\n");\r
 346 > +     return TAG_PARSE_INVALID;\r
 347 > +    }\r
 348 > +\r
 349 > +    return TAG_PARSE_SUCCESS;\r
 350 > +}\r
 351 > +\r
 352 > +\r
 353 >  static inline void\r
 354 >  message_error (notmuch_message_t *message,\r
 355 >              notmuch_status_t status,\r
 356 > diff --git a/tag-util.h b/tag-util.h\r
 357 > index c07bfde..246de85 100644\r
 358 > --- a/tag-util.h\r
 359 > +++ b/tag-util.h\r
 360 > @@ -72,6 +72,21 @@ parse_tag_line (void *ctx, char *line,\r
 361 >               tag_op_flag_t flags,\r
 362 >               char **query_str, tag_op_list_t *ops);\r
 363 >  \r
 364 > +\r
 365 > +\r
 366 > +/* Parse a command line of the following format:\r
 367 > + *\r
 368 > + * +<tag>|-<tag> [...] [--] <search-terms>\r
 369 > + *\r
 370 > + * Output Parameters:\r
 371 > + *   ops     contains a list of tag operations\r
 372 > + *   query_str the search terms.\r
 373 > + */\r
 374 > +\r
 375 > +tag_parse_status_t\r
 376 > +parse_tag_command_line (void *ctx, int argc, char **argv,\r
 377 > +                     char **query_str, tag_op_list_t *ops);\r
 378 > +\r
 379 >  /*\r
 380 >   * Create an empty list of tag operations\r
 381 >   *\r
 382 > -- \r
 383 > 1.7.10.4\r
 384 >\r
 385 > _______________________________________________\r
 386 > notmuch mailing list\r
 387 > notmuch@notmuchmail.org\r
 388 > http://notmuchmail.org/mailman/listinfo/notmuch\r