[PATCH v2 2/2] cli: add support for pre and post notmuch new hooks
Jani Nikula
jani at nikula.org
Sun Dec 4 11:36:21 PST 2011
On Sat, 3 Dec 2011 23:00:47 -0500, Austin Clements <amdragon at MIT.EDU> wrote:
> Quoth Jani Nikula on Dec 04 at 1:16 am:
> > Run notmuch new pre and post hooks, named "pre-new" and "post-new", if
> > present in the notmuch hooks directory. The hooks will be run before and
> > after incorporating new messages to the database.
> >
> > Typical use cases for pre-new and post-new hooks are fetching or delivering
> > new mail to the maildir, and custom tagging of the mail incorporated to the
> > database.
> >
> > Also add command line option --no-hooks to notmuch new to bypass the hooks.
> >
> > Signed-off-by: Jani Nikula <jani at nikula.org>
> > ---
> > notmuch-new.c | 12 ++++++++++++
> > notmuch.1 | 50 +++++++++++++++++++++++++++++++++++++++++++++++++-
> > 2 files changed, 61 insertions(+), 1 deletions(-)
> >
> > diff --git a/notmuch-new.c b/notmuch-new.c
> > index 81a9350..27dde0c 100644
> > --- a/notmuch-new.c
> > +++ b/notmuch-new.c
> > @@ -811,6 +811,7 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
> > _filename_node_t *f;
> > int i;
> > notmuch_bool_t timer_is_active = FALSE;
> > + int run_hooks = 1;
>
> notmuch_bool_t?
Yes.
> > add_files_state.verbose = 0;
> > add_files_state.output_is_a_tty = isatty (fileno (stdout));
> > @@ -820,6 +821,8 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
> > for (i = 0; i < argc && argv[i][0] == '-'; i++) {
> > if (STRNCMP_LITERAL (argv[i], "--verbose") == 0) {
> > add_files_state.verbose = 1;
> > + } else if (STRNCMP_LITERAL (argv[i], "--no-hooks") == 0) {
>
> I see this mistake all over notmuch, so maybe it's better to
> perpetuate it here and fix it everywhere in another patch, but this
> should be strcmp, not STRNCMP_LITERAL. STRNCMP_LITERAL is the right
> thing for options that take values, but for boolean options like this,
> it will accept
> notmuch new --no-hooks-just-kidding
Oops. I just took it from the --verbose handling above without
checking. I'll fix this one, as I don't think everyone else making the
same mistake is a good reason to repeat it. The rest will be taken care
of in the Great Argument Parsing Overhaul which is in the works...
> > + run_hooks = 0;
> > } else {
> > fprintf (stderr, "Unrecognized option: %s\n", argv[i]);
> > return 1;
> > @@ -833,6 +836,12 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
> > add_files_state.synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config);
> > db_path = notmuch_config_get_database_path (config);
> >
> > + if (run_hooks) {
> > + ret = notmuch_run_hook (db_path, "pre-new");
> > + if (ret)
> > + return ret;
> > + }
> > +
> > dot_notmuch_path = talloc_asprintf (ctx, "%s/%s", db_path, ".notmuch");
> >
> > if (stat (dot_notmuch_path, &st)) {
> > @@ -981,5 +990,8 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
> >
> > notmuch_database_close (notmuch);
> >
> > + if (run_hooks && !ret && !interrupted)
> > + ret = notmuch_run_hook (db_path, "post-new");
>
> Does it matter at this point if the hook fails? I'm not sure.
I wasn't sure either, but I ended up thinking that the hooks become part
of 'notmuch new' and claiming success when a hook fails is not quite
right. This might have importance if scripting 'notmuch new'.
> > +
> > return ret || interrupted;
> > }
> > diff --git a/notmuch.1 b/notmuch.1
> > index 92931d7..66f82e9 100644
> > --- a/notmuch.1
> > +++ b/notmuch.1
>
> I am willfully ignorant of nroff, so somebody else will have to
> comment if any of the nroff code/formatting is wrong.
That makes two of us; I shamelessly admit I'm just following what
everyone else seems to be doing... cargo cult.
> > @@ -85,7 +85,7 @@ The
> > command is used to incorporate new mail into the notmuch database.
> > .RS 4
> > .TP 4
> > -.B new
> > +.BR new " [options...]"
> >
> > Find and import any new messages to the database.
> >
> > @@ -118,6 +118,22 @@ if
> > has previously been completed, but
> > .B "notmuch new"
> > has not previously been run.
> > +
> > +The
> > +.B new
> > +command supports hooks. See the
> > +.B "HOOKS"
> > +section below for more details on hooks.
> > +
> > +Supported options for
> > +.B new
> > +include
> > +.RS 4
> > +.TP 4
> > +.BR \-\-no\-hooks
> > +
> > +Prevents hooks from being run.
> > +.RE
> > .RE
> >
> > Several of the notmuch commands accept search terms with a common
> > @@ -705,6 +721,38 @@ specify a date range to return messages from 2009\-10\-01 until the
> > current time:
> >
> > $(date +%s \-d 2009\-10\-01)..$(date +%s)
> > +.SH HOOKS
> > +Hooks are scripts (or arbitrary executables or symlinks to such) you can place
> > +in the notmuch hooks directory to trigger action at certain points. The hooks
> > +directory is .notmuch/hooks within the database directory. The user must have
> > +executable permission set on the scripts.
>
> Could be more concise. Maybe something like "Hooks are scripts (or
> arbitrary executables or symlinks to such) that notmuch invokes before
> and after certain actions. These scripts reside in the .notmuch/hooks
> directory within the database directory and must have executable
> permissions."
Better, thanks.
> > +
> > +The currently available hooks are described below.
> > +.RS 4
> > +.TP 4
> > +.B pre\-new
> > +This hook is invoked by the
> > +.B new
> > +command before scanning or importing new messages into the database. Any errors
> > +in running the hook will abort further processing of the
>
> "If this script exits with a non-zero status, notmuch will abort ..."?
Yeah, I was trying to cover also the errors that may happen before the
script is actually run, but perhaps that's not important.
> > +.B new
> > +command.
> > +
> > +Typical use case for this hook is fetching or delivering new mail to be imported
> > +into the database.
>
> Perhaps "Typically this hook is used for ..."?
Not being a native speaker, I'll take your word for it. :)
> > +.RE
> > +.RS 4
> > +.TP 4
> > +.B post\-new
> > +This hook is invoked by the
> > +.B new
> > +command after new messages have been imported into the database and initial tags
> > +have been applied. The hook will not be run if there have been any errors during
> > +the scan or import.
> > +
> > +Typical use case for this hook is performing additional query based tagging on
> > +the imported messages.
>
> Same thing. "Typically this hook is used to perform ..."? Also,
> "query-based".
Ditto.
>
> > +.RE
> > .SH ENVIRONMENT
> > The following environment variables can be used to control the
> > behavior of notmuch.
Many thanks for your thorough review, as always!
BR,
Jani.
More information about the notmuch
mailing list