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

2014-01-22 Thread Sebastian Schuberth

On 02.01.2014 22:05, Sebastian Schuberth wrote:


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.


Right, I'll change this in a re-roll (using single-quotes for the command name).


Sorry for not coming up with the re-roll until now, and now it's too 
late to fixup the commit as it's already on master (3f784a4). Since this 
is just a minor wording issue I'll not follow this up anymore.


--
Sebastian Schuberth
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-01-02 Thread Jonathan Nieder
Hi,

Sebastian Schuberth wrote:

[...]
 --- 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)
[...]
   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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-01-02 Thread Sebastian Schuberth
On Thu, Jan 2, 2014 at 9:31 PM, Jonathan Nieder jrnie...@gmail.com wrote:

 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.

Right, I'll change this in a re-roll (using single-quotes for the command name).

 The rest of the patch looks good.

Thanks for the review.

-- 
Sebastian Schuberth
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html