Re: [PATCH v3 1/2] add interface for /dev/tty interaction
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 5, 2012, 20:11 UTC
- Message-ID
- <7vsjc12j5o.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <1344192340-19415-2-git-send-email-rctay89@gmail.com>
Tay Ray Chuan <rctay89@gmail.com> writes:
Show 9 quoted lines
> Factor out the opening and closing of /dev/tty from > git_terminal_prompt(), so that callers may first test if a controlling > terminal is available before proceeding with prompting proper. > > When HAVE_DEV_TTY is not defined, terminal_open() falls back to checking > tty-ness of stdin and stderr, as getpass() uses them both. > > Signed-off-by: Tay Ray Chuan <rctay89@gmail.com> > ---
This is not your fault but seeing term_t made me go "eek, yuck".
As far as I can see, use of "FILE *" in existing compat/terminal.c is not buying us anything useful. The stdio calls made on FILE *fh are only fopen(), fputs(), fflush() and fclose(), and everything else goes through fileno(fh).
So perhaps it is a saner approach to fix that function first before this patch so that it works on file descriptors.
Show 108 quoted lines
> compat/terminal.c | 52 ++++++++++++++++++++++++++++++++++++++++++++--------
> compat/terminal.h | 10 ++++++++++
> 2 files changed, 54 insertions(+), 8 deletions(-)
>
> diff --git a/compat/terminal.c b/compat/terminal.c
> index 6d16c8f..c85d5c7 100644
> --- a/compat/terminal.c
> +++ b/compat/terminal.c
> @@ -24,15 +24,21 @@ static void restore_term_on_signal(int sig)
> raise(sig);
> }
>
> -char *git_terminal_prompt(const char *prompt, int echo)
> +term_t terminal_open(void)
> +{
> + return fopen("/dev/tty", "w+");
> +}
> +
> +int terminal_close(term_t term)
> +{
> + return fclose(term);
> +}
> +
> +char *terminal_prompt(term_t term, const char *prompt, int echo)
> {
> static struct strbuf buf = STRBUF_INIT;
> int r;
> - FILE *fh;
> -
> - fh = fopen("/dev/tty", "w+");
> - if (!fh)
> - return NULL;
> + FILE *fh = term;
>
> if (!echo) {
> struct termios t;
> @@ -64,18 +70,48 @@ char *git_terminal_prompt(const char *prompt, int echo)
> }
>
> restore_term();
> - fclose(fh);
>
> if (r == EOF)
> return NULL;
> return buf.buf;
> }
>
> +char *git_terminal_prompt(const char *prompt, int echo)
> +{
> + char *ret;
> + term_t term;
> +
> + term = terminal_open();
> + if (!term)
> + return NULL;
> +
> + ret = terminal_prompt(term, prompt, echo);
> +
> + terminal_close(term);
> +
> + return ret;
> +}
> +
> #else
>
> -char *git_terminal_prompt(const char *prompt, int echo)
> +term_t terminal_open()
> +{
> + return isatty(0) && isatty(2);
> +}
> +
> +int terminal_close(term_t term)
> +{
> + return 0;
> +}
> +
> +char *terminal_prompt(term_t term, const char *prompt, int echo)
> {
> return getpass(prompt);
> }
>
> +char *git_terminal_prompt(const char *prompt, int echo)
> +{
> + return terminal_prompt(prompt, echo);
> +}
> +
> #endif
> diff --git a/compat/terminal.h b/compat/terminal.h
> index 97db7cd..cf2aa10 100644
> --- a/compat/terminal.h
> +++ b/compat/terminal.h
> @@ -1,6 +1,16 @@
> #ifndef COMPAT_TERMINAL_H
> #define COMPAT_TERMINAL_H
>
> +#ifdef HAVE_DEV_TTY
> +typedef FILE *term_t;
> +#else
> +typedef int term_t;
> +#endif
> +
> +term_t terminal_open();
> +int terminal_close(term_t term);
> +char *terminal_prompt(term_t term, const char *prompt, int echo);
> +
> char *git_terminal_prompt(const char *prompt, int echo);
>
> #endif /* COMPAT_TERMINAL_H */