Re: [PATCH] Start conforming code to "git subcmd" style

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 30, 2008, 20:49 UTC
Message-ID: <7vwshyfctu.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <20080830111253.GA9148@zakalwe.fi>

Heikki Orsila <heikki.orsila@iki.fi> writes:

> User notifications are presented as 'git cmd', and code comments
> are presented as '"cmd"' or 'git's cmd', rather than 'git-cmd'.

Thanks.

The part I will _not_ comment on in your patch looked all good, so to reduce further back-and-forth, I'd apply them to 'maint', excluding the parts I did comment on in this message.

Show 13 quoted lines

> diff --git a/builtin-apply.c b/builtin-apply.c
> ...
> @@ -506,17 +506,17 @@ static char *gitdiff_verify_name(const char *li
> ...
> -			die("git-apply: bad git-diff - expected /dev/nu...
> +			die("git apply: bad git-diff - expected /dev/nu...
> ...
> -			die("git-apply: bad git-diff - inconsistent %s ...
> +			die("git apply: bad git-diff - inconsistent %s ...
> ...
> -			die("git-apply: bad git-diff - expected /dev/nu...
> +			die("git apply: bad git-diff - expected /dev/nu...
> ...

I'd vote for doing "s/git-diff/patch/" here. After looking at builtin-apply.c, there is no other error/die messages that would become ambiguous, so such a rewording won't make it harder to help people who saw any of these error messages (or other error messages from the "git-apply" program).

Show 10 quoted lines

> diff --git a/builtin-blame.c b/builtin-blame.c
> ...
> @@ -2299,12 +2299,12 @@ int cmd_blame(int argc, const char **argv, co...
> ...
> -		OPT_BIT(..."Use the ... as git-annotate (Default: off)"...
> +		OPT_BIT(..."Use the ... as git annotate (Default: off)"...
> ...
> -		OPT_STRING(..."Use ...instead of calling git-rev-list"),
> +		OPT_STRING(..."Use ...instead of calling git rev-list"),
> ...

A two-word command name in a prose is hard to read; "rev-list" is not a word and that makes the problem less serious, but it would be easier to read if these two word command names are quoted or grouped together in some way to make it clear they form a single noun and the sentence is talking about a single "thing".

The old "git-foo" spelling was good for that purpose, but it will invite user confusion so we cannot use it anymore. Perhaps we can say "instead of calling 'git rev-list'"?

The command name at the beginning of die message does not have this issue. E.g. the colon in:

	die("git foo: I hate you");

is sufficient to make it clear that these two words form a single noun; i.e. "I'm 'git foo' program, and I am telling you that I hate you".

But it might be just me, so before asking you to reroll another round, I'd like to hear opinions from the list.

 (1) No, JC is worrying too much about readability; Heikki's patch is good;

 (2) JC's right -- "instead of calling 'git rev-list'" is much better;

 (3) Something else?

Show 6 quoted lines

> diff --git a/builtin-branch.c b/builtin-branch.c
> @@ -526,7 +526,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
> ...
> -		OPT_GROUP("Specific git-branch actions:"),
> +		OPT_GROUP("Specific git branch actions:"),
> ...

Likewise.