All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>, git@vger.kernel.org
Subject: Re: What's cooking in git.git (Jan 2016, #02; Mon, 11)
Date: Wed, 13 Jan 2016 15:07:13 -0800	[thread overview]
Message-ID: <xmqqtwmhkrj2.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20160112214909.GD2841@sigill.intra.peff.net> (Jeff King's message of "Tue, 12 Jan 2016 16:49:09 -0500")

Jeff King <peff@peff.net> writes:

> On Tue, Jan 12, 2016 at 10:47:25AM -0800, Junio C Hamano wrote:
>
>> I think strbuf_getline() that handles the payload as "text" without
>> having _crlf() suffix is an ideal endgame in the longer term, but I
>> do not think it is a good idea to do that as a flag-day change.  So
>> I am inclined not to change the function names around that feature
>> in this series.  Others can do the wholesale rename as a separate
>> follow-up topic when the tree is quiescent.
>
> Yeah, I think we would want to catch topics in flight. Should the end of
> this series then be to _remove_ strbuf_getline()? Callers should be
> using strbuf_getline_crlf() if they want text lines, and
> strbuf_getdelim() if they do not.
>
> Topics in flight will need fixed up, but that's OK; the breakage (and
> the fix) will be obvious.
>
> And then after a quiet period we can drop the "_crlf()" and have
> strbuf_getline() back.

Actually, I think a patch that

 - renames strbuf_getline() to strbuf_getdelim(); and
 - renames strbuf_getline_crlf() to strbuf_getline() 

on top of the series we already have is sufficient to bring the
endgame state to us.  The new strbuf_getline() has a different
function signature from the traditional one, so any topic in flight
that is unaware of this series can easily be caught, and we can do
this without a quiet period.

A more interesting question is if strbuf_getdelim() should take an
arbitrary byte as its third parameter.  As I said elsewhere, the
only reason why it is not a "do we use LF or do we use NUL?"
boolean is because I wrote these codepaths anticipating that there
might be a value other than NUL and LF that could be useful when I
introduced line_termination long time ago, but no useful caller that
uses other useful value has emerged, so I think the interface was
too broad and too general for its own good.

It becomes very tempting not to do strbuf_getdelim() at all, but
instead rewrite the current calls to strbuf_getline() to call one of
two functions, i.e. strbuf_getline_lf() and strbuf_getline_nul(),
when we rename strbuf_getline_crlf() to strbuf_getline().

By going that route, those who want to help CRLF situation further
can then concentrate on output from "git grep strbuf_getline_lf()",
identify the ones that can be safely turned into strbuf_getline(),
and do the conversion.

  reply	other threads:[~2016-01-13 23:07 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-11 23:45 What's cooking in git.git (Jan 2016, #02; Mon, 11) Junio C Hamano
2016-01-12  0:06 ` Mike Hommey
2016-01-12  3:04   ` Junio C Hamano
2016-01-12  4:34 ` Edmundo Carmona Antoranz
2016-01-12  8:39 ` Johannes Schindelin
2016-01-12 18:47   ` Junio C Hamano
2016-01-12 21:49     ` Jeff King
2016-01-13 23:07       ` Junio C Hamano [this message]
2016-01-13 23:22         ` Jeff King
2016-01-13 23:44           ` Junio C Hamano
2016-01-13 23:54             ` Junio C Hamano
2016-01-14 10:21               ` Jeff King
2016-01-13  2:56 ` David A. Greene
2016-01-18 13:35 ` Michael J Gruber
2016-01-18 17:06   ` Jeff King
2016-01-18 21:39     ` Eric Wong
2016-01-19  7:07       ` Michael J Gruber
2016-01-25  9:56 ` Duy Nguyen
2016-01-25 22:03   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqtwmhkrj2.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.