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

[Michael Haggerty <mhagger@alum.mit.edu>]

On 05/14/2013 04:24 PM, Johan Herland wrote:
> On Mon, May 13, 2013 at 10:34 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Junio C Hamano <gitster@pobox.com> writes:
>>> Johan Herland <johan@herland.net> writes:
>>>> Obviously, I named it '%1' since it expands into the _first_ component
>>>> of the (slash-separated) shorthand.
>>>
>>> OK, I can buy something like
>>>
>>>       %*
>>>         refs/%*
>>>         refs/heads/%*
>>>         ...
>>>         refs/remotes/%*/HEAD
>>>         refs/remotes/%1/%2
>>>         refs/peers/%1/heads/%2
>>>
>>> that is, for a pattern that has %*, we feed the end-user string as a
>>> whole, and for a pattern that has %1 thru %N, we find an appropriate
>>> way to chop the end-user string into N pieces (e.g. nick/name would
>>> be split into %1 = nick, %2 = name, while foo/bar/baz might have two
>>> possibilities, <%1, %2> = <foo, bar/baz> or <foo/bar, baz>).  The
>>> earlier ones on the above list can even be written with their %*
>>> substituted with %1 if we go that route.
>>
>> Just to make sure.
>>
>> Please do not let "two possibilities" stop you.  As I said in the
>> nearby thread, I do not necessarily insist that we must try all N
>> possibilities.  "We find an appropriate way" could be just "we
>> always chop at the first slash, and %1 is what comes before it, %2
>> is what comes after it".
>>
>> That will close the possibility for us to use %1 thru %N (N is
>> limited to 2), but it still is "We have %1 and we have %2, both fall
>> into the same 'path components, numbered from left to right'
>> category", and justifies the use of %1 ("one", not "el").
>>
>> So still no objection to %1 from me.
> 
> I think I like "refs/peers/%1/heads/%*" better than
> "refs/peers/%1/heads/%2", since the latter sort of makes me wonder
> whether the 3rd, 4th, etc. components would be discarded. That said,
> having %* mean "the rest of the shorthand" means that we must adjust
> the expansion of %* for every preceding %N, which prevents us from
> simplifying the code by using strbuf_expand_dict_cb() with a static
> dictionary [1].
> 
> I am not sure why we would want "refs/remotes/%1/%2" instead of
> "refs/remote/%*". Maybe I've been staring at this for too long, but I
> find the latter shorter and more descriptive and the "%1/%2" notation
> needlessly cumbersome, especially if it's also supposed to match
> "foo/bar/baz"

"refs/remotes/%1/%2" (or "refs/remotes/%1/%*") might be a nice way to
imply that the rule should only be attempted if the input has at least
two components, whereas something like "refs/heads/%*" would be applied
even for inputs with no slashes.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/