Jonathan Nieder [off-list ref] writes:
Message should go on stderr and be guarded with an advice option (see
advice.c).
Like this:
const char *arg;
...
if (argc != 1 || !advice_old_fashioned_set_upstream)
return 0; /* ok. */
arg = argv[0];
advise("If you wanted to make '%s' track '%s', do this:",
head, arg);
if (branch_existed)
advise(" $ git branch --set-upstream-to='%s' '%s'",
old_upstream, arg);
else
advise(" $ git branch -d '%s'", arg);
advise(" $ git branch --set-upstream-to='%s'", arg);
If an argument contains single-quotes, the quoting will be wrong, but
that's probably not worth worrying about.
In principle, I would agree that this is a kind of thing that falls
into the "advice" categiry, but with the fact that we plan to
deprecate "--set-upstream", combined with the fact that [PATCH 1/3]
introduced the new option --set-upstream-to together with a short
and sweet -u synonym already at this point in the series, I think it
is better to leave them emitted unconditionally to the standard
error stream, in order to train users away from using the old option
that has its arguments wrong (the option does not take an argument
it should, and makes the command line to look as if it takes two
branch arguments in the wrong order).
Actually, we should probably add the deprecation warning in this
commit.
Junio C Hamano wrote:
I think it
is better to leave them emitted unconditionally to the standard
error stream, in order to train users away from using the old option
that has its arguments wrong (the option does not take an argument
it should, and makes the command line to look as if it takes two
branch arguments in the wrong order).
I thought we already discussed that that is a side-issue?
The option is a mode option for the command, like "-m", "-d", or
"--edit-description". I genuinely don't think the order of options it
takes is counter-intuitive. The second argument defaulting to HEAD
and the behavior of creating the branch named by the first argument
when it does not exist are quite counter-intuitive.
Transitioning to a different argument order seems like it would just
make the command more complicated. After the transition, there are
two options to explain, and during the transition, it is easy to make
scripts with gratuitous incompatibilities that won't work on older
systems.
Where is my thinking going wrong?
Jonathan
On Tue, 2012-07-10 at 18:00 -0500, Jonathan Nieder wrote:
Junio C Hamano wrote:
quoted
I think it
is better to leave them emitted unconditionally to the standard
error stream, in order to train users away from using the old option
that has its arguments wrong (the option does not take an argument
it should, and makes the command line to look as if it takes two
branch arguments in the wrong order).
I thought we already discussed that that is a side-issue?
The current --set-upstream is the whole reason for this series existing.
The option is a mode option for the command, like "-m", "-d", or
"--edit-description". I genuinely don't think the order of options it
takes is counter-intuitive. The second argument defaulting to HEAD
and the behavior of creating the branch named by the first argument
when it does not exist are quite counter-intuitive.
This is confusing. First you say that you don't think it's
counter-intuitive but then you say it is? Or is the first part about -m
and -d?
The second part of the paragraph is indeed what I'm trying to solve with
this series. If you want to create a new branch, you should be using -t.
Transitioning to a different argument order seems like it would just
make the command more complicated. After the transition, there are
two options to explain, and during the transition, it is easy to make
scripts with gratuitous incompatibilities that won't work on older
systems.
Where is my thinking going wrong?
We're not transitioning to a new order as such, particularly not with
the same option name. The incompatibilities would ensue with the other
patch I send which did change the order for --set-upstream, but what
this does is _add_ --set-upstream-to=<upstream> such that the option
takes one argument and the command takes one optional argument, which
makes it closer to what one would expect, specially as changing the
upstream information is something you're most likely to do on the
current branch.
cmn