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

From: Jeff King <peff@peff.net>
Date: Nov 3, 2021, 11:57 UTC
Message-ID: <YYJ5IpvGRoDvp8V6@coredump.intra.peff.net>
In-Reply-To: <patch-2.2-964e7f4531f-20211102T155046Z-avarab@gmail.com>

On Tue, Nov 02, 2021 at 05:48:10PM +0100, Ævar Arnfjörð Bjarmason wrote:

Show 16 quoted lines

> In 97387c8bdd9 (am: read interactive input from stdin, 2019-05-20) we
> we fixed a behavior change in the conversion of git-am from a
> shellscript to a C program by changing it from using git_prompt() to
> using fgets(..., stdin). This ensured that we could run:
> 
>     echo y | git am --interactive [...]
> 
> But along with that in the subsequent 6e7baf246a2 (am: drop tty
> requirement for --interactive, 2019-05-20) we had to remove support
> for:
> 
>     git am --interactive </dev/null
> 
> This change builds on the refactoring of git_prompt() into "normal
> prompt" and "wants password" functions in the preceding commit, and
> moves "git am --interactive" back to using the prompt function.

Why do we want to do that? The only reason I mentioned that "/dev/null" thing in the earlier commit is that it's pointless.

IMHO nothing should be using git_prompt() outside of the credential code. They should just be reading from stdin, which is much more flexible. If a caller knows that stdin is coming from elsewhere, they can redirect from /dev/tty.

> It seems to me that using /dev/tty is desirable over using stdin,
> these prompts are meant to be interactive, and our acceptance of stdin
> was an artifact of how these commands were originally implemented in
> shellscript.

Basically, I think I just disagree with this paragraph entirely. Moving to stdin in the commits you referenced was done to help testing, but I also think it's just a more flexible direction overall.

-Peff