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

From: Heiko Voigt <hvoigt@hvoigt.net>
Date: Dec 15, 2010, 07:48 UTC
Message-ID: <20101215074832.GC14646@book.hvoigt.net>
In-Reply-To: <7vk4jbrjxn.fsf@alter.siamese.dyndns.org>

Hi,

On Tue, Dec 14, 2010 at 03:52:52PM -0800, Junio C Hamano wrote:

Show 15 quoted lines

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

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?

Show 18 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 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