[PATCH] git-gui: give more advice when detaching HEAD

The current advice is a little sparse and dates back to
d41b43e (git-gui: Refactor branch switch to support detached
head, 2007-07-08). In the meantime, command-line git grew
much more detailed advice for this situation, especially in
13be3e3 (Reword "detached HEAD" notification, 2010-01-29).

Let's use that more detailed advice here.

Signed-off-by: Jeff King <redacted>
---
I recently helped somebody who had detached HEAD via git-gui, made a
bunch of commits, switched to another branch, and then became confused
about where his work went.

After working through what happened with him, I think this is one place
where we could have prevented the problem. And given that we saw the
need for more advice in the CLI, I think this change is a no-brainer.

I also think we could have saved him by doing one or more of:

  1. Give some indication or warning during commit that you're in a
     detached state. The CLI template says "You are not on any branch"
     when editing the commit message, and mentions "detached HEAD" as
     the branch in the post-commit summary. As far as I can tell,
     git-gui says nothing at all.

  2. When leaving the detached state, notice that we have commits not
     contained in any other ref and pop up an "are you sure you want to
     lose these commits" dialog, with an option to create a branch. This
     is something we considered and rejected for the CLI, but I wonder
     if it makes more sense for git-gui.

  3. Make it easier to create a new branch from the checkout dialog.
     Obviously I can go to "Branch->Create" and make a new branch from a
     remote one. But if my mental model is "Checkout", then I pick a
     remote branch, we may want to present the user with the decision
     _then_ about detaching or creating. Something as simple as a "make
     local branch from remote" checkbox would help. Or perhaps the
     "you're going to a detached HEAD" dialog should actually have a
     button to create a local branch right then and there instead. I
     dunno.

All of those things are too far beyond my scope of caring about git-gui
and wanting to write tcl to actually implement myself. But I thought I
would share them as thoughts that came from a real confused-user
interaction. Feel free to ignore.

 lib/checkout_op.tcl |   13 ++++++++++---
 1 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/lib/checkout_op.tcl b/lib/checkout_op.tcl
index 9e7412c..9c95208 100644
--- a/lib/checkout_op.tcl
+++ b/lib/checkout_op.tcl

@@ -449,9 +449,16 @@ method _after_readtree {} {
 	}
 
 	if {$is_detached} {
-		info_popup [mc "You are no longer on a local branch.
-
-If you wanted to be on a branch, create one now starting from 'This Detached Checkout'."]
+		info_popup [mc \
+"You are no longer on a local branch. You can look
+around, make experimental changes and commit,
+and you can discard any commits you make in this
+state without impacting any branches by
+performing another checkout.
+
+If you want to create a new branch to retain
+commits you create, you may do so (now or later)
+by starting from 'This Detached Checkout'."]
 	}
 
 	# -- Run the post-checkout hook.

-- 
1.7.4

Hi,

On Sat, Feb 12, 2011 at 02:05:38AM -0500, Jeff King wrote:

  1. Give some indication or warning during commit that you're in a
     detached state. The CLI template says "You are not on any branch"
     when editing the commit message, and mentions "detached HEAD" as
     the branch in the post-commit summary. As far as I can tell,
     git-gui says nothing at all.

How about something like this:

---8<----
From 8e2b61cd5e8d85f43ed6f00935a757f0dfa56b3b Mon Sep 17 00:00:00 2001
From: Heiko Voigt <redacted>
Date: Sun, 13 Feb 2011 13:25:04 +0100
Subject: [PATCH] git-gui: warn when trying to commit on a detached head

The commandline is already warning when checking out a detached head.
Since the only thing thats potentially dangerous is to create commits
on a detached head lets warn in case the user is about to do that.

Signed-off-by: Heiko Voigt <redacted>
---
The wording of the warning might need some cleanup and documentation of
the configuration variable is still missing but if you like it I will
add it.

 git-gui/git-gui.sh     |    1 +
 git-gui/lib/commit.tcl |   14 ++++++++++++++
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index d3acf0d..5314a3f 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh

@@ -831,6 +831,7 @@ set default_config(gui.fontdiff) [font configure font_diff]
 # TODO: this option should be added to the git-config documentation
 set default_config(gui.maxfilesdisplayed) 5000
 set default_config(gui.usettk) 1
+set default_config(gui.warndetachedcommit) 1
 set font_descs {
 	{fontui   font_ui   {mc "Main Font"}}
 	{fontdiff font_diff {mc "Diff/Console Font"}}

diff --git a/git-gui/lib/commit.tcl b/git-gui/lib/commit.tcl
index 7f459cd..9bef8ee 100644
--- a/git-gui/lib/commit.tcl
+++ b/git-gui/lib/commit.tcl

@@ -259,8 +259,22 @@ proc commit_prehook_wait {fd_ph curHEAD msg_p} {
 }
 
 proc commit_commitmsg {curHEAD msg_p} {
+	global is_detached repo_config
 	global pch_error
 
+	if {$is_detached && $repo_config(gui.warndetachedcommit)} {
+		set msg [mc "You are about to commit on a detached head.
+This is a potentially dangerous thing to do because
+if you switch to another branch you will loose your
+changes and it can be difficult to get them back.
+
+Do you really want to proceed?"]
+		if {[ask_popup $msg] ne yes} {
+			unlock_index
+			return
+		}
+	}
+
 	# -- Run the commit-msg hook.
 	#
 	set fd_ph [githook_read commit-msg $msg_p]

-- 
1.7.4.rc3.4.g155c4

On Sun, Feb 13, 2011 at 01:31:52PM +0100, Heiko Voigt wrote:

On Sat, Feb 12, 2011 at 02:05:38AM -0500, Jeff King wrote:

quoted

  1. Give some indication or warning during commit that you're in a
     detached state. The CLI template says "You are not on any branch"
     when editing the commit message, and mentions "detached HEAD" as
     the branch in the post-commit summary. As far as I can tell,
     git-gui says nothing at all.

How about something like this:
[...]
Subject: [PATCH] git-gui: warn when trying to commit on a detached head

The commandline is already warning when checking out a detached head.
Since the only thing thats potentially dangerous is to create commits
on a detached head lets warn in case the user is about to do that.

It seems a little heavy-handed to have a dialog pop up for each commit.
It's not actually dangerous to create a commit on a detached HEAD; it's
just dangerous to _leave_ without referencing your new commits.

So I think for making commits, something informational that doesn't
require a click-through would be the more appropriate level (similar to
what the CLI does; it just mentions it in the commit template). I guess
there isn't a commit template in the same way for git gui; instead, it
is always showing you the current state. And indeed, it does switch from
"Current Branch: master" to "Current Branch: HEAD" when you are on a
detached head. Maybe we should beef that up a bit to "You are not on any
branch." or something that is more self-explanatory. I dunno. I am just
guessing here about what users would want.

I do think a pop-up is appropriate when you try to check something else
out, and commits you have made on the detached HEAD are about to become
unreferenced. But this is something even the CLI doesn't do, so it would
make sense to see how the check is implemented there first before doing
anything in git-gui.

-Peff

Hi,

On Tue, Feb 15, 2011 at 01:39:03AM -0500, Jeff King wrote:

On Sun, Feb 13, 2011 at 01:31:52PM +0100, Heiko Voigt wrote:

quoted

On Sat, Feb 12, 2011 at 02:05:38AM -0500, Jeff King wrote:

quoted

  1. Give some indication or warning during commit that you're in a
     detached state. The CLI template says "You are not on any branch"
     when editing the commit message, and mentions "detached HEAD" as
     the branch in the post-commit summary. As far as I can tell,
     git-gui says nothing at all.

How about something like this:
[...]
Subject: [PATCH] git-gui: warn when trying to commit on a detached head

The commandline is already warning when checking out a detached head.
Since the only thing thats potentially dangerous is to create commits
on a detached head lets warn in case the user is about to do that.

It seems a little heavy-handed to have a dialog pop up for each commit.
It's not actually dangerous to create a commit on a detached HEAD; it's
just dangerous to _leave_ without referencing your new commits.

Hmm, how about adding a checkbox:

  [ ] Do not ask again

In my experience anything other than a popup will be overseen so I would
suggest doing it at least once to prepare the user for the possible
consequences.

IMO such a message is a good thing for the GUI regardless whether we
implement the leaving detached HEAD state warning. First I think a
typical GUI user does not commit on a detached head that often since
there is currently no way to use these commits from the GUI (e.g.
format-patch, rebase, ...). Second because a detached head is very
practical for testing work on a remote branch the message box would
remind most users to switch to their development branch first. If they
only get that message after a series of commits it might become a hassle
for them to get these commits onto another branch (remember no
format-patch or rebase currently).

So I think for making commits, something informational that doesn't
require a click-through would be the more appropriate level (similar to
what the CLI does; it just mentions it in the commit template). I guess
there isn't a commit template in the same way for git gui; instead, it
is always showing you the current state. And indeed, it does switch from
"Current Branch: master" to "Current Branch: HEAD" when you are on a
detached head. Maybe we should beef that up a bit to "You are not on any
branch." or something that is more self-explanatory. I dunno. I am just
guessing here about what users would want.

I do think a pop-up is appropriate when you try to check something else
out, and commits you have made on the detached HEAD are about to become
unreferenced. But this is something even the CLI doesn't do, so it would
make sense to see how the check is implemented there first before doing
anything in git-gui.

From what I read in this thread it currently seems to be not so easy to

precisely find out whether some commit is referenced. (If we care about
stuff outside of remotes, heads and tags). But maybe we do not need
that for the GUI.

If a commit is referenced from non typical refs the worst we do is issue
a false warning. Meaning we warn the user even though the commit is
referenced. For a GUI I think being a little more restrictive is the
right thing to do since it should guide the user much more into a safe
workflow. If he wants to do special things than there still is the CLI
to fall back on. And its just a warning so we are not preventing
anything.

Now it depends on what we would want for the CLI if we are going to
implement a thorough check over everything in refs/ than there is no
reason for not applying the same thing to git-gui. In case the current
behavior is deemed sufficient we should go with the check mention

Just to give you a practical example:

At $dayjob we are currently even more restrictive and completely forbid
commits on a detached head by a pre-commit hook. This was mainly done
due to the lack of warnings but I do not recall a single incident where a
user actually complained about this restriction (~90% GUI users).

Cheers Heiko

On Tue, Feb 15, 2011 at 08:16:21PM +0100, Heiko Voigt wrote:

quoted

It seems a little heavy-handed to have a dialog pop up for each commit.
It's not actually dangerous to create a commit on a detached HEAD; it's
just dangerous to _leave_ without referencing your new commits.

Hmm, how about adding a checkbox:

  [ ] Do not ask again

In my experience anything other than a popup will be overseen so I would
suggest doing it at least once to prepare the user for the possible
consequences.

Yeah, that's much better IMHO because at least clueful people can
dismiss it after the first time.

IMO such a message is a good thing for the GUI regardless whether we
implement the leaving detached HEAD state warning. First I think a
typical GUI user does not commit on a detached head that often since
there is currently no way to use these commits from the GUI (e.g.
format-patch, rebase, ...).

Fair enough. I really have no idea what sorts of things gui users do, or
how they perceive the system.

Second because a detached head is very practical for testing work on a
remote branch the message box would remind most users to switch to
their development branch first. If they only get that message after a
series of commits it might become a hassle for them to get these
commits onto another branch (remember no format-patch or rebase
currently).

Good point.

quoted

I do think a pop-up is appropriate when you try to check something else
out, and commits you have made on the detached HEAD are about to become
unreferenced. But this is something even the CLI doesn't do, so it would
make sense to see how the check is implemented there first before doing
anything in git-gui.

From what I read in this thread it currently seems to be not so easy to
precisely find out whether some commit is referenced. (If we care about
stuff outside of remotes, heads and tags). But maybe we do not need
that for the GUI.

Yeah, I think there is still some question about how it should happen,
and any check in the gui should probably be the same as in the cli.  But
from the rest of what you say, that shouldn't impact whether a
per-commit warning is worth doing.

-Peff

Hi,

On Tue, Feb 15, 2011 at 10:46:06PM -0500, Jeff King wrote:

On Tue, Feb 15, 2011 at 08:16:21PM +0100, Heiko Voigt wrote:

quoted

quoted

It seems a little heavy-handed to have a dialog pop up for each commit.
It's not actually dangerous to create a commit on a detached HEAD; it's
just dangerous to _leave_ without referencing your new commits.

Hmm, how about adding a checkbox:

  [ ] Do not ask again

In my experience anything other than a popup will be overseen so I would
suggest doing it at least once to prepare the user for the possible
consequences.

Yeah, that's much better IMHO because at least clueful people can
dismiss it after the first time.

I tried to implement such a dialog yesterday. Unfortunately my tcl/tk
fu seems not sufficient enough. I managed to create the dialog but did
not get the results which button was clicked. It has something to do
with the scope of variables in tcl. Pat you would probably be able to
fix this. I can send you the code if you are interested?

quoted

quoted

I do think a pop-up is appropriate when you try to check something else
out, and commits you have made on the detached HEAD are about to become
unreferenced. But this is something even the CLI doesn't do, so it would
make sense to see how the check is implemented there first before doing
anything in git-gui.

From what I read in this thread it currently seems to be not so easy to
precisely find out whether some commit is referenced. (If we care about
stuff outside of remotes, heads and tags). But maybe we do not need
that for the GUI.

Yeah, I think there is still some question about how it should happen,
and any check in the gui should probably be the same as in the cli.  But
from the rest of what you say, that shouldn't impact whether a
per-commit warning is worth doing.

Will someone be working on this? Otherwise: Implementing the previously
suggested solution is quite straightforward for git-gui. I could
implement that check in git-gui first and once the CLI has some
mechanism for this check we could switch to that.

Cheers Heiko