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

> Philippe Blain <levraiphilippeblain@gmail.com> writes:
> 
>> +	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?