threads / discuss / 19383

[buglet] misleading error message from git checkout

Subject: [buglet] misleading error message from git checkout

## tl;dr

4 messages between May 16, 2009 and May 17, 2009.

replies: 3people: 3as markdown or json

Nanako Shiraishi· May 16, 2009, 10:25 UTC · lore
When you aren't on any branch, have changes in your working files, and try to switch to another branch, you get an error message like this:
 % git checkout another
 Previous HEAD position was 0beee4c... git-add--interactive: remove hunk coalescing
 error: You have local changes to 'git-add--interactive.perl'; cannot switch branches.
Because the first line says "Previous ... was", I was mislead to believe that I sucsessfully switched branches, but I was mistaken.
If git-checkout were still a shell script probably I would have looked into fixing this myself, but now it is rewritten in C... Can somebody fix this please?
-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/
Daniel Cordero· May 16, 2009, 17:54 UTC · re: Nanako Shiraishi · lore

[PATCH] builtin-checkout: Don't tell user that HEAD has moved before it has

Previously, checkout would tell the user this message before moving HEAD, without regard to whether the upcoming move will result in success. If the move failed, this causes confusion.

Show the message after the move, unless the move failed.
Signed-off-by: Daniel Cordero <theappleman@gmail.com>
---
 builtin-checkout.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/builtin-checkout.c b/builtin-checkout.c
index dc4bfb5..f2d7ef0 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -541,14 +541,6 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new)
 		parse_commit(new->commit);
 	}
 
-	/*
-	 * If we were on a detached HEAD, but we are now moving to
-	 * a new commit, we want to mention the old commit once more
-	 * to remind the user that it might be lost.
-	 */
-	if (!opts->quiet && !old.path && old.commit && new->commit != old.commit)
-		describe_detached_head("Previous HEAD position was", old.commit);
-
 	if (!old.commit && !opts->force) {
 		if (!opts->quiet) {
 			warning("You appear to be on a branch yet to be born.");
@@ -561,6 +553,14 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new)
 	if (ret)
 		return ret;
 
+	/*
+	 * If we were on a detached HEAD, but have now moved to
+	 * a new commit, we want to mention the old commit once more
+	 * to remind the user that it might be lost.
+	 */
+	if (!opts->quiet && !old.path && old.commit && new->commit != old.commit)
+		describe_detached_head("Previous HEAD position was", old.commit);
+
 	update_refs_for_switch(opts, &old, new);
 
 	ret = post_checkout_hook(old.commit, new->commit, 1);
-- 
1.6.3.1.93.g8d138
Junio C Hamano· May 16, 2009, 19:57 UTC · re: Daniel Cordero · lore

Re: [PATCH] builtin-checkout: Don't tell user that HEAD has moved before it has

Daniel Cordero <theappleman@gmail.com> writes:
Show 7 quoted lines
> Previously, checkout would tell the user this message before moving HEAD,
> without regard to whether the upcoming move will result in success.
> If the move failed, this causes confusion.
>
> Show the message after the move, unless the move failed.
>
> Signed-off-by: Daniel Cordero <theappleman@gmail.com>
Thanks; the patch looks obviously good.  Can we have tests?
Nanako Shiraishi· May 17, 2009, 02:43 UTC · re: Junio C Hamano · lore

[PATCH] test: checkout shouldn't say that HEAD has moved if it didn't

Signed-off-by: しらいしななこ <nanako3@lavabit.com>
---
Quoting Junio C Hamano <gitster@pobox.com>:
Show 11 quoted lines
> Daniel Cordero <theappleman@gmail.com> writes:
>
>> Previously, checkout would tell the user this message before moving HEAD,
>> without regard to whether the upcoming move will result in success.
>> If the move failed, this causes confusion.
>>
>> Show the message after the move, unless the move failed.
>>
>> Signed-off-by: Daniel Cordero <theappleman@gmail.com>
>
> Thanks; the patch looks obviously good.  Can we have tests?
Here is one.
 t/t7201-co.sh |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index bdb808a..ebfd34d 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -534,4 +534,12 @@ test_expect_success 'failing checkout -b should not break working tree' '
 
 '
 
+test_expect_success 'switch out of non-branch' '
+	git reset --hard master &&
+	git checkout master^0 &&
+	echo modified >one &&
+	test_must_fail git checkout renamer 2>error.log &&
+	! grep "^Previous HEAD" error.log
+'
+
 test_done
-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

← back to recent threads