From: Johannes Sixt <j6t@kdbg.org>
Date: Wed, 12 Aug 2015 18:24:49 GMT
Subject: Re: [PATCH] revisions --stdin: accept CRLF line terminators
Message-ID: <55CB8F71.1020402@kdbg.org>
In-Reply-To: <xmqqtws5o4lp.fsf@gitster.dls.corp.google.com>

Am 12.08.2015 um 00:14 schrieb Junio C Hamano:
> Now, I am wondering if it makes sense to do these two things:
>
>   * Teach revision.c::read_revisions_from_stdin() to use
>     strbuf_getline() instead of strbuf_getwholeline().
>
>   * Teach strbuf_getline() to remove CR at the end when stripping the
>     LF at the end, only if "term" parameter is set to LF.
>
> Doing so would solve 1. and 2., but we obviously need to audit all
> the other uses of strbuf_getline() to see if they can benefit (or if
> some of them may be broken because they _always_ need LF terminated
> lines, i.e. CRLF terminated input is illegal to them).

I can see what I can do with these. Don't hold your breath, though.

> As to 3., I think it is OK.  The code structure of 4. is too ugly
> and needs to be revamped to go one line at a time first before even
> thinking about how to proceed, I would think.

Regarding update-ref --stdin (your 4.), I notice that the input format 
is very strict, so the solution is to allow an optional CR before the 
LF. I alread have a patch, but it skips all trailing space, which is 
probably too lenient. (I only needed the patch once for a debug 
sesssion, but there is no obvious breakage without the patch.)

-- Hannes