hello,

I hope I don't bother you but so far I didn't get any reply to my last mail with the patch for tag-command hooks.
Did you accept the patch or not ? If not, please tell my if I can improve it.
And do you have a opinion about my suggestion to pass the message-ids of retagged mails to the hook ?

regards

dominik



On Wed 2012-07-18 20:09, Dominik Peteler 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 ?
> 
> regards
> 
> dominik
> 
> 
> 
> 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.
> ---
>  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++) {
>  	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, &notmuch))
>  	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
>