On Tuesday 15 August 2017 01:49 AM, Junio C Hamano wrote:
I wonder if these two lines add any value here. Those who know the
reason would not be helped, and those who don't know have to view
"git show b347d06bf" anyway.
That's right.
I somehow think the above wastes bits a bit too much. Wouldn't it
be sufficient to say
In order to prevent "--set-upstream" on a command line from
being taken as an abbreviated form of "--set-upstream-to",
explicitly catch "--set-upstream" option and die, instead of
just removing it from the list of options.
Thanks for the shorter version. I'll use this :)
Because from the end-user's point of view, it has already been
removed, I'd phrase it more like
The --set-upstream option has been removed. Use --track or ...
I thought I changed it. It seems to have gone missing. Thanks for
noticing this.
and make sure we do not list "--set-upstream" in the list of
supported options in
git branch -h
output.
I guess the instructions given below are enough. They do seem to be
doing hiding it
from 'git branch -h'
Here we would want to use something like
{ OPTION_SET_INT, 0, "set-upstream", &track, NULL, N_("do not use"),
PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, BRANCH_TRACK_OVERRIDE },
in order to hide the option from "git branch -h" output.
All review comments from Martin were also good ones, and I won't
repeat them here.
A query,
I see the following code in the code path a little above the die statement
added in this change,
if (!strcmp(argv[0], "HEAD"))
die(_("it does not make sense to create 'HEAD'
manually"));
It does seem to be doing quite a nice job of avoiding an ambiguity that
could
have bad consequences but it's still possible to create a branch named
'HEAD'
using the '-b' option of 'checkout'. Should 'git checkout -b HEAD' actually
fail(it does not currently) for the same reason 'git branch HEAD' fails?
My guess is that people would use 'git checkout -b <new_branch_name>
<starting_point>'
more than it's 'git branch' counterpart.
Thanks for noticing. I offhand see no reason not to do what you
suggest above.
There are two ways in which this could be done.
1. Duplicate the check done in 'builtin/branch.c' in
'builtin/checkout.c'. This doesn't
sound good to me.
2. Do the check in 'branch.c::validate_new_branchname' to ensure there's
no way to
create a branch with the name of 'HEAD'.
Which one is preferred?
