Hi,

On Wed, Dec 15, 2010 at 01:11:03AM +0100, Johannes Schindelin wrote:
quoted text
> On Tue, 14 Dec 2010, Erik Faye-Lund wrote:
> 
> > On Tue, Dec 14, 2010 at 11:21 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
[...]
quoted text
> > > +       if (answer[0] == 'y' && strlen(answer) == 1)
> > > +               return 1;
> > > +       if (!strncasecmp(answer, "yes", sizeof(answer)))
> > > +               return 1;
> > > +       if (answer[0] == 'n' && strlen(answer) == 1)
> > > +               return 0;
> > > +       if (!strncasecmp(answer, "no", sizeof(answer)))
> > > +               return 0;
> > 
> > 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?
> 
> Makes sense to me.

Will do in the next iteration.

quoted text
> > > +static int ask_user_yes_no(const char *format, ...)
[...]
quoted text
> > 
> > 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. Perhaps it should be called something that reflects this?
> > "maybe_ask_yes_no", "ask_yes_no_if_tty", "should_retry"? I don't have
> > a non-ugly suggestion, but I suspect something like that might leave
> > other people less puzzled when reading the code.
> 
> I like ask_yes_no_if_tty.

How about ask_yes_no_if_possible() ? Since we do not just rely on tty
but still have GIT_ASK_YESNO to get to the user I think this would be
a closer description of what happens.

Cheers Heiko
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html