Re: [PATCH v2 3/6] clean: read user input with strbuf_getline()
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Feb 22, 2016, 02:27 UTC
- Message-ID
- <CAPig+cSi-4R-a=HVmpCWAZ3kr=yQtJ9GdT-JZ4hJ2kmqg-edVA@mail.gmail.com>
- In-Reply-To
- <56CA6264.1040400@moritzneeb.de>
On Sun, Feb 21, 2016 at 8:20 PM, Moritz Neeb <lists@moritzneeb.de> wrote:
Show 9 quoted lines
> 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/
Show 39 quoted lines
> 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@moritzneeb.de>
> ---
> 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