Subject: Re: [PATCH v2 1/3] fetch set_head: move warn advice into advise_if_enabled

From: Junio C Hamano <[email-masked]>
Date: Thu, 05 Dec 2024 04:28:41 +0900

Bence Ferdinandy <[email-masked]> writes:

> +static void set_head_advice_msg(const char *remote, const char *head_name)
> +{
> +	const char message_advice_set_head[] =
> +	N_("Run 'git remote set-head %s %s' to follow the change, or set\n"
> +	   "'remote.%s.followRemoteHEAD' configuration option to a different value\n"
> +	   "if you do not want to see this message.");
> +
> +	advise_if_enabled(ADVICE_FETCH_SET_HEAD_WARN, _(message_advice_set_head),
> +			remote, head_name, remote);
> +}
> ...
>  	else if (updateres && buf_prev->len) {
>  		printf("'HEAD' at '%s' is '%s', "
>  			"but we have a detached HEAD pointing to '%s' locally.\n",
>  			remote, head_name, buf_prev->buf);
> -		printf("Run 'git remote set-head %s %s' to follow the change.\n",
> -			remote, head_name);
> +		set_head_advice_msg(remote, head_name);
>  	}
>  	strbuf_release(&buf_prefix);
>  }

Looking good.

> diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
> index ccb74428bc..5d7ee1b550 100755
> --- a/t/t5510-fetch.sh
> +++ b/t/t5510-fetch.sh
> @@ -123,7 +123,6 @@ test_expect_success "fetch test followRemoteHEAD warn no change" '
>  		git fetch >output &&
>  		echo "${SQ}HEAD${SQ} at ${SQ}origin${SQ} is ${SQ}main${SQ}," \
>  			"but we have ${SQ}other${SQ} locally." >expect &&
> -		echo "Run ${SQ}git remote set-head origin main${SQ} to follow the change." >>expect &&
>  		test_cmp expect output &&

OK.  We just lose this part of the test, not because the advice
messages are squelched but because they go to the standard error
stream.  An obvious alternative is to capture it in a separate
file and grep, i.e.

	git fetch >output 2>error &&
	echo ... >expect &&
	test_grep "Run ${SQ}git remote set-head" error &&
	test_cmp expect output

If we were testing that the advice mechanism is used, do that with
and without the ADVICE_FETCH_SET_HEAD_WARN squelched, but probably
it is a bit too much, in the sense that we are not in the business
of testing that the advice mechanism works correctly in this series.