Thread (11 messages) flat view 11 messages, 3 authors, 2016-06-15

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

From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:59:32

Hi,

Sebastian Schuberth wrote:

[...]
quoted hunk ↗ jump to hunk
--- 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.

[...]
quoted hunk ↗ jump to hunk
+++ b/git.c
[...]
quoted hunk ↗ jump to hunk
@@ -563,14 +563,14 @@ int main(int argc, char **av)
[...]
 	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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help