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

From: Junio C Hamano <gitster@pobox.com>
Date: May 29, 2015, 21:52 UTC
Message-ID: <xmqqk2vraw6p.fsf@gitster.dls.corp.google.com>
In-Reply-To: <CAGZ79kbELvZ6otnNwGHsgiC9EjgS2vrDGU2KCgF2c_Azm=-rWg@mail.gmail.com>

Stefan Beller <sbeller@google.com> writes:

Show 25 quoted lines

>>> +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").