42/d60343bb2df9f46f31c3a8c34d26aa59f1a39c

   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 D35DE431FAF\r
   6         for <notmuch@notmuchmail.org>; Tue, 31 Jul 2012 06:19:14 -0700 (PDT)\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 hQo9YQGvdlyC for <notmuch@notmuchmail.org>;\r
  16         Tue, 31 Jul 2012 06:19:11 -0700 (PDT)\r
  17 Received: from mail-gh0-f181.google.com (mail-gh0-f181.google.com\r
  18         [209.85.160.181]) (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 1489E431FBD\r
  21         for <notmuch@notmuchmail.org>; Tue, 31 Jul 2012 06:19:11 -0700 (PDT)\r
  22 Received: by ghbz13 with SMTP id z13so7322316ghb.26\r
  23         for <notmuch@notmuchmail.org>; Tue, 31 Jul 2012 06:19:09 -0700 (PDT)\r
  24 X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;\r
  25         d=google.com; s=20120113;\r
  26         h=from:to:cc:subject:in-reply-to:references:user-agent:date\r
  27         :message-id:mime-version:content-type:x-gm-message-state;\r
  28         bh=OF3KX6cPhIB+K9k/6XQeI3217uNYLyNmFl7+SdzElXM=;\r
  29         b=FcA6fJhD9e8lOQOUMpuVox0Xo8IladheSZTS+KGKEO0B14fpjNX5/3Dw//466Wxen9\r
  30         qFR+keM4uk0qZoi5EFCR3EyTEizdPS6KiKA6rL1ljUiAG03BfBuMqD4H2Up60bbiKr7i\r
  31         3IO8jRsJa2UtdV2TB0ekM82zylRbXQSFEgRDpjlqIKMoDKKw46OplYwlM5f76PdiDF8V\r
  32         PrhoNHvfUvPyLS5npfjm725ilrnc24zWBvHqLjtMWPvqszZZRRu7Qt2N+8CdIOlH8jKY\r
  33         g25TbS50XZFWVTWDM8JJ7/3o3KWEdAu8a1XiShJvfDAD/z7O42UmFsieAfgkH+oUbB/X\r
  34         GiNQ==\r
  35 Received: by 10.236.182.228 with SMTP id o64mr13323006yhm.85.1343740749265;\r
  36         Tue, 31 Jul 2012 06:19:09 -0700 (PDT)\r
  37 Received: from localhost ([2001:4b98:dc0:43:216:3eff:fe1b:25f3])\r
  38         by mx.google.com with ESMTPS id d21sm100098yhe.22.2012.07.31.06.19.08\r
  39         (version=SSLv3 cipher=OTHER); Tue, 31 Jul 2012 06:19:08 -0700 (PDT)\r
  40 From: Jani Nikula <jani@nikula.org>\r
  41 To: Dominik Peteler <dominik@with-h.at>, notmuch@notmuchmail.org\r
  42 Subject: Re: [PATCHv2] cli: Hooks for tag-command\r
  43 In-Reply-To: <1342634970-12991-1-git-send-email-dominik@with-h.at>\r
  44 References: <1342634970-12991-1-git-send-email-dominik@with-h.at>\r
  45 User-Agent: Notmuch/0.13.2+106~gb810aee (http://notmuchmail.org) Emacs/23.2.1\r
  46         (x86_64-pc-linux-gnu)\r
  47 Date: Tue, 31 Jul 2012 15:19:04 +0200\r
  48 Message-ID: <87txwo9ign.fsf@nikula.org>\r
  49 MIME-Version: 1.0\r
  50 Content-Type: text/plain; charset=us-ascii\r
  51 X-Gm-Message-State:\r
  52  ALoCoQmcbn1wesTObaNE7RNf32SWL1nmU9nFDXss/xDHOV9IuGfchZfCBl6rOaLx8eRB7+5hBYr6\r
  53 Cc: Dominik Peteler <dominik@with-h.at>\r
  54 X-BeenThere: notmuch@notmuchmail.org\r
  55 X-Mailman-Version: 2.1.13\r
  56 Precedence: list\r
  57 List-Id: "Use and development of the notmuch mail system."\r
  58         <notmuch.notmuchmail.org>\r
  59 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  60         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  61 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  62 List-Post: <mailto:notmuch@notmuchmail.org>\r
  63 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  64 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  65         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  66 X-List-Received-Date: Tue, 31 Jul 2012 13:19:15 -0000\r
  67 \r
  68 On Wed, 18 Jul 2012, Dominik Peteler <dominik@with-h.at> wrote:\r
  69 > hello,\r
  70 >\r
  71 > I improved my patch according to Janis mail:\r
  72 >  * new cli syntax: notmuch tag [ --no-hooks ] -- <tag ops> [ -- ] <search terms>\r
  73 >  * adjusted man pages and wrote tests\r
  74 >\r
  75 > I had the idea to improve this feature by passing the message-ids or the filename to the hooks.\r
  76 > What's your opinion about that ? Any suggestions ?\r
  77 \r
  78 There are probably races there that are difficult to solve. Imagine two\r
  79 concurrent 'notmuch tag' invocations. The db can't be locked across the\r
  80 hooks. Also, if you relied solely on the passed message-ids or filenames\r
  81 to do whatever your hook would do, interrupting 'notmuch tag' before the\r
  82 hook is run would leave you in an inconsistent state.\r
  83 \r
  84 >\r
  85 > regards\r
  86 >\r
  87 > dominik\r
  88 \r
  89 Note that this part of the patch ends up as the commit message in the\r
  90 git repository. Please have a look at\r
  91 http://notmuchmail.org/patchformatting/.\r
  92 \r
  93 >\r
  94 >\r
  95 >\r
  96 > There are two hooks:\r
  97 >  * pre-tag: Run before tagging\r
  98 >  * post-tag: Run after\r
  99 >\r
 100 > This allows users to react on changes of tags. For example,\r
 101 > you might want to move a message to a special Maildir\r
 102 > depending on its notmuch tags.\r
 103 \r
 104 I am not sure if the example is such a hot idea. Moving a file in the\r
 105 maildir means you'd need to run 'notmuch new' again to inform notmuch of\r
 106 the change. I.e. after tagging you couldn't view the message before\r
 107 you've run 'notmuch new'.\r
 108 \r
 109 Are there any other, perhaps more compelling reasons to have hooks in\r
 110 'notmuch tag'? Can you give us some other examples, please?\r
 111 \r
 112 > ---\r
 113 >  man/man1/notmuch-tag.1   | 22 +++++++++++++++++++-\r
 114 >  man/man5/notmuch-hooks.5 | 19 ++++++++++++++++++\r
 115 >  notmuch-tag.c            | 52 +++++++++++++++++++++++++++++++++++++++++++++---\r
 116 >  test/hooks               | 36 +++++++++++++++++++++++++++++++++\r
 117 >  test/tagging             | 28 ++++++++++++++++++++++++++\r
 118 >  5 files changed, 153 insertions(+), 4 deletions(-)\r
 119 >\r
 120 > diff --git a/man/man1/notmuch-tag.1 b/man/man1/notmuch-tag.1\r
 121 > index d810e1b..e00e189 100644\r
 122 > --- a/man/man1/notmuch-tag.1\r
 123 > +++ b/man/man1/notmuch-tag.1\r
 124 > @@ -4,7 +4,11 @@ notmuch-tag \- add/remove tags for all messages matching the search terms\r
 125 >  \r
 126 >  .SH SYNOPSIS\r
 127 >  .B notmuch tag\r
 128 > -.RI  "+<" tag> "|\-<" tag "> [...] [\-\-] <" search-term ">..."\r
 129 > +.RI  "+<" tag "> |\-<" tag "> [...] [\-\-] <" search-term ">..."\r
 130 > +\r
 131 > +.B notmuch tag\r
 132 > +.RB "[" --no-hooks "]"\r
 133 > +.RI "\-\- +<" tag "> |\-<" tag "> [...] \-\- <" search-term ">..."\r
 134 >  \r
 135 >  .SH DESCRIPTION\r
 136 >  \r
 137 > @@ -29,6 +33,22 @@ updates the maildir flags according to tag changes if the\r
 138 >  configuration option is enabled. See \fBnotmuch-config\fR(1) for\r
 139 >  details.\r
 140 >  \r
 141 > +The\r
 142 > +.B tag\r
 143 > +command supports hooks. See  \fBnotmuch-hooks(5)\fR\r
 144 > +for more details on hooks.\r
 145 > +\r
 146 > +Supported options for\r
 147 > +.B tag\r
 148 > +include\r
 149 > +.RS 4\r
 150 > +.TP 4\r
 151 > +.BR \-\-no\-hooks\r
 152 > +\r
 153 > +Prevents hooks from being run.\r
 154 > +.RE\r
 155 > +.RE\r
 156 > +\r
 157 >  .SH SEE ALSO\r
 158 >  \r
 159 >  \fBnotmuch\fR(1), \fBnotmuch-config\fR(1), \fBnotmuch-count\fR(1),\r
 160 > diff --git a/man/man5/notmuch-hooks.5 b/man/man5/notmuch-hooks.5\r
 161 > index b914a29..e193ef5 100644\r
 162 > --- a/man/man5/notmuch-hooks.5\r
 163 > +++ b/man/man5/notmuch-hooks.5\r
 164 > @@ -38,6 +38,25 @@ the scan or import.\r
 165 >  Typically this hook is used to perform additional query\-based tagging on the\r
 166 >  imported messages.\r
 167 >  .RE\r
 168 > +.RS 4\r
 169 > +.TP 4\r
 170 > +.B pre\-tag\r
 171 > +This hook is invoked by the\r
 172 > +.B tag\r
 173 > +command before tagging messages. If this\r
 174 > +hook exits with a non-zero status, notmuch will abort further processing of the\r
 175 > +.B tag\r
 176 > +command.\r
 177 > +.RE\r
 178 > +.RS 4\r
 179 > +.TP 4\r
 180 > +.B post\-tag\r
 181 > +This hook is invoked by the\r
 182 > +.B tag\r
 183 > +command after messages have been tagged. The hook will not be run if there have been any errors during\r
 184 > +the tagging.\r
 185 > +.RE\r
 186 > +\r
 187 >  \r
 188 >  .SH SEE ALSO\r
 189 >  \r
 190 > diff --git a/notmuch-tag.c b/notmuch-tag.c\r
 191 > index 7d18639..7572059 100644\r
 192 > --- a/notmuch-tag.c\r
 193 > +++ b/notmuch-tag.c\r
 194 > @@ -174,9 +174,17 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])\r
 195 >      int tag_ops_count = 0;\r
 196 >      char *query_string;\r
 197 >      notmuch_config_t *config;\r
 198 > +    const char *db_path;\r
 199 >      notmuch_database_t *notmuch;\r
 200 >      struct sigaction action;\r
 201 >      notmuch_bool_t synchronize_flags;\r
 202 > +    /* Points to the position of the "--" delimiters, e. g.\r
 203 > +     *    <optional arguments> arg_delimiters[0] <tag ops> arg_delimiters[1] <search terms>\r
 204 > +     *\r
 205 > +     * arg_delimiters[0] may remain -1 if there are no arguments given\r
 206 > +     * arg_delimiters[0] may remain -1 if there is no delimiter between tag ops and search terms */\r
 207 > +    int arg_delimiters[2] = {-1, -1};\r
 208 > +    notmuch_bool_t run_hooks = TRUE;\r
 209 >      int i;\r
 210 >      int ret;\r
 211 >  \r
 212 > @@ -197,11 +205,37 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])\r
 213 >       return 1;\r
 214 >      }\r
 215 >  \r
 216 > +    /* Determine position of delimiters */\r
 217 >      for (i = 0; i < argc; i++) {\r
 218 >       if (strcmp (argv[i], "--") == 0) {\r
 219 > -         i++;\r
 220 > -         break;\r
 221 > +         if (arg_delimiters[1] == -1) {\r
 222 > +             arg_delimiters[1] = i;\r
 223 > +         } else if (arg_delimiters[0] == -1) {\r
 224 > +             arg_delimiters[0] = arg_delimiters[1];\r
 225 > +             arg_delimiters[1] = i;\r
 226 > +         } else {\r
 227 > +             fprintf (stderr, "Error: 'notmuch tag' requires delimiter \"--\" at most two times.\n");\r
 228 > +             return 1;\r
 229 > +         }\r
 230 >       }\r
 231 > +    }\r
 232 > +\r
 233 > +    /* Process arguments if present */\r
 234 > +    for (i = 0; i < arg_delimiters[0]; i++) {\r
 235 > +     if (strcmp (argv[i], "--no-hooks") == 0) {\r
 236 > +         run_hooks = FALSE;\r
 237 > +     } else {\r
 238 > +         fprintf (stderr, "Error: 'notmuch tag' doesn't recognize argument '%s'.\n", argv[i]);\r
 239 > +         return 1;\r
 240 > +     }\r
 241 > +    }\r
 242 > +\r
 243 > +    /* Set arg_delimiters[1] to argc if no delimiters at all are present */\r
 244 > +    if (arg_delimiters[1] == -1)\r
 245 > +         arg_delimiters[1] = argc;\r
 246 > +\r
 247 > +    /* Read tag ops */\r
 248 > +    for (i = arg_delimiters[0]+1; i < arg_delimiters[1]; i++) {\r
 249 \r
 250 As I said before, you should parse the arguments using the notmuch\r
 251 argument parser. The above is getting needlessly complicated. Please see\r
 252 the other notmuch commands for examples, and see the relevant part of\r
 253 [1] for hints how to add this to 'notmuch tag'.\r
 254 \r
 255 [1]\r
 256 id:"c560dc17781ea2e725cc75e00cb328822a65537f.1334404979.git.jani@nikula.org" \r
 257 or\r
 258 http://mid.gmane.org/c560dc17781ea2e725cc75e00cb328822a65537f.1334404979.git.jani@nikula.org\r
 259 \r
 260 >       if (argv[i][0] == '+' || argv[i][0] == '-') {\r
 261 >           tag_ops[tag_ops_count].tag = argv[i] + 1;\r
 262 >           tag_ops[tag_ops_count].remove = (argv[i][0] == '-');\r
 263 > @@ -229,7 +263,15 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])\r
 264 >      if (config == NULL)\r
 265 >       return 1;\r
 266 >  \r
 267 > -    if (notmuch_database_open (notmuch_config_get_database_path (config),\r
 268 > +    db_path = notmuch_config_get_database_path (config);\r
 269 > +\r
 270 > +    if (run_hooks) {\r
 271 > +     ret = notmuch_run_hook (db_path, "pre-tag");\r
 272 > +     if (ret)\r
 273 > +         return ret;\r
 274 > +    }\r
 275 > +\r
 276 > +    if (notmuch_database_open (db_path,\r
 277 >                              NOTMUCH_DATABASE_MODE_READ_WRITE, &notmuch))\r
 278 >       return 1;\r
 279 >  \r
 280 > @@ -239,5 +281,9 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])\r
 281 >  \r
 282 >      notmuch_database_destroy (notmuch);\r
 283 >  \r
 284 > +    if (!ret && run_hooks) {\r
 285 > +     ret = notmuch_run_hook (db_path, "post-tag");\r
 286 > +    }\r
 287 > +\r
 288 >      return ret;\r
 289 >  }\r
 290 > diff --git a/test/hooks b/test/hooks\r
 291 > index 77e8569..ae857cc 100755\r
 292 > --- a/test/hooks\r
 293 > +++ b/test/hooks\r
 294 > @@ -31,6 +31,7 @@ rm_hooks () {\r
 295 >  # add a message to generate mail dir and database\r
 296 >  add_message\r
 297 >  \r
 298 > +# {pre,post}-new hooks\r
 299 >  test_begin_subtest "pre-new is run"\r
 300 >  rm_hooks\r
 301 >  generate_message\r
 302 > @@ -101,4 +102,39 @@ EOF\r
 303 >  chmod +x "${HOOK_DIR}/pre-new"\r
 304 >  test_expect_code 1 "hook execution failure" "notmuch new"\r
 305 >  \r
 306 > +\r
 307 > +\r
 308 > +# {pre,post}-tag hooks\r
 309 > +test_begin_subtest "pre-tag is run"\r
 310 > +rm_hooks\r
 311 > +generate_message\r
 312 > +create_echo_hook "pre-tag" expected output\r
 313 > +notmuch tag +foo -- '*' > /dev/null\r
 314 > +test_expect_equal_file expected output\r
 315 > +\r
 316 > +test_begin_subtest "post-tag is run"\r
 317 > +rm_hooks\r
 318 > +generate_message\r
 319 > +create_echo_hook "post-tag" expected output\r
 320 > +notmuch tag +foo -- '*'  > /dev/null\r
 321 > +test_expect_equal_file expected output\r
 322 > +\r
 323 > +test_begin_subtest "pre-tag is run before post-new"\r
 324 > +rm_hooks\r
 325 > +generate_message\r
 326 > +create_echo_hook "pre-tag" pre-tag.expected pre-tag.output\r
 327 > +create_echo_hook "post-tag" post-tag.expected post-tag.output\r
 328 > +notmuch tag +foo -- '*'  > /dev/null\r
 329 > +test_expect_equal_file post-tag.expected post-tag.output\r
 330 > +\r
 331 > +test_begin_subtest "pre-tag non-zero exit status (hook status)"\r
 332 > +rm_hooks\r
 333 > +generate_message\r
 334 > +create_failing_hook "pre-tag"\r
 335 > +output=`notmuch tag +foo -- '*'  2>&1`\r
 336 > +test_expect_equal "$output" "Error: pre-tag hook failed with status 13"\r
 337 > +\r
 338 > +# depends on the previous subtest leaving broken hook behind\r
 339 > +test_expect_code 1 "pre-tag non-zero exit status (notmuch status)" "notmuch tag +foo -- '*'"\r
 340 > +\r
 341 >  test_done\r
 342 > diff --git a/test/tagging b/test/tagging\r
 343 > index e4782ed..5167f4f 100755\r
 344 > --- a/test/tagging\r
 345 > +++ b/test/tagging\r
 346 > @@ -46,4 +46,32 @@ test_expect_equal "$output" "\\r
 347 >  thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; One (:\"  inbox tag1 unread)\r
 348 >  thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag1 tag4 unread)"\r
 349 >  \r
 350 > +test_begin_subtest "Arguments mixed with tag ops"\r
 351 > +notmuch tag +-no-hooks --no-hooks -- One\r
 352 > +notmuch tag --no-hooks +-no-hooks -tag4 -- Two\r
 353 > +output=$(notmuch search \* | notmuch_search_sanitize)\r
 354 > +test_expect_equal "$output" "\\r
 355 > +thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; One (:\"  inbox tag1 unread)\r
 356 > +thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Two (-no-hooks inbox tag1 unread)"\r
 357 > +notmuch tag --no-hooks -- Two\r
 358 > +\r
 359 > +test_begin_subtest "Arguments with correct position"\r
 360 > +notmuch tag --no-hooks -- +tag4 -tag4 -- One\r
 361 > +output=$(notmuch search \* | notmuch_search_sanitize)\r
 362 > +test_expect_equal "$output" "\\r
 363 > +thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; One (:\"  inbox tag1 unread)\r
 364 > +thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag1 unread)"\r
 365 > +\r
 366 > +test_begin_subtest "Missing arguments"\r
 367 > +notmuch tag -- +tag4 -tag4 -- One\r
 368 > +output=$(notmuch search \* | notmuch_search_sanitize)\r
 369 > +test_expect_equal "$output" "\\r
 370 > +thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; One (:\"  inbox tag1 unread)\r
 371 > +thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag1 unread)"\r
 372 > +\r
 373 > +test_begin_subtest "Unknown argument"\r
 374 > +output=$(notmuch tag --no-blubb -- +tag4 -tag4 -- One 2>&1)\r
 375 > +test_expect_equal "$output" "\\r
 376 > +Error: 'notmuch tag' doesn't recognize argument '--no-blubb'."\r
 377 > +\r
 378 >  test_done\r
 379 > -- \r
 380 > 1.7.11.2\r
 381 >\r
 382 > _______________________________________________\r
 383 > notmuch mailing list\r
 384 > notmuch@notmuchmail.org\r
 385 > http://notmuchmail.org/mailman/listinfo/notmuch\r