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

Jani Nikula jani at nikula.org
Sun Oct 23 11:49:40 PDT 2016


On Sun, 23 Oct 2016, Mark Walters <markwalters1009 at gmail.com> wrote:
> On Sat, 22 Oct 2016, Jani Nikula <jani at 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.

FWIW this is the same as in git.

>>
>> ---
>>
>> 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.

That's mentioned right there in the commit message, you are into reading
such things. ;)

> 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?

I wouldn't worry about it. I'm inclined to say that's the risk people
have to take when they add their own notmuch-stuff.

We could guarantee we don't touch some namespace, but making things
easier is part of the point here. I don't think people are going to name
their subcommands notmuch-local-stuff just to avoid potential problems
later.

BR,
Jani.


>
> 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 at notmuchmail.org
>> https://notmuchmail.org/mailman/listinfo/notmuch


More information about the notmuch mailing list