Thread (22 messages) flat view 22 messages, 4 authors, 2016-06-15

Re: [PATCHv2 03/10] refs.c: Refactor code for mapping between shorthand names and full refnames

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:57:13

Johan Herland [off-list ref] 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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help