Re: [PATCH v5 2/2] revision: implement `git log --merge` also for rebase/cherry-pick/revert

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 26, 2024, 04:35 UTC
Message-ID: <xmqqa5nnj10v.fsf@gitster.g>
In-Reply-To: <20240225-ml-log-merge-with-cherry-pick-and-other-pseudo-heads-v5-2-af1ef2d9e44d@gmail.com>

Philippe Blain <levraiphilippeblain@gmail.com> writes:

Show 11 quoted lines

> +	for (i = 0; i < ARRAY_SIZE(other_head); i++)
> +		if (!read_ref_full(other_head[i],
> +				RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,
> +				oid, NULL)) {
> +			if (is_null_oid(oid))
> +				die(_("%s is a symbolic ref?"), other_head[i]);
> +			return other_head[i];
> +		}
> +
> +	die(_("--merge requires one of the pseudorefs MERGE_HEAD, CHERRY_PICK_HEAD, REVERT_HEAD or REBASE_HEAD"));
> +}

Just a minor nit, but reacting to recent "passive-aggressive" message change in another thread, perhaps we should stop asking a rhetorical question like the new message and instead state what we detected and what we consider is an error condition as a fact in them.

The last die() in the above helper function used to be such a rhetorical question "--merge without HEAD?" but now it reads much better. The one about symbolic ref is new in this series, and we can avoid making it rhetorical from the get go. Perhaps "%s exists but it is a symbolic ref" or something?