Re: [PATCH 3/6] Remove return undef from ask()

From: Jay Soffian <jaysoffian@gmail.com>
Date: May 4, 2009, 02:26 UTC
Message-ID: <76718490905031926i771b0234ua7b45d5e0d827913@mail.gmail.com>
In-Reply-To: <20090503202625.GC20468@coredump.intra.peff.net>

On Sun, May 3, 2009 at 4:26 PM, Jeff King <peff@peff.net> wrote:

> On a side note, while looking at this function, I wonder if that "return
> undef" is correct after all. We get there only if the user has failed to
> give valid input 10 times, so presumably it is a sanity check to
> prevent runaway input errors

Correct, that is why it is there.

> (and I am cc'ing Jay, who added the
> function not too long ago). Should we be respecting the default here, as
> we do when we get EOF?

The original motivation was a user who was running send-email from cron and it was looping forever. That case is now actually handled before the loop, and all other normal cases are handled inside the loop.

So the only thing that can cause the loop to exit (AFAIK) is when $valid_re is passed in and the user provides invalid input 10x.

> Although I tend to think if the user is
> repeatedly giving us bogus input that we should not just proceed, but
> should probably die. Because otherwise we are guessing at what they
> might have wanted.

Well, it returns undef, at which point it's up to the caller to figure out what to do. You'll notice the one caller which passes in $valid_re dies:

die "Send this email reply required" unless defined $_;

Letting the caller decide what to do provides more flexibility.

j.