On Mon, May 13, 2013 at 10:34 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Junio C Hamano <gitster@xxxxxxxxx> writes:
>> Johan Herland <johan@xxxxxxxxxxx> 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"
When it comes to multi-level remote names, I guess I could drop the
patch that disallows them, and still just have "%1" only map to the
first component (i.e. "foo/bar/baz" would always be interpreted as %1
= "foo", and never %1 = "foo/bar"). This would mean that the
"foo/bar/baz" shorthand notation would simply not work against
remote-tracking branch "baz" from remote "foo/bar", but we might say
that's ok, because (a) multi-level remote names are so rare, and (b)
the simple workaround of explicitly saying
"refs/peers/foo/bar/heads/baz" would always be available in any case.
...Johan
[1]: Maybe we could use '%N+' to mean "everything starting from
component #N"? Then we could construct the following dictionary the
shorthand "foo/bar/baz":
"*" -> "foo/bar/baz"
"1" -> "foo"
"1+" -> "foo/bar/baz"
"2" -> "bar"
"2+" -> "bar/baz"
etc.
But I really think this is overkill. Avoiding having to write our own
expansion helper is not _that_ important.
--
Johan Herland, <johan@xxxxxxxxxxx>
www.herland.net
--
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