From: Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com>
Date: Tue, 27 Jan 2026 20:35:43 GMT
Subject: Re: [PATCH 2/2] checkout: tell "parse_remote_branch" which command is calling it
Message-ID: <fa7f1648-3cf6-4e5f-bee9-fb5e8700d01d@app.fastmail.com>
In-Reply-To: <20260127192936.904719-3-gitster@pobox.com>

On Tue, Jan 27, 2026, at 20:29, Junio C Hamano wrote:
> When "git checkout <dwim>" and "git switch <dwim>" need to error out
> due to ambiguity of the branch name <dwim>, these command give an

s/these command/these two commands/ or something.

> advise message that tells a sample command to show how to
> disambiguate from the parse_remote_branch() function.  The sample
> command hardcodes "git checkout", since this feature predates "git
> switch" by a large margin.  To a user who said "git switch <dwim>"
> and got this message, it is confusing.
>
> Pass the "enum checkout_command", which was invented in the previous
> step for this exact purpose, down the call chain leading to
> parse_remote_branch() function to change the sample command shown to
> the user in this advise message.

The commit message could also say that it adds a regression test for
this advice (for the first time).

>
> Reported-by: Simon Cheng <cyqsimon@gmail.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  builtin/checkout.c        | 29 ++++++++++++++++++++++++-----
>  t/t2027-checkout-track.sh | 21 +++++++++++++++++++++
>  2 files changed, 45 insertions(+), 5 deletions(-)
>[snip]
> diff --git a/t/t2027-checkout-track.sh b/t/t2027-checkout-track.sh
> index a397790df5..e9f8d8ec48 100755
> --- a/t/t2027-checkout-track.sh
> +++ b/t/t2027-checkout-track.sh
> @@ -47,4 +47,25 @@ test_expect_success 'checkout --track -b overrides
> autoSetupMerge=inherit' '
>  	test_cmp_config refs/heads/main branch.b4.merge
>  '
>
> +test_expect_success 'ambiguous tracking info' '
> +	# Set up a few remote repositories
> +	git init --bare --initial-branch=trunk src1 &&
> +	git init --bare --initial-branch=trunk src2 &&
> +	git push src1 one:refs/heads/trunk &&
> +	git push src2 two:refs/heads/trunk &&
> +
> +	git remote add -f src1 "file://$PWD/src1" &&
> +	git remote add -f src2 "file://$PWD/src2" &&
> +
> +	# DWIM
> +	test_must_fail git checkout trunk 2>hint &&
> +	test_grep "hint: *git checkout --track" hint &&
> +	test_grep ! "hint: *git switch --track" hint &&
> +
> +	{ git update-ref -d refs/heads/trunk || :; } &&

I don’t understand what the purpose of this is after `git checkout` but
before `git switch`. I can delete it and the test still passes. Is it
post-test cleanup?

> +	test_must_fail git switch trunk 2>hint &&
> +	test_grep ! "hint: *git checkout --track" hint &&
> +	test_grep "hint: *git switch --track" hint
> +'

Maybe just the positive greps are enough. I read these a few times
because I thought the order was wrong, i.e. that `hint` was overwritten
before it got tested. The regression that they test are unlikely and
these negative greps might not make immediate sense for future
readers. I dunno.

I was about to suggest parameterizing the command but that makes it
harder to grep for that part of the advice.

    checkout_with_cmd () {
            cmd="$1"
            test_must_fail git $cmd trunk 2>hint &&
            test_grep --fixed-string \
                      "hint:     git $cmd --track origin/<name>" hint
    }


> +
>  test_done
> --
> 2.53.0-rc2-135-gb1217c0133