Re: [PATCHv2] cli: Hooks for tag-command
authorJani Nikula <jani@nikula.org>
Tue, 31 Jul 2012 13:19:04 +0000 (15:19 +0200)
committerW. Trevor King <wking@tremily.us>
Fri, 7 Nov 2014 17:48:41 +0000 (09:48 -0800)
42/d60343bb2df9f46f31c3a8c34d26aa59f1a39c [new file with mode: 0644]

diff --git a/42/d60343bb2df9f46f31c3a8c34d26aa59f1a39c b/42/d60343bb2df9f46f31c3a8c34d26aa59f1a39c
new file mode 100644 (file)
index 0000000..956d645
--- /dev/null
@@ -0,0 +1,385 @@
+Return-Path: <jani@nikula.org>\r
+X-Original-To: notmuch@notmuchmail.org\r
+Delivered-To: notmuch@notmuchmail.org\r
+Received: from localhost (localhost [127.0.0.1])\r
+       by olra.theworths.org (Postfix) with ESMTP id D35DE431FAF\r
+       for <notmuch@notmuchmail.org>; Tue, 31 Jul 2012 06:19:14 -0700 (PDT)\r
+X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
+X-Spam-Flag: NO\r
+X-Spam-Score: -0.7\r
+X-Spam-Level: \r
+X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5\r
+       tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\r
+Received: from olra.theworths.org ([127.0.0.1])\r
+       by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
+       with ESMTP id hQo9YQGvdlyC for <notmuch@notmuchmail.org>;\r
+       Tue, 31 Jul 2012 06:19:11 -0700 (PDT)\r
+Received: from mail-gh0-f181.google.com (mail-gh0-f181.google.com\r
+       [209.85.160.181]) (using TLSv1 with cipher RC4-SHA (128/128 bits))\r
+       (No client certificate requested)\r
+       by olra.theworths.org (Postfix) with ESMTPS id 1489E431FBD\r
+       for <notmuch@notmuchmail.org>; Tue, 31 Jul 2012 06:19:11 -0700 (PDT)\r
+Received: by ghbz13 with SMTP id z13so7322316ghb.26\r
+       for <notmuch@notmuchmail.org>; Tue, 31 Jul 2012 06:19:09 -0700 (PDT)\r
+X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;\r
+       d=google.com; s=20120113;\r
+       h=from:to:cc:subject:in-reply-to:references:user-agent:date\r
+       :message-id:mime-version:content-type:x-gm-message-state;\r
+       bh=OF3KX6cPhIB+K9k/6XQeI3217uNYLyNmFl7+SdzElXM=;\r
+       b=FcA6fJhD9e8lOQOUMpuVox0Xo8IladheSZTS+KGKEO0B14fpjNX5/3Dw//466Wxen9\r
+       qFR+keM4uk0qZoi5EFCR3EyTEizdPS6KiKA6rL1ljUiAG03BfBuMqD4H2Up60bbiKr7i\r
+       3IO8jRsJa2UtdV2TB0ekM82zylRbXQSFEgRDpjlqIKMoDKKw46OplYwlM5f76PdiDF8V\r
+       PrhoNHvfUvPyLS5npfjm725ilrnc24zWBvHqLjtMWPvqszZZRRu7Qt2N+8CdIOlH8jKY\r
+       g25TbS50XZFWVTWDM8JJ7/3o3KWEdAu8a1XiShJvfDAD/z7O42UmFsieAfgkH+oUbB/X\r
+       GiNQ==\r
+Received: by 10.236.182.228 with SMTP id o64mr13323006yhm.85.1343740749265;\r
+       Tue, 31 Jul 2012 06:19:09 -0700 (PDT)\r
+Received: from localhost ([2001:4b98:dc0:43:216:3eff:fe1b:25f3])\r
+       by mx.google.com with ESMTPS id d21sm100098yhe.22.2012.07.31.06.19.08\r
+       (version=SSLv3 cipher=OTHER); Tue, 31 Jul 2012 06:19:08 -0700 (PDT)\r
+From: Jani Nikula <jani@nikula.org>\r
+To: Dominik Peteler <dominik@with-h.at>, notmuch@notmuchmail.org\r
+Subject: Re: [PATCHv2] cli: Hooks for tag-command\r
+In-Reply-To: <1342634970-12991-1-git-send-email-dominik@with-h.at>\r
+References: <1342634970-12991-1-git-send-email-dominik@with-h.at>\r
+User-Agent: Notmuch/0.13.2+106~gb810aee (http://notmuchmail.org) Emacs/23.2.1\r
+       (x86_64-pc-linux-gnu)\r
+Date: Tue, 31 Jul 2012 15:19:04 +0200\r
+Message-ID: <87txwo9ign.fsf@nikula.org>\r
+MIME-Version: 1.0\r
+Content-Type: text/plain; charset=us-ascii\r
+X-Gm-Message-State:\r
+ ALoCoQmcbn1wesTObaNE7RNf32SWL1nmU9nFDXss/xDHOV9IuGfchZfCBl6rOaLx8eRB7+5hBYr6\r
+Cc: Dominik Peteler <dominik@with-h.at>\r
+X-BeenThere: notmuch@notmuchmail.org\r
+X-Mailman-Version: 2.1.13\r
+Precedence: list\r
+List-Id: "Use and development of the notmuch mail system."\r
+       <notmuch.notmuchmail.org>\r
+List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
+       <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
+List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
+List-Post: <mailto:notmuch@notmuchmail.org>\r
+List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
+List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
+       <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
+X-List-Received-Date: Tue, 31 Jul 2012 13:19:15 -0000\r
+\r
+On Wed, 18 Jul 2012, Dominik Peteler <dominik@with-h.at> wrote:\r
+> hello,\r
+>\r
+> I improved my patch according to Janis mail:\r
+>  * new cli syntax: notmuch tag [ --no-hooks ] -- <tag ops> [ -- ] <search terms>\r
+>  * adjusted man pages and wrote tests\r
+>\r
+> I had the idea to improve this feature by passing the message-ids or the filename to the hooks.\r
+> What's your opinion about that ? Any suggestions ?\r
+\r
+There are probably races there that are difficult to solve. Imagine two\r
+concurrent 'notmuch tag' invocations. The db can't be locked across the\r
+hooks. Also, if you relied solely on the passed message-ids or filenames\r
+to do whatever your hook would do, interrupting 'notmuch tag' before the\r
+hook is run would leave you in an inconsistent state.\r
+\r
+>\r
+> regards\r
+>\r
+> dominik\r
+\r
+Note that this part of the patch ends up as the commit message in the\r
+git repository. Please have a look at\r
+http://notmuchmail.org/patchformatting/.\r
+\r
+>\r
+>\r
+>\r
+> There are two hooks:\r
+>  * pre-tag: Run before tagging\r
+>  * post-tag: Run after\r
+>\r
+> This allows users to react on changes of tags. For example,\r
+> you might want to move a message to a special Maildir\r
+> depending on its notmuch tags.\r
+\r
+I am not sure if the example is such a hot idea. Moving a file in the\r
+maildir means you'd need to run 'notmuch new' again to inform notmuch of\r
+the change. I.e. after tagging you couldn't view the message before\r
+you've run 'notmuch new'.\r
+\r
+Are there any other, perhaps more compelling reasons to have hooks in\r
+'notmuch tag'? Can you give us some other examples, please?\r
+\r
+> ---\r
+>  man/man1/notmuch-tag.1   | 22 +++++++++++++++++++-\r
+>  man/man5/notmuch-hooks.5 | 19 ++++++++++++++++++\r
+>  notmuch-tag.c            | 52 +++++++++++++++++++++++++++++++++++++++++++++---\r
+>  test/hooks               | 36 +++++++++++++++++++++++++++++++++\r
+>  test/tagging             | 28 ++++++++++++++++++++++++++\r
+>  5 files changed, 153 insertions(+), 4 deletions(-)\r
+>\r
+> diff --git a/man/man1/notmuch-tag.1 b/man/man1/notmuch-tag.1\r
+> index d810e1b..e00e189 100644\r
+> --- a/man/man1/notmuch-tag.1\r
+> +++ b/man/man1/notmuch-tag.1\r
+> @@ -4,7 +4,11 @@ notmuch-tag \- add/remove tags for all messages matching the search terms\r
+>  \r
+>  .SH SYNOPSIS\r
+>  .B notmuch tag\r
+> -.RI  "+<" tag> "|\-<" tag "> [...] [\-\-] <" search-term ">..."\r
+> +.RI  "+<" tag "> |\-<" tag "> [...] [\-\-] <" search-term ">..."\r
+> +\r
+> +.B notmuch tag\r
+> +.RB "[" --no-hooks "]"\r
+> +.RI "\-\- +<" tag "> |\-<" tag "> [...] \-\- <" search-term ">..."\r
+>  \r
+>  .SH DESCRIPTION\r
+>  \r
+> @@ -29,6 +33,22 @@ updates the maildir flags according to tag changes if the\r
+>  configuration option is enabled. See \fBnotmuch-config\fR(1) for\r
+>  details.\r
+>  \r
+> +The\r
+> +.B tag\r
+> +command supports hooks. See  \fBnotmuch-hooks(5)\fR\r
+> +for more details on hooks.\r
+> +\r
+> +Supported options for\r
+> +.B tag\r
+> +include\r
+> +.RS 4\r
+> +.TP 4\r
+> +.BR \-\-no\-hooks\r
+> +\r
+> +Prevents hooks from being run.\r
+> +.RE\r
+> +.RE\r
+> +\r
+>  .SH SEE ALSO\r
+>  \r
+>  \fBnotmuch\fR(1), \fBnotmuch-config\fR(1), \fBnotmuch-count\fR(1),\r
+> diff --git a/man/man5/notmuch-hooks.5 b/man/man5/notmuch-hooks.5\r
+> index b914a29..e193ef5 100644\r
+> --- a/man/man5/notmuch-hooks.5\r
+> +++ b/man/man5/notmuch-hooks.5\r
+> @@ -38,6 +38,25 @@ the scan or import.\r
+>  Typically this hook is used to perform additional query\-based tagging on the\r
+>  imported messages.\r
+>  .RE\r
+> +.RS 4\r
+> +.TP 4\r
+> +.B pre\-tag\r
+> +This hook is invoked by the\r
+> +.B tag\r
+> +command before tagging messages. If this\r
+> +hook exits with a non-zero status, notmuch will abort further processing of the\r
+> +.B tag\r
+> +command.\r
+> +.RE\r
+> +.RS 4\r
+> +.TP 4\r
+> +.B post\-tag\r
+> +This hook is invoked by the\r
+> +.B tag\r
+> +command after messages have been tagged. The hook will not be run if there have been any errors during\r
+> +the tagging.\r
+> +.RE\r
+> +\r
+>  \r
+>  .SH SEE ALSO\r
+>  \r
+> diff --git a/notmuch-tag.c b/notmuch-tag.c\r
+> index 7d18639..7572059 100644\r
+> --- a/notmuch-tag.c\r
+> +++ b/notmuch-tag.c\r
+> @@ -174,9 +174,17 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])\r
+>      int tag_ops_count = 0;\r
+>      char *query_string;\r
+>      notmuch_config_t *config;\r
+> +    const char *db_path;\r
+>      notmuch_database_t *notmuch;\r
+>      struct sigaction action;\r
+>      notmuch_bool_t synchronize_flags;\r
+> +    /* Points to the position of the "--" delimiters, e. g.\r
+> +     *    <optional arguments> arg_delimiters[0] <tag ops> arg_delimiters[1] <search terms>\r
+> +     *\r
+> +     * arg_delimiters[0] may remain -1 if there are no arguments given\r
+> +     * arg_delimiters[0] may remain -1 if there is no delimiter between tag ops and search terms */\r
+> +    int arg_delimiters[2] = {-1, -1};\r
+> +    notmuch_bool_t run_hooks = TRUE;\r
+>      int i;\r
+>      int ret;\r
+>  \r
+> @@ -197,11 +205,37 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])\r
+>      return 1;\r
+>      }\r
+>  \r
+> +    /* Determine position of delimiters */\r
+>      for (i = 0; i < argc; i++) {\r
+>      if (strcmp (argv[i], "--") == 0) {\r
+> -        i++;\r
+> -        break;\r
+> +        if (arg_delimiters[1] == -1) {\r
+> +            arg_delimiters[1] = i;\r
+> +        } else if (arg_delimiters[0] == -1) {\r
+> +            arg_delimiters[0] = arg_delimiters[1];\r
+> +            arg_delimiters[1] = i;\r
+> +        } else {\r
+> +            fprintf (stderr, "Error: 'notmuch tag' requires delimiter \"--\" at most two times.\n");\r
+> +            return 1;\r
+> +        }\r
+>      }\r
+> +    }\r
+> +\r
+> +    /* Process arguments if present */\r
+> +    for (i = 0; i < arg_delimiters[0]; i++) {\r
+> +    if (strcmp (argv[i], "--no-hooks") == 0) {\r
+> +        run_hooks = FALSE;\r
+> +    } else {\r
+> +        fprintf (stderr, "Error: 'notmuch tag' doesn't recognize argument '%s'.\n", argv[i]);\r
+> +        return 1;\r
+> +    }\r
+> +    }\r
+> +\r
+> +    /* Set arg_delimiters[1] to argc if no delimiters at all are present */\r
+> +    if (arg_delimiters[1] == -1)\r
+> +        arg_delimiters[1] = argc;\r
+> +\r
+> +    /* Read tag ops */\r
+> +    for (i = arg_delimiters[0]+1; i < arg_delimiters[1]; i++) {\r
+\r
+As I said before, you should parse the arguments using the notmuch\r
+argument parser. The above is getting needlessly complicated. Please see\r
+the other notmuch commands for examples, and see the relevant part of\r
+[1] for hints how to add this to 'notmuch tag'.\r
+\r
+[1]\r
+id:"c560dc17781ea2e725cc75e00cb328822a65537f.1334404979.git.jani@nikula.org" \r
+or\r
+http://mid.gmane.org/c560dc17781ea2e725cc75e00cb328822a65537f.1334404979.git.jani@nikula.org\r
+\r
+>      if (argv[i][0] == '+' || argv[i][0] == '-') {\r
+>          tag_ops[tag_ops_count].tag = argv[i] + 1;\r
+>          tag_ops[tag_ops_count].remove = (argv[i][0] == '-');\r
+> @@ -229,7 +263,15 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])\r
+>      if (config == NULL)\r
+>      return 1;\r
+>  \r
+> -    if (notmuch_database_open (notmuch_config_get_database_path (config),\r
+> +    db_path = notmuch_config_get_database_path (config);\r
+> +\r
+> +    if (run_hooks) {\r
+> +    ret = notmuch_run_hook (db_path, "pre-tag");\r
+> +    if (ret)\r
+> +        return ret;\r
+> +    }\r
+> +\r
+> +    if (notmuch_database_open (db_path,\r
+>                             NOTMUCH_DATABASE_MODE_READ_WRITE, &notmuch))\r
+>      return 1;\r
+>  \r
+> @@ -239,5 +281,9 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])\r
+>  \r
+>      notmuch_database_destroy (notmuch);\r
+>  \r
+> +    if (!ret && run_hooks) {\r
+> +    ret = notmuch_run_hook (db_path, "post-tag");\r
+> +    }\r
+> +\r
+>      return ret;\r
+>  }\r
+> diff --git a/test/hooks b/test/hooks\r
+> index 77e8569..ae857cc 100755\r
+> --- a/test/hooks\r
+> +++ b/test/hooks\r
+> @@ -31,6 +31,7 @@ rm_hooks () {\r
+>  # add a message to generate mail dir and database\r
+>  add_message\r
+>  \r
+> +# {pre,post}-new hooks\r
+>  test_begin_subtest "pre-new is run"\r
+>  rm_hooks\r
+>  generate_message\r
+> @@ -101,4 +102,39 @@ EOF\r
+>  chmod +x "${HOOK_DIR}/pre-new"\r
+>  test_expect_code 1 "hook execution failure" "notmuch new"\r
+>  \r
+> +\r
+> +\r
+> +# {pre,post}-tag hooks\r
+> +test_begin_subtest "pre-tag is run"\r
+> +rm_hooks\r
+> +generate_message\r
+> +create_echo_hook "pre-tag" expected output\r
+> +notmuch tag +foo -- '*' > /dev/null\r
+> +test_expect_equal_file expected output\r
+> +\r
+> +test_begin_subtest "post-tag is run"\r
+> +rm_hooks\r
+> +generate_message\r
+> +create_echo_hook "post-tag" expected output\r
+> +notmuch tag +foo -- '*'  > /dev/null\r
+> +test_expect_equal_file expected output\r
+> +\r
+> +test_begin_subtest "pre-tag is run before post-new"\r
+> +rm_hooks\r
+> +generate_message\r
+> +create_echo_hook "pre-tag" pre-tag.expected pre-tag.output\r
+> +create_echo_hook "post-tag" post-tag.expected post-tag.output\r
+> +notmuch tag +foo -- '*'  > /dev/null\r
+> +test_expect_equal_file post-tag.expected post-tag.output\r
+> +\r
+> +test_begin_subtest "pre-tag non-zero exit status (hook status)"\r
+> +rm_hooks\r
+> +generate_message\r
+> +create_failing_hook "pre-tag"\r
+> +output=`notmuch tag +foo -- '*'  2>&1`\r
+> +test_expect_equal "$output" "Error: pre-tag hook failed with status 13"\r
+> +\r
+> +# depends on the previous subtest leaving broken hook behind\r
+> +test_expect_code 1 "pre-tag non-zero exit status (notmuch status)" "notmuch tag +foo -- '*'"\r
+> +\r
+>  test_done\r
+> diff --git a/test/tagging b/test/tagging\r
+> index e4782ed..5167f4f 100755\r
+> --- a/test/tagging\r
+> +++ b/test/tagging\r
+> @@ -46,4 +46,32 @@ test_expect_equal "$output" "\\r
+>  thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; One (:\"  inbox tag1 unread)\r
+>  thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag1 tag4 unread)"\r
+>  \r
+> +test_begin_subtest "Arguments mixed with tag ops"\r
+> +notmuch tag +-no-hooks --no-hooks -- One\r
+> +notmuch tag --no-hooks +-no-hooks -tag4 -- Two\r
+> +output=$(notmuch search \* | notmuch_search_sanitize)\r
+> +test_expect_equal "$output" "\\r
+> +thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; One (:\"  inbox tag1 unread)\r
+> +thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Two (-no-hooks inbox tag1 unread)"\r
+> +notmuch tag --no-hooks -- Two\r
+> +\r
+> +test_begin_subtest "Arguments with correct position"\r
+> +notmuch tag --no-hooks -- +tag4 -tag4 -- One\r
+> +output=$(notmuch search \* | notmuch_search_sanitize)\r
+> +test_expect_equal "$output" "\\r
+> +thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; One (:\"  inbox tag1 unread)\r
+> +thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag1 unread)"\r
+> +\r
+> +test_begin_subtest "Missing arguments"\r
+> +notmuch tag -- +tag4 -tag4 -- One\r
+> +output=$(notmuch search \* | notmuch_search_sanitize)\r
+> +test_expect_equal "$output" "\\r
+> +thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; One (:\"  inbox tag1 unread)\r
+> +thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag1 unread)"\r
+> +\r
+> +test_begin_subtest "Unknown argument"\r
+> +output=$(notmuch tag --no-blubb -- +tag4 -tag4 -- One 2>&1)\r
+> +test_expect_equal "$output" "\\r
+> +Error: 'notmuch tag' doesn't recognize argument '--no-blubb'."\r
+> +\r
+>  test_done\r
+> -- \r
+> 1.7.11.2\r
+>\r
+> _______________________________________________\r
+> notmuch mailing list\r
+> notmuch@notmuchmail.org\r
+> http://notmuchmail.org/mailman/listinfo/notmuch\r