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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Feb 21, 2016 at 8:20 PM, Moritz Neeb <lists@xxxxxxxxxxxxx> 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. 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.

> For the case of filtering by patterns the input is still trimmed in an
> untouched codepath after it is split up into multiple patterns.
> This is considered as desirable, because of two reasons:

s/, because of/ for/

> First this fitering is not part of similar UIs and it is way more likely
> to accidentally type a space in this way of interacting.
>
> Signed-off-by: Moritz Neeb <lists@xxxxxxxxxxxxx>
> ---
> diff --git a/builtin/clean.c b/builtin/clean.c
> @@ -570,9 +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) {
> -                       strbuf_trim(&choice);
> -               } else {
> +               if (strbuf_getline(&choice, stdin) == EOF) {
>                         eof = 1;
>                         break;
>                 }
> @@ -652,9 +650,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)
> -                       strbuf_trim(&confirm);
> -               else
> +               if (strbuf_getline(&confirm, stdin) == EOF)
>                         putchar('\n');
>                 /* quit filter_by_pattern mode if press ENTER or Ctrl-D */
> @@ -750,9 +746,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) {
> -                               strbuf_trim(&confirm);
> -                       } else {
> +                       if (strbuf_getline(&confirm, stdin) == EOF) {
>                                 putchar('\n');
>                                 eof = 1;
>                         }
> --
> 2.7.1.345.gc14003e
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]