From: Junio C Hamano <gitster@pobox.com>
Date: Fri, 10 Apr 2020 17:44:59 GMT
Subject: Re: [PATCH v2 1/2] Refactor code asking the user for input interactively
Message-ID: <xmqqftdbjkv8.fsf@gitster.c.googlers.com>
In-Reply-To: <9d2ee78a9e414c0b6aacbc9c878ab08eb70703d5.1586518072.git.gitgitgadget@gmail.com>

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> -		if (strbuf_getline(&input, stdin) == EOF) {
> +		if (git_read_line_interactively(&input) == EOF) {

It's not like we are mimicking or giving a thin wrapper to improve
an existing read_line_interactively() from a third-party, so I do
not see much point in giving "git_" prefix here.  On the other hand,
"strbuf_" prefix may not hurt (but the type of its first parameter
is sufficient so it is not exactly required).

>  			printf(_("Remove %s [y/N]? "), qname);
> -			if (strbuf_getline_lf(&confirm, stdin) != EOF) {
> -				strbuf_trim(&confirm);
> -			} else {
> +			if (git_read_line_interactively(&confirm) == EOF) {
>  				putchar('\n');
>  				eof = 1;

A fat-finger that gave an answer " yes <RET>" used to be still taken
as a yes but now it is interpreted as "no", because the new helper
trims a lot less.  In general, the existing code should be already
choosing the safer default, so such a change in behaviour brought in
by this change, even if they were not intentional, should probably
be safe.

Thanks.