All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Johannes Schindelin <johannes.schindelin@gmx.de>,
	Junio C Hamano <gitster@pobox.com>,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: [PATCH 7/9] built-in add -p: handle Escape sequences in interactive.singlekey mode
Date: Sat, 21 Dec 2019 22:41:58 +0000	[thread overview]
Message-ID: <fd5a12977609f69790fd2405baf5836add434cef.1576968120.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.175.git.1576968120.gitgitgadget@gmail.com>

From: Johannes Schindelin <johannes.schindelin@gmx.de>

This recapitulates part of b5cc003253c8 (add -i: ignore terminal escape
sequences, 2011-05-17):

    add -i: ignore terminal escape sequences

    On the author's terminal, the up-arrow input sequence is ^[[A, and
    thus fat-fingering an up-arrow into 'git checkout -p' is quite
    dangerous: git-add--interactive.perl will ignore the ^[ and [
    characters and happily treat A as "discard everything".

    As a band-aid fix, use Term::Cap to get all terminal capabilities.
    Then use the heuristic that any capability value that starts with ^[
    (i.e., \e in perl) must be a key input sequence.  Finally, given an
    input that starts with ^[, read more characters until we have read a
    full escape sequence, then return that to the caller.  We use a
    timeout of 0.5 seconds on the subsequent reads to avoid getting stuck
    if the user actually input a lone ^[.

    Since none of the currently recognized keys start with ^[, the net
    result is that the sequence as a whole will be ignored and the help
    displayed.

Note that we leave part for later which uses "Term::Cap to get all
terminal capabilities", for several reasons:

1. it is actually not really necessary, as the timeout of 0.5 seconds
   should be plenty sufficient to catch Escape sequences,

2. it is cleaner to keep the change to special-case Escape sequences
   separate from the change that reads all terminal capabilities to
   speed things up, and

3. in practice, relying on the terminal capabilities is a bit overrated,
   as the information could be incomplete, or plain wrong. For example,
   in this developer's tmux sessions, the terminal capabilities claim
   that the "cursor up" sequence is ^[M, but the actual sequence
   produced by the "cursor up" key is ^[[A.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 compat/terminal.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 55 insertions(+), 1 deletion(-)

diff --git a/compat/terminal.c b/compat/terminal.c
index 1b2564042a..b7f58d1781 100644
--- a/compat/terminal.c
+++ b/compat/terminal.c
@@ -161,6 +161,37 @@ static int enable_non_canonical(void)
 	return disable_bits(ENABLE_ECHO_INPUT | ENABLE_LINE_INPUT | ENABLE_PROCESSED_INPUT);
 }
 
+/*
+ * Override `getchar()`, as the default implementation does not use
+ * `ReadFile()`.
+ *
+ * This poses a problem when we want to see whether the standard
+ * input has more characters, as the default of Git for Windows is to start the
+ * Bash in a MinTTY, which uses a named pipe to emulate a pty, in which case
+ * our `poll()` emulation calls `PeekNamedPipe()`, which seems to require
+ * `ReadFile()` to be called first to work properly (it only reports 0
+ * available bytes, otherwise).
+ *
+ * So let's just override `getchar()` with a version backed by `ReadFile()` and
+ * go our merry ways from here.
+ */
+static int mingw_getchar(void)
+{
+	DWORD read = 0;
+	unsigned char ch;
+
+	if (!ReadFile(GetStdHandle(STD_INPUT_HANDLE), &ch, 1, &read, NULL))
+		return EOF;
+
+	if (!read) {
+		error("Unexpected 0 read");
+		return EOF;
+	}
+
+	return ch;
+}
+#define getchar mingw_getchar
+
 #endif
 
 #ifndef FORCE_TEXT
@@ -228,8 +259,31 @@ int read_key_without_echo(struct strbuf *buf)
 		restore_term();
 		return EOF;
 	}
-
 	strbuf_addch(buf, ch);
+
+	if (ch == '\033' /* ESC */) {
+		/*
+		 * We are most likely looking at an Escape sequence. Let's try
+		 * to read more bytes, waiting at most half a second, assuming
+		 * that the sequence is complete if we did not receive any byte
+		 * within that time.
+		 *
+		 * Start by replacing the Escape byte with ^[ */
+		strbuf_splice(buf, buf->len - 1, 1, "^[", 2);
+
+		for (;;) {
+			struct pollfd pfd = { .fd = 0, .events = POLLIN };
+
+			if (poll(&pfd, 1, 500) < 1)
+				break;
+
+			ch = getchar();
+			if (ch == EOF)
+				return 0;
+			strbuf_addch(buf, ch);
+		}
+	}
+
 	restore_term();
 	return 0;
 }
-- 
gitgitgadget


  parent reply	other threads:[~2019-12-21 22:42 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-21 22:41 [PATCH 0/9] built-in add -p: add support for the same config settings as the Perl version Johannes Schindelin via GitGitGadget
2019-12-21 22:41 ` [PATCH 1/9] built-in add -p: support interactive.diffFilter Johannes Schindelin via GitGitGadget
2019-12-21 22:41 ` [PATCH 2/9] built-in add -p: handle diff.algorithm Johannes Schindelin via GitGitGadget
2019-12-21 22:41 ` [PATCH 3/9] terminal: make the code of disable_echo() reusable Johannes Schindelin via GitGitGadget
2019-12-21 22:41 ` [PATCH 4/9] terminal: accommodate Git for Windows' default terminal Johannes Schindelin via GitGitGadget
2019-12-21 22:41 ` [PATCH 5/9] terminal: add a new function to read a single keystroke Johannes Schindelin via GitGitGadget
2019-12-21 22:41 ` [PATCH 6/9] built-in add -p: respect the `interactive.singlekey` config setting Johannes Schindelin via GitGitGadget
2019-12-21 22:41 ` Johannes Schindelin via GitGitGadget [this message]
2019-12-21 22:41 ` [PATCH 8/9] built-in add -p: handle Escape sequences more efficiently Johannes Schindelin via GitGitGadget
2019-12-21 22:42 ` [PATCH 9/9] ci: include the built-in `git add -i` in the `linux-gcc` job Johannes Schindelin via GitGitGadget
2019-12-21 22:53   ` SZEDER Gábor
2019-12-25 11:56     ` Johannes Schindelin
2019-12-22  0:11   ` Junio C Hamano
2019-12-25 11:57     ` Johannes Schindelin
2019-12-24 18:23 ` [PATCH 0/9] built-in add -p: add support for the same config settings as the Perl version Junio C Hamano
2019-12-24 18:39   ` Junio C Hamano
2019-12-25  8:46     ` Simon Ruderich
2019-12-25 12:09     ` Johannes Schindelin
2019-12-25 12:02   ` Johannes Schindelin
2019-12-25 11:56 ` [PATCH v2 " Johannes Schindelin via GitGitGadget
2019-12-25 11:56   ` [PATCH v2 1/9] built-in add -p: support interactive.diffFilter Johannes Schindelin via GitGitGadget
2020-01-07 22:57     ` SZEDER Gábor
2020-01-13  6:47       ` Johannes Schindelin
2019-12-25 11:56   ` [PATCH v2 2/9] built-in add -p: handle diff.algorithm Johannes Schindelin via GitGitGadget
2019-12-25 11:56   ` [PATCH v2 3/9] terminal: make the code of disable_echo() reusable Johannes Schindelin via GitGitGadget
2019-12-25 11:56   ` [PATCH v2 4/9] terminal: accommodate Git for Windows' default terminal Johannes Schindelin via GitGitGadget
2019-12-25 11:56   ` [PATCH v2 5/9] terminal: add a new function to read a single keystroke Johannes Schindelin via GitGitGadget
2019-12-25 11:56   ` [PATCH v2 6/9] built-in add -p: respect the `interactive.singlekey` config setting Johannes Schindelin via GitGitGadget
2019-12-25 11:56   ` [PATCH v2 7/9] built-in add -p: handle Escape sequences in interactive.singlekey mode Johannes Schindelin via GitGitGadget
2019-12-25 11:56   ` [PATCH v2 8/9] built-in add -p: handle Escape sequences more efficiently Johannes Schindelin via GitGitGadget
2019-12-25 11:57   ` [PATCH v2 9/9] ci: include the built-in `git add -i` in the `linux-gcc` job Johannes Schindelin via GitGitGadget
2019-12-26 20:48     ` Derrick Stolee
2020-01-01 22:10       ` Johannes Schindelin
2019-12-26 20:45   ` [PATCH v2 0/9] built-in add -p: add support for the same config settings as the Perl version Junio C Hamano
2020-01-13  8:29   ` [PATCH v3 00/10] " Johannes Schindelin via GitGitGadget
2020-01-13  8:29     ` [PATCH v3 01/10] built-in add -i/-p: treat SIGPIPE as EOF Johannes Schindelin via GitGitGadget
2020-01-13 17:04       ` SZEDER Gábor
2020-01-13 18:33         ` Jeff King
2020-01-15 18:32           ` Junio C Hamano
2020-01-15 19:03             ` Jeff King
2020-01-14 12:47         ` Johannes Schindelin
2020-01-17 14:32         ` SZEDER Gábor
2020-01-17 18:58           ` Jeff King
2020-01-13  8:29     ` [PATCH v3 02/10] built-in add -p: support interactive.diffFilter Johannes Schindelin via GitGitGadget
2020-01-13  8:29     ` [PATCH v3 03/10] built-in add -p: handle diff.algorithm Johannes Schindelin via GitGitGadget
2020-01-13  8:29     ` [PATCH v3 04/10] terminal: make the code of disable_echo() reusable Johannes Schindelin via GitGitGadget
2020-01-13  8:29     ` [PATCH v3 05/10] terminal: accommodate Git for Windows' default terminal Johannes Schindelin via GitGitGadget
2020-01-13  8:29     ` [PATCH v3 06/10] terminal: add a new function to read a single keystroke Johannes Schindelin via GitGitGadget
2020-01-13  8:29     ` [PATCH v3 07/10] built-in add -p: respect the `interactive.singlekey` config setting Johannes Schindelin via GitGitGadget
2020-01-13  8:29     ` [PATCH v3 08/10] built-in add -p: handle Escape sequences in interactive.singlekey mode Johannes Schindelin via GitGitGadget
2020-01-13  8:29     ` [PATCH v3 09/10] built-in add -p: handle Escape sequences more efficiently Johannes Schindelin via GitGitGadget
2020-01-13  8:29     ` [PATCH v3 10/10] ci: include the built-in `git add -i` in the `linux-gcc` job Johannes Schindelin via GitGitGadget
2020-01-14 18:43     ` [PATCH v4 00/10] built-in add -p: add support for the same config settings as the Perl version Johannes Schindelin via GitGitGadget
2020-01-14 18:43       ` [PATCH v4 01/10] t3701: adjust difffilter test Johannes Schindelin via GitGitGadget
2020-01-14 18:43       ` [PATCH v4 02/10] built-in add -p: support interactive.diffFilter Johannes Schindelin via GitGitGadget
2020-01-14 18:43       ` [PATCH v4 03/10] built-in add -p: handle diff.algorithm Johannes Schindelin via GitGitGadget
2020-01-14 18:43       ` [PATCH v4 04/10] terminal: make the code of disable_echo() reusable Johannes Schindelin via GitGitGadget
2020-01-14 18:43       ` [PATCH v4 05/10] terminal: accommodate Git for Windows' default terminal Johannes Schindelin via GitGitGadget
2020-01-14 18:43       ` [PATCH v4 06/10] terminal: add a new function to read a single keystroke Johannes Schindelin via GitGitGadget
2020-01-14 18:43       ` [PATCH v4 07/10] built-in add -p: respect the `interactive.singlekey` config setting Johannes Schindelin via GitGitGadget
2020-01-14 18:43       ` [PATCH v4 08/10] built-in add -p: handle Escape sequences in interactive.singlekey mode Johannes Schindelin via GitGitGadget
2020-01-14 18:43       ` [PATCH v4 09/10] built-in add -p: handle Escape sequences more efficiently Johannes Schindelin via GitGitGadget
2020-01-14 18:43       ` [PATCH v4 10/10] ci: include the built-in `git add -i` in the `linux-gcc` job Johannes Schindelin via GitGitGadget

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=fd5a12977609f69790fd2405baf5836add434cef.1576968120.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    /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.