[PATCHv2] cli: Hooks for tag-command
Jani Nikula
jani at nikula.org
Tue Jul 31 06:19:04 PDT 2012
On Wed, 18 Jul 2012, Dominik Peteler <dominik at with-h.at> wrote:
> hello,
>
> I improved my patch according to Janis mail:
> * new cli syntax: notmuch tag [ --no-hooks ] -- <tag ops> [ -- ] <search terms>
> * adjusted man pages and wrote tests
>
> I had the idea to improve this feature by passing the message-ids or the filename to the hooks.
> What's your opinion about that ? Any suggestions ?
There are probably races there that are difficult to solve. Imagine two
concurrent 'notmuch tag' invocations. The db can't be locked across the
hooks. Also, if you relied solely on the passed message-ids or filenames
to do whatever your hook would do, interrupting 'notmuch tag' before the
hook is run would leave you in an inconsistent state.
>
> regards
>
> dominik
Note that this part of the patch ends up as the commit message in the
git repository. Please have a look at
http://notmuchmail.org/patchformatting/.
>
>
>
> There are two hooks:
> * pre-tag: Run before tagging
> * post-tag: Run after
>
> This allows users to react on changes of tags. For example,
> you might want to move a message to a special Maildir
> depending on its notmuch tags.
I am not sure if the example is such a hot idea. Moving a file in the
maildir means you'd need to run 'notmuch new' again to inform notmuch of
the change. I.e. after tagging you couldn't view the message before
you've run 'notmuch new'.
Are there any other, perhaps more compelling reasons to have hooks in
'notmuch tag'? Can you give us some other examples, please?
> ---
> man/man1/notmuch-tag.1 | 22 +++++++++++++++++++-
> man/man5/notmuch-hooks.5 | 19 ++++++++++++++++++
> notmuch-tag.c | 52 +++++++++++++++++++++++++++++++++++++++++++++---
> test/hooks | 36 +++++++++++++++++++++++++++++++++
> test/tagging | 28 ++++++++++++++++++++++++++
> 5 files changed, 153 insertions(+), 4 deletions(-)
>
> diff --git a/man/man1/notmuch-tag.1 b/man/man1/notmuch-tag.1
> index d810e1b..e00e189 100644
> --- a/man/man1/notmuch-tag.1
> +++ b/man/man1/notmuch-tag.1
> @@ -4,7 +4,11 @@ notmuch-tag \- add/remove tags for all messages matching the search terms
>
> .SH SYNOPSIS
> .B notmuch tag
> -.RI "+<" tag> "|\-<" tag "> [...] [\-\-] <" search-term ">..."
> +.RI "+<" tag "> |\-<" tag "> [...] [\-\-] <" search-term ">..."
> +
> +.B notmuch tag
> +.RB "[" --no-hooks "]"
> +.RI "\-\- +<" tag "> |\-<" tag "> [...] \-\- <" search-term ">..."
>
> .SH DESCRIPTION
>
> @@ -29,6 +33,22 @@ updates the maildir flags according to tag changes if the
> configuration option is enabled. See \fBnotmuch-config\fR(1) for
> details.
>
> +The
> +.B tag
> +command supports hooks. See \fBnotmuch-hooks(5)\fR
> +for more details on hooks.
> +
> +Supported options for
> +.B tag
> +include
> +.RS 4
> +.TP 4
> +.BR \-\-no\-hooks
> +
> +Prevents hooks from being run.
> +.RE
> +.RE
> +
> .SH SEE ALSO
>
> \fBnotmuch\fR(1), \fBnotmuch-config\fR(1), \fBnotmuch-count\fR(1),
> diff --git a/man/man5/notmuch-hooks.5 b/man/man5/notmuch-hooks.5
> index b914a29..e193ef5 100644
> --- a/man/man5/notmuch-hooks.5
> +++ b/man/man5/notmuch-hooks.5
> @@ -38,6 +38,25 @@ the scan or import.
> Typically this hook is used to perform additional query\-based tagging on the
> imported messages.
> .RE
> +.RS 4
> +.TP 4
> +.B pre\-tag
> +This hook is invoked by the
> +.B tag
> +command before tagging messages. If this
> +hook exits with a non-zero status, notmuch will abort further processing of the
> +.B tag
> +command.
> +.RE
> +.RS 4
> +.TP 4
> +.B post\-tag
> +This hook is invoked by the
> +.B tag
> +command after messages have been tagged. The hook will not be run if there have been any errors during
> +the tagging.
> +.RE
> +
>
> .SH SEE ALSO
>
> diff --git a/notmuch-tag.c b/notmuch-tag.c
> index 7d18639..7572059 100644
> --- a/notmuch-tag.c
> +++ b/notmuch-tag.c
> @@ -174,9 +174,17 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
> int tag_ops_count = 0;
> char *query_string;
> notmuch_config_t *config;
> + const char *db_path;
> notmuch_database_t *notmuch;
> struct sigaction action;
> notmuch_bool_t synchronize_flags;
> + /* Points to the position of the "--" delimiters, e. g.
> + * <optional arguments> arg_delimiters[0] <tag ops> arg_delimiters[1] <search terms>
> + *
> + * arg_delimiters[0] may remain -1 if there are no arguments given
> + * arg_delimiters[0] may remain -1 if there is no delimiter between tag ops and search terms */
> + int arg_delimiters[2] = {-1, -1};
> + notmuch_bool_t run_hooks = TRUE;
> int i;
> int ret;
>
> @@ -197,11 +205,37 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
> return 1;
> }
>
> + /* Determine position of delimiters */
> for (i = 0; i < argc; i++) {
> if (strcmp (argv[i], "--") == 0) {
> - i++;
> - break;
> + if (arg_delimiters[1] == -1) {
> + arg_delimiters[1] = i;
> + } else if (arg_delimiters[0] == -1) {
> + arg_delimiters[0] = arg_delimiters[1];
> + arg_delimiters[1] = i;
> + } else {
> + fprintf (stderr, "Error: 'notmuch tag' requires delimiter \"--\" at most two times.\n");
> + return 1;
> + }
> }
> + }
> +
> + /* Process arguments if present */
> + for (i = 0; i < arg_delimiters[0]; i++) {
> + if (strcmp (argv[i], "--no-hooks") == 0) {
> + run_hooks = FALSE;
> + } else {
> + fprintf (stderr, "Error: 'notmuch tag' doesn't recognize argument '%s'.\n", argv[i]);
> + return 1;
> + }
> + }
> +
> + /* Set arg_delimiters[1] to argc if no delimiters at all are present */
> + if (arg_delimiters[1] == -1)
> + arg_delimiters[1] = argc;
> +
> + /* Read tag ops */
> + for (i = arg_delimiters[0]+1; i < arg_delimiters[1]; i++) {
As I said before, you should parse the arguments using the notmuch
argument parser. The above is getting needlessly complicated. Please see
the other notmuch commands for examples, and see the relevant part of
[1] for hints how to add this to 'notmuch tag'.
[1]
id:"c560dc17781ea2e725cc75e00cb328822a65537f.1334404979.git.jani at nikula.org"
or
http://mid.gmane.org/c560dc17781ea2e725cc75e00cb328822a65537f.1334404979.git.jani@nikula.org
> if (argv[i][0] == '+' || argv[i][0] == '-') {
> tag_ops[tag_ops_count].tag = argv[i] + 1;
> tag_ops[tag_ops_count].remove = (argv[i][0] == '-');
> @@ -229,7 +263,15 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
> if (config == NULL)
> return 1;
>
> - if (notmuch_database_open (notmuch_config_get_database_path (config),
> + db_path = notmuch_config_get_database_path (config);
> +
> + if (run_hooks) {
> + ret = notmuch_run_hook (db_path, "pre-tag");
> + if (ret)
> + return ret;
> + }
> +
> + if (notmuch_database_open (db_path,
> NOTMUCH_DATABASE_MODE_READ_WRITE, ¬much))
> return 1;
>
> @@ -239,5 +281,9 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
>
> notmuch_database_destroy (notmuch);
>
> + if (!ret && run_hooks) {
> + ret = notmuch_run_hook (db_path, "post-tag");
> + }
> +
> return ret;
> }
> diff --git a/test/hooks b/test/hooks
> index 77e8569..ae857cc 100755
> --- a/test/hooks
> +++ b/test/hooks
> @@ -31,6 +31,7 @@ rm_hooks () {
> # add a message to generate mail dir and database
> add_message
>
> +# {pre,post}-new hooks
> test_begin_subtest "pre-new is run"
> rm_hooks
> generate_message
> @@ -101,4 +102,39 @@ EOF
> chmod +x "${HOOK_DIR}/pre-new"
> test_expect_code 1 "hook execution failure" "notmuch new"
>
> +
> +
> +# {pre,post}-tag hooks
> +test_begin_subtest "pre-tag is run"
> +rm_hooks
> +generate_message
> +create_echo_hook "pre-tag" expected output
> +notmuch tag +foo -- '*' > /dev/null
> +test_expect_equal_file expected output
> +
> +test_begin_subtest "post-tag is run"
> +rm_hooks
> +generate_message
> +create_echo_hook "post-tag" expected output
> +notmuch tag +foo -- '*' > /dev/null
> +test_expect_equal_file expected output
> +
> +test_begin_subtest "pre-tag is run before post-new"
> +rm_hooks
> +generate_message
> +create_echo_hook "pre-tag" pre-tag.expected pre-tag.output
> +create_echo_hook "post-tag" post-tag.expected post-tag.output
> +notmuch tag +foo -- '*' > /dev/null
> +test_expect_equal_file post-tag.expected post-tag.output
> +
> +test_begin_subtest "pre-tag non-zero exit status (hook status)"
> +rm_hooks
> +generate_message
> +create_failing_hook "pre-tag"
> +output=`notmuch tag +foo -- '*' 2>&1`
> +test_expect_equal "$output" "Error: pre-tag hook failed with status 13"
> +
> +# depends on the previous subtest leaving broken hook behind
> +test_expect_code 1 "pre-tag non-zero exit status (notmuch status)" "notmuch tag +foo -- '*'"
> +
> test_done
> diff --git a/test/tagging b/test/tagging
> index e4782ed..5167f4f 100755
> --- a/test/tagging
> +++ b/test/tagging
> @@ -46,4 +46,32 @@ test_expect_equal "$output" "\
> thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; One (:\" inbox tag1 unread)
> thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag1 tag4 unread)"
>
> +test_begin_subtest "Arguments mixed with tag ops"
> +notmuch tag +-no-hooks --no-hooks -- One
> +notmuch tag --no-hooks +-no-hooks -tag4 -- Two
> +output=$(notmuch search \* | notmuch_search_sanitize)
> +test_expect_equal "$output" "\
> +thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; One (:\" inbox tag1 unread)
> +thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; Two (-no-hooks inbox tag1 unread)"
> +notmuch tag --no-hooks -- Two
> +
> +test_begin_subtest "Arguments with correct position"
> +notmuch tag --no-hooks -- +tag4 -tag4 -- One
> +output=$(notmuch search \* | notmuch_search_sanitize)
> +test_expect_equal "$output" "\
> +thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; One (:\" inbox tag1 unread)
> +thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag1 unread)"
> +
> +test_begin_subtest "Missing arguments"
> +notmuch tag -- +tag4 -tag4 -- One
> +output=$(notmuch search \* | notmuch_search_sanitize)
> +test_expect_equal "$output" "\
> +thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; One (:\" inbox tag1 unread)
> +thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag1 unread)"
> +
> +test_begin_subtest "Unknown argument"
> +output=$(notmuch tag --no-blubb -- +tag4 -tag4 -- One 2>&1)
> +test_expect_equal "$output" "\
> +Error: 'notmuch tag' doesn't recognize argument '--no-blubb'."
> +
> test_done
> --
> 1.7.11.2
>
> _______________________________________________
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
More information about the notmuch
mailing list