Re: [PATCH] pack-protocol: document newline behavior in push commands

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 3, 2015, 22:17 UTC
Message-ID: <xmqq4mjbi1w1.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20150903080205.GA20182@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> Frankly, this feels a bit like a step backwards to me. I am tempted to
> suggest instead that git start sending the newlines, but I'm not sure
> it's worth any potential fallout.

I actually think we should do both in the longer term.

If we say sender "SHOULD" and we know no existing receiver violates the "MUST NOT reject", our sender should follow that "SHOULD".

This documentation update is good in that it makes the examples easier to read (by the way, the first pre-context line ends with '\n', which we would eventually also address) by making the reader understand that the convention used in this S:/C: exchange illustration the optional LF is not shown.

Show 6 quoted lines

> I'm also tempted to scrap this and say it just falls under the rule
> that every PKT-LINE is "sender SHOULD include LF" and "receiver MUST NOT
> complain about missing LF" (which does appear earlier in the document,
> though in a different context). Or maybe we should write out that rule
> explicitly and drop the "LF" from all parts of the grammar (which is
> what the thread above advocates).

Hmm, I view this patch as a first step in that direction.

Show 29 quoted lines

>
>  Documentation/technical/pack-protocol.txt | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt
> index 4064fc7..9ce53b4 100644
> --- a/Documentation/technical/pack-protocol.txt
> +++ b/Documentation/technical/pack-protocol.txt
> @@ -469,8 +469,8 @@ references.
>  
>    shallow           =  PKT-LINE("shallow" SP obj-id LF)
>  
> -  command-list      =  PKT-LINE(command NUL capability-list LF)
> -		       *PKT-LINE(command LF)
> +  command-list      =  PKT-LINE(command NUL capability-list)
> +		       *PKT-LINE(command)
>  		       flush-pkt
>  
>    command           =  create / delete / update
> @@ -586,8 +586,8 @@ An example client/server communication might look like this:
>     S: 003f74730d410fcb6603ace96f1dc55ea6196122532d refs/heads/team\n
>     S: 0000
>  
> -   C: 003e7d1665144a3a975c05f1f43902ddaf084e784dbe 74730d410fcb6603ace96f1dc55ea6196122532d refs/heads/debug\n
> -   C: 003e74730d410fcb6603ace96f1dc55ea6196122532d 5a3f6be755bbb7deae50065988cbfa1ffa9ab68a refs/heads/master\n
> +   C: 003e7d1665144a3a975c05f1f43902ddaf084e784dbe 74730d410fcb6603ace96f1dc55ea6196122532d refs/heads/debug
> +   C: 003e74730d410fcb6603ace96f1dc55ea6196122532d 5a3f6be755bbb7deae50065988cbfa1ffa9ab68a refs/heads/master
>     C: 0000
>     C: [PACKDATA]