Re: [PATCH 2/5] refs: make `is_pseudoref_syntax()` stricter

[Junio C Hamano <gitster@pobox.com>]

Karthik Nayak <karthik.188@gmail.com> writes:

> Using this information we make the `is_pseudoref_syntax()` function
> stricter, by adding the check for "HEAD" suffix and for refs which don't
> end with the HEAD suffix, matching them with a predetermined list.

OK, so this partly answers my question on the previous step.  Before
making it more strict, the function worked only on the "syntax", so
a random string that can be a pseudo ref passed the check.

But stepping back a bit, if we call this function is_pseudoref_syntax(),
wouldn't it be what we want to have anyway?  You seem to want a
separate function called is_pseudoref() that rejects bogus uppercase
string "FOO_BAR" while accepting the known-good pseudoref you add
tests for, plus the ${FOO}_HEAD for any value of ${FOO} we currently
have and we may want to add in the future.

>  int is_pseudoref_syntax(const char *refname)
>  {
> +	/* TODO: move these pseudorefs to have _HEAD suffix */
> +	static const char *const irregular_pseudorefs[] = {
> +		"BISECT_EXPECTED_REV",
> +		"NOTES_MERGE_PARTIAL",
> +		"NOTES_MERGE_REF",
> +		"AUTO_MERGE"
> +	};
> +	size_t i;
>  	const char *c;
>  
>  	for (c = refname; *c; c++) {
> @@ -837,10 +845,17 @@ int is_pseudoref_syntax(const char *refname)
>  	}
>  
>  	/*
> -	 * HEAD is not a pseudoref, but it certainly uses the
> -	 * pseudoref syntax.
> +	 * Most pseudorefs end with _HEAD. HEAD itself is not a
> +	 * pseudoref, but it certainly uses the pseudoref syntax.
>  	 */
> -	return 1;
> +	if (ends_with(refname, "HEAD"))
> +		return 1;

I would imagine that at the final stage in which something like this
will be named is_pseudoref(), asking is_pseudoref("HEAD") would
return "No" (even though "is_pseudoref_syntax()", if the helper
function remains, may say "Yes" to "HEAD").  And this ends_with()
will use "_HEAD", instead of "HEAD".  But I am reading ahead of
myself, so let's keep going.

> +	for (i = 0; i < ARRAY_SIZE(irregular_pseudorefs); i++)
> +		if (!strcmp(refname, irregular_pseudorefs[i]))
> +			return 1;
> +
> +	return 0;
>  }