diff mbox series

[2/4] terminal: don't assume stdin is /dev/tty

Message ID 20220304131126.8293-3-phillip.wood123@gmail.com (mailing list archive)
State Superseded
Headers show
Series builtin add -p: hopefully final readkey fixes | expand

Commit Message

Phillip Wood March 4, 2022, 1:11 p.m. UTC 
From: Phillip Wood <phillip.wood@dunelm.org.uk>

read_key_without_echo() reads from stdin but uses /dev/tty when it
disables echo. This is unfortunate as there no guarantee that stdin is
the same device as /dev/tty. The perl version of "add -p" uses stdin
when it sets the terminal mode, this commit does the same for the
builtin version. There is still a difference between the perl and
builtin versions though - the perl version will ignore any errors when
setting the terminal mode[1] and will still read single bytes when
stdin is not a terminal. The builtin version displays a warning if
setting the terminal mode fails and switches to reading a line at a
time.

[1] https://github.com/jonathanstowe/TermReadKey/blob/b061c913bbf7ff9bad9b4eea6caae189eacd6063/ReadKey.xs#L1090

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 compat/terminal.c | 5 +++--
 compat/terminal.h | 3 ++-
 2 files changed, 5 insertions(+), 3 deletions(-)

Comments

Ramsay Jones March 4, 2022, 8:42 p.m. UTC | #1 
On 04/03/2022 13:11, Phillip Wood wrote:
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
> 
[snip]
> diff --git a/compat/terminal.h b/compat/terminal.h
> index f24b91390d..f5655d0d0b 100644
> --- a/compat/terminal.h
> +++ b/compat/terminal.h
> @@ -3,7 +3,8 @@
>  
>  /* Save input and output settings */
>  #define SAVE_TERM_DUPLEX (1u << 0)
> -
> +/* Save stdin rather than /dev/tty (fails is stdin is not a terminal) */

s/fails is/fails if/ ?

ATB,
Ramsay Jones

> +#define SAVE_TERM_STDIN  (1u << 1)
>  /*
>   * Save the terminal attributes so they can be restored later by a
>   * call to restore_term(). Note that every successful call to
diff mbox series

Patch

diff --git a/compat/terminal.c b/compat/terminal.c
index bad8e04cd8..249836e78f 100644
--- a/compat/terminal.c
+++ b/compat/terminal.c
@@ -37,7 +37,8 @@  void restore_term(void)
 int save_term(unsigned flags)
 {
 	if (term_fd < 0)
-		term_fd = open("/dev/tty", O_RDWR);
+		term_fd = (flags & SAVE_TERM_STDIN) ? 0
+						    : open("/dev/tty", O_RDWR);
 	if (term_fd < 0)
 		return -1;
 	if (tcgetattr(term_fd, &old_term) < 0)
@@ -362,7 +363,7 @@  int read_key_without_echo(struct strbuf *buf)
 	static int warning_displayed;
 	int ch;
 
-	if (warning_displayed || enable_non_canonical(0) < 0) {
+	if (warning_displayed || enable_non_canonical(SAVE_TERM_STDIN) < 0) {
 		if (!warning_displayed) {
 			warning("reading single keystrokes not supported on "
 				"this platform; reading line instead");
diff --git a/compat/terminal.h b/compat/terminal.h
index f24b91390d..f5655d0d0b 100644
--- a/compat/terminal.h
+++ b/compat/terminal.h
@@ -3,7 +3,8 @@ 
 
 /* Save input and output settings */
 #define SAVE_TERM_DUPLEX (1u << 0)
-
+/* Save stdin rather than /dev/tty (fails is stdin is not a terminal) */
+#define SAVE_TERM_STDIN  (1u << 1)
 /*
  * Save the terminal attributes so they can be restored later by a
  * call to restore_term(). Note that every successful call to