[PATCH] gitk: disable checkout of remote branch

At the command line, this gives you a detailed warning message, but the
GUI currently allows it without any fuss.

Since the GUI is often used by people much less familiar with git, it
seems reasonable to make the GUI more restrictive than the command line,
not less.

This prevents a lot of detached HEAD commits by new users.

Signed-off-by: Sitaram Chamarty <redacted>
---
 gitk |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/gitk b/gitk
index db5ec54..95e27d4 100755
--- a/gitk
+++ b/gitk

@@ -8897,6 +8897,9 @@ proc headmenu {x y id head} {
     set headmenuid $id
     set headmenuhead $head
     set state normal
+    if {[string match "remotes/*" $head]} {
+	set state disabled
+    }
     if {$head eq $mainhead} {
 	set state disabled
     }

-- 
1.6.5

Heya,

On Tue, Nov 3, 2009 at 17:00, Sitaram Chamarty [off-list ref] wrote:

At the command line, this gives you a detailed warning message, but the
GUI currently allows it without any fuss.

This is even better than an annoying popup dialog, as we all know
those are just ignored anyway :).

-- 
Cheers,

Sverre Rabbelier

Sverre Rabbelier-2 wrote:

On Tue, Nov 3, 2009 at 17:00, Sitaram Chamarty [off-list ref] wrote:

quoted

At the command line, this gives you a detailed warning message, but the
GUI currently allows it without any fuss.

This is even better than an annoying popup dialog, as we all know
those are just ignored anyway :).

Might be better to include a configuration option to allow this, for those
that know what they're doing. Most of the people that know what they're
doing will use the command line, anyway, but it may irritate some people.
-- 
View this message in context: http://n2.nabble.com/PATCH-gitk-disable-checkout-of-remote-branch-tp3939363p3942366.html
Sent from the git mailing list archive at Nabble.com.

On Wed, Nov 4, 2009 at 6:18 AM, Tim Mazid [off-list ref] wrote:

Might be better to include a configuration option to allow this, for those
that know what they're doing. Most of the people that know what they're
doing will use the command line, anyway, but it may irritate some people.

I considered that but found my tcl fu was seriously lacking.  These
are literally the first 3 lines of tcl I ever wrote in my life, and
this program is one huge 11,000+ line monolith, so I'm naturally
scared to make more than very, very, small changes :)

In any case, as you said, most people who know what they're doing can
use the CLI to get there anyway...

Sitaram Chamarty wrote:

On Wed, Nov 4, 2009 at 6:18 AM, Tim Mazid [off-list ref] wrote:

quoted

Might be better to include a configuration option to allow this, for
those
that know what they're doing. Most of the people that know what they're
doing will use the command line, anyway, but it may irritate some people.

I considered that but found my tcl fu was seriously lacking.  These
are literally the first 3 lines of tcl I ever wrote in my life, and
this program is one huge 11,000+ line monolith, so I'm naturally
scared to make more than very, very, small changes :)

In any case, as you said, most people who know what they're doing can
use the CLI to get there anyway...

Hm, now that I think about it, what might be better is to just do what
should be done for them. :P

So instead of invoking 'git checkout REMOTE/BRANCH', do 'git checkout -b
BRANCH REMOTE/BRANCH'.

I know you said, you don't know, tcl, but just throwing it out there for
anyone else.

Cheers,
Tim.
-- 
View this message in context: http://n2.nabble.com/PATCH-gitk-disable-checkout-of-remote-branch-tp3939363p3943388.html
Sent from the git mailing list archive at Nabble.com.

Heya,

On Wed, Nov 4, 2009 at 07:17, Tim Mazid [off-list ref] 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.

-- 
Cheers,

Sverre Rabbelier

On Wed, Nov 04, 2009 at 07:41:28AM +0100, Sverre Rabbelier wrote:

On Wed, Nov 4, 2009 at 07:17, Tim Mazid [off-list ref] wrote:

quoted

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. And for the record, my complaint about its behavior
turned out to be partially because I was an idiot. I am still not
convinced that we won't later regret leaving the stale local branch
sitting around, or that users won't find it confusing to see:

  $ git checkout foo
  Branch foo set up to track remote branch foo from origin.
  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.

(i.e., you do the same thing, but get two very different results, and
you have to know how to do the fast-forward. Trivial if you are used to
working with branches, but perhaps not if you are just sightseeing).

But I am no longer planning on writing a long-winded rant about the
feature. ;)

-Peff

Jeff King wrote:

On Wed, Nov 04, 2009 at 07:41:28AM +0100, Sverre Rabbelier wrote:

quoted

On Wed, Nov 4, 2009 at 07:17, Tim Mazid [off-list ref] wrote:

quoted

So instead of invoking 'git checkout REMOTE/BRANCH', do 'git checkout

-b

quoted

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. And for the record, my complaint about its behavior
turned out to be partially because I was an idiot. I am still not
convinced that we won't later regret leaving the stale local branch
sitting around, or that users won't find it confusing to see:

  $ git checkout foo
  Branch foo set up to track remote branch foo from origin.
  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.

Hm. I actually meant inside gitk, not git itself. As in, when you click
inside gitk and try to checkout a remote, it automatically creates a
tracking branch and checks THAT out instead, whereas command-line git works
the same way.
Does that even make sense? :P
-- 
View this message in context: http://n2.nabble.com/PATCH-gitk-disable-checkout-of-remote-branch-tp3939363p3943894.html
Sent from the git mailing list archive at Nabble.com.

On Wed, Nov 4, 2009 at 2:38 PM, Tim Mazid [off-list ref] wrote:

Hm. I actually meant inside gitk, not git itself. As in, when you click
inside gitk and try to checkout a remote, it automatically creates a
tracking branch and checks THAT out instead, whereas command-line git works
the same way.
Does that even make sense? :P

It certainly does.  And if gitk is internally running the same
"checkout" command that may happen automatically (in 1.7 or whatever).

I will "watch this space" as they say :)  I stated only my minimum
needs (prevent detached HEAD with even less warning [i.e., zero
warning] than in CLI) and managed minimum code for it.  But any other
solution that achives the same effect is also fine!

Jeff King [off-list ref] writes:

On Wed, Nov 04, 2009 at 07:41:28AM +0100, Sverre Rabbelier wrote:

quoted

On Wed, Nov 4, 2009 at 07:17, Tim Mazid [off-list ref] wrote:

quoted

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:

 - he says 'git checkout BRANCH'; and

 - BRANCH does not yet exist; and

 - BRANCH does not name a commit so the request cannot be to detach HEAD
   at some commit (like REMOTE/BRANCH); and

 - there is a unique REMOTE that has BRANCH.  

The user wants to check out his own BRANCH (the request lacks REMOTE to
start with) but such a branch does not exist yet, and there is only one
sensible commit to start that new branch, hence we DWIM it and helpfully
run "git branch -t BRANCH REMOTE/BRANCH" automatically before performing
"git checkout BRANCH" that was asked.

We never claim to allow checking out the remote tracking branch itself.
The new guessing is only about a local branch that does not exist yet.

... I am still not convinced that we won't later regret leaving the
stale local branch sitting around, or that users won't find it confusing
to see:

  $ git checkout foo
  Branch foo set up to track remote branch foo from origin.
  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.

(i.e., you do the same thing, but get two very different results,...

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'

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.

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

Jeff King [off-list ref] writes:

quoted

On Wed, Nov 04, 2009 at 07:41:28AM +0100, Sverre Rabbelier wrote:

quoted

On Wed, Nov 4, 2009 at 07:17, Tim Mazid [off-list ref] wrote:

quoted

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

Sitaram Chamarty writes:

This prevents a lot of detached HEAD commits by new users.

Signed-off-by: Sitaram Chamarty <redacted>

Thanks, applied.

Paul.