On Mon, May 13, 2013 at 6:56 AM, Junio C Hamano <gits...@pobox.com> wrote: > 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.
Obviously, I named it '%1' since it expands into the _first_ component of the (slash-separated) shorthand. There is no further parsing or verification that it actually corresponds to a remote (and as far as I currently understand, we do not want to do such verification), so I thought it better not to make such assumptions in the placeholder name. That said, I could go with '%r' for "remote", although we have plenty of other concepts in Git that use 'r' as the initial letter. I could maybe use '%remote' instead? Also, about the '%*': When used alone, it means "the entire shorthand", but when preceded with a '%1' it subtly changes meaning into 'the remainder of the shorthand after extracting the first component'. I believe the two interpretations are compatible and unambiguous, but if we want to be very explicit about what's happening, we could use something like '%all' and '%the_rest' for the two cases, respectively? ...Johan -- Johan Herland, <jo...@herland.net> www.herland.net -- 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
