Johan Herland <jo...@herland.net> writes:

> The refname_expand() function no longer uses mkpath()/mksnpath() to
> perform the pattern expansion. Instead, it uses strbuf_expand(), which
> removes the need for using fixed-length buffers from the code.

It is a brilliant idea to use strbuf_expand() for this. I like it.

I notice that you later introduce %1 (that is 'one', not 'el'), but
unless you are planning to introduce %2 and %3 that semantically
fall into a similar category as %1, I would rather see a different
letter used that is mnemonic to what the placeholder _means_.  

The choice of the letter is arbitrary and may not look like it
matters that much, because it is not exposed to the end user.  But
by switching from the sprintf() semantics that shows things given to
it in the order they were given, without knowing what they mean, and
introducing a strbuf_expand() machinery tailored for refnames (and
refnames only), the new code assigns meanings to each part of the
refname, and we can afford to be more descriptive.

The choice of '%*' is justifiable, "it is the closest to the '*' we
traditionally used to replace only one thing", but '%1' does not
look the best placeholder to use, at least to me.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to