Re: [PATCH v2 3/6] clean: read user input with strbuf_getline()

From: Moritz Neeb <lists@moritzneeb.de>
Date: Feb 22, 2016, 07:40 UTC
Message-ID: <56CABB8B.2070502@moritzneeb.de>
In-Reply-To: <CAPig+cSi-4R-a=HVmpCWAZ3kr=yQtJ9GdT-JZ4hJ2kmqg-edVA@mail.gmail.com>

On 02/22/2016 03:27 AM, Eric Sunshine wrote:

Show 13 quoted lines

> On Sun, Feb 21, 2016 at 8:20 PM, Moritz Neeb <lists@moritzneeb.de> wrote:
>> The inputs that are read are all answers that are given by the user
>> when interacting with git on the commandline. As these answers are
>> not supposed to contain a meaningful CR it is safe to
>> replace strbuf_getline_lf() can be replaced by strbuf_getline().
>>
>> Before the user input was trimmed to remove the CR. This would be now
>> redundant. Another effect of the trimming was that some (accidentally)
>> typed spaces were filtered. But here we want to be consistent with similar UIs
>> like interactive adding, which only accepts space-less input.
> 
> I don't at all insist upon it, but this behavior change feels somewhat
> like it ought to be in its own commit.

You're right, two commits would be nicer. I was also thinking about splitting up the three codepaths, but I decided all of the clean-interaction belongs together.

Show 6 quoted lines

> I'm also not convinced that
> making this consistent with the less forgiving behavior of
> "interactive adding" is desirable (rather the reverse: that that case
> should be more flexible). However, I wasn't following the discussion
> with Junio closely, and perhaps missed you two agreeing that this is
> preferable.

To summarize the discussion with Junio: We were not directly talking about that. Two aspects from the whole discussion were that I should decide something and justify a stance (which I did) and that it's also beneficial to think aloud (which I forgot).

In fact, I was surprised, that interactive adding is that strict. I should've added that to the discussion. I am at the moment sometimes unsure whether I find things weird because git standards are different than what I'd expect or because things really should be changed. So I went for the former and decided to go for consistency with the base. I'd expect, from my own behaviour, interactive adding is used by far more than interactive cleaning, which might be an argument to adapt the latter.

But now that we're discussing this, I don't really see a benefit from the user perspective, it's more code cleanup.