Re: [PATCH 2/2] prompt.c: add and use a GIT_TEST_TERMINAL_PROMPT=true

From: Jeff King <peff@peff.net>
Date: Nov 4, 2021, 09:58 UTC
Message-ID: <YYOu2i48Br9sOz4V@coredump.intra.peff.net>
In-Reply-To: <211103.864k8tjmmp.gmgdl@evledraar.gmail.com>

On Wed, Nov 03, 2021 at 04:12:43PM +0100, Ævar Arnfjörð Bjarmason wrote:

Show 7 quoted lines

> I'm fine with it either way, my reading of your 2019-ish commits was
> that you did that not to intentionally get that behavior, but to work
> around that test issue.
> 
> So we do always want the "read from stdin" behavior, so I can get those
> bisect tests by just changing its behavior too, with no need for the
> test variable? If so I'm fine with that.

Yes. The thing that makes it OK to do from a backwards-compatibility standpoint is that even though the tools read from /dev/tty now, they also insist that stdin is a tty. So in practice it will always be the same thing (technically you _could_ redirect stdin from a different tty, but that's sufficiently bizarre that I think we can discount it).

Show 5 quoted lines

> I think it's a good thing in general to have a not-for-password
> git_prompt() API, because it makes it easy to make that use some
> readline-like API, i.e. one that would have tab completion, and handle
> the loop some (but not all) callers have around handling retries etc,
> we'd also be able to translate the "Y" "n" characters...

Yeah, I'd be OK with that direction, but I think it is not really the same thing that the existing git_prompt() was designed for (where you most certainly don't want readline-ish things like history for your password).

Perhaps a good first step is not so much splitting git_prompt() into two pieces, as simply renaming it in place. It really was designed for the credential request and not much else, but the name is a bit more generic.

But I'd probably wait on that until we were looking at actually adding a functional readline-ish interface before even doing that.

-Peff