Re: [PATCH v2 1/4] Consistently use the term "builtin" instead of "internal command"

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jan 2, 2014, 20:31 UTC
Message-ID: <20140102203132.GQ20443@google.com>
In-Reply-To: <52C590B0.1020702@gmail.com>

Hi,

Sebastian Schuberth wrote:

[...]

Show 8 quoted lines

> --- a/Documentation/technical/api-builtin.txt
> +++ b/Documentation/technical/api-builtin.txt
> @@ -14,7 +14,7 @@ Git:
>  
>  . Add the external declaration for the function to `builtin.h`.
>  
> -. Add the command to `commands[]` table in `handle_internal_command()`,
> +. Add the command to `commands[]` table in `handle_builtin()`,

Makes sense.  Using consistent jargon makes for easier reading.

[...]

> +++ b/git.c

[...]

> @@ -563,14 +563,14 @@ int main(int argc, char **av)

[...]

Show 7 quoted lines

>  	if (starts_with(cmd, "git-")) {
>  		cmd += 4;
>  		argv[0] = cmd;
> -		handle_internal_command(argc, argv);
> +		handle_builtin(argc, argv);
> -		die("cannot handle %s internally", cmd);
> +		die("cannot handle %s as a builtin", cmd);

I think this makes the user-visible message less clear.

Before when the user had a stale git-whatever link lingering in gitexecdir, git would say

	fatal: cannot handle whatever internally

which tells me git was asked to handle the whatever command internally and was unable to. Afterward, it becomes

	fatal: cannot handle whatever as a builtin

which requires that I learn the jargon use of "builtin" as a noun. busybox's analogous message is "applet not found". It's less likely to come up when using git because it requires having a stray link to "git". A message like

	$ git whatever
	fatal: whatever: no such built-in command

would just leave me wondering "I never claimed it was built-in; what's going on?" I think it would be simplest to keep it as

	$ git whatever
	fatal: cannot handle "whatever" internally

which at least makes it clear that this is a low-level error.

The rest of the patch looks good.

Thanks, Jonathan