threads / patch / 43286

patch, 2 partsOptionally check for uncommitted changes before switching branches.

Subject: [PATCH 2/2] Optionally check for uncommitted changes before switching branches.

## tl;dr

4 messages between Dec 7, 2006 and Dec 7, 2006. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Shawn O. Pearce· Dec 7, 2006, 10:02 UTC · lore

Lately I have noticed a number of users are forgetting that they have uncommitted changes in their working directory when they switch to another branch. This causes the user to accidentally carry those changes into the new branch, which is usually not where they wanted to commit them. A correlation does appear to exist between the user being interrupted in the middle of their task and the branch switch, indicating they simply got distracted and forgot what was going on.

Git shouldn't cause the user to make mistakes when it can help to prevent them. So now users may set checkout.requireCleanDirectory to true in their config file to have git-checkout verify the working directory is clean before switching branches.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 Documentation/config.txt |    8 ++++++++
 git-checkout.sh          |   10 ++++++++++
 t/t3200-branch.sh        |   11 +++++++++++
 3 files changed, 29 insertions(+), 0 deletions(-)
Show changes to 3 files +29 −0

Documentation/config.txt, git-checkout.sh, t/t3200-branch.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 9d754c8..f10e8ac 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -136,6 +136,14 @@ checkout.requireSourceBranch::
 	be the source version if only one argument is supplied.
 	Default is false, to stay compatible with prior behavior.
 
+checkout.requireCleanDirectory::
+	If true tells git-checkout to verify there are no uncommitted
+	changes still in the index or working directory before
+	switching branches.  If uncommitted changes exist the -m
+	flag can be used to skip the check if the user really wanted
+	to carry those onto the new branch.  Default is false,
+	to stay compatible with prior behavior.
+
 pager.color::
 	A boolean to enable/disable colored output when the pager is in
 	use (default is true).
diff --git a/git-checkout.sh b/git-checkout.sh
index 5f9fb6e..c04b8c1 100755
--- a/git-checkout.sh
+++ b/git-checkout.sh
@@ -171,6 +171,16 @@ then
     git-read-tree --reset -u $new
 else
     git-update-index --refresh >/dev/null
+	if [ -n "$old" ] &&
+	   [ -z "$merge" ] &&
+	   [ Xtrue = "X`git-repo-config --bool checkout.requireCleanDirectory`" ]
+	then
+		if [ `git-diff-index --cached $old | wc -l` -gt 0 ] ||
+		   [ `git-diff-files | wc -l` -gt 0 ]
+		then
+			die "Working directory has uncommitted changes; commit, reset or use -m"
+		fi
+	fi
     merge_error=$(git-read-tree -m -u $old $new 2>&1) || (
 	case "$merge" in
 	'')
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 7e0c48b..9429827 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -84,4 +84,15 @@ test_expect_success \
 	'git-repo-config checkout.requireSourceBranch false
 	 git-checkout -b N'
 
+test_expect_failure \
+	'git checkout -b O works only if tree is clean' \
+	'git-repo-config checkout.requireCleanDirectory true
+	 echo atest >atest
+	 git add atest
+	 git-checkout -b O'
+
+test_expect_success \
+	'git checkout -m -b O works' \
+	'git-checkout -m -b O'
+
 test_done
Junio C Hamano· Dec 7, 2006, 19:38 UTC · re: Shawn O. Pearce · lore

Re: [PATCH 2/2] Optionally check for uncommitted changes before switching branches.

"Shawn O. Pearce" <spearce@spearce.org> writes:
> Git shouldn't cause the user to make mistakes when it can help to
> prevent them.  So now users may set checkout.requireCleanDirectory
> to true in their config file to have git-checkout verify the working
> directory is clean before switching branches.

A lot of times the reason to switch branches is because the user starts to make a trivial change in the worktree and realizes that the change belongs to another branch. Other times it is done by mistake and making it easier to notice that mistake is a laudable goal.

Your patch allows -m to override this, but that destroys one very useful feature of -m. In the above "ah, this trivial thing belongs to the other branch, so let's switch to the branch to commit only that trivial piece and come back to the current branch to continue what I'm doing" workflow, I usually first say "git checkout" without -m to switch, and if it does not allow me to switch, it is an indication that "the trivial thing" I thought was trivial was not trivial. I take it as a cue that I should instead do it "the right way" (i.e. finish or stash away what I am doing, switch to the branch in a clean state and fix it properly).

Other times, when all (or most of) the changes in the work tree logically should have started on a different branch, I do force it with -m (and take the conflict markers in my worktree), but being able to forbid the worktree merge by not giving -m is important.

People with this new configuration set and has to override it with a command line switch will lose this ability if you overload that on '-m'.

Shawn Pearce· Dec 7, 2006, 19:43 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/2] Optionally check for uncommitted changes before switching branches.

Junio C Hamano <junkio@cox.net> wrote:
Show 6 quoted lines
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> 
> > Git shouldn't cause the user to make mistakes when it can help to
> > prevent them.  So now users may set checkout.requireCleanDirectory
> > to true in their config file to have git-checkout verify the working
> > directory is clean before switching branches.
[...snip...]
> People with this new configuration set and has to override it
> with a command line switch will lose this ability if you
> overload that on '-m'.
Yes.  That's a problem with this patch.

What about when this option is enabled then -m means do what we did before, and -m -m (or -mm, or -m2) does what -m does when the option is false?

Junio C Hamano· Dec 7, 2006, 23:01 UTC · re: Shawn Pearce · lore

Re: [PATCH 2/2] Optionally check for uncommitted changes before switching branches.

Shawn Pearce <spearce@spearce.org> writes:
> What about when this option is enabled then -m means do what we
> did before, and -m -m (or -mm, or -m2) does what -m does when the
> option is false?
Wouldn't --force be more appropriate?

← back to recent threads