Re: [PATCH v2 1/4] Consistently use the term builtin instead of internal command
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
[PATCH v2 1/4] Consistently use the term builtin instead of internal command
Signed-off-by: Sebastian Schuberth sschube...@gmail.com
---
Documentation/technical/api-builtin.txt | 2 +-
git.c | 14 +++---
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/Documentation/technical/api-builtin.txt
b/Documentation/technical/api-builtin.txt
index f3c1357..150a02a 100644
--- 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()`,
defined in `git.c`. The entry should look like:
{ foo, cmd_foo, options },
diff --git a/git.c b/git.c
index 3799514..89ab5d7 100644
--- a/git.c
+++ b/git.c
@@ -332,7 +332,7 @@ static int run_builtin(struct cmd_struct *p, int argc,
const char **argv)
return 0;
}
-static void handle_internal_command(int argc, const char **argv)
+static void handle_builtin(int argc, const char **argv)
{
const char *cmd = argv[0];
static struct cmd_struct commands[] = {
@@ -517,8 +517,8 @@ static int run_argv(int *argcp, const char ***argv)
int done_alias = 0;
while (1) {
- /* See if it's an internal command */
- handle_internal_command(*argcp, *argv);
+ /* See if it's a builtin */
+ handle_builtin(*argcp, *argv);
/* .. then try the external ones */
execv_dashed_external(*argv);
@@ -563,14 +563,14 @@ int main(int argc, char **av)
* - cannot execute it externally (since it would just do
*the same thing over again)
*
-* So we just directly call the internal command handler, and
-* die if that one cannot handle it.
+* So we just directly call the builtin handler, and die if
+* that one cannot handle it.
*/
if (starts_with(cmd, git-)) {
cmd += 4;
argv[0] = cmd;
- handle_internal_command(argc, argv);
- die(cannot handle %s internally, cmd);
+ handle_builtin(argc, argv);
+ die(cannot handle %s as a builtin, cmd);
}
/* Look for flags.. */
--
1.8.3-mingw-1
--
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
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
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
