From: Moritz Neeb <lists@moritzneeb.de>
Date: Sun, 28 Feb 2016 07:36:48 GMT
Subject: Re: [PATCH v3 3/7] clean: read user input with strbuf_getline()
Message-ID: <56D2A390.9010804@moritzneeb.de>
In-Reply-To: <CAPig+cSkV0KMG7gONabPxgQkzWgPU+1YGTMb4SDmpg1QrHxhSA@mail.gmail.com>

On 02/28/2016 07:36 AM, Eric Sunshine wrote:
> On Sun, Feb 28, 2016 at 12:13 AM, 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().
> 
> Grammo: "it is safe to replace ... can be replaced by ..."
> 

I dropped the second duplication.

>> In the subsequent codepath, the input is trimmed. This leads to
> 
> How about?
> 
>     After being read, the input is trimmed.

Yep, sounds better.

Thanks.

> 
>> accepting user input with spaces, e.g. "  y ", as a valid answer in
>> the interactive cleaning process.
>>
>> Although trimming would not be required anymore to remove a potential CR,
>> we don't want to change the existing behavior with this patch.
>> Thus, the trimming is kept in place.
>>
>> Signed-off-by: Moritz Neeb <lists@moritzneeb.de>
>> ---
>> diff --git a/builtin/clean.c b/builtin/clean.c
>> @@ -570,7 +570,7 @@ static int *list_and_choose(struct menu_opts *opts, struct menu_stuff *stuff)
>>                                clean_get_color(CLEAN_COLOR_RESET));
>>                 }
>>
>> -               if (strbuf_getline_lf(&choice, stdin) != EOF) {
>> +               if (strbuf_getline(&choice, stdin) != EOF) {
>>                         strbuf_trim(&choice);
>>                 } else {
>>                         eof = 1;
>> @@ -652,7 +652,7 @@ static int filter_by_patterns_cmd(void)
>>                 clean_print_color(CLEAN_COLOR_PROMPT);
>>                 printf(_("Input ignore patterns>> "));
>>                 clean_print_color(CLEAN_COLOR_RESET);
>> -               if (strbuf_getline_lf(&confirm, stdin) != EOF)
>> +               if (strbuf_getline(&confirm, stdin) != EOF)
>>                         strbuf_trim(&confirm);
>>                 else
>>                         putchar('\n');
>> @@ -750,7 +750,7 @@ static int ask_each_cmd(void)
>>                         qname = quote_path_relative(item->string, NULL, &buf);
>>                         /* TRANSLATORS: Make sure to keep [y/N] as is */
>>                         printf(_("Remove %s [y/N]? "), qname);
>> -                       if (strbuf_getline_lf(&confirm, stdin) != EOF) {
>> +                       if (strbuf_getline(&confirm, stdin) != EOF) {
>>                                 strbuf_trim(&confirm);
>>                         } else {
>>                                 putchar('\n');
>> --
>> 2.4.3