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

[Jakub Narebski <jnareb@gmail.com>]

Junio C Hamano <gitster@pobox.com> writes:

> 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'.
...
> > 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).

I agree.  git-apply in general is presented a patch output, not
necessary git-diff output (it could be output generated by GNU diff,
or by 'scm diff' from some SCM...).

> > 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'"?

Either "git-rev-list" or "'git rev-list'" is fine; "git rev-list"
is not, as it requires careful reading to notice which part is
proposed git command, and which the rest of message.

> 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?

I think that "git foo: message" is unambiguous, and I guess _that_
could be even in one single large patch.  Other cases I guess need
careful review and thinking about in a case by case basis,
unfortunately.

Better to be careful about that change than to make change and then
notice that it is not good...

-- 
Jakub Narebski
Poland
ShadeHawk on #git