checkout: special case error messages during noop switching

"git checkout" ran with no branch and no pathspec behaves like
switching the branch to the current branch (in other words, a
no-op, except that it gives a side-effect "here are the modified
paths" report).  But unlike "git checkout HEAD" or "git checkout
main" (when you are on the 'main' branch), the user is much less
conscious that they are "switching" to the current branch.

This twists end-user expectation in a strange way.  There are
options (like "--ours") that make sense only when we are checking
out paths out of either the tree-ish or out of the index.  So the
error message the command below gives

    $ git checkout --ours
    fatal: '--ours/theirs' cannot be used with switching branches

is technically correct, but because the end-user may not even be
aware of the fact that the command they are issuing is about no-op
branch switching [*], they may find the error confusing.

Let's refactor the code to make it easier to special case the "no-op
branch switching" situation, and then customize the exact error
message for "--ours/--theirs".  Since it is more likely that the
end-user forgot to give pathspec that is required by the option,
let's make it say

    $ git checkout --ours
    fatal: '--ours/theirs' needs the paths to check out

instead.

Among the other options that are incompatible with branch switching,
there may be some that benefit by having messages tweaked when a
no-op branch switching is done, but I'll leave them as #leftoverbits
material.

[Footnote]

 * Yes, the end-users are irrational.  When they did not give
   "--ours", they take it granted that "git checkout" gives a short
   status, e.g..

    $ git checkout
    M	builtin/checkout.c
    M	t/t7201-co.sh

   exactly as a branch switching command.

Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 3a7bfde158801653932f4769927dd4ce717d9457..2ec873a61249c5ca5f8f9bad3f681cd503a48d84 100644 (file)
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1518,6 +1518,10 @@ static void die_if_some_operation_in_progress(void)
 static int checkout_branch(struct checkout_opts *opts,
                           struct branch_info *new_branch_info)
 {
+       int noop_switch = (!new_branch_info->name &&
+                          !opts->new_branch &&
+                          !opts->force_detach);
+
        if (opts->pathspec.nr)
                die(_("paths cannot be used with switching branches"));
 
@@ -1529,9 +1533,14 @@ static int checkout_branch(struct checkout_opts *opts,
                die(_("'%s' cannot be used with switching branches"),
                    "--[no]-overlay");
 
-       if (opts->writeout_stage)
-               die(_("'%s' cannot be used with switching branches"),
-                   "--ours/--theirs");
+       if (opts->writeout_stage) {
+               const char *msg;
+               if (noop_switch)
+                       msg = _("'%s' needs the paths to check out");
+               else
+                       msg = _("'%s' cannot be used with switching branches");
+               die(msg, "--ours/--theirs");
+       }
 
        if (opts->force && opts->merge)
                die(_("'%s' cannot be used with '%s'"), "-f", "-m");
@@ -1558,10 +1567,8 @@ static int checkout_branch(struct checkout_opts *opts,
                die(_("Cannot switch branch to a non-commit '%s'"),
                    new_branch_info->name);
 
-       if (!opts->switch_branch_doing_nothing_is_ok &&
-           !new_branch_info->name &&
-           !opts->new_branch &&
-           !opts->force_detach)
+       if (noop_switch &&
+           !opts->switch_branch_doing_nothing_is_ok)
                die(_("missing branch or commit argument"));
 
        if (!opts->implicit_detach &&

diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 10cc6c46051e95e8e8d52fae23396574f319b802..42acf965c8fedab0cf5bb4a4e79c2651bd863b3b 100755 (executable)
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -497,6 +497,19 @@ test_expect_success 'checkout unmerged stage' '
        test ztheirside = "z$(cat file)"
 '
 
+test_expect_success 'checkout --ours is incompatible with switching' '
+       test_must_fail git checkout --ours 2>error &&
+       test_grep "needs the paths to check out" error &&
+
+       test_must_fail git checkout --ours HEAD 2>error &&
+       test_grep "cannot be used with switching" error &&
+
+       test_must_fail git checkout --ours main 2>error &&
+       test_grep "cannot be used with switching" error &&
+
+       git checkout --ours file
+'
+
 test_expect_success 'checkout path with --merge from tree-ish is a no-no' '
        setup_conflicting_index &&
        test_must_fail git checkout -m HEAD -- file