Re: Custom subcommand help handlers

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Mon, Dec 20 2021, Johannes Schindelin wrote:

> Hi Sean,
>
> On Sat, 18 Dec 2021, Sean Allred wrote:
>
>> I've got a custom subcommand I'm distributing in my company to integrate
>> with our bug-tracker. It's a pretty robust utility and has its own help
>> function, but running `git foo --help` doesn't pass --help to my git-foo
>> utility. I asked a question[1] about this scenario on the Windows fork
>> and they directed me upstream.
>>
>> It sounds like `git foo --help` is internally consumed as `git help
>> foo`, which forwards requests to info/man/web handlers per config.
>> Being on Windows and knowing my peers as I do, the vast majority of my
>> users won't be familiar with info or man. The HTML documentation used
>> by the web handler is in a Git4Win-controlled installation directory
>> that I'd really rather not touch/maintain. I really just want `git foo
>> --help` to call `git-foo --help`.
>>
>> What's the best way to go about this?
>>
>> In the event the best next step is to start a patch, does it sound
>> reasonable to simply not perform this `git foo --help` -> `git help
>> foo` transformation for non-builtins? Or, while I don't relish the
>> idea, would some kind of config option be needed?
>
> I think you might need to be a bit more careful than just looking whether
> the command in question is a built-in or not. It could be delivered as a
> script or executable inside `libexec/git-core`. So maybe check that,
> something like this:
>
> -- snip --
> diff --git a/git.c b/git.c
> index c802dfe98004..d609f90cc117 100644
> --- a/git.c
> +++ b/git.c
> @@ -688,6 +688,33 @@ static void strip_extension(const char **argv)
>  #define strip_extension(cmd)
>  #endif
>
> +static int is_in_git_exec_path(const char *command_name)
> +{
> +	struct strbuf path = STRBUF_INIT;
> +	int ret = 0;
> +
> +	if (!command_name)
> +		return 0;
> +
> +	strbuf_addf(&path, "%s/git-%s", git_exec_path(), command_name);
> +	ret = !access(path.buf, X_OK);
> +
> +#ifdef STRIP_EXTENSION
> +	if (!ret) {
> +		/*
> +		 * If `command_name` ended in `.exe`, strip it, otherwise
> +		 * append it.
> +		 */
> +		if (!strbuf_strip_suffix(&path, STRIP_EXTENSION))
> +			strbuf_addstr(&path, STRIP_EXTENSION);
> +		ret = !access(path.buf, X_OK);
> +	}
> +#endif
> +
> +	strbuf_release(&path);
> +	return ret;
> +}
> +
>  static void handle_builtin(int argc, const char **argv)
>  {
>  	struct strvec args = STRVEC_INIT;
> @@ -697,8 +724,11 @@ static void handle_builtin(int argc, const char **argv)
>  	strip_extension(argv);
>  	cmd = argv[0];
>
> +	builtin = get_builtin(cmd);
> +
>  	/* Turn "git cmd --help" into "git help --exclude-guides cmd" */
> -	if (argc > 1 && !strcmp(argv[1], "--help")) {
> +	if (argc > 1 && !strcmp(argv[1], "--help") &&
> +	    (builtin || is_in_git_exec_path(argv[0]))) {
>  		int i;
>
>  		argv[1] = argv[0];
> @@ -714,7 +744,6 @@ static void handle_builtin(int argc, const char **argv)
>  		argv = args.v;
>  	}
>
> -	builtin = get_builtin(cmd);
>  	if (builtin)
>  		exit(run_builtin(builtin, argc, argv));
>  	strvec_clear(&args);
> -- snap --
>
> Of course, this might break existing users' setups where they ship a Git
> command together with a manual page.
>
> A potential remedy against that would be, as you say, a config option.
> Maybe defaulting to the manual page if `help.format` is `man`, otherwise
> defaulting to passing `--help` to the command.

What are the cases that require us to inexpect our --exec-path at
runtime, as opposed to having a list of commands we know we put there at
"install" time?

The only ones I can think of are e.g. Debian's packaging which might
compile the git with "git-send-email", but it won't be there unless you
install "git-email" in addition to "git".

But for those cases any such logic would presumably want the hardcoded
full list over the dynamic access() check, since e.g. "git-doc" on that
platform orthagonally installs "git-send-email.html" and the like, and
"git help send-email" would presumably like to error saying that we know
about git-send-email, we just can't find its documentation, even if we
can't find it in --exec-path.