Re: [PATCH] cli: try to run external notmuch- prefixed commands as subcommands

Subject: Re: [PATCH] cli: try to run external notmuch- prefixed commands as subcommands

Date: Sun, 23 Oct 2016 04:22:08 +0100

To: Jani Nikula, notmuch@notmuchmail.org

Cc:

From: Mark Walters


On Sat, 22 Oct 2016, Jani Nikula <jani@nikula.org> wrote:
> If the given subcommand is not known to notmuch, try to execute
> external notmuch-<subcommand> instead. This allows users to have their
> own notmuch related tools be run via the notmuch command, not unlike
> git does. Also notmuch-emacs-mua will be executable via 'notmuch
> emacs-mua'.
>
> By design, this does not allow notmuch's own subcommands to be
> overriden using external commands.
>
> ---
>
> Whether internal subcommands can be overridden or not is up to debate,
> can be consider either a bug or a feature depending on how you look at
> it.

This looks good to me +1 (note my C is not great, and I haven't tested
yet). It took me a while to realise that a nice feature of this was that
we can have some commands in languages other than C, like the emacs-mua
command in the next patch.

My only query is whether we are setting ourselves up to annoy people if
we add official commands later. For example if we add a notmuch delete
command then it could break a user's setup if they have a notmuch-delete
command. Would it be worth saying that we guarantee some namespace such
as local-* (so commands called notmuch-local-*) won't be touched?

Best wishes

Mark




> ---
>  notmuch.c | 39 +++++++++++++++++++++++++++++++++++++--
>  1 file changed, 37 insertions(+), 2 deletions(-)
>
> diff --git a/notmuch.c b/notmuch.c
> index 38b73c1d2acc..b9c320329dd5 100644
> --- a/notmuch.c
> +++ b/notmuch.c
> @@ -363,6 +363,39 @@ notmuch_command (notmuch_config_t *config,
>      return EXIT_SUCCESS;
>  }
>  
> +/*
> + * Try to run subcommand in argv[0] as notmuch- prefixed external
> + * command. argv must be NULL terminated (argv passed to main always
> + * is).
> + *
> + * Does not return if the external command is found and
> + * executed. Return TRUE if external command is not found. Return
> + * FALSE on errors.
> + */
> +static notmuch_bool_t try_external_command(char *argv[])
> +{
> +    char *old_argv0 = argv[0];
> +    notmuch_bool_t ret = TRUE;
> +
> +    argv[0] = talloc_asprintf (NULL, "notmuch-%s", old_argv0);
> +
> +    /*
> +     * This will only return on errors. Not finding an external
> +     * command (ENOENT) is not an error from our perspective.
> +     */
> +    execvp (argv[0], argv);
> +    if (errno != ENOENT) {
> +	fprintf (stderr, "Error: Running external command '%s' failed: %s\n",
> +		 argv[0], strerror(errno));
> +	ret = FALSE;
> +    }
> +
> +    talloc_free (argv[0]);
> +    argv[0] = old_argv0;
> +
> +    return ret;
> +}
> +
>  int
>  main (int argc, char *argv[])
>  {
> @@ -406,8 +439,10 @@ main (int argc, char *argv[])
>  
>      command = find_command (command_name);
>      if (!command) {
> -	fprintf (stderr, "Error: Unknown command '%s' (see \"notmuch help\")\n",
> -		 command_name);
> +	/* This won't return if the external command is found. */
> +	if (try_external_command(argv + opt_index))
> +	    fprintf (stderr, "Error: Unknown command '%s' (see \"notmuch help\")\n",
> +		     command_name);
>  	ret = EXIT_FAILURE;
>  	goto DONE;
>      }
> -- 
> 2.1.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch

Thread: