Hi,
On Tue, Dec 14, 2010 at 03:52:52PM -0800, Junio C Hamano wrote:
> 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 ;-)
Will change as stated in the previous email but of course using this
code for efficiency ;) Can anybody estimate how fast the user would need
to type to actually make this noticeable and how much heat that would
produce on the keyboard?
> >> + 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 do not mind to change this function name to make it more match what
its doing. Since code is read way more often than written I think this
makes sense. See my other email about the suggestion.
> 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?
Do you mean that I append " (y/n)? " ? For the (y/n) you can think of it
as the tui implementation of the yes/no button. I can see that the ?
might need to go into the question string itself. Is it that what you
meant?
Cheers Heiko
--
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