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

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

 



Erik Faye-Lund <kusmabite@xxxxxxxxx> 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?
--
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]