Re: [PATCH v3 1/2 / RFC] builtin/branch: stop supporting the use of --set-upstream option

Kaartic Sivaraam [off-list ref] writes:

The '--set-upstream' option of branch was deprecated in,

    b347d06bf branch: deprecate --set-upstream and show help if we
    detect possible mistaken use (Thu, 30 Aug 2012 19:23:13 +0200)

It was deprecated for the reasons specified in the commit message of the
referenced commit.

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.

Make 'branch' die with an appropraite error message when the '--set-upstream'
option is used.

OK.

Note that there's a reason behind "dying with an error message" instead of
"not accepting the option". 'git branch' would *accept* '--set-upstream'
even after it's removal as a consequence of,

        Unique prefix can be abbrievated in option names

                          AND

    '--set-upstream' is a unique prefix of '--set-upstream-to'
       (when the '--set-upstream' option has been removed)

In order to smooth the transition for users and to avoid them being affected
by the "prefix issue" it was decided to make branch die when seeing the
'--set-upstream' flag for a few years and let the users know that it would be
removed some time in the future.

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.

    $ git branch --set-upstream origin/master
    The --set-upstream flag is deprecated and will be removed. Consider using --track or --set-upstream-to
    Branch origin/master set up to track local branch master.

After,

    $ git branch
    * master

    $ git branch --set-upstream origin/master
    fatal: the '--set-upstream' flag is no longer supported and will be removed. Consider using '--track' or '--set-upstream-to'

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

and make sure we do not list "--set-upstream" in the list of
supported options in

	git branch -h

output.

 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.

-		OPT_SET_INT( 0, "set-upstream",  &track, N_("change upstream info"),
+		OPT_SET_INT( 0, "set-upstream",  &track, N_("no longer supported"),
 			BRANCH_TRACK_OVERRIDE),

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.

Thanks.