Subject: Re: [PATCH v5 3/6] set-head: better output for --auto

From: Junio C Hamano <[email-masked]>
Date: Wed, 09 Oct 2024 13:53:07 -0700

Bence Ferdinandy <[email-masked]> writes:

> +static void report_auto(const char *remote, const char *head_name,
> +			struct strbuf *buf_prev) {
> +	struct strbuf buf_prefix = STRBUF_INIT;
> +	const char *prev_head;

I think we need to initialize prev_head to NULL.

> +	strbuf_addf(&buf_prefix, "refs/remotes/%s/", remote);
> +	skip_prefix(buf_prev->buf, buf_prefix.buf, &prev_head);

If the symref was not pointing into the "refs/remotes/<remote>/"
hierarchy previously, skip_prefix() comes back without touching
prev_head (i.e. not starting with the prefix does not clear it).

Assuming that we fix the initialization, the rest of the function
looks more or less correct.

> +	if (prev_head && !strcmp(prev_head, head_name))
> +		printf("'%s/HEAD' is unchanged and points to '%s'\n",
> +			remote, head_name);
> +	else if (prev_head)
> +		printf("'%s/HEAD' has changed from '%s' and now points to '%s'\n",
> +			remote, prev_head, head_name);
> +	else
> +		printf("'%s/HEAD' is now created and points to '%s'\n",
> +			remote, head_name);

The "more or less" part is that the message does not let you tell
between refs/remotes/<name>/HEAD that did not exist, and
refs/remotes/<name>/HEAD that used to point at somewhere unexpected,
outside refs/remotes/<name>/ hierarchy.

For that, we can check if buf_prev->len is 0 (in which case, the
"now created and points at" applies) or non-zero (in which case, we
say something like "used to point at '%s' (which is unusual), but
now points at '%s' as requested").