Re: [PATCH 3/7] pack-protocol.txt: Mark all LFs in push-cert as required

From: Dave Borowitz <dborowitz@google.com>
Date: Jul 6, 2015, 18:08 UTC
Message-ID: <CAD0k6qQH1t2QzjQydjqzse+caib4Z+yCtJd9eDy=hBukxLKMhQ@mail.gmail.com>
In-Reply-To: <xmqq7fqd17qn.fsf@gitster.dls.corp.google.com>

On Mon, Jul 6, 2015 at 2:06 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 29 quoted lines

> Dave Borowitz <dborowitz@google.com> writes:
>
>> On Mon, Jul 6, 2015 at 1:34 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Dave Borowitz <dborowitz@google.com> writes:
>>>
>>>> Another way of looking at the problem with my assumptions is, I was
>>>> assuming "pkt-line framing" was the same thing as "pkt-line header".
>>>> You seem to be saying the definition of "pkt-line framing" is "header,
>>>> and optional trailing newline".
>>>
>>> Yes.  I thought that was what "Server SHOULD terminate with LF;
>>> client MUST NOT require it" in the existing text meant.
>>
>> Unfortunately, the existing text is littered with examples of
>> "PKT-LINE(foo SP bar LF)". If we assume "PKT-LINE(...)" means "apply
>> pkt-line framing to the [...]", then this strongly implies that
>> "pkt-line framing" does _not_ include the trailing LF. (Or the logical
>> but bizarre alternative reading that such an example might have _two_
>> trailing LFs :)
>
> Yes,  But I never viewed PKT-LINE() as an element that strictly
> defines the grammar of the packet protocol ;-)
>
> By clarifying that "sender SHOULD terminate with LF, receiver MUST
> NOT require it" is the rule (and fixing the existing implementations
> at places where they violate the "MUST NOT" part, which I think are
> very small number of places), I think we can drop these LF (or LF?
> for that matter) from all of the PKT-LINE() in the construction in
> the pack-protocol.txt, which would be a very good thing to do.

Completely agree, and that is what I meant when I said "The additional upside [to explicitly defining pkt-line framing in this way] is that we could then potentially remove all or almost all LFs from this document."

> The example in your sentence will become PKT-LINE(foo SP bar) and
> the "there may be an LF at the end" would only be at one place, as a
> part of the definition of PKT-LINE().