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

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 9, 2024, 20:53 UTC
Message-ID: <xmqqjzehq9qk.fsf@gitster.g>
In-Reply-To: <20241009135747.3563204-3-bence@ferdinandy.com>

Bence Ferdinandy <bence@ferdinandy.com> 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.

Show 9 quoted lines

> +	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").