git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Carlos Martín Nieto" <cmn@elego.de>
To: Jonathan Nieder <jrnieder@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] branch: add -u as a shortcut for --set-upstream
Date: Sun, 08 Jul 2012 07:48:15 +0200	[thread overview]
Message-ID: <1341726495.10752.78.camel@flaca.cmartin.tk> (raw)
In-Reply-To: <20120706153239.GA29362@burratino>

On Fri, 2012-07-06 at 10:32 -0500, Jonathan Nieder wrote:
> Hi Carlos,
> 
> Carlos Martín Nieto wrote:
> 
> > Add this shortcut just like git-push has it.
> [...]
> > --- a/builtin/branch.c
> > +++ b/builtin/branch.c
> > @@ -724,7 +724,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
> >  		OPT__QUIET(&quiet, "suppress informational messages"),
> >  		OPT_SET_INT('t', "track",  &track, "set up tracking mode (see git-pull(1))",
> >  			BRANCH_TRACK_EXPLICIT),
> > -		OPT_SET_INT( 0, "set-upstream",  &track, "change upstream info",
> > +		OPT_SET_INT('u', "set-upstream",  &track, "change upstream info",
> >  			BRANCH_TRACK_OVERRIDE),
> 
> I think this is a bad idea.  The --set-upstream option is confusing:
> 
> 	$ git branch --set-upstream=foo
> 	error: option 'set-upstream' takes no value
> 	$ ???

It is confusing, see the other thread (about making --set-upstream
behave more sanely) for the second part of this. I wanted to add a hack
so

    git branch --set-upstream foo

would set the current branch to track foo.

> 
> -u to set the corresponding upstream branch at the same time as
> creating a branch, analagous to "git push -u" might seem intuitive:
> 
> 	# create foo branch, setting its upstream at the same time
> 	git branch -u foo origin/foo
> 
> But that is what --track does and is not what --set-upstream is for.

Right, it's for altering the configuration after the branch has already
been created. I wasn't thinking about creating the branch. That usage of
--set-upstream seems unintuitive to me, but changing the upstream for a
branch that already exists is quite intuitive and what push -u does.

> 
> Unlike --track, I don't think --set-upstream is a common enough
> operation to deserve a one-letter synonym.

I find myself using it surprisingly often, though it's certainly still
not in the top-10.

> 
> Instead, I'd suggest the following changes:
> 
>  1) Add support for
> 
> 	# change upstream info
> 	git branch --set-upstream=origin/foo foo
> 
>     for existing branches only.

I submitted the patch I mentioned above which changes --set-upstream to
something closer to what the user probably expects, which behaves mostly
like this, except that you'd have to omit the '=' as it still expected
the branch as an argument to the command. Not the cleanest wrt how
options should take arguments, but it got the job done without much code
churn.

However, Junio doesn't like that it changes existing behaviour which is
even consistent (as long as you know that --set-upstream is a flag,
rather than an option with an argument) and some people might depend on
it behaving like it does with a single argument. He'd accept a
--set-upstream-to that behaves just like you describe. This could do
with having a short alias, as the name is quite long-winded. Another
reason for adding -u is that it would add confusion if --set-upstream
has the short alias -u in push, but it means something else in branch,
even though they have the same option (though it would be
--set-upstream-to here, but we'll end up deprecating --set-upstream, so
it works out in the long run).

> 
>  2) Introduce an --unset-upstream option which removes the
>     "corresponding upstream branch" configuration for an existing
>     branch.

This is a good idea. I'll add it to the patch series.

> 
>  3) Warn on
> 
> 	# acts just like --track
> 	git branch --set-upstream foo origin/foo
> 
>     for branches that do not exist yet.  Plan to make this a hard
>     error in the future.
> 
>  4) Warn on
> 
> 	# sets upstream for "foo" to the current branch
> 	git branch --set-upstream foo
> 
>     and plan to make it a hard error in the future.
> 
>  5) Warn on
> 
> 	git branch --set-upstream origin/foo foo
> 
>   which is almost certainly a typo for
> 
> 	git branch --set-upstream=origin/foo foo

This is roughly what Junio suggested. His proposal was to also show how
to undo the --set-upstream if it was invoked the wrong way.

> 
>  6) Perhaps, make -u a synonym for -t for consistency with "git push".

I don't really see -u and -t being consistent with push -u. The push
might create a branch, but that would be in another repository. I look
at it as modifying the upstream information for an existing branch in
the local repository, and -t does not do that, --set-upstream(-to) does
that. It can also create a new local branch, but that's another bug in
the interface.

   cmn

      reply	other threads:[~2012-07-08 10:48 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-05  7:57 [PATCH] branch: add -u as a shortcut for --set-upstream Carlos Martín Nieto
2012-07-06 15:32 ` Jonathan Nieder
2012-07-08  5:48   ` Carlos Martín Nieto [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1341726495.10752.78.camel@flaca.cmartin.tk \
    --to=cmn@elego.de \
    --cc=git@vger.kernel.org \
    --cc=jrnieder@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).