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:36 UTC
Message-ID: <20101215073639.GB14646@book.hvoigt.net>
In-Reply-To: <alpine.DEB.1.00.1012150109340.1461@bonsai2>

Hi,

On Wed, Dec 15, 2010 at 01:11:03AM +0100, Johannes Schindelin wrote:

> On Tue, 14 Dec 2010, Erik Faye-Lund wrote:
> 
> > On Tue, Dec 14, 2010 at 11:21 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:

[...]

Show 18 quoted lines

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

> > > +static int ask_user_yes_no(const char *format, ...)

[...]

Show 9 quoted lines

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