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

Nanako Shiraishi [off-list ref] writes:

Quoting Junio C Hamano [off-list ref]:

quoted

Sitaram Chamarty [off-list ref] writes:

quoted

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 patch helps me a lot.

The patch seems to disable checkout unconditionally, but it at least needs
an "expert mode" switch to bypass the patch's logic, or (better yet) a
"training wheel" switch for you to set in repositories of the people you
manage.

It will be more helpful if it checked out a new local branch that tracks
the remote branch, instead of refusing what the user asked to do. It may
need a new dialog that asks to confirm (and allows the user to change)
the name of the new branch.

Heh, stop, step back and think a bit.  I admit that it wasn't just you; we
were both blind.

If we unconditionally disable "check out this branch" from the context
sensitive menu for the tip of a remote tracking branch, I do not think we
lose anything.  If one wants to start a new local branch from there, one
can use the context sensitive menu for an arbitrary commit (rowmenu) and
say "Create new branch".

If we wanted users of gitk to use it to detach HEAD, the current UI is not
a good way to do so anyway --- it only allows detaching the tip of remote
tracking branches and not an arbitrary commit.

	Side note.  I am not arguing it is a good idea.  I am only saying
	that if it were a good idea, such an action should be in rowmenu
	that applies to any commit, not headmenu that applies only to the
	tips of refs. 

So I retract my earlier objection entirely.  I do not think the feature
Sitaram is disabling was meant to allow detaching HEAD at all and it can
be safely disabled for remote tracking branches to make the GUI experience
safer.

On 2009-06-21 23:27:13, Junio C Hamano [off-list ref] wrote:

If we wanted users of gitk to use it to detach HEAD, the current UI is not
a good way to do so anyway --- it only allows detaching the tip of remote
tracking branches and not an arbitrary commit.

Aaah -- excellent; wish I'd thought of it :-)  Thanks

So I retract my earlier objection entirely.  I do not think the feature
Sitaram is disabling was meant to allow detaching HEAD at all and it can
be safely disabled for remote tracking branches to make the GUI experience
safer.

Thanks.  I will resubmit to you and Paul again with the diff
header changed to suit his tree.

Regards,

Sitaram

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

Paul,

I have amended the diff header to fit the gitk repo as opposed to the
git repo.  To summarise the email discussions, the feature that is
being disabled gets you a detached HEAD, but if that were really the
intent we should allow a detached HEAD from any commit, not just a
head commit.  Therefore this particular disablement is useful and
makes the GUI experience safer.

Please accept.  Thanks.

 gitk |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/gitk b/gitk
index 4604c83..70b6abc 100755
--- a/gitk
+++ b/gitk

@@ -8833,6 +8833,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.3.2