Re: [PATCH] revisions --stdin: accept CRLF line terminators

From: Johannes Sixt <j6t@kdbg.org>
Date: Aug 12, 2015, 18:24 UTC
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:

Show 12 quoted lines

> 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