Date: Mon, 30 Jul 2012 14:39:28 +0200
From: Dominik Peteler <dominik@with-h.at>
To: notmuch@notmuchmail.org
Subject: Re: [PATCHv2] cli: Hooks for tag-command
Message-ID: <20120730123928.GA749@fuckup.uni-wuerzburg.de>
References:
 <CAB+hUn_XHQQ9MDaVXvOMzfM8SdKCWUgnjNQvxJaQWWyc3H621Q@mail.gmail.com>
	<1342634970-12991-1-git-send-email-dominik@with-h.at>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha1;
	protocol="application/pgp-signature"; boundary="5vNYLRcllDrimb99"
Content-Disposition: inline
In-Reply-To: <1342634970-12991-1-git-send-email-dominik@with-h.at>
User-Agent: Mutt/1.5.21 (2011-07-01)
Precedence: list


--5vNYLRcllDrimb99
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

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 i=
t.
And do you have a opinion about my suggestion to pass the message-ids of re=
tagged mails to the hook ?

regards

dominik



On Wed 2012-07-18 20:09, Dominik Peteler wrote:
> hello,
>=20
> 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
>=20
> 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 ?
>=20
> regards
>=20
> dominik
>=20
>=20
>=20
> There are two hooks:
>  * pre-tag: Run before tagging
>  * post-tag: Run after
>=20
> 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(-)
>=20
> 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
> =20
>  .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 ">..."
> =20
>  .SH DESCRIPTION
> =20
> @@ -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.
> =20
> +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
> =20
>  \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 o=
n 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 the=
re have been any errors during
> +the tagging.
> +.RE
> +
> =20
>  .SH SEE ALSO
> =20
> 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 =3D 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_delimiter=
s[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] =3D {-1, -1};
> +    notmuch_bool_t run_hooks =3D TRUE;
>      int i;
>      int ret;
> =20
> @@ -197,11 +205,37 @@ notmuch_tag_command (void *ctx, int argc, char *arg=
v[])
>  	return 1;
>      }
> =20
> +    /* Determine position of delimiters */
>      for (i =3D 0; i < argc; i++) {
>  	if (strcmp (argv[i], "--") =3D=3D 0) {
> -	    i++;
> -	    break;
> +	    if (arg_delimiters[1] =3D=3D -1) {
> +		arg_delimiters[1] =3D i;
> +	    } else if (arg_delimiters[0] =3D=3D -1) {
> +		arg_delimiters[0] =3D arg_delimiters[1];
> +		arg_delimiters[1] =3D i;
> +	    } else {
> +		fprintf (stderr, "Error: 'notmuch tag' requires delimiter \"--\" at mo=
st two times.\n");
> +		return 1;
> +	    }
>  	}
> +    }
> +
> +    /* Process arguments if present */
> +    for (i =3D 0; i < arg_delimiters[0]; i++) {
> +	if (strcmp (argv[i], "--no-hooks") =3D=3D 0) {
> +	    run_hooks =3D 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] =3D=3D -1)
> +	    arg_delimiters[1] =3D argc;
> +
> +    /* Read tag ops */
> +    for (i =3D arg_delimiters[0]+1; i < arg_delimiters[1]; i++) {
>  	if (argv[i][0] =3D=3D '+' || argv[i][0] =3D=3D '-') {
>  	    tag_ops[tag_ops_count].tag =3D argv[i] + 1;
>  	    tag_ops[tag_ops_count].remove =3D (argv[i][0] =3D=3D '-');
> @@ -229,7 +263,15 @@ notmuch_tag_command (void *ctx, int argc, char *argv=
[])
>      if (config =3D=3D NULL)
>  	return 1;
> =20
> -    if (notmuch_database_open (notmuch_config_get_database_path (config),
> +    db_path =3D notmuch_config_get_database_path (config);
> +
> +    if (run_hooks) {
> +	ret =3D 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;
> =20
> @@ -239,5 +281,9 @@ notmuch_tag_command (void *ctx, int argc, char *argv[=
])
> =20
>      notmuch_database_destroy (notmuch);
> =20
> +    if (!ret && run_hooks) {
> +	ret =3D 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
> =20
> +# {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"
> =20
> +
> +
> +# {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=3D`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)" "notm=
uch 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 u=
nread)
>  thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag1 tag4 u=
nread)"
> =20
> +test_begin_subtest "Arguments mixed with tag ops"
> +notmuch tag +-no-hooks --no-hooks -- One
> +notmuch tag --no-hooks +-no-hooks -tag4 -- Two
> +output=3D$(notmuch search \* | notmuch_search_sanitize)
> +test_expect_equal "$output" "\
> +thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; One (:\"  inbox tag1 u=
nread)
> +thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Two (-no-hooks inbox t=
ag1 unread)"
> +notmuch tag --no-hooks -- Two
> +
> +test_begin_subtest "Arguments with correct position"
> +notmuch tag --no-hooks -- +tag4 -tag4 -- One
> +output=3D$(notmuch search \* | notmuch_search_sanitize)
> +test_expect_equal "$output" "\
> +thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; One (:\"  inbox tag1 u=
nread)
> +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=3D$(notmuch search \* | notmuch_search_sanitize)
> +test_expect_equal "$output" "\
> +thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; One (:\"  inbox tag1 u=
nread)
> +thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag1 unread=
)"
> +
> +test_begin_subtest "Unknown argument"
> +output=3D$(notmuch tag --no-blubb -- +tag4 -tag4 -- One 2>&1)
> +test_expect_equal "$output" "\
> +Error: 'notmuch tag' doesn't recognize argument '--no-blubb'."
> +
>  test_done
> --=20
> 1.7.11.2
>=20

--5vNYLRcllDrimb99
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)

iQIcBAEBAgAGBQJQFoCAAAoJEFxmf0CweVkTFrkQAJtoYSf5F5C4+sRjc4QAqzZN
Sa9ZUgDpacVhx5jcPgEwJpyAmTIoBl70jpZtUyz/r2/jb9ofcaRhRWi+SUhZUSDi
cA0lSq4W9HoRvz0AiuggdgzE0ydungL+CJsrWwQZrVYnXMcpoVf+NIKCnkGU2dZo
I7fGRWeSklfGysKehsKHX3G8UUzvicnr7EBd7gWN5+x9PE6RJZXNR/cx9YiRS03s
n1kvfEPyyK3J8l2Gz42Vh16/RmFPAFc07/aIpylOp8l4qBz6WDWQy7gGFDjwFscU
P4NlPC0fugCLawnF4z+H/5zffhLN9BL5LvsQ3p9AZ0PLLlmfVuke54y/g07PuIOO
a4HyIeyhHsXdJcJYut7xFxzFN8kt89yfXtLVUSRO6OOAOwcGF8UEhdf6UYGKn3wZ
zLMkF+vXmkDIKq7RfwvNX++pR/NXU14zDfZ5s1ywuZ0tBmmiqjfVa8hh1zh3HrdB
PQGh2AnSFrZRCCcvXsmVVvCsybC6fKrldrNRB/q7lNyn6W8TyeKZObGu0cshHEi0
KKNl5olunoiKUN2cSc8uoa9Kc1BFek91SLz0+gdOlV3FKlEGGBPnmihDanDFZATg
07qw8XvcdK3/pSgNVVGRc6Lcr+xyYLJ1dC0dBB+jDQBlE4dj0guChF93IEa/MXTv
Od3M+3RH8oSHRGMUvL1a
=eWDC
-----END PGP SIGNATURE-----

--5vNYLRcllDrimb99--