git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
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 07:36:03 +0900	[thread overview]
Message-ID: <xmqqv97m8dnw.fsf@gitster.g> (raw)
In-Reply-To: <20210513214152.34792-2-firminmartin24@gmail.com> (Firmin Martin's message of "Thu, 13 May 2021 23:41:51 +0200")

Firmin Martin <firminmartin24@gmail.com> writes:

> +/*
> + * This function should not be used for regular prompts (i.e., asking user for
> + * confirmation or picking an option from an interactive menu) as it only
> + * accepts input from /dev/tty, thus making it impossible to test with the
> + * current test suite.  Please instead use git_read_line_interactively for that
> + * purpose.  See 97387c8bdd (am: read interactive input from stdin, 2019-05-20)
> + * for historical context.
> + *
> + */

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);

  reply	other threads:[~2021-05-13 22:36 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 [this message]
2021-05-13 23:03     ` Junio C Hamano
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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqv97m8dnw.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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).