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

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

[Jeff King]

Our pack-protocol spec indicates that a pushing client
should send ref update commands like:

  $old_sha1 $new_sha1 $ref\n

with each ref update in its own pktline, with a trailing
newline. However, git itself does not follow this behavior;
it omits the trailing newline.

For the most part the distinction is harmless. The
`receive-pack` on the other end will call
`packet_read_line`, which strips off the newline if it is
there, and we are fine either way.

Things are more complicated for the initial ref, which also
has capabilities. The spec says to send:

  $old_sha1 $new_sha1 $ref\0 $caps\n

which is also OK by the current `receive-pack code (the
newline is at the end of the packet, so we still strip it).

As an aside, it would _not_ be OK to send:

  $old_sha1 $new_sha1 $ref\n\0 $caps\n

The spec does not allow that and receive-pack would reject
it, but it is perhaps a mistake that a naive client
implementation might make.

So what is in the current spec is quite reasonable, and
simply follows the normal rules for pkt-line framing (we say
LF, but it really is optional). But it does not document
how git actually behaves.

Signed-off-by: Jeff King <peff@peff.net>
---
+cc Junio, Dave, and Shawn, as this is somewhat related to the
discussion in
http://thread.gmane.org/gmane.comp.version-control.git/273175/focus=273444

I happened to be looking into some protocol stuff recently and noticed
that we do not follow the spec here (but interestingly, libgit2 does!).

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'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).

 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]
 
-- 
2.5.1.812.ge796bff

[PATCH] pack-protocol: clarify LF-handling in PKT-LINE()

[Jeff King]

On Thu, Sep 03, 2015 at 04:02:05AM -0400, Jeff King wrote:

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

Hmph, I just noticed that patch 2/7 from Dave's original series is
essentially the same as what I just sent.

Since the discussion seemed to end up in this "let's make PKT-LINE more
clear" direction, here is an attempt to do that.

-- >8 --
Subject: pack-protocol: clarify LF-handling in PKT-LINE()

The spec is very inconsistent about which PKT-LINE() parts
of the grammar include a LF. On top of that, the code is not
consistent, either (e.g., send-pack does not put newlines
into the ref-update commands it sends).

Let's make explicit the long-standing expectation that we
generally expect pkt-lines to end in a newline, but that
receivers should be lenient. This makes the spec consistent,
and matches what git already does (though it does not always
fulfill the SHOULD).

We do make an exception for the push-cert, where the
receiving code is currently a bit pickier. This is a
reasonable way to be, as the data needs to be byte-for-byte
compatible with what was signed. We _could_ make up some
rules about signing a canonicalized version including
newlines, but that would require a code change, and is out
of scope for this patch.

Signed-off-by: Jeff King <peff@peff.net>
---
 Documentation/technical/pack-protocol.txt   | 46 ++++++++++++++++-------------
 Documentation/technical/protocol-common.txt |  5 +++-
 2 files changed, 30 insertions(+), 21 deletions(-)

diff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt
index 4064fc7..c6977bb 100644
--- a/Documentation/technical/pack-protocol.txt
+++ b/Documentation/technical/pack-protocol.txt
@@ -14,6 +14,14 @@ data.  The protocol functions to have a server tell a client what is
 currently on the server, then for the two to negotiate the smallest amount
 of data to send in order to fully update one or the other.
 
+pkt-line Format
+---------------
+
+The descriptions below build on the pkt-line format described in
+protocol-common.txt. When the grammar indicate `PKT-LINE(...)`, unless
+otherwise noted the usual pkt-line LF rules apply: the sender SHOULD
+include a LF, but the receiver MUST NOT complain if it is not present.
+
 Transports
 ----------
 There are three transports over which the packfile protocol is
@@ -143,9 +151,6 @@ with the object name that each reference currently points to.
    003fe92df48743b7bc7d26bcaabfddde0a1e20cae47c refs/tags/v1.0^{}
    0000
 
-Server SHOULD terminate each non-flush line using LF ("\n") terminator;
-client MUST NOT complain if there is no terminator.
-
 The returned response is a pkt-line stream describing each ref and
 its current value.  The stream MUST be sorted by name according to
 the C locale ordering.
@@ -165,15 +170,15 @@ MUST peel the ref if it's an annotated tag.
 		      flush-pkt
 
   no-refs          =  PKT-LINE(zero-id SP "capabilities^{}"
-		      NUL capability-list LF)
+		      NUL capability-list)
 
   list-of-refs     =  first-ref *other-ref
   first-ref        =  PKT-LINE(obj-id SP refname
-		      NUL capability-list LF)
+		      NUL capability-list)
 
   other-ref        =  PKT-LINE(other-tip / other-peeled)
-  other-tip        =  obj-id SP refname LF
-  other-peeled     =  obj-id SP refname "^{}" LF
+  other-tip        =  obj-id SP refname
+  other-peeled     =  obj-id SP refname "^{}"
 
   shallow          =  PKT-LINE("shallow" SP obj-id)
 
@@ -216,8 +221,8 @@ out of what the server said it could do with the first 'want' line.
 
   depth-request     =  PKT-LINE("deepen" SP depth)
 
-  first-want        =  PKT-LINE("want" SP obj-id SP capability-list LF)
-  additional-want   =  PKT-LINE("want" SP obj-id LF)
+  first-want        =  PKT-LINE("want" SP obj-id SP capability-list)
+  additional-want   =  PKT-LINE("want" SP obj-id)
 
   depth             =  1*DIGIT
 ----
@@ -284,7 +289,7 @@ so that there is always a block of 32 "in-flight on the wire" at a time.
 		       compute-end
 
   have-list         =  *have-line
-  have-line         =  PKT-LINE("have" SP obj-id LF)
+  have-line         =  PKT-LINE("have" SP obj-id)
   compute-end       =  flush-pkt / PKT-LINE("done")
 ----
 
@@ -348,10 +353,10 @@ Then the server will start sending its packfile data.
 
 ----
   server-response = *ack_multi ack / nak
-  ack_multi       = PKT-LINE("ACK" SP obj-id ack_status LF)
+  ack_multi       = PKT-LINE("ACK" SP obj-id ack_status)
   ack_status      = "continue" / "common" / "ready"
-  ack             = PKT-LINE("ACK SP obj-id LF)
-  nak             = PKT-LINE("NAK" LF)
+  ack             = PKT-LINE("ACK" SP obj-id)
+  nak             = PKT-LINE("NAK")
 ----
 
 A simple clone may look like this (with no 'have' lines):
@@ -467,10 +472,10 @@ references.
 ----
   update-request    =  *shallow ( command-list | push-cert ) [packfile]
 
-  shallow           =  PKT-LINE("shallow" SP obj-id LF)
+  shallow           =  PKT-LINE("shallow" SP obj-id)
 
-  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
@@ -521,7 +526,8 @@ Push Certificate
 
 A push certificate begins with a set of header lines.  After the
 header and an empty line, the protocol commands follow, one per
-line.
+line. Note that the the trailing LF in push-cert PKT-LINEs is _not_
+optional; it must be present.
 
 Currently, the following header fields are defined:
 
@@ -560,12 +566,12 @@ update was successful, or 'ng [refname] [error]' if the update was not.
 		      1*(command-status)
 		      flush-pkt
 
-  unpack-status     = PKT-LINE("unpack" SP unpack-result LF)
+  unpack-status     = PKT-LINE("unpack" SP unpack-result)
   unpack-result     = "ok" / error-msg
 
   command-status    = command-ok / command-fail
-  command-ok        = PKT-LINE("ok" SP refname LF)
-  command-fail      = PKT-LINE("ng" SP refname SP error-msg LF)
+  command-ok        = PKT-LINE("ok" SP refname)
+  command-fail      = PKT-LINE("ng" SP refname SP error-msg)
 
   error-msg         = 1*(OCTECT) ; where not "ok"
 ----
diff --git a/Documentation/technical/protocol-common.txt b/Documentation/technical/protocol-common.txt
index 889985f..bf30167 100644
--- a/Documentation/technical/protocol-common.txt
+++ b/Documentation/technical/protocol-common.txt
@@ -62,7 +62,10 @@ A pkt-line MAY contain binary data, so implementors MUST ensure
 pkt-line parsing/formatting routines are 8-bit clean.
 
 A non-binary line SHOULD BE terminated by an LF, which if present
-MUST be included in the total length.
+MUST be included in the total length. Receivers MUST treat pkt-lines
+with non-binary data the same whether or not they contain the trailing
+LF (stripping the LF if present, and not complaining when it is
+missing).
 
 The maximum length of a pkt-line's data component is 65520 bytes.
 Implementations MUST NOT send pkt-line whose length exceeds 65524
-- 
2.5.1.812.ge796bff

Re: [PATCH] pack-protocol: clarify LF-handling in PKT-LINE()

[Jeff King]

On Thu, Sep 03, 2015 at 04:24:09AM -0400, Jeff King wrote:

> diff --git a/Documentation/technical/protocol-common.txt b/Documentation/technical/protocol-common.txt
> index 889985f..bf30167 100644
> --- a/Documentation/technical/protocol-common.txt
> +++ b/Documentation/technical/protocol-common.txt
> @@ -62,7 +62,10 @@ A pkt-line MAY contain binary data, so implementors MUST ensure
>  pkt-line parsing/formatting routines are 8-bit clean.
>  
>  A non-binary line SHOULD BE terminated by an LF, which if present
> -MUST be included in the total length.
> +MUST be included in the total length. Receivers MUST treat pkt-lines
> +with non-binary data the same whether or not they contain the trailing
> +LF (stripping the LF if present, and not complaining when it is
> +missing).

I noticed that this section leaves vague the question of "what is a
binary pkt-line". All of the PKT-LINE calls I tweaked in the grammar are
for non-binary instances. The only binary ones are described in the
sideband discussion of the "Packfile data" section, and there we do not
have a grammar.

So I think the result is reasonably clear, but I have far too much
intimate knowledge of git's protocol to be a good judge.

-Peff

Re: [PATCH] pack-protocol: clarify LF-handling in PKT-LINE()

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Since the discussion seemed to end up in this "let's make PKT-LINE more
> clear" direction, here is an attempt to do that.

Ah, I was confused while reading the other one ;-)

Will queue this one instead.  Thanks.

> -- >8 --
> Subject: pack-protocol: clarify LF-handling in PKT-LINE()
>
> The spec is very inconsistent about which PKT-LINE() parts
> of the grammar include a LF. On top of that, the code is not
> consistent, either (e.g., send-pack does not put newlines
> into the ref-update commands it sends).
>
> Let's make explicit the long-standing expectation that we
> generally expect pkt-lines to end in a newline, but that
> receivers should be lenient. This makes the spec consistent,
> and matches what git already does (though it does not always
> fulfill the SHOULD).
>
> We do make an exception for the push-cert, where the
> receiving code is currently a bit pickier. This is a
> reasonable way to be, as the data needs to be byte-for-byte
> compatible with what was signed. We _could_ make up some
> rules about signing a canonicalized version including
> newlines, but that would require a code change, and is out
> of scope for this patch.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  Documentation/technical/pack-protocol.txt   | 46 ++++++++++++++++-------------
>  Documentation/technical/protocol-common.txt |  5 +++-
>  2 files changed, 30 insertions(+), 21 deletions(-)
>
> diff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt
> index 4064fc7..c6977bb 100644
> --- a/Documentation/technical/pack-protocol.txt
> +++ b/Documentation/technical/pack-protocol.txt
> @@ -14,6 +14,14 @@ data.  The protocol functions to have a server tell a client what is
>  currently on the server, then for the two to negotiate the smallest amount
>  of data to send in order to fully update one or the other.
>  
> +pkt-line Format
> +---------------
> +
> +The descriptions below build on the pkt-line format described in
> +protocol-common.txt. When the grammar indicate `PKT-LINE(...)`, unless
> +otherwise noted the usual pkt-line LF rules apply: the sender SHOULD
> +include a LF, but the receiver MUST NOT complain if it is not present.
> +
>  Transports
>  ----------
>  There are three transports over which the packfile protocol is
> @@ -143,9 +151,6 @@ with the object name that each reference currently points to.
>     003fe92df48743b7bc7d26bcaabfddde0a1e20cae47c refs/tags/v1.0^{}
>     0000
>  
> -Server SHOULD terminate each non-flush line using LF ("\n") terminator;
> -client MUST NOT complain if there is no terminator.
> -
>  The returned response is a pkt-line stream describing each ref and
>  its current value.  The stream MUST be sorted by name according to
>  the C locale ordering.
> @@ -165,15 +170,15 @@ MUST peel the ref if it's an annotated tag.
>  		      flush-pkt
>  
>    no-refs          =  PKT-LINE(zero-id SP "capabilities^{}"
> -		      NUL capability-list LF)
> +		      NUL capability-list)
>  
>    list-of-refs     =  first-ref *other-ref
>    first-ref        =  PKT-LINE(obj-id SP refname
> -		      NUL capability-list LF)
> +		      NUL capability-list)
>  
>    other-ref        =  PKT-LINE(other-tip / other-peeled)
> -  other-tip        =  obj-id SP refname LF
> -  other-peeled     =  obj-id SP refname "^{}" LF
> +  other-tip        =  obj-id SP refname
> +  other-peeled     =  obj-id SP refname "^{}"
>  
>    shallow          =  PKT-LINE("shallow" SP obj-id)
>  
> @@ -216,8 +221,8 @@ out of what the server said it could do with the first 'want' line.
>  
>    depth-request     =  PKT-LINE("deepen" SP depth)
>  
> -  first-want        =  PKT-LINE("want" SP obj-id SP capability-list LF)
> -  additional-want   =  PKT-LINE("want" SP obj-id LF)
> +  first-want        =  PKT-LINE("want" SP obj-id SP capability-list)
> +  additional-want   =  PKT-LINE("want" SP obj-id)
>  
>    depth             =  1*DIGIT
>  ----
> @@ -284,7 +289,7 @@ so that there is always a block of 32 "in-flight on the wire" at a time.
>  		       compute-end
>  
>    have-list         =  *have-line
> -  have-line         =  PKT-LINE("have" SP obj-id LF)
> +  have-line         =  PKT-LINE("have" SP obj-id)
>    compute-end       =  flush-pkt / PKT-LINE("done")
>  ----
>  
> @@ -348,10 +353,10 @@ Then the server will start sending its packfile data.
>  
>  ----
>    server-response = *ack_multi ack / nak
> -  ack_multi       = PKT-LINE("ACK" SP obj-id ack_status LF)
> +  ack_multi       = PKT-LINE("ACK" SP obj-id ack_status)
>    ack_status      = "continue" / "common" / "ready"
> -  ack             = PKT-LINE("ACK SP obj-id LF)
> -  nak             = PKT-LINE("NAK" LF)
> +  ack             = PKT-LINE("ACK" SP obj-id)
> +  nak             = PKT-LINE("NAK")
>  ----
>  
>  A simple clone may look like this (with no 'have' lines):
> @@ -467,10 +472,10 @@ references.
>  ----
>    update-request    =  *shallow ( command-list | push-cert ) [packfile]
>  
> -  shallow           =  PKT-LINE("shallow" SP obj-id LF)
> +  shallow           =  PKT-LINE("shallow" SP obj-id)
>  
> -  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
> @@ -521,7 +526,8 @@ Push Certificate
>  
>  A push certificate begins with a set of header lines.  After the
>  header and an empty line, the protocol commands follow, one per
> -line.
> +line. Note that the the trailing LF in push-cert PKT-LINEs is _not_
> +optional; it must be present.
>  
>  Currently, the following header fields are defined:
>  
> @@ -560,12 +566,12 @@ update was successful, or 'ng [refname] [error]' if the update was not.
>  		      1*(command-status)
>  		      flush-pkt
>  
> -  unpack-status     = PKT-LINE("unpack" SP unpack-result LF)
> +  unpack-status     = PKT-LINE("unpack" SP unpack-result)
>    unpack-result     = "ok" / error-msg
>  
>    command-status    = command-ok / command-fail
> -  command-ok        = PKT-LINE("ok" SP refname LF)
> -  command-fail      = PKT-LINE("ng" SP refname SP error-msg LF)
> +  command-ok        = PKT-LINE("ok" SP refname)
> +  command-fail      = PKT-LINE("ng" SP refname SP error-msg)
>  
>    error-msg         = 1*(OCTECT) ; where not "ok"
>  ----
> diff --git a/Documentation/technical/protocol-common.txt b/Documentation/technical/protocol-common.txt
> index 889985f..bf30167 100644
> --- a/Documentation/technical/protocol-common.txt
> +++ b/Documentation/technical/protocol-common.txt
> @@ -62,7 +62,10 @@ A pkt-line MAY contain binary data, so implementors MUST ensure
>  pkt-line parsing/formatting routines are 8-bit clean.
>  
>  A non-binary line SHOULD BE terminated by an LF, which if present
> -MUST be included in the total length.
> +MUST be included in the total length. Receivers MUST treat pkt-lines
> +with non-binary data the same whether or not they contain the trailing
> +LF (stripping the LF if present, and not complaining when it is
> +missing).
>  
>  The maximum length of a pkt-line's data component is 65520 bytes.
>  Implementations MUST NOT send pkt-line whose length exceeds 65524

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

[Junio C Hamano]

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.

> 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.

>
>  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]

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

[Jeff King]

On Thu, Sep 03, 2015 at 03:17:18PM -0700, Junio C Hamano wrote:

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

Right, it was the second "we know..." that made me worry. It is really
"we assume". :) Whether it is right according to the spec or not, the
real world is sometimes more complicated. And given that there is no
real advantage to changing the sending behavior now, I didn't think it
worth doing.

> 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.

In the second patch I left them all intact, but I agree that we could
drop the "\n" entirely from the example conversations, as it is implied
(and GIT_PACKET_TRACE, for example, does not even show it).

-Peff

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

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Right, it was the second "we know..." that made me worry. It is really
> "we assume". :) Whether it is right according to the spec or not, the
> real world is sometimes more complicated. And given that there is no
> real advantage to changing the sending behavior now, I didn't think it
> worth doing.

Agreed.

>>> 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.
>
> In the second patch I left them all intact, but I agree that we could
> drop the "\n" entirely from the example conversations, as it is implied
> (and GIT_PACKET_TRACE, for example, does not even show it).

Sure.