# [buglet] misleading error message from git checkout 4 messages from 2009-05-16 to 2009-05-17. Participants: Nanako Shiraishi, Daniel Cordero, Junio C Hamano. Thread: https://gitlist.dev/t/19383 ## Nanako Shiraishi, 2009-05-16 10:25 Subject: [buglet] misleading error message from git checkout Message-ID: <20090516192531.6117@nanako3.lavabit.com> URL: https://gitlist.dev/e/20090516192531.6117%40nanako3.lavabit.com ``` 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, 2009-05-16 17:54 Subject: [PATCH] builtin-checkout: Don't tell user that HEAD has moved before it has Message-ID: <20090516175444.GA1000@cumin.applehq.eu> URL: https://gitlist.dev/e/20090516175444.GA1000%40cumin.applehq.eu In-Reply-To: <20090516192531.6117@nanako3.lavabit.com> ``` 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 --- 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, 2009-05-16 19:57 Subject: Re: [PATCH] builtin-checkout: Don't tell user that HEAD has moved before it has Message-ID: <7vr5yolt1j.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vr5yolt1j.fsf%40alter.siamese.dyndns.org In-Reply-To: <20090516175444.GA1000@cumin.applehq.eu> ``` Daniel Cordero 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 Thanks; the patch looks obviously good. Can we have tests? ``` ## Nanako Shiraishi, 2009-05-17 02:43 Subject: [PATCH] test: checkout shouldn't say that HEAD has moved if it didn't Message-ID: <20090517114308.6117@nanako3.lavabit.com> URL: https://gitlist.dev/e/20090517114308.6117%40nanako3.lavabit.com In-Reply-To: <7vr5yolt1j.fsf@alter.siamese.dyndns.org> ``` Signed-off-by: しらいしななこ --- Quoting Junio C Hamano : > Daniel Cordero 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 > > 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/ ```