[buglet] misleading error message from git checkout

[buglet] misleading error message from git checkout

[Nanako Shiraishi]

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/

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

[Daniel Cordero]

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

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

[Junio C Hamano]

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?

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

[Nanako Shiraishi]

Signed-off-by: しらいしななこ <nanako3@lavabit.com>
---

Quoting Junio C Hamano <gitster@pobox.com>:

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