From: Junio C Hamano <gitster@pobox.com>
To: Firmin Martin <firminmartin24@gmail.com>
Cc: git@vger.kernel.org, Jeff King <peff@peff.net>
Subject: Re: [PATCH 1/2] prompt.h: clarify the non-use of git_prompt
Date: Fri, 14 May 2021 08:03:42 +0900 [thread overview]
Message-ID: <xmqqmtsy8cdt.fsf@gitster.g> (raw)
In-Reply-To: <xmqqv97m8dnw.fsf@gitster.g> (Junio C. Hamano's message of "Fri, 14 May 2021 07:36:03 +0900")
Junio C Hamano <gitster@pobox.com> writes:
> I have a strong objection against the above phrasing.
>
> If we are asking user for interactive input, this SHOULD be used,
> especially if we might be reading the data to work on from the
> standard input and we may need to ask the user to interactively
> instruct us what to do to that data. The only plausible reason that
> we may want to avoid it and instead prefer the (misnamed)
> read_line_interactively() to read whatever from the standard input
> (which may not be "interactive" at all, which is why I said
> "misnamed") is because our test framework does not use setsid (and
> setsid(1) may not be universally available) with pty to emulate tty
> input, isn't it?
>
>> char *git_prompt(const char *prompt, int flags);
>>
>> int git_read_line_interactively(struct strbuf *line);
So, here is an alternative that nudges users away from this helper,
but with honesty. I also suggest a better name for that misnamed
"interactively" function in the comment, but will leave it as an
exercise to readers to come up with a patch to rename the function.
/*
* Give prompt to the user and accept interactive input from the
* controlling terminal (/dev/tty). This function can be used even
* when the standard input is being used to feed us real data to
* operate on, as we open /dev/tty ourselves for user interaction.
*
* In a codepath that never uses the standard input for real data,
* consider using git_read_line_from_standard_input() instead, as it
* is easier to write tests for (our test framework currently does not
* make it easy to simulate end-user input coming from /dev/tty).
*/
next prev parent reply other threads:[~2021-05-13 23:03 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-13 21:41 [PATCH 0/2] Refactor some prompts Firmin Martin
2021-05-13 21:41 ` [PATCH 1/2] prompt.h: clarify the non-use of git_prompt Firmin Martin
2021-05-13 22:36 ` Junio C Hamano
2021-05-13 23:03 ` Junio C Hamano [this message]
2021-05-15 10:12 ` Jeff King
2021-05-13 21:41 ` [PATCH 2/2] builtin: use git_read_line_interactively to prompt Firmin Martin
2021-05-13 22:51 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqmtsy8cdt.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=firminmartin24@gmail.com \
--cc=git@vger.kernel.org \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.