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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Johan Herland <johan@xxxxxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]