Subject: Re: [RFC/WIP PATCH 11/11] Document protocol version 2

From: Junio C Hamano <[email-masked]>
Date: Fri, 29 May 2015 14:52:14 -0700

Stefan Beller <[email-masked]> writes:

>>> +Capability discovery (v2)
>>> +-------------------------
>>> ...
>>> +  capability-list  =  *(capability) [agent LF] flush-pkt
>>> +  capability       =  PKT-LINE("capability:" keyvaluepair LF)
>>> +  agent            =  keyvaluepair LF
>>> +  keyvaluepair     =  1*(LC_ALPHA / DIGIT / "-" / "_" / "=")
>>
>> What is the "=" doing there?  If you meant to cover things like
>> "lang=en" with this, I do not think it is a good idea.  Rather, it
>> should be more like this:
>>
>>         capability = 1*(LC_ALPHA / DIGIT / "-" / "_") [ "=" value ]
>>         value = 0*( any octet other than LF, NUL )
>>
>> in order to leave us wiggle room to have more than very limited
>> subset of US-ASCII in 'value'.  I suspect that we may want to allow
>> anything other than LF (unlike v1 that allowed anything other than
>> SP and LF).
>
> Currently we can do a = as part of the line after the first ref, such as
>
>     symref=HEAD:refs/heads/master agent=git/2:2.4.0
>
> so I thought we want to keep this.

I do not understand that statement.

Capability exchange in v2 is one packet per cap, so the above
example would be expressed as:

	symref=HEAD:refs/heads/master
        agent=git/2:2.4.0

right?  Your "keyvaluepair" is limited to [a-z0-9-_=]*, and neither
of the above two can be expressed with that, which was why I said
you need two different set of characters before and after "=".  Left
hand side of "=" is tightly limited and that is OK.  Right hand side
may contain characters like ':', '.' and '/', so your alphabet need
to be more lenient, even in v1 (which I would imagine would be "any
octet other than SP, LF and NUL").