[PATCH 9/10] Remove cmd_usage() routine and re-organize the help/usage code.

[PATCH 9/10] Remove cmd_usage() routine and re-organize the help/usage code.

[Ramsay Jones]

[-- Attachment #1: Type: text/plain, Size: 4146 bytes --]

The cmd_usage() routine was causing warning messages due to a NULL
format parameter being passed in three out of four calls. This is a
problem if you want to compile with -Werror. A simple solution is to
simply remove the GNU __attribute__ format pragma from the cmd_usage()
declaration in the header file. The function interface was somewhat
muddled anyway, so re-write the code to finesse the problem.

Signed-off-by: Ramsay Allan Jones <ramsay@ramsay1.demon.co.uk>
---
 builtin-help.c |   54
+++++++++++++++++++++++-------------------------------
 builtin.h      |    7 ++-----
 git.c          |    7 +++++--
 3 files changed, 30 insertions(+), 38 deletions(-)

diff --git a/builtin-help.c b/builtin-help.c
index 7470faa..006da05 100644
--- a/builtin-help.c
+++ b/builtin-help.c
@@ -9,8 +9,6 @@ #include "builtin.h"
 #include "exec_cmd.h"
 #include "common-cmds.h"

-static const char git_usage[] =
-	"Usage: git [--version] [--exec-path[=GIT_EXEC_PATH]] [--help] COMMAND
 ARGS ]";

 /* most gui terms set COLUMNS (although some don't export it) */
 static int term_columns(void)
@@ -178,31 +176,6 @@ static void list_common_cmds_help(void)
 	puts("(use 'git help -a' to get a list of all installed git commands)");
 }

-void cmd_usage(int show_all, const char *exec_path, const char *fmt, ...)
-{
-	if (fmt) {
-		va_list ap;
-
-		va_start(ap, fmt);
-		printf("git: ");
-		vprintf(fmt, ap);
-		va_end(ap);
-		putchar('\n');
-	}
-	else
-		puts(git_usage);
-
-	if (exec_path) {
-		putchar('\n');
-		if (show_all)
-			list_commands(exec_path, "git-*");
-		else
-			list_common_cmds_help();
-        }
-
-	exit(1);
-}
-
 static void show_man_page(const char *git_cmd)
 {
 	const char *page;
@@ -221,6 +194,13 @@ static void show_man_page(const char *gi
 	execlp("man", "man", page, NULL);
 }

+void help_unknown_cmd(const char *cmd)
+{
+	printf("git: '%s' is not a git-command\n\n", cmd);
+	list_common_cmds_help();
+	exit(1);
+}
+
 int cmd_version(int argc, const char **argv, char **envp)
 {
 	printf("git version %s\n", git_version_string);
@@ -230,12 +210,24 @@ int cmd_version(int argc, const char **a
 int cmd_help(int argc, const char **argv, char **envp)
 {
 	const char *help_cmd = argv[1];
-	if (!help_cmd)
-		cmd_usage(0, git_exec_path(), NULL);
-	else if (!strcmp(help_cmd, "--all") || !strcmp(help_cmd, "-a"))
-		cmd_usage(1, git_exec_path(), NULL);
+	const char *exec_path = git_exec_path();
+
+	if (!help_cmd) {
+		printf("usage: %s\n\n", git_usage_string);
+		list_common_cmds_help();
+		exit(1);
+	}
+
+	else if (!strcmp(help_cmd, "--all") || !strcmp(help_cmd, "-a")) {
+		printf("usage: %s\n\n", git_usage_string);
+		if(exec_path)
+			list_commands(exec_path, "git-*");
+		exit(1);
+	}
+
 	else
 		show_man_page(help_cmd);
+
 	return 0;
 }

diff --git a/builtin.h b/builtin.h
index 7bfff11..b6cf5be 100644
--- a/builtin.h
+++ b/builtin.h
@@ -5,12 +5,9 @@ #include <stdio.h>
 #include <limits.h>

 extern const char git_version_string[];
+extern const char git_usage_string[];

-void cmd_usage(int show_all, const char *exec_path, const char *fmt, ...)
-#ifdef __GNUC__
-	__attribute__((__format__(__printf__, 3, 4), __noreturn__))
-#endif
-	;
+extern void help_unknown_cmd(const char *cmd);

 extern int cmd_help(int argc, const char **argv, char **envp);
 extern int cmd_version(int argc, const char **argv, char **envp);
diff --git a/git.c b/git.c
index ca8961f..f414df9 100644
--- a/git.c
+++ b/git.c
@@ -14,6 +14,9 @@ #include "cache.h"

 #include "builtin.h"

+const char git_usage_string[] =
+	"git [--version] [--exec-path[=GIT_EXEC_PATH]] [--help] COMMAND [ ARGS ]";
+
 static void prepend_to_path(const char *dir, int len)
 {
 	const char *old_path = getenv("PATH");
@@ -279,7 +282,7 @@ int main(int argc, const char **argv, ch
 			puts(git_exec_path());
 			exit(0);
 		}
-		cmd_usage(0, NULL, NULL);
+		usage(git_usage_string);
 	}
 	argv[0] = cmd;

@@ -312,7 +315,7 @@ int main(int argc, const char **argv, ch
 	}

 	if (errno == ENOENT)
-		cmd_usage(0, exec_path, "'%s' is not a git-command", cmd);
+		help_unknown_cmd(cmd);

 	fprintf(stderr, "Failed to run command '%s': %s\n",
 		cmd, strerror(errno));
--
1.4.1

[-- Attachment #2: P0009.TXT --]
[-- Type: text/plain, Size: 4412 bytes --]

From 1ce42e2b5b65b03657b3ca9a3b06dc97cc66573c Mon Sep 17 00:00:00 2001
From: Ramsay Allan Jones <ramsay@ramsay1.demon.co.uk>
Date: Sun, 30 Jul 2006 22:42:25 +0100
Subject: [PATCH 9/10] Remove cmd_usage() routine and re-organize the help/usage code.

The cmd_usage() routine was causing warning messages due to a NULL
format parameter being passed in three out of four calls. This is a
problem if you want to compile with -Werror. A simple solution is to
simply remove the GNU __attribute__ format pragma from the cmd_usage()
declaration in the header file. The function interface was somewhat
muddled anyway, so re-write the code to finesse the problem.

Signed-off-by: Ramsay Allan Jones <ramsay@ramsay1.demon.co.uk>
---
 builtin-help.c |   54 +++++++++++++++++++++++-------------------------------
 builtin.h      |    7 ++-----
 git.c          |    7 +++++--
 3 files changed, 30 insertions(+), 38 deletions(-)

diff --git a/builtin-help.c b/builtin-help.c
index 7470faa..006da05 100644
--- a/builtin-help.c
+++ b/builtin-help.c
@@ -9,8 +9,6 @@ #include "builtin.h"
 #include "exec_cmd.h"
 #include "common-cmds.h"
 
-static const char git_usage[] =
-	"Usage: git [--version] [--exec-path[=GIT_EXEC_PATH]] [--help] COMMAND [ ARGS ]";
 
 /* most gui terms set COLUMNS (although some don't export it) */
 static int term_columns(void)
@@ -178,31 +176,6 @@ static void list_common_cmds_help(void)
 	puts("(use 'git help -a' to get a list of all installed git commands)");
 }
 
-void cmd_usage(int show_all, const char *exec_path, const char *fmt, ...)
-{
-	if (fmt) {
-		va_list ap;
-
-		va_start(ap, fmt);
-		printf("git: ");
-		vprintf(fmt, ap);
-		va_end(ap);
-		putchar('\n');
-	}
-	else
-		puts(git_usage);
-
-	if (exec_path) {
-		putchar('\n');
-		if (show_all)
-			list_commands(exec_path, "git-*");
-		else
-			list_common_cmds_help();
-        }
-
-	exit(1);
-}
-
 static void show_man_page(const char *git_cmd)
 {
 	const char *page;
@@ -221,6 +194,13 @@ static void show_man_page(const char *gi
 	execlp("man", "man", page, NULL);
 }
 
+void help_unknown_cmd(const char *cmd)
+{
+	printf("git: '%s' is not a git-command\n\n", cmd);
+	list_common_cmds_help();
+	exit(1);
+}
+
 int cmd_version(int argc, const char **argv, char **envp)
 {
 	printf("git version %s\n", git_version_string);
@@ -230,12 +210,24 @@ int cmd_version(int argc, const char **a
 int cmd_help(int argc, const char **argv, char **envp)
 {
 	const char *help_cmd = argv[1];
-	if (!help_cmd)
-		cmd_usage(0, git_exec_path(), NULL);
-	else if (!strcmp(help_cmd, "--all") || !strcmp(help_cmd, "-a"))
-		cmd_usage(1, git_exec_path(), NULL);
+	const char *exec_path = git_exec_path();
+
+	if (!help_cmd) {
+		printf("usage: %s\n\n", git_usage_string);
+		list_common_cmds_help();
+		exit(1);
+	}
+
+	else if (!strcmp(help_cmd, "--all") || !strcmp(help_cmd, "-a")) {
+		printf("usage: %s\n\n", git_usage_string);
+		if(exec_path)
+			list_commands(exec_path, "git-*");
+		exit(1);
+	}
+
 	else
 		show_man_page(help_cmd);
+
 	return 0;
 }
 
diff --git a/builtin.h b/builtin.h
index 7bfff11..b6cf5be 100644
--- a/builtin.h
+++ b/builtin.h
@@ -5,12 +5,9 @@ #include <stdio.h>
 #include <limits.h>
 
 extern const char git_version_string[];
+extern const char git_usage_string[];
 
-void cmd_usage(int show_all, const char *exec_path, const char *fmt, ...)
-#ifdef __GNUC__
-	__attribute__((__format__(__printf__, 3, 4), __noreturn__))
-#endif
-	;
+extern void help_unknown_cmd(const char *cmd);
 
 extern int cmd_help(int argc, const char **argv, char **envp);
 extern int cmd_version(int argc, const char **argv, char **envp);
diff --git a/git.c b/git.c
index ca8961f..f414df9 100644
--- a/git.c
+++ b/git.c
@@ -14,6 +14,9 @@ #include "cache.h"
 
 #include "builtin.h"
 
+const char git_usage_string[] =
+	"git [--version] [--exec-path[=GIT_EXEC_PATH]] [--help] COMMAND [ ARGS ]";
+
 static void prepend_to_path(const char *dir, int len)
 {
 	const char *old_path = getenv("PATH");
@@ -279,7 +282,7 @@ int main(int argc, const char **argv, ch
 			puts(git_exec_path());
 			exit(0);
 		}
-		cmd_usage(0, NULL, NULL);
+		usage(git_usage_string);
 	}
 	argv[0] = cmd;
 
@@ -312,7 +315,7 @@ int main(int argc, const char **argv, ch
 	}
 
 	if (errno == ENOENT)
-		cmd_usage(0, exec_path, "'%s' is not a git-command", cmd);
+		help_unknown_cmd(cmd);
 
 	fprintf(stderr, "Failed to run command '%s': %s\n",
 		cmd, strerror(errno));
-- 
1.4.1

Re: [PATCH 9/10] Remove cmd_usage() routine and re-organize the help/usage code.

[Martin Waitz]

[-- Attachment #1: Type: text/plain, Size: 1087 bytes --]

hoi :)

On Wed, Aug 02, 2006 at 02:03:44AM +0100, Ramsay Jones wrote:
>  builtin-help.c |   54
> +++++++++++++++++++++++-------------------------------
>  builtin.h      |    7 ++-----
>  git.c          |    7 +++++--
>  3 files changed, 30 insertions(+), 38 deletions(-)

this patch is at the tip of "master" now, but with one more change:
builtin-help.c           |   54 ++++++++++++++++++++--------------------------
builtin.h                |    7 ++----
git.c                    |    7 ++++--
t/t9100-git-svn-basic.sh |    7 +++---
4 files changed, 33 insertions(+), 42 deletions(-)

diff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh
index bf1d638..34a3ccd 100755
--- a/t/t9100-git-svn-basic.sh
+++ b/t/t9100-git-svn-basic.sh
@@ -170,7 +170,7 @@ then
             test -L $SVN_TREE/exec-2.sh"

        name='modify a symlink to become a file'
-       git help > help || true
+       echo git help > help || true
        rm exec-2.sh
        cp help exec-2.sh
        git update-index exec-2.sh

this looks strange.

-- 
Martin Waitz

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 9/10] Remove cmd_usage() routine and re-organize the help/usage code.

[Junio C Hamano]

Martin Waitz <tali@admingilde.org> writes:

> On Wed, Aug 02, 2006 at 02:03:44AM +0100, Ramsay Jones wrote:
>>  builtin-help.c |   54
>> +++++++++++++++++++++++-------------------------------
>>  builtin.h      |    7 ++-----
>>  git.c          |    7 +++++--
>>  3 files changed, 30 insertions(+), 38 deletions(-)
>
> this patch is at the tip of "master" now, but with one more change:
>...
> diff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh
> index bf1d638..34a3ccd 100755
> --- a/t/t9100-git-svn-basic.sh
> +++ b/t/t9100-git-svn-basic.sh
>...
> this looks strange.

Ramsay's patch to cmd_help() broke this test because the test
relied on the details of output from "git help", which the patch
subtly changed.

I considered making the fix for broken test a separate commit,
but the fix for the test was simple enough, so I rolled it in,
with the additional comment in the log to explain what was going
on -- I suspect the explanation was not clear enough.

I could have committed the fix for the test first and then this
one.

RE: [PATCH 9/10] Remove cmd_usage() routine and re-organize the help/usage code.

[Ramsay Jones]

[-- Attachment #1: Type: text/plain, Size: 2984 bytes --]


On Wed 2006-08-02 at 19:18, Junio C Hamano wrote:
> 
> Martin Waitz <tali@admingilde.org> writes:
> 
> > On Wed, Aug 02, 2006 at 02:03:44AM +0100, Ramsay Jones wrote:
> >>  builtin-help.c |   54
> >> +++++++++++++++++++++++-------------------------------
> >>  builtin.h      |    7 ++-----
> >>  git.c          |    7 +++++--
> >>  3 files changed, 30 insertions(+), 38 deletions(-)
> >
> > this patch is at the tip of "master" now, but with one more change:
> >...
> > diff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh
> > index bf1d638..34a3ccd 100755
> > --- a/t/t9100-git-svn-basic.sh
> > +++ b/t/t9100-git-svn-basic.sh
> >...
> > this looks strange.
> 
> Ramsay's patch to cmd_help() broke this test because the test
> relied on the details of output from "git help", which the patch
> subtly changed.
> 
> I considered making the fix for broken test a separate commit,
> but the fix for the test was simple enough, so I rolled it in,
> with the additional comment in the log to explain what was going
> on -- I suspect the explanation was not clear enough.
> 
> I could have committed the fix for the test first and then this
> one.
> 

I was surprised and concerned to read that (how did I miss the
failing test?), until I found that I didn't have this test! (Phew)
I assume this is a post 1.4.1 test, and from the name, I suppose
they would have mostly failed for me anyway since I don't have
svn installed.

In any event, I'm sorry to have broken your test, I should have
documented the changed behaviour. The change should be limited to
a lower-case usage prefix and a change in exit code from 1 to 129.
The change from "Usage:" to "usage:" was for consistency with every
other command (ignoring the test-delta program). The exit code
change only applies to one of the four original cmd_usage() call
sites; namely the one changed to a usage() call.  Hmmm, should
the other three "call sites" change to exit(129) as well?

Any other change in behavior was unintentional. Did I miss
something?

Speaking of consistency, I noticed some inconsistent command
names in various usage strings. I attach a patch (p0011.txt)
for your consideration.

During one of my "grep-fests", I also noticed some calls to
die(usage_string) rather than usage(usage_string). Calling die()
rather than usage() means that a "fatal: " (rather than "usage: ")
prefix is output prior the usage string, and the exit code
is 128 (rather than 129).

It looks like to choice of die() was deliberate, particularly in
builtin-rm.c and hash-object.c since they call both die() and
usage(). However, It's not clear to me why this choice was made.
(Which is not to say there isn't a perfectly good reason for the
choice; it's just that I don't see it.)

I attach a patch (p0012.txt) which replaces these calls to die()
with usage(). Since it is possible, however unlikely, that there
are some scripts out there that depend on the current behavior,
you may not want to apply it.

Cheers,

Ramsay

[-- Attachment #2: P0011.TXT --]
[-- Type: text/plain, Size: 2727 bytes --]

From 564d11cd55388f6a8de82b34b362543f73e0096c Mon Sep 17 00:00:00 2001
From: Ramsay Allan Jones <ramsay@ramsay1.demon.co.uk>
Date: Thu, 3 Aug 2006 16:38:39 +0100
Subject: [PATCH] Fixup command names in some usage strings.

Most usage strings, such as for command xxx, start with "git-xxx".
This updates the rebels to conform to the general pattern.
(The git wrapper is an exception to this, of course ...)

Signed-off-by: Ramsay Allan Jones <ramsay@ramsay1.demon.co.uk>
---
 blame.c        |    2 +-
 builtin-diff.c |    2 +-
 builtin-push.c |    2 +-
 mktag.c        |    2 +-
 mktree.c       |    2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/blame.c b/blame.c
index c86e2fd..2f19b4d 100644
--- a/blame.c
+++ b/blame.c
@@ -20,7 +20,7 @@ #include "xdiff-interface.h"
 
 #define DEBUG 0
 
-static const char blame_usage[] = "[-c] [-l] [-t] [-S <revs-file>] [--] file [commit]\n"
+static const char blame_usage[] = "git-blame [-c] [-l] [-t] [-S <revs-file>] [--] file [commit]\n"
 	"  -c, --compability Use the same output mode as git-annotate (Default: off)\n"
 	"  -l, --long        Show long commit SHA1 (Default: off)\n"
 	"  -t, --time        Show raw timestamp (Default: off)\n"
diff --git a/builtin-diff.c b/builtin-diff.c
index 99a2f76..f934dc0 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -23,7 +23,7 @@ struct blobinfo {
 };
 
 static const char builtin_diff_usage[] =
-"diff <options> <rev>{0,2} -- <path>*";
+"git-diff <options> <rev>{0,2} -- <path>*";
 
 static int builtin_diff_files(struct rev_info *revs,
 			      int argc, const char **argv)
diff --git a/builtin-push.c b/builtin-push.c
index 66b9407..57db489 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -8,7 +8,7 @@ #include "builtin.h"
 
 #define MAX_URI (16)
 
-static const char push_usage[] = "git push [--all] [--tags] [--force] <repository> [<refspec>...]";
+static const char push_usage[] = "git-push [--all] [--tags] [--force] <repository> [<refspec>...]";
 
 static int all = 0, tags = 0, force = 0, thin = 1;
 static const char *execute = NULL;
diff --git a/mktag.c b/mktag.c
index be41513..1d0cb17 100644
--- a/mktag.c
+++ b/mktag.c
@@ -123,7 +123,7 @@ int main(int argc, char **argv)
 	unsigned char result_sha1[20];
 
 	if (argc != 1)
-		usage("cat <signaturefile> | git-mktag");
+		usage("git-mktag < signaturefile");
 
 	setup_git_directory();
 
diff --git a/mktree.c b/mktree.c
index ab63cd9..9a6f0d2 100644
--- a/mktree.c
+++ b/mktree.c
@@ -71,7 +71,7 @@ static void write_tree(unsigned char *sh
 	write_sha1_file(buffer, offset, tree_type, sha1);
 }
 
-static const char mktree_usage[] = "mktree [-z]";
+static const char mktree_usage[] = "git-mktree [-z]";
 
 int main(int ac, char **av)
 {
-- 
1.4.1


[-- Attachment #3: P0012.TXT --]
[-- Type: text/plain, Size: 2649 bytes --]

From d5c3c850036f06c35416b8a515652c248bf4a73a Mon Sep 17 00:00:00 2001
From: Ramsay Allan Jones <ramsay@ramsay1.demon.co.uk>
Date: Thu, 3 Aug 2006 16:48:41 +0100
Subject: [PATCH] Replace some calls to die(usage_str) with usage(usage_str).

The only change in behaviour should be having a "usage: " prefix
on the output string rather than "fatal: ", and an exit code of
129 rather than 128.

Signed-off-by: Ramsay Allan Jones <ramsay@ramsay1.demon.co.uk>
---
 builtin-add.c        |    2 +-
 builtin-init-db.c    |    2 +-
 builtin-rm.c         |    2 +-
 builtin-write-tree.c |    2 +-
 hash-object.c        |    4 ++--
 5 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/builtin-add.c b/builtin-add.c
index bfbbb1b..ced84ea 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -158,7 +158,7 @@ int cmd_add(int argc, const char **argv,
 			verbose = 1;
 			continue;
 		}
-		die(builtin_add_usage);
+		usage(builtin_add_usage);
 	}
 	git_config(git_default_config);
 	pathspec = get_pathspec(prefix, argv + i);
diff --git a/builtin-init-db.c b/builtin-init-db.c
index 7fdd2fa..85a2afd 100644
--- a/builtin-init-db.c
+++ b/builtin-init-db.c
@@ -267,7 +267,7 @@ int cmd_init_db(int argc, const char **a
 		else if (!strncmp(arg, "--shared=", 9))
 			shared_repository = git_config_perm("arg", arg+9);
 		else
-			die(init_db_usage);
+			usage(init_db_usage);
 	}
 
 	/*
diff --git a/builtin-rm.c b/builtin-rm.c
index 4d56a1f..bfacbb2 100644
--- a/builtin-rm.c
+++ b/builtin-rm.c
@@ -81,7 +81,7 @@ int cmd_rm(int argc, const char **argv, 
 			force = 1;
 			continue;
 		}
-		die(builtin_rm_usage);
+		usage(builtin_rm_usage);
 	}
 	if (argc <= i)
 		usage(builtin_rm_usage);
diff --git a/builtin-write-tree.c b/builtin-write-tree.c
index 70e9b6f..bbe1f07 100644
--- a/builtin-write-tree.c
+++ b/builtin-write-tree.c
@@ -74,7 +74,7 @@ int cmd_write_tree(int argc, const char 
 		else if (!strncmp(arg, "--prefix=", 9))
 			prefix = arg + 9;
 		else
-			die(write_tree_usage);
+			usage(write_tree_usage);
 		argc--; argv++;
 	}
 
diff --git a/hash-object.c b/hash-object.c
index 43bd93b..bf0b492 100644
--- a/hash-object.c
+++ b/hash-object.c
@@ -46,7 +46,7 @@ int main(int argc, char **argv)
 		if (!no_more_flags && argv[i][0] == '-') {
 			if (!strcmp(argv[i], "-t")) {
 				if (argc <= ++i)
-					die(hash_object_usage);
+					usage(hash_object_usage);
 				type = argv[i];
 			}
 			else if (!strcmp(argv[i], "-w")) {
@@ -66,7 +66,7 @@ int main(int argc, char **argv)
 				hash_stdin(type, write_object);
 			}
 			else
-				die(hash_object_usage);
+				usage(hash_object_usage);
 		} 
 		else {
 			const char *arg = argv[i];
-- 
1.4.1

Re: [PATCH 9/10] Remove cmd_usage() routine and re-organize the help/usage code.

[Junio C Hamano]

"Ramsay Jones" <ramsay@ramsay1.demon.co.uk> writes:

> In any event, I'm sorry to have broken your test,

Don't worry.  My primary function while wearing the "maintainer"
hat is to catch mistakes like this; even if this test breakage
slipped through, it would not have been your fault but mine.

> ... I should have
> documented the changed behaviour. The change should be limited to
> a lower-case usage prefix and a change in exit code from 1 to 129.

I think both are good changes.  The native usage() exits with 129
so it would be consistent for the git wrapper to exit with 129
when it gives usage message, and in lowercase.

Will take a look at your patches when I go home (I am at day-job
today).

Re: [PATCH 9/10] Remove cmd_usage() routine and re-organize the help/usage code.

[Junio C Hamano]

"Ramsay Jones" <ramsay@ramsay1.demon.co.uk> writes:

> Speaking of consistency, I noticed some inconsistent command
> names in various usage strings. I attach a patch (p0011.txt)
> for your consideration.

Thanks, all look good.

> During one of my "grep-fests", I also noticed some calls to
> die(usage_string) rather than usage(usage_string). Calling die()
> rather than usage() means that a "fatal: " (rather than "usage: ")
> prefix is output prior the usage string, and the exit code
> is 128 (rather than 129).
>
> It looks like to choice of die() was deliberate, particularly in
> builtin-rm.c and hash-object.c since they call both die() and
> usage(). However, It's not clear to me why this choice was made.
> (Which is not to say there isn't a perfectly good reason for the
> choice; it's just that I don't see it.)

I think these were all sloppy coding.  The fix you did with
p0012 look good -- all of them are complaining that the command
line parsing found something unexpected, so I think usage() is
more appropriate.

RE: [PATCH 9/10] Remove cmd_usage() routine and re-organize the help/usage code.

[Ramsay Jones]

On Wed, 2006-08-02 at 14:22 Martin Waitz wrote:
> hoi :)
> 
> On Wed, Aug 02, 2006 at 02:03:44AM +0100, Ramsay Jones wrote:
> >  builtin-help.c |   54
> > +++++++++++++++++++++++-------------------------------
> >  builtin.h      |    7 ++-----
> >  git.c          |    7 +++++--
> >  3 files changed, 30 insertions(+), 38 deletions(-)
> 
> this patch is at the tip of "master" now, but with one more change:
> builtin-help.c           |   54 
> ++++++++++++++++++++--------------------------
> builtin.h                |    7 ++----
> git.c                    |    7 ++++--
> t/t9100-git-svn-basic.sh |    7 +++---
> 4 files changed, 33 insertions(+), 42 deletions(-)
> 
> diff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh
> index bf1d638..34a3ccd 100755
> --- a/t/t9100-git-svn-basic.sh
> +++ b/t/t9100-git-svn-basic.sh
> @@ -170,7 +170,7 @@ then
>              test -L $SVN_TREE/exec-2.sh"
> 
>         name='modify a symlink to become a file'
> -       git help > help || true
> +       echo git help > help || true
>         rm exec-2.sh
>         cp help exec-2.sh
>         git update-index exec-2.sh
> 
> this looks strange.
> 

Erm, well maybe. I dunno. Junio?

Ramsay