Re: [PATCH 4/5] refs: introduce `refs_for_each_all_refs()`

Karthik Nayak [off-list ref] writes:

On Fri, Jan 19, 2024 at 9:57 PM Junio C Hamano [off-list ref] wrote:

quoted

This looks more like add_pseudoref_entries() given that the general
direction is to have an "allow" list of pseudo refs (at this point
after the previous step of the series, is_pseudoref_syntax() is the
is_pseudoref() function, and uses ends_with("_HEAD") as a mere
optimization to avoid listing all the possible pseudo refs that
exists or will be added in the future whose name ends with "_HEAD").

Other than the naming, I think these two steps make sense.

I think overall the naming is correct, I would change the comments in
`is_pseudoref_syntax()`.

Because, apart from pseudorefs, we also want to print HEAD. This is also
why the pattern matches "HEAD" instead of "_HEAD". I'll add some more
comments to clarify this.

With the hardcoded "these are definitely pseudorefs" list in the
function, it no longer is is_pseudoref_SYNTAX() at all.  I would
rather prefer to see is_pseudoref() that says no to HEAD and have
the callers check

	-	if (is_pseudoref_syntax(foo))
	+	if (is_pseudoref(foo) || is_headref(foo))

than keeping the messy semantics we have.  My second preference is
to call it is_pseudoref_or_head() that says yes to "HEAD" and
pseudorefs, even though I like it much less.

Similarly, between giving the function under discussion a more
descriptive name add_pseudoref_and_head_entries(), or adding a new
function add_head_entry() to make the callers call add_head_entry()
and add_pseudoref_entries() separately, I have a slight preference
for the latter.

Thanks.