Re: [PATCH v2 1/3] credential.c: fix credential reading with regards to CR/LF

From: Jeff King <peff@peff.net>
Date: Oct 2, 2020, 12:01 UTC
Message-ID: <20201002120126.GA2255314@coredump.intra.peff.net>
In-Reply-To: <nycvar.QRO.7.76.6.2010021011540.50@tvgsbejvaqbjf.bet>

On Fri, Oct 02, 2020 at 01:37:23PM +0200, Johannes Schindelin wrote:

Show 13 quoted lines

> But I would highly doubt that the empty lines were the biggest problem:
> Sure, we would fail to recognize an empty line with CR/LF line endings
> when reading with `strbuf_getline_lf()`, but we would totally
> misunderstand the entire rest of the lines, too. For example, we would
> mistake `quit\r` for an unknown command, and hence simply ignore it.
> 
> I do agree, however, that your confusion validly points out a flaw in the
> commit message: the "empty line" comment is a red herring.
> 
> Therefore, I spent some time pouring over the commit message. This is my
> current version:
> [...]
> What do you think?

I think we are on the same page, and this revision does a good job of fixing my complaint about the commit message. Thanks. One minor typo:

>     While we do this for the credential helper protocol, we do _not_ do
>     adjust `git credential-cache--daemon` (which won't work on Windows,

s/do$//

-Peff