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>

Junio C Hamano committed Jul 2, 2024 at 13:51 UTC d1e6c61272737926cd049a6126431fe9c12e4a54
2 files changed +27 -7
builtin/checkout.c
+14 -7
@@ -1518,6 +1518,10 @@ static void die_if_some_operation_in_progress(void)
1518 static int checkout_branch(struct checkout_opts *opts,
1519 struct branch_info *new_branch_info)
1520 {
1521 + int noop_switch = (!new_branch_info->name &&
1522 + !opts->new_branch &&
1523 + !opts->force_detach);
1524 +
1525 if (opts->pathspec.nr)
1526 die(_("paths cannot be used with switching branches"));
1527
@@ -1529,9 +1533,14 @@ static int checkout_branch(struct checkout_opts *opts,
1533 die(_("'%s' cannot be used with switching branches"),
1534 "--[no]-overlay");
1535
1532 - if (opts->writeout_stage)
1533 - die(_("'%s' cannot be used with switching branches"),
1534 - "--ours/--theirs");
1536 + if (opts->writeout_stage) {
1537 + const char *msg;
1538 + if (noop_switch)
1539 + msg = _("'%s' needs the paths to check out");
1540 + else
1541 + msg = _("'%s' cannot be used with switching branches");
1542 + die(msg, "--ours/--theirs");
1543 + }
1544
1545 if (opts->force && opts->merge)
1546 die(_("'%s' cannot be used with '%s'"), "-f", "-m");
@@ -1558,10 +1567,8 @@ static int checkout_branch(struct checkout_opts *opts,
1567 die(_("Cannot switch branch to a non-commit '%s'"),
1568 new_branch_info->name);
1569
1561 - if (!opts->switch_branch_doing_nothing_is_ok &&
1562 - !new_branch_info->name &&
1563 - !opts->new_branch &&
1564 - !opts->force_detach)
1570 + if (noop_switch &&
1571 + !opts->switch_branch_doing_nothing_is_ok)
1572 die(_("missing branch or commit argument"));
1573
1574 if (!opts->implicit_detach &&
t/t7201-co.sh
+13
@@ -497,6 +497,19 @@ test_expect_success 'checkout unmerged stage' '
497 test ztheirside = "z$(cat file)"
498 '
499
500 +test_expect_success 'checkout --ours is incompatible with switching' '
501 + test_must_fail git checkout --ours 2>error &&
502 + test_grep "needs the paths to check out" error &&
503 +
504 + test_must_fail git checkout --ours HEAD 2>error &&
505 + test_grep "cannot be used with switching" error &&
506 +
507 + test_must_fail git checkout --ours main 2>error &&
508 + test_grep "cannot be used with switching" error &&
509 +
510 + git checkout --ours file
511 +'
512 +
513 test_expect_success 'checkout path with --merge from tree-ish is a no-no' '
514 setup_conflicting_index &&
515 test_must_fail git checkout -m HEAD -- file