Re: [PATCH v3 3/8] mingw: make failures to unlink or move raise a question

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 14, 2010, 23:52 UTC
Message-ID: <7vk4jbrjxn.fsf@alter.siamese.dyndns.org>
In-Reply-To: <AANLkTi=cHb2kV2MaYu72nXVOksO7O9HhJLEo-fU0sV5N@mail.gmail.com>

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

Show 7 quoted lines

> 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 ;-)

Show 6 quoted lines

>> +       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?