1f/5a8986d5c7a7161efcb7defbcee352fa9089ad

   1 Return-Path: <jani@nikula.org>\r
   2 X-Original-To: notmuch@notmuchmail.org\r
   3 Delivered-To: notmuch@notmuchmail.org\r
   4 Received: from localhost (localhost [127.0.0.1])\r
   5         by olra.theworths.org (Postfix) with ESMTP id D13FE429E25\r
   6         for <notmuch@notmuchmail.org>; Sun,  4 Dec 2011 11:36:28 -0800 (PST)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: -0.7\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5\r
  12         tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\r
  13 Received: from olra.theworths.org ([127.0.0.1])\r
  14         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  15         with ESMTP id L+kP6ntp60sE for <notmuch@notmuchmail.org>;\r
  16         Sun,  4 Dec 2011 11:36:28 -0800 (PST)\r
  17 Received: from mail-ww0-f45.google.com (mail-ww0-f45.google.com\r
  18  [74.125.82.45])        (using TLSv1 with cipher RC4-SHA (128/128 bits))        (No client\r
  19  certificate requested) by olra.theworths.org (Postfix) with ESMTPS id\r
  20  A35B8429E21    for <notmuch@notmuchmail.org>; Sun,  4 Dec 2011 11:36:27 -0800\r
  21  (PST)\r
  22 Received: by wgbds13 with SMTP id ds13so4711419wgb.2\r
  23         for <notmuch@notmuchmail.org>; Sun, 04 Dec 2011 11:36:25 -0800 (PST)\r
  24 Received: by 10.216.182.193 with SMTP id o43mr1837324wem.87.1323027384882;\r
  25         Sun, 04 Dec 2011 11:36:24 -0800 (PST)\r
  26 Received: from localhost (dsl-hkibrasgw4-fe5cdc00-23.dhcp.inet.fi.\r
  27         [80.220.92.23])\r
  28         by mx.google.com with ESMTPS id 6sm23853781wby.22.2011.12.04.11.36.22\r
  29         (version=SSLv3 cipher=OTHER); Sun, 04 Dec 2011 11:36:23 -0800 (PST)\r
  30 From: Jani Nikula <jani@nikula.org>\r
  31 To: Austin Clements <amdragon@MIT.EDU>\r
  32 Subject: Re: [PATCH v2 2/2] cli: add support for pre and post notmuch new\r
  33  hooks\r
  34 In-Reply-To: <20111204040047.GB16405@mit.edu>\r
  35 References:\r
  36  <6688b09fffa2a66b496af78008102f88ab4e9450.1322953841.git.jani@nikula.org>\r
  37         <6ccaa31da55b0dfc9e339780e43e24e1489235e8.1322953841.git.jani@nikula.org>\r
  38         <20111204040047.GB16405@mit.edu>\r
  39 User-Agent: Notmuch/0.10+59~g7f77e5e (http://notmuchmail.org) Emacs/23.3.1\r
  40         (i686-pc-linux-gnu)\r
  41 Date: Sun, 04 Dec 2011 21:36:21 +0200\r
  42 Message-ID: <87pqg4ku2y.fsf@nikula.org>\r
  43 MIME-Version: 1.0\r
  44 Content-Type: text/plain; charset=us-ascii\r
  45 Cc: notmuch@notmuchmail.org\r
  46 X-BeenThere: notmuch@notmuchmail.org\r
  47 X-Mailman-Version: 2.1.13\r
  48 Precedence: list\r
  49 List-Id: "Use and development of the notmuch mail system."\r
  50         <notmuch.notmuchmail.org>\r
  51 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  52         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  53 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  54 List-Post: <mailto:notmuch@notmuchmail.org>\r
  55 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  56 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  57         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  58 X-List-Received-Date: Sun, 04 Dec 2011 19:36:29 -0000\r
  59 \r
  60 On Sat, 3 Dec 2011 23:00:47 -0500, Austin Clements <amdragon@MIT.EDU> wrote:\r
  61 > Quoth Jani Nikula on Dec 04 at  1:16 am:\r
  62 > > Run notmuch new pre and post hooks, named "pre-new" and "post-new", if\r
  63 > > present in the notmuch hooks directory. The hooks will be run before and\r
  64 > > after incorporating new messages to the database.\r
  65 > > \r
  66 > > Typical use cases for pre-new and post-new hooks are fetching or delivering\r
  67 > > new mail to the maildir, and custom tagging of the mail incorporated to the\r
  68 > > database.\r
  69 > > \r
  70 > > Also add command line option --no-hooks to notmuch new to bypass the hooks.\r
  71 > > \r
  72 > > Signed-off-by: Jani Nikula <jani@nikula.org>\r
  73 > > ---\r
  74 > >  notmuch-new.c |   12 ++++++++++++\r
  75 > >  notmuch.1     |   50 +++++++++++++++++++++++++++++++++++++++++++++++++-\r
  76 > >  2 files changed, 61 insertions(+), 1 deletions(-)\r
  77 > > \r
  78 > > diff --git a/notmuch-new.c b/notmuch-new.c\r
  79 > > index 81a9350..27dde0c 100644\r
  80 > > --- a/notmuch-new.c\r
  81 > > +++ b/notmuch-new.c\r
  82 > > @@ -811,6 +811,7 @@ notmuch_new_command (void *ctx, int argc, char *argv[])\r
  83 > >      _filename_node_t *f;\r
  84 > >      int i;\r
  85 > >      notmuch_bool_t timer_is_active = FALSE;\r
  86 > > +    int run_hooks = 1;\r
  87 > \r
  88 > notmuch_bool_t?\r
  89 \r
  90 Yes.\r
  91 \r
  92 > >      add_files_state.verbose = 0;\r
  93 > >      add_files_state.output_is_a_tty = isatty (fileno (stdout));\r
  94 > > @@ -820,6 +821,8 @@ notmuch_new_command (void *ctx, int argc, char *argv[])\r
  95 > >      for (i = 0; i < argc && argv[i][0] == '-'; i++) {\r
  96 > >     if (STRNCMP_LITERAL (argv[i], "--verbose") == 0) {\r
  97 > >         add_files_state.verbose = 1;\r
  98 > > +   } else if (STRNCMP_LITERAL (argv[i], "--no-hooks") == 0) {\r
  99 > \r
 100 > I see this mistake all over notmuch, so maybe it's better to\r
 101 > perpetuate it here and fix it everywhere in another patch, but this\r
 102 > should be strcmp, not STRNCMP_LITERAL.  STRNCMP_LITERAL is the right\r
 103 > thing for options that take values, but for boolean options like this,\r
 104 > it will accept\r
 105 >   notmuch new --no-hooks-just-kidding\r
 106 \r
 107 Oops. I just took it from the --verbose handling above without\r
 108 checking. I'll fix this one, as I don't think everyone else making the\r
 109 same mistake is a good reason to repeat it. The rest will be taken care\r
 110 of in the Great Argument Parsing Overhaul which is in the works...\r
 111 \r
 112 > > +       run_hooks = 0;\r
 113 > >     } else {\r
 114 > >         fprintf (stderr, "Unrecognized option: %s\n", argv[i]);\r
 115 > >         return 1;\r
 116 > > @@ -833,6 +836,12 @@ notmuch_new_command (void *ctx, int argc, char *argv[])\r
 117 > >      add_files_state.synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config);\r
 118 > >      db_path = notmuch_config_get_database_path (config);\r
 119 > >  \r
 120 > > +    if (run_hooks) {\r
 121 > > +   ret = notmuch_run_hook (db_path, "pre-new");\r
 122 > > +   if (ret)\r
 123 > > +       return ret;\r
 124 > > +    }\r
 125 > > +\r
 126 > >      dot_notmuch_path = talloc_asprintf (ctx, "%s/%s", db_path, ".notmuch");\r
 127 > >  \r
 128 > >      if (stat (dot_notmuch_path, &st)) {\r
 129 > > @@ -981,5 +990,8 @@ notmuch_new_command (void *ctx, int argc, char *argv[])\r
 130 > >  \r
 131 > >      notmuch_database_close (notmuch);\r
 132 > >  \r
 133 > > +    if (run_hooks && !ret && !interrupted)\r
 134 > > +   ret = notmuch_run_hook (db_path, "post-new");\r
 135 > \r
 136 > Does it matter at this point if the hook fails?  I'm not sure.\r
 137 \r
 138 I wasn't sure either, but I ended up thinking that the hooks become part\r
 139 of 'notmuch new' and claiming success when a hook fails is not quite\r
 140 right. This might have importance if scripting 'notmuch new'.\r
 141 \r
 142 > > +\r
 143 > >      return ret || interrupted;\r
 144 > >  }\r
 145 > > diff --git a/notmuch.1 b/notmuch.1\r
 146 > > index 92931d7..66f82e9 100644\r
 147 > > --- a/notmuch.1\r
 148 > > +++ b/notmuch.1\r
 149 > \r
 150 > I am willfully ignorant of nroff, so somebody else will have to\r
 151 > comment if any of the nroff code/formatting is wrong.\r
 152 \r
 153 That makes two of us; I shamelessly admit I'm just following what\r
 154 everyone else seems to be doing... cargo cult.\r
 155 \r
 156 > > @@ -85,7 +85,7 @@ The\r
 157 > >  command is used to incorporate new mail into the notmuch database.\r
 158 > >  .RS 4\r
 159 > >  .TP 4\r
 160 > > -.B new\r
 161 > > +.BR new " [options...]"\r
 162 > >  \r
 163 > >  Find and import any new messages to the database.\r
 164 > >  \r
 165 > > @@ -118,6 +118,22 @@ if\r
 166 > >  has previously been completed, but\r
 167 > >  .B "notmuch new"\r
 168 > >  has not previously been run.\r
 169 > > +\r
 170 > > +The\r
 171 > > +.B new\r
 172 > > +command supports hooks. See the\r
 173 > > +.B "HOOKS"\r
 174 > > +section below for more details on hooks.\r
 175 > > +\r
 176 > > +Supported options for\r
 177 > > +.B new\r
 178 > > +include\r
 179 > > +.RS 4\r
 180 > > +.TP 4\r
 181 > > +.BR \-\-no\-hooks\r
 182 > > +\r
 183 > > +Prevents hooks from being run.\r
 184 > > +.RE\r
 185 > >  .RE\r
 186 > >  \r
 187 > >  Several of the notmuch commands accept search terms with a common\r
 188 > > @@ -705,6 +721,38 @@ specify a date range to return messages from 2009\-10\-01 until the\r
 189 > >  current time:\r
 190 > >  \r
 191 > >     $(date +%s \-d 2009\-10\-01)..$(date +%s)\r
 192 > > +.SH HOOKS\r
 193 > > +Hooks are scripts (or arbitrary executables or symlinks to such) you can place\r
 194 > > +in the notmuch hooks directory to trigger action at certain points. The hooks\r
 195 > > +directory is .notmuch/hooks within the database directory. The user must have\r
 196 > > +executable permission set on the scripts.\r
 197 > \r
 198 > Could be more concise.  Maybe something like "Hooks are scripts (or\r
 199 > arbitrary executables or symlinks to such) that notmuch invokes before\r
 200 > and after certain actions.  These scripts reside in the .notmuch/hooks\r
 201 > directory within the database directory and must have executable\r
 202 > permissions."\r
 203 \r
 204 Better, thanks.\r
 205 \r
 206 > > +\r
 207 > > +The currently available hooks are described below.\r
 208 > > +.RS 4\r
 209 > > +.TP 4\r
 210 > > +.B pre\-new\r
 211 > > +This hook is invoked by the\r
 212 > > +.B new\r
 213 > > +command before scanning or importing new messages into the database. Any errors\r
 214 > > +in running the hook will abort further processing of the\r
 215 > \r
 216 > "If this script exits with a non-zero status, notmuch will abort ..."?\r
 217 \r
 218 Yeah, I was trying to cover also the errors that may happen before the\r
 219 script is actually run, but perhaps that's not important.\r
 220 \r
 221 > > +.B new\r
 222 > > +command.\r
 223 > > +\r
 224 > > +Typical use case for this hook is fetching or delivering new mail to be imported\r
 225 > > +into the database.\r
 226 > \r
 227 > Perhaps "Typically this hook is used for ..."?\r
 228 \r
 229 Not being a native speaker, I'll take your word for it. :)\r
 230 \r
 231 > > +.RE\r
 232 > > +.RS 4\r
 233 > > +.TP 4\r
 234 > > +.B post\-new\r
 235 > > +This hook is invoked by the\r
 236 > > +.B new\r
 237 > > +command after new messages have been imported into the database and initial tags\r
 238 > > +have been applied. The hook will not be run if there have been any errors during\r
 239 > > +the scan or import.\r
 240 > > +\r
 241 > > +Typical use case for this hook is performing additional query based tagging on\r
 242 > > +the imported messages.\r
 243 > \r
 244 > Same thing.  "Typically this hook is used to perform ..."?  Also,\r
 245 > "query-based".\r
 246 \r
 247 Ditto.\r
 248 \r
 249 > \r
 250 > > +.RE\r
 251 > >  .SH ENVIRONMENT\r
 252 > >  The following environment variables can be used to control the\r
 253 > >  behavior of notmuch.\r
 254 \r
 255 Many thanks for your thorough review, as always!\r
 256 \r
 257 \r
 258 BR,\r
 259 Jani.\r