From: Junio C Hamano <gitster@pobox.com>
Date: Tue, 14 Dec 2010 23:52:52 GMT
Subject: Re: [PATCH v3 3/8] mingw: make failures to unlink or move raise a question
Message-ID: <7vk4jbrjxn.fsf@alter.siamese.dyndns.org>
In-Reply-To: <AANLkTi=cHb2kV2MaYu72nXVOksO7O9HhJLEo-fU0sV5N@mail.gmail.com>

Erik Faye-Lund <kusmabite@gmail.com> writes:

> Since you're doing case insensitive checks for "yes" and "no", perhaps
> it'd make sense to allow upper case 'Y' and 'N' also? Something like:
>
> -       if (answer[0] == 'n' && strlen(answer) == 1)
> +       if (tolower(answer[0]) == 'n' && strlen(answer) == 1)
>
> hm?

Why not

	if (tolower(answer[0]) == 'n' && !answer[1])

think of the case answer[] is very long ;-)

>> +       if (!isatty(_fileno(stdin)) || !isatty(_fileno(stderr)))
>> +               return 0;
>
> I'm wondering, doesn't this make the semantics a bit wrong? The
> function is called "ask_user_yes_no", but it might end up not asking
> after all.

I don't think that is such a big issue.

Imagine you had only getenv("GIT_ASK_YESNO") codepath, and no fallback
"tty" codepath.  And you ship with a separate program as a default
"asker".

The implementation of that asker happens to read yes/no from the tty, but
it defauts to "no" if there is no tty interaction available.

If you view it that way, the code we see above is just an optimization to
avoid spawning that default "asker" as a separate process.

I was more puzzled by the code to formulate question[]; why doesn't it
build the same question for both codepaths and spit that out to stderr
with fputs() in the fallvack asker?