Re: [PATCH v1 1/8] compat/terminal: let prompt accept input from pipe

Jeff King [off-list ref] writes:

On Tue, May 11, 2021 at 12:38:10PM +0900, Junio C Hamano wrote:

quoted

quoted

quoted

I actually inspired myself from the two occurrences of git_prompt in
builtin/bisect--helper.c introduced in 09535f056b (bisect--helper:
reimplement `bisect_autostart` shell function in C, 2020-09-24).
Not sure if they should also be converted to a simple fgets.

Yes, I think they should be switched.

OK, that is because in the context of a "bisect" session, we won't
be feeding any real data from its standard input, unlike "git am"
that may well be eating a patch stream from its standard input
stream.  If so, makes sense.

Yes, though even in "git am", we forbid using interactive mode with
patches on stdin (and did so even when we were reading from the tty;
presumably the rule dates back to when it was a shell script and was
using stdin).

As long as the "prompt and accept an single-line answer from the end
user" is restricted to "git am -i", I'll be perfectly OK with that.
I just do not want my regular "type '|' in my MUA to pipe the
current article to a command, and give 'git am -s' as the command"
workflow to get broken in the future when somebody blindly follows a
carelessly written direction to use a helper that reads from the
standard input for confirmation.  The condition under which use of
that helper is appropriate needs to be clearly spelled out.

Thanks.

On Tue, May 11, 2021 at 03:17:13PM +0900, Junio C Hamano wrote:

quoted

quoted

OK, that is because in the context of a "bisect" session, we won't
be feeding any real data from its standard input, unlike "git am"
that may well be eating a patch stream from its standard input
stream.  If so, makes sense.

Yes, though even in "git am", we forbid using interactive mode with
patches on stdin (and did so even when we were reading from the tty;
presumably the rule dates back to when it was a shell script and was
using stdin).

As long as the "prompt and accept an single-line answer from the end
user" is restricted to "git am -i", I'll be perfectly OK with that.
I just do not want my regular "type '|' in my MUA to pipe the
current article to a command, and give 'git am -s' as the command"
workflow to get broken in the future when somebody blindly follows a
carelessly written direction to use a helper that reads from the
standard input for confirmation.  The condition under which use of
that helper is appropriate needs to be clearly spelled out.

Yeah, I don't think anybody is proposing to change the behavior of "git
am" here (we might swap out the current fgets(stdin) for a helper which
does the equivalent). But I agree that any comment recommending one
versus the other should probably remind people to think about how stdin
is otherwise used in the program, and whether that will cause any
conflicts.

-Peff