Re: [PATCH] gitk: disable checkout of remote branch

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Nov 04, 2009 at 10:03:49AM -0800, Junio C Hamano wrote:

> Jeff King <peff@xxxxxxxx> writes:
> 
> > On Wed, Nov 04, 2009 at 07:41:28AM +0100, Sverre Rabbelier wrote:
> >
> >> On Wed, Nov 4, 2009 at 07:17, Tim Mazid <timmazid@xxxxxxxxxxx> wrote:
> >> > So instead of invoking 'git checkout REMOTE/BRANCH', do 'git checkout -b
> >> > BRANCH REMOTE/BRANCH'.
> >> 
> >> Automagically doing 'git checkout -t remote/branch' when asked to do
> >> 'git checkout remote/branch' was suggested earlier on the list and I
> >> think there was even a patch that implemented it, not sure what the
> >> outcome of the series was. I do remember that Peff was annoyed by it
> >> at the GitTogether though so it might be a bad idea.
> >
> > It's in 'next' now.
> 
> Isn't it quite different?  What's in 'next' for 1.7.0 is to guess the
> user's intention when:

Sorry, yes, I just saw Sverre's comment and misread the original
proposal.  Checking out "$remote/$branch" will still detach the HEAD,
and I don't think anybody has a previous proposal to change that.

> I think this is primarily because the way this DWIM is totally silent in
> the transcript is misleading.  If you explain it the way I outlined above,
> I do not think there is any confusion.  That is, there is no way for the
> user to get confused if the command sequence were like so:
> 
>    $ git branch -t foo origin/foo
>    Branch foo set up to track remote branch foo from origin.
>    $ git checkout foo
>    Switched to a new branch 'foo'
> 
>    ... time passes ...
> 
>    $ git checkout foo
>    Switched to branch 'foo'
>    Your branch is behind 'origin/foo' by 1 commit, and can be fast-forwarded.
> 
> It could just be a matter of telling what we are doing a bit more
> explicitly when this DWIM kicks in.  How about this?
>
>    $ git checkout foo
>    (first forking your own 'foo' from 'origin/foo')
>    Branch foo set up to track remote branch foo from origin.
>    Switched to a new branch 'foo'

This is much better than the current behavior, IMHO. It at least says
what is going on, so a user who actually reads the message will have a
chance of knowing what happened.

The devil's advocate argument is that the difference between the "branch
-t" and the DWIM is that in the former, the user intentionally asks for
a new branch, whereas in the latter, they must realize (by reading and
understanding) that a new branch has been created.

Maybe that difference isn't relevant, and people actually read and
understand everything git says. Maybe not. I dunno. I don't think we
have any real data yet on how people will perceive the feature over
time, and I suspect the only way to get it is to release with it and see
what happens.

> In any case, I do not think the DWIM would kick in when you try to detach
> at remote branch head.  I did not check gitk code to find out the exact
> command line it uses, but I do not think it runs "checkout BRANCH".  The
> command needs to be at least "checkout REMOTE/BRANCH" to work the way it
> does now with any released version of git, and I would not be surprised if
> paulus was cautious enough to have spelled it as "refs/REMOTE/BRANCH" to
> avoid any potential ambiguity issues.

Yes, I was confused when I wrote the original. I agree that "checkout
REMOTE/BRANCH" from the command line should still detach. If gitk wants
to prevent people accidentally detaching HEAD, the context menu for
remote branch boxes should probably detect remote branches and say
something like "Create local branch 'foo' from 'origin/foo'".

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]