{"thread":{"id":"52505","subject":"[PATCH 0/9] built-in add -p: add support for the same config settings as the Perl version","startedAt":"2019-12-21T22:42:04Z","lastAt":"2020-01-17T18:58:39Z","messageCount":63,"participants":["Johannes Schindelin via GitGitGadget","SZEDER Gábor","Junio C Hamano","Simon Ruderich","Johannes Schindelin","Derrick Stolee","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":9},"messages":[{"id":"388755","messageId":"pull.175.git.1576968120.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":null,"subject":"[PATCH 0/9] built-in add -p: add support for the same config settings as the Perl version","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-21T22:41:51Z","receivedAt":"2019-12-21T22:42:04Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"This is the final leg of the journey to a fully built-in git add: the git\nadd -i and git add -p modes were re-implemented in C, but they lacked\nsupport for a couple of config settings.\n\nThe one that sticks out most is the interactive.singleKey setting: it was\nparticularly hard to get to work, especially on Windows.\n\nIt also seems to be the setting that is incomplete already in the Perl\nversion of the interactive add command: while the name of the config setting\nsuggests that it applies to all of the interactive add, including the main\nloop of git add --interactive and to the file selections in that command, it\ndoes not. Only the git add --patch mode respects that setting.\n\nAs it is outside the purpose of the conversion of git-add--interactive.perl \nto C, we will leave that loose end for some future date.\n\nJohannes Schindelin (9):\n built-in add -p: support interactive.diffFilter\n built-in add -p: handle diff.algorithm\n terminal: make the code of disable_echo() reusable\n terminal: accommodate Git for Windows' default terminal\n terminal: add a new function to read a single keystroke\n built-in add -p: respect the `interactive.singlekey` config setting\n built-in add -p: handle Escape sequences in interactive.singlekey mode\n built-in add -p: handle Escape sequences more efficiently\n ci: include the built-in `git add -i` in the `linux-gcc` job\n\n add-interactive.c | 19 +++\n add-interactive.h | 4 +\n add-patch.c | 57 ++++++++-\n ci/run-build-and-tests.sh | 1 +\n compat/terminal.c | 249 +++++++++++++++++++++++++++++++++++++-\n compat/terminal.h | 3 +\n 6 files changed, 325 insertions(+), 8 deletions(-)\n\n\nbase-commit: 2d4b85ddc76af3e703e6e3a6a72319b5e79c2d8b\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-175%2Fdscho%2Fadd-p-in-c-config-settings-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-175/dscho/add-p-in-c-config-settings-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/175\n-- \ngitgitgadget\n"},{"id":"388756","messageId":"a7355776d6aef9d731de27c02caca728ff579bac.1576968120.git.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.git.1576968120.gitgitgadget@gmail.com","subject":"[PATCH 1/9] built-in add -p: support interactive.diffFilter","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-21T22:41:52Z","receivedAt":"2019-12-21T22:42:11Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThe Perl version supports post-processing the colored diff (that is\ngenerated in addition to the uncolored diff, intended to offer a\nprettier user experience) by a command configured via that config\nsetting, and now the built-in version does that, too.\n\nSigned-off-by: Johannes Schindelin \n---\n add-interactive.c | 12 ++++++++++++\n add-interactive.h | 3 +++\n add-patch.c | 33 +++++++++++++++++++++++++++++++++\n 3 files changed, 48 insertions(+)\n\ndiff --git a/add-interactive.c b/add-interactive.c\nindex 0e753d2acc..00c3bc9a1b 100644\n--- a/add-interactive.c\n+++ b/add-interactive.c\n@@ -52,6 +52,17 @@ void init_add_i_state(struct add_i_state *s, struct repository *r)\n \t\tdiff_get_color(s->use_color, DIFF_FILE_OLD));\n \tinit_color(r, s, \"new\", s->file_new_color,\n \t\tdiff_get_color(s->use_color, DIFF_FILE_NEW));\n+\n+\tFREE_AND_NULL(s->interactive_diff_filter);\n+\tgit_config_get_string(\"interactive.difffilter\",\n+\t\t\t &s->interactive_diff_filter);\n+}\n+\n+void clear_add_i_state(struct add_i_state *s)\n+{\n+\tFREE_AND_NULL(s->interactive_diff_filter);\n+\tmemset(s, 0, sizeof(*s));\n+\ts->use_color = -1;\n }\n \n /*\n@@ -1149,6 +1160,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps)\n \tstrbuf_release(&print_file_item_data.worktree);\n \tstrbuf_release(&header);\n \tprefix_item_list_clear(&commands);\n+\tclear_add_i_state(&s);\n \n \treturn res;\n }\ndiff --git a/add-interactive.h b/add-interactive.h\nindex 4895ed1df5..7299cf6e04 100644\n--- a/add-interactive.h\n+++ b/add-interactive.h\n@@ -15,9 +15,12 @@ struct add_i_state {\n \tchar context_color[COLOR_MAXLEN];\n \tchar file_old_color[COLOR_MAXLEN];\n \tchar file_new_color[COLOR_MAXLEN];\n+\n+\tchar *interactive_diff_filter;\n };\n \n void init_add_i_state(struct add_i_state *s, struct repository *r);\n+void clear_add_i_state(struct add_i_state *s);\n \n struct repository;\n struct pathspec;\ndiff --git a/add-patch.c b/add-patch.c\nindex 2ad18dc3cb..73bf2caca2 100644\n--- a/add-patch.c\n+++ b/add-patch.c\n@@ -397,6 +397,7 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n \n \tif (want_color_fd(1, -1)) {\n \t\tstruct child_process colored_cp = CHILD_PROCESS_INIT;\n+\t\tconst char *diff_filter = s->s.interactive_diff_filter;\n \n \t\tsetup_child_process(s, &colored_cp, NULL);\n \t\txsnprintf((char *)args.argv[color_arg_index], 8, \"--color\");\n@@ -406,6 +407,24 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n \t\targv_array_clear(&args);\n \t\tif (res)\n \t\t\treturn error(_(\"could not parse colored diff\"));\n+\n+\t\tif (diff_filter) {\n+\t\t\tstruct child_process filter_cp = CHILD_PROCESS_INIT;\n+\n+\t\t\tsetup_child_process(s, &filter_cp,\n+\t\t\t\t\t diff_filter, NULL);\n+\t\t\tfilter_cp.git_cmd = 0;\n+\t\t\tfilter_cp.use_shell = 1;\n+\t\t\tstrbuf_reset(&s->buf);\n+\t\t\tif (pipe_command(&filter_cp,\n+\t\t\t\t\t colored->buf, colored->len,\n+\t\t\t\t\t &s->buf, colored->len,\n+\t\t\t\t\t NULL, 0) < 0)\n+\t\t\t\treturn error(_(\"failed to run '%s'\"),\n+\t\t\t\t\t diff_filter);\n+\t\t\tstrbuf_swap(colored, &s->buf);\n+\t\t}\n+\n \t\tstrbuf_complete_line(colored);\n \t\tcolored_p = colored->buf;\n \t\tcolored_pend = colored_p + colored->len;\n@@ -530,6 +549,9 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n \t\t\t\t\t\t colored_pend - colored_p);\n \t\t\tif (colored_eol)\n \t\t\t\tcolored_p = colored_eol + 1;\n+\t\t\telse if (p != pend)\n+\t\t\t\t/* colored shorter than non-colored? */\n+\t\t\t\tgoto mismatched_output;\n \t\t\telse\n \t\t\t\tcolored_p = colored_pend;\n \n@@ -554,6 +576,15 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n \t\t */\n \t\thunk->splittable_into++;\n \n+\t/* non-colored shorter than colored? */\n+\tif (colored_p != colored_pend) {\n+mismatched_output:\n+\t\terror(_(\"mismatched output from interactive.diffFilter\"));\n+\t\tadvise(_(\"Your filter must maintain a one-to-one correspondence\\n\"\n+\t\t\t \"between its input and output lines.\"));\n+\t\treturn -1;\n+\t}\n+\n \treturn 0;\n }\n \n@@ -1611,6 +1642,7 @@ int run_add_p(struct repository *r, enum add_p_mode mode,\n \t parse_diff(&s, ps) < 0) {\n \t\tstrbuf_release(&s.plain);\n \t\tstrbuf_release(&s.colored);\n+\t\tclear_add_i_state(&s.s);\n \t\treturn -1;\n \t}\n \n@@ -1629,5 +1661,6 @@ int run_add_p(struct repository *r, enum add_p_mode mode,\n \tstrbuf_release(&s.buf);\n \tstrbuf_release(&s.plain);\n \tstrbuf_release(&s.colored);\n+\tclear_add_i_state(&s.s);\n \treturn 0;\n }\n-- \ngitgitgadget\n\n"},{"id":"388757","messageId":"74958419f62ee5a04b5fbc3ce6b399353f0791c8.1576968120.git.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.git.1576968120.gitgitgadget@gmail.com","subject":"[PATCH 2/9] built-in add -p: handle diff.algorithm","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-21T22:41:53Z","receivedAt":"2019-12-21T22:42:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThe Perl version of `git add -p` reads the config setting\n`diff.algorithm` and if set, uses it to generate the diff using the\nspecified algorithm.\n\nThis patch ports that functionality to the C version.\n\nNote: just like `git-add--interactive.perl`, we do _not_ respect this\nconfig setting in `git add -i`'s `diff` command, but _only_ in the\n`patch` command.\n\nSigned-off-by: Johannes Schindelin \n---\n add-interactive.c | 5 +++++\n add-interactive.h | 2 +-\n add-patch.c | 3 +++\n 3 files changed, 9 insertions(+), 1 deletion(-)\n\ndiff --git a/add-interactive.c b/add-interactive.c\nindex 00c3bc9a1b..77762d75d6 100644\n--- a/add-interactive.c\n+++ b/add-interactive.c\n@@ -56,11 +56,16 @@ void init_add_i_state(struct add_i_state *s, struct repository *r)\n \tFREE_AND_NULL(s->interactive_diff_filter);\n \tgit_config_get_string(\"interactive.difffilter\",\n \t\t\t &s->interactive_diff_filter);\n+\n+\tFREE_AND_NULL(s->interactive_diff_algorithm);\n+\tgit_config_get_string(\"diff.algorithm\",\n+\t\t\t &s->interactive_diff_algorithm);\n }\n \n void clear_add_i_state(struct add_i_state *s)\n {\n \tFREE_AND_NULL(s->interactive_diff_filter);\n+\tFREE_AND_NULL(s->interactive_diff_algorithm);\n \tmemset(s, 0, sizeof(*s));\n \ts->use_color = -1;\n }\ndiff --git a/add-interactive.h b/add-interactive.h\nindex 7299cf6e04..21389851aa 100644\n--- a/add-interactive.h\n+++ b/add-interactive.h\n@@ -16,7 +16,7 @@ struct add_i_state {\n \tchar file_old_color[COLOR_MAXLEN];\n \tchar file_new_color[COLOR_MAXLEN];\n \n-\tchar *interactive_diff_filter;\n+\tchar *interactive_diff_filter, *interactive_diff_algorithm;\n };\n \n void init_add_i_state(struct add_i_state *s, struct repository *r);\ndiff --git a/add-patch.c b/add-patch.c\nindex 73bf2caca2..fdfaa76c3c 100644\n--- a/add-patch.c\n+++ b/add-patch.c\n@@ -359,6 +359,7 @@ static int is_octal(const char *p, size_t len)\n static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n {\n \tstruct argv_array args = ARGV_ARRAY_INIT;\n+\tconst char *diff_algorithm = s->s.interactive_diff_algorithm;\n \tstruct strbuf *plain = &s->plain, *colored = NULL;\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n \tchar *p, *pend, *colored_p = NULL, *colored_pend = NULL, marker = '\\0';\n@@ -368,6 +369,8 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n \tint res;\n \n \targv_array_pushv(&args, s->mode->diff);\n+\tif (diff_algorithm)\n+\t\targv_array_pushf(&args, \"--diff-algorithm=%s\", diff_algorithm);\n \tif (s->revision) {\n \t\tstruct object_id oid;\n \t\targv_array_push(&args,\n-- \ngitgitgadget\n\n"},{"id":"388758","messageId":"6d6794089d23169c7ea20f4d9ad1351ddcd33e51.1576968120.git.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.git.1576968120.gitgitgadget@gmail.com","subject":"[PATCH 6/9] built-in add -p: respect the `interactive.singlekey` config setting","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-21T22:41:57Z","receivedAt":"2019-12-21T22:42:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThe Perl version of `git add -p` supports this config setting to allow\nusers to input commands via single characters (as opposed to having to\npress the key afterwards).\n\nThis is an opt-in feature because it requires Perl packages\n(Term::ReadKey and Term::Cap, where it tries to handle an absence of the\nlatter package gracefully) to work. Note that at least on Ubuntu, that\nPerl package is not installed by default (it needs to be installed via\n`sudo apt-get install libterm-readkey-perl`), so this feature is\nprobably not used a whole lot.\n\nIn C, we obviously do not have these packages available, but we just\nintroduced `read_single_keystroke()` that is similar to what\nTerm::ReadKey provides, and we use that here.\n\nSigned-off-by: Johannes Schindelin \n---\n add-interactive.c | 2 ++\n add-interactive.h | 1 +\n add-patch.c | 21 +++++++++++++++++----\n 3 files changed, 20 insertions(+), 4 deletions(-)\n\ndiff --git a/add-interactive.c b/add-interactive.c\nindex 77762d75d6..01a2f92f0c 100644\n--- a/add-interactive.c\n+++ b/add-interactive.c\n@@ -60,6 +60,8 @@ void init_add_i_state(struct add_i_state *s, struct repository *r)\n \tFREE_AND_NULL(s->interactive_diff_algorithm);\n \tgit_config_get_string(\"diff.algorithm\",\n \t\t\t &s->interactive_diff_algorithm);\n+\n+\tgit_config_get_bool(\"interactive.singlekey\", &s->use_single_key);\n }\n \n void clear_add_i_state(struct add_i_state *s)\ndiff --git a/add-interactive.h b/add-interactive.h\nindex 21389851aa..3450359685 100644\n--- a/add-interactive.h\n+++ b/add-interactive.h\n@@ -16,6 +16,7 @@ struct add_i_state {\n \tchar file_old_color[COLOR_MAXLEN];\n \tchar file_new_color[COLOR_MAXLEN];\n \n+\tint use_single_key;\n \tchar *interactive_diff_filter, *interactive_diff_algorithm;\n };\n \ndiff --git a/add-patch.c b/add-patch.c\nindex fdfaa76c3c..ef0469d398 100644\n--- a/add-patch.c\n+++ b/add-patch.c\n@@ -6,6 +6,7 @@\n #include \"pathspec.h\"\n #include \"color.h\"\n #include \"diff.h\"\n+#include \"compat/terminal.h\"\n \n enum prompt_mode_type {\n \tPROMPT_MODE_CHANGE = 0, PROMPT_DELETION, PROMPT_HUNK\n@@ -1148,14 +1149,27 @@ static int run_apply_check(struct add_p_state *s,\n \treturn 0;\n }\n \n+static int read_single_character(struct add_p_state *s)\n+{\n+\tif (s->s.use_single_key) {\n+\t\tint res = read_key_without_echo(&s->answer);\n+\t\tprintf(\"%s\\n\", res == EOF ? \"\" : s->answer.buf);\n+\t\treturn res;\n+\t}\n+\n+\tif (strbuf_getline(&s->answer, stdin) == EOF)\n+\t\treturn EOF;\n+\tstrbuf_trim_trailing_newline(&s->answer);\n+\treturn 0;\n+}\n+\n static int prompt_yesno(struct add_p_state *s, const char *prompt)\n {\n \tfor (;;) {\n \t\tcolor_fprintf(stdout, s->s.prompt_color, \"%s\", _(prompt));\n \t\tfflush(stdout);\n-\t\tif (strbuf_getline(&s->answer, stdin) == EOF)\n+\t\tif (read_single_character(s) == EOF)\n \t\t\treturn -1;\n-\t\tstrbuf_trim_trailing_newline(&s->answer);\n \t\tswitch (tolower(s->answer.buf[0])) {\n \t\tcase 'n': return 0;\n \t\tcase 'y': return 1;\n@@ -1395,9 +1409,8 @@ static int patch_update_file(struct add_p_state *s,\n \t\t\t _(s->mode->prompt_mode[prompt_mode_type]),\n \t\t\t s->buf.buf);\n \t\tfflush(stdout);\n-\t\tif (strbuf_getline(&s->answer, stdin) == EOF)\n+\t\tif (read_single_character(s) == EOF)\n \t\t\tbreak;\n-\t\tstrbuf_trim_trailing_newline(&s->answer);\n \n \t\tif (!s->answer.len)\n \t\t\tcontinue;\n-- \ngitgitgadget\n\n"},{"id":"388759","messageId":"af9b59873833ad56d803d101a6c0d7f049017bfe.1576968120.git.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.git.1576968120.gitgitgadget@gmail.com","subject":"[PATCH 8/9] built-in add -p: handle Escape sequences more efficiently","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-21T22:41:59Z","receivedAt":"2019-12-21T22:42:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWhen `interactive.singlekey = true`, we react immediately to keystrokes,\neven to Escape sequences (e.g. when pressing a cursor key).\n\nThe problem with Escape sequences is that we do not really know when\nthey are done, and as a heuristic we poll standard input for half a\nsecond to make sure that we got all of it.\n\nWhile waiting half a second is not asking for a whole lot, it can become\nquite annoying over time, therefore with this patch, we read the\nterminal capabilities (if available) and extract known Escape sequences\nfrom there, then stop polling immediately when we detected that the user\npressed a key that generated such a known sequence.\n\nThis recapitulates the remaining part of b5cc003253c8 (add -i: ignore\nterminal escape sequences, 2011-05-17).\n\nNote: We do *not* query the terminal capabilities directly. That would\neither require a lot of platform-specific code, or it would require\nlinking to a library such as ncurses.\n\nLinking to a library in the built-ins is something we try very hard to\navoid (we even kicked the libcurl dependency to a non-built-in remote\nhelper, just to shave off a tiny fraction of a second from Git's startup\ntime). And the platform-specific code would be a maintenance nightmare.\n\nEven worse: in Git for Windows' case, we would need to query MSYS2\npseudo terminals, which `git.exe` simply cannot do (because it is\nintentionally *not* an MSYS2 program).\n\nTo address this, we simply spawn `infocmp -L -1` and parse its output\n(which works even in Git for Windows, because that helper is included in\nthe end-user facing installations).\n\nThis is done only once, as in the Perl version, but it is done only when\nthe first Escape sequence is encountered, not upon startup of `git add\n-i`; This saves on startup time, yet makes reacting to the first Escape\nsequence slightly more sluggish. But it allows us to keep the\nterminal-related code encapsulated in the `compat/terminal.c` file.\n\nSigned-off-by: Johannes Schindelin \n---\n compat/terminal.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++-\n 1 file changed, 72 insertions(+), 1 deletion(-)\n\ndiff --git a/compat/terminal.c b/compat/terminal.c\nindex b7f58d1781..35bca03d14 100644\n--- a/compat/terminal.c\n+++ b/compat/terminal.c\n@@ -4,6 +4,7 @@\n #include \"strbuf.h\"\n #include \"run-command.h\"\n #include \"string-list.h\"\n+#include \"hashmap.h\"\n \n #if defined(HAVE_DEV_TTY) || defined(GIT_WINDOWS_NATIVE)\n \n@@ -238,6 +239,71 @@ char *git_terminal_prompt(const char *prompt, int echo)\n \treturn buf.buf;\n }\n \n+/*\n+ * The `is_known_escape_sequence()` function returns 1 if the passed string\n+ * corresponds to an Escape sequence that the terminal capabilities contains.\n+ *\n+ * To avoid depending on ncurses or other platform-specific libraries, we rely\n+ * on the presence of the `infocmp` executable to do the job for us (failing\n+ * silently if the program is not available or refused to run).\n+ */\n+struct escape_sequence_entry {\n+\tstruct hashmap_entry entry;\n+\tchar sequence[FLEX_ARRAY];\n+};\n+\n+static int sequence_entry_cmp(const void *hashmap_cmp_fn_data,\n+\t\t\t const struct escape_sequence_entry *e1,\n+\t\t\t const struct escape_sequence_entry *e2,\n+\t\t\t const void *keydata)\n+{\n+\treturn strcmp(e1->sequence, keydata ? keydata : e2->sequence);\n+}\n+\n+static int is_known_escape_sequence(const char *sequence)\n+{\n+\tstatic struct hashmap sequences;\n+\tstatic int initialized;\n+\n+\tif (!initialized) {\n+\t\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\t\tstruct strbuf buf = STRBUF_INIT;\n+\t\tchar *p, *eol;\n+\n+\t\thashmap_init(&sequences, (hashmap_cmp_fn)sequence_entry_cmp,\n+\t\t\t NULL, 0);\n+\n+\t\targv_array_pushl(&cp.args, \"infocmp\", \"-L\", \"-1\", NULL);\n+\t\tif (pipe_command(&cp, NULL, 0, &buf, 0, NULL, 0))\n+\t\t\tstrbuf_setlen(&buf, 0);\n+\n+\t\tfor (eol = p = buf.buf; *p; p = eol + 1) {\n+\t\t\tp = strchr(p, '=');\n+\t\t\tif (!p)\n+\t\t\t\tbreak;\n+\t\t\tp++;\n+\t\t\teol = strchrnul(p, '\\n');\n+\n+\t\t\tif (starts_with(p, \"\\\\E\")) {\n+\t\t\t\tchar *comma = memchr(p, ',', eol - p);\n+\t\t\t\tstruct escape_sequence_entry *e;\n+\n+\t\t\t\tp[0] = '^';\n+\t\t\t\tp[1] = '[';\n+\t\t\t\tFLEX_ALLOC_MEM(e, sequence, p, comma - p);\n+\t\t\t\thashmap_entry_init(&e->entry,\n+\t\t\t\t\t\t strhash(e->sequence));\n+\t\t\t\thashmap_add(&sequences, &e->entry);\n+\t\t\t}\n+\t\t\tif (!*eol)\n+\t\t\t\tbreak;\n+\t\t}\n+\t\tinitialized = 1;\n+\t}\n+\n+\treturn !!hashmap_get_from_hash(&sequences, strhash(sequence), sequence);\n+}\n+\n int read_key_without_echo(struct strbuf *buf)\n {\n \tstatic int warning_displayed;\n@@ -271,7 +337,12 @@ int read_key_without_echo(struct strbuf *buf)\n \t\t * Start by replacing the Escape byte with ^[ */\n \t\tstrbuf_splice(buf, buf->len - 1, 1, \"^[\", 2);\n \n-\t\tfor (;;) {\n+\t\t/*\n+\t\t * Query the terminal capabilities once about all the Escape\n+\t\t * sequences it knows about, so that we can avoid waiting for\n+\t\t * half a second when we know that the sequence is complete.\n+\t\t */\n+\t\twhile (!is_known_escape_sequence(buf->buf)) {\n \t\t\tstruct pollfd pfd = { .fd = 0, .events = POLLIN };\n \n \t\t\tif (poll(&pfd, 1, 500) < 1)\n-- \ngitgitgadget\n\n"},{"id":"388760","messageId":"a77fa914da14c84ff2ebadd26fbcac97456aae04.1576968120.git.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.git.1576968120.gitgitgadget@gmail.com","subject":"[PATCH 4/9] terminal: accommodate Git for Windows' default terminal","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-21T22:41:55Z","receivedAt":"2019-12-21T22:42:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nGit for Windows' Git Bash runs in MinTTY by default, which does not have\na Win32 Console instance, but uses MSYS2 pseudo terminals instead.\n\nThis is a problem, as Git for Windows does not want to use the MSYS2\nemulation layer for Git itself, and therefore has no direct way to\ninteract with that pseudo terminal.\n\nAs a workaround, use the `stty` utility (which is included in Git for\nWindows, and which *is* an MSYS2 program, so it knows how to deal with\nthe pseudo terminal).\n\nNote: If Git runs in a regular CMD or PowerShell window, there *is* a\nregular Win32 Console to work with. This is not a problem for the MSYS2\n`stty`: it copes with this scenario just fine.\n\nAlso note that we introduce support for more bits than would be\nnecessary for a mere `disable_echo()` here, in preparation for the\nupcoming `enable_non_canonical()` function.\n\nSigned-off-by: Johannes Schindelin \n---\n compat/terminal.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 50 insertions(+)\n\ndiff --git a/compat/terminal.c b/compat/terminal.c\nindex 1fb40b3a0a..16e9949da1 100644\n--- a/compat/terminal.c\n+++ b/compat/terminal.c\n@@ -2,6 +2,8 @@\n #include \"compat/terminal.h\"\n #include \"sigchain.h\"\n #include \"strbuf.h\"\n+#include \"run-command.h\"\n+#include \"string-list.h\"\n \n #if defined(HAVE_DEV_TTY) || defined(GIT_WINDOWS_NATIVE)\n \n@@ -64,11 +66,28 @@ static int disable_echo(void)\n #define OUTPUT_PATH \"CONOUT$\"\n #define FORCE_TEXT \"t\"\n \n+static int use_stty = 1;\n+static struct string_list stty_restore = STRING_LIST_INIT_DUP;\n static HANDLE hconin = INVALID_HANDLE_VALUE;\n static DWORD cmode;\n \n static void restore_term(void)\n {\n+\tif (use_stty) {\n+\t\tint i;\n+\t\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\n+\t\tif (stty_restore.nr == 0)\n+\t\t\treturn;\n+\n+\t\targv_array_push(&cp.args, \"stty\");\n+\t\tfor (i = 0; i < stty_restore.nr; i++)\n+\t\t\targv_array_push(&cp.args, stty_restore.items[i].string);\n+\t\trun_command(&cp);\n+\t\tstring_list_clear(&stty_restore, 0);\n+\t\treturn;\n+\t}\n+\n \tif (hconin == INVALID_HANDLE_VALUE)\n \t\treturn;\n \n@@ -79,6 +98,37 @@ static void restore_term(void)\n \n static int disable_bits(DWORD bits)\n {\n+\tif (use_stty) {\n+\t\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\n+\t\targv_array_push(&cp.args, \"stty\");\n+\n+\t\tif (bits & ENABLE_LINE_INPUT) {\n+\t\t\tstring_list_append(&stty_restore, \"icanon\");\n+\t\t\targv_array_push(&cp.args, \"-icanon\");\n+\t\t}\n+\n+\t\tif (bits & ENABLE_ECHO_INPUT) {\n+\t\t\tstring_list_append(&stty_restore, \"echo\");\n+\t\t\targv_array_push(&cp.args, \"-echo\");\n+\t\t}\n+\n+\t\tif (bits & ENABLE_PROCESSED_INPUT) {\n+\t\t\tstring_list_append(&stty_restore, \"-ignbrk\");\n+\t\t\tstring_list_append(&stty_restore, \"intr\");\n+\t\t\tstring_list_append(&stty_restore, \"^c\");\n+\t\t\targv_array_push(&cp.args, \"ignbrk\");\n+\t\t\targv_array_push(&cp.args, \"intr\");\n+\t\t\targv_array_push(&cp.args, \"\");\n+\t\t}\n+\n+\t\tif (run_command(&cp) == 0)\n+\t\t\treturn 0;\n+\n+\t\t/* `stty` could not be executed; access the Console directly */\n+\t\tuse_stty = 0;\n+\t}\n+\n \thconin = CreateFile(\"CONIN$\", GENERIC_READ | GENERIC_WRITE,\n \t FILE_SHARE_READ, NULL, OPEN_EXISTING,\n \t FILE_ATTRIBUTE_NORMAL, NULL);\n-- \ngitgitgadget\n\n"},{"id":"388761","messageId":"7631c1ea8c82154581eaaed9dcc278a011897282.1576968120.git.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.git.1576968120.gitgitgadget@gmail.com","subject":"[PATCH 3/9] terminal: make the code of disable_echo() reusable","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-21T22:41:54Z","receivedAt":"2019-12-21T22:42:13Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWe are about to introduce the function `enable_non_canonical()`, which\nshares almost the complete code with `disable_echo()`.\n\nLet's prepare for that, by refactoring out that shared code.\n\nSigned-off-by: Johannes Schindelin \n---\n compat/terminal.c | 19 +++++++++++++++----\n 1 file changed, 15 insertions(+), 4 deletions(-)\n\ndiff --git a/compat/terminal.c b/compat/terminal.c\nindex fa13ee672d..1fb40b3a0a 100644\n--- a/compat/terminal.c\n+++ b/compat/terminal.c\n@@ -32,7 +32,7 @@ static void restore_term(void)\n \tterm_fd = -1;\n }\n \n-static int disable_echo(void)\n+static int disable_bits(tcflag_t bits)\n {\n \tstruct termios t;\n \n@@ -43,7 +43,7 @@ static int disable_echo(void)\n \told_term = t;\n \tsigchain_push_common(restore_term_on_signal);\n \n-\tt.c_lflag &= ~ECHO;\n+\tt.c_lflag &= ~bits;\n \tif (!tcsetattr(term_fd, TCSAFLUSH, &t))\n \t\treturn 0;\n \n@@ -53,6 +53,11 @@ static int disable_echo(void)\n \treturn -1;\n }\n \n+static int disable_echo(void)\n+{\n+\treturn disable_bits(ECHO);\n+}\n+\n #elif defined(GIT_WINDOWS_NATIVE)\n \n #define INPUT_PATH \"CONIN$\"\n@@ -72,7 +77,7 @@ static void restore_term(void)\n \thconin = INVALID_HANDLE_VALUE;\n }\n \n-static int disable_echo(void)\n+static int disable_bits(DWORD bits)\n {\n \thconin = CreateFile(\"CONIN$\", GENERIC_READ | GENERIC_WRITE,\n \t FILE_SHARE_READ, NULL, OPEN_EXISTING,\n@@ -82,7 +87,7 @@ static int disable_echo(void)\n \n \tGetConsoleMode(hconin, &cmode);\n \tsigchain_push_common(restore_term_on_signal);\n-\tif (!SetConsoleMode(hconin, cmode & (~ENABLE_ECHO_INPUT))) {\n+\tif (!SetConsoleMode(hconin, cmode & ~bits)) {\n \t\tCloseHandle(hconin);\n \t\thconin = INVALID_HANDLE_VALUE;\n \t\treturn -1;\n@@ -91,6 +96,12 @@ static int disable_echo(void)\n \treturn 0;\n }\n \n+static int disable_echo(void)\n+{\n+\treturn disable_bits(ENABLE_ECHO_INPUT);\n+}\n+\n+\n #endif\n \n #ifndef FORCE_TEXT\n-- \ngitgitgadget\n\n"},{"id":"388762","messageId":"fd5a12977609f69790fd2405baf5836add434cef.1576968120.git.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.git.1576968120.gitgitgadget@gmail.com","subject":"[PATCH 7/9] built-in add -p: handle Escape sequences in interactive.singlekey mode","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-21T22:41:58Z","receivedAt":"2019-12-21T22:42:14Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThis recapitulates part of b5cc003253c8 (add -i: ignore terminal escape\nsequences, 2011-05-17):\n\n add -i: ignore terminal escape sequences\n\n On the author's terminal, the up-arrow input sequence is ^[[A, and\n thus fat-fingering an up-arrow into 'git checkout -p' is quite\n dangerous: git-add--interactive.perl will ignore the ^[ and [\n characters and happily treat A as \"discard everything\".\n\n As a band-aid fix, use Term::Cap to get all terminal capabilities.\n Then use the heuristic that any capability value that starts with ^[\n (i.e., \\e in perl) must be a key input sequence. Finally, given an\n input that starts with ^[, read more characters until we have read a\n full escape sequence, then return that to the caller. We use a\n timeout of 0.5 seconds on the subsequent reads to avoid getting stuck\n if the user actually input a lone ^[.\n\n Since none of the currently recognized keys start with ^[, the net\n result is that the sequence as a whole will be ignored and the help\n displayed.\n\nNote that we leave part for later which uses \"Term::Cap to get all\nterminal capabilities\", for several reasons:\n\n1. it is actually not really necessary, as the timeout of 0.5 seconds\n should be plenty sufficient to catch Escape sequences,\n\n2. it is cleaner to keep the change to special-case Escape sequences\n separate from the change that reads all terminal capabilities to\n speed things up, and\n\n3. in practice, relying on the terminal capabilities is a bit overrated,\n as the information could be incomplete, or plain wrong. For example,\n in this developer's tmux sessions, the terminal capabilities claim\n that the \"cursor up\" sequence is ^[M, but the actual sequence\n produced by the \"cursor up\" key is ^[[A.\n\nSigned-off-by: Johannes Schindelin \n---\n compat/terminal.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++-\n 1 file changed, 55 insertions(+), 1 deletion(-)\n\ndiff --git a/compat/terminal.c b/compat/terminal.c\nindex 1b2564042a..b7f58d1781 100644\n--- a/compat/terminal.c\n+++ b/compat/terminal.c\n@@ -161,6 +161,37 @@ static int enable_non_canonical(void)\n \treturn disable_bits(ENABLE_ECHO_INPUT | ENABLE_LINE_INPUT | ENABLE_PROCESSED_INPUT);\n }\n \n+/*\n+ * Override `getchar()`, as the default implementation does not use\n+ * `ReadFile()`.\n+ *\n+ * This poses a problem when we want to see whether the standard\n+ * input has more characters, as the default of Git for Windows is to start the\n+ * Bash in a MinTTY, which uses a named pipe to emulate a pty, in which case\n+ * our `poll()` emulation calls `PeekNamedPipe()`, which seems to require\n+ * `ReadFile()` to be called first to work properly (it only reports 0\n+ * available bytes, otherwise).\n+ *\n+ * So let's just override `getchar()` with a version backed by `ReadFile()` and\n+ * go our merry ways from here.\n+ */\n+static int mingw_getchar(void)\n+{\n+\tDWORD read = 0;\n+\tunsigned char ch;\n+\n+\tif (!ReadFile(GetStdHandle(STD_INPUT_HANDLE), &ch, 1, &read, NULL))\n+\t\treturn EOF;\n+\n+\tif (!read) {\n+\t\terror(\"Unexpected 0 read\");\n+\t\treturn EOF;\n+\t}\n+\n+\treturn ch;\n+}\n+#define getchar mingw_getchar\n+\n #endif\n \n #ifndef FORCE_TEXT\n@@ -228,8 +259,31 @@ int read_key_without_echo(struct strbuf *buf)\n \t\trestore_term();\n \t\treturn EOF;\n \t}\n-\n \tstrbuf_addch(buf, ch);\n+\n+\tif (ch == '\\033' /* ESC */) {\n+\t\t/*\n+\t\t * We are most likely looking at an Escape sequence. Let's try\n+\t\t * to read more bytes, waiting at most half a second, assuming\n+\t\t * that the sequence is complete if we did not receive any byte\n+\t\t * within that time.\n+\t\t *\n+\t\t * Start by replacing the Escape byte with ^[ */\n+\t\tstrbuf_splice(buf, buf->len - 1, 1, \"^[\", 2);\n+\n+\t\tfor (;;) {\n+\t\t\tstruct pollfd pfd = { .fd = 0, .events = POLLIN };\n+\n+\t\t\tif (poll(&pfd, 1, 500) < 1)\n+\t\t\t\tbreak;\n+\n+\t\t\tch = getchar();\n+\t\t\tif (ch == EOF)\n+\t\t\t\treturn 0;\n+\t\t\tstrbuf_addch(buf, ch);\n+\t\t}\n+\t}\n+\n \trestore_term();\n \treturn 0;\n }\n-- \ngitgitgadget\n\n"},{"id":"388764","messageId":"3996d7997a66aec61ebdd7831879aa8522f9938a.1576968120.git.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.git.1576968120.gitgitgadget@gmail.com","subject":"[PATCH 5/9] terminal: add a new function to read a single keystroke","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-21T22:41:56Z","receivedAt":"2019-12-21T22:42:14Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nTypically, input on the command-line is line-based. It is actually not\nreally easy to get single characters (or better put: keystrokes).\n\nWe provide two implementations here:\n\n- One that handles `/dev/tty` based systems as well as native Windows.\n The former uses the `tcsetattr()` function to put the terminal into\n \"raw mode\", which allows us to read individual keystrokes, one by one.\n The latter uses `stty.exe` to do the same, falling back to direct\n Win32 Console access.\n\n Thanks to the refactoring leading up to this commit, this is a single\n function, with the platform-specific details hidden away in\n conditionally-compiled code blocks.\n\n- A fall-back which simply punts and reads back an entire line.\n\nNote that the function writes the keystroke into an `strbuf` rather than\na `char`, in preparation for reading Escape sequences (e.g. when the\nuser hit an arrow key). This is also required for UTF-8 sequences in\ncase the keystroke corresponds to a non-ASCII letter.\n\nSigned-off-by: Johannes Schindelin \n---\n compat/terminal.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++\n compat/terminal.h | 3 +++\n 2 files changed, 58 insertions(+)\n\ndiff --git a/compat/terminal.c b/compat/terminal.c\nindex 16e9949da1..1b2564042a 100644\n--- a/compat/terminal.c\n+++ b/compat/terminal.c\n@@ -60,6 +60,11 @@ static int disable_echo(void)\n \treturn disable_bits(ECHO);\n }\n \n+static int enable_non_canonical(void)\n+{\n+\treturn disable_bits(ICANON | ECHO);\n+}\n+\n #elif defined(GIT_WINDOWS_NATIVE)\n \n #define INPUT_PATH \"CONIN$\"\n@@ -151,6 +156,10 @@ static int disable_echo(void)\n \treturn disable_bits(ENABLE_ECHO_INPUT);\n }\n \n+static int enable_non_canonical(void)\n+{\n+\treturn disable_bits(ENABLE_ECHO_INPUT | ENABLE_LINE_INPUT | ENABLE_PROCESSED_INPUT);\n+}\n \n #endif\n \n@@ -198,6 +207,33 @@ char *git_terminal_prompt(const char *prompt, int echo)\n \treturn buf.buf;\n }\n \n+int read_key_without_echo(struct strbuf *buf)\n+{\n+\tstatic int warning_displayed;\n+\tint ch;\n+\n+\tif (warning_displayed || enable_non_canonical() < 0) {\n+\t\tif (!warning_displayed) {\n+\t\t\twarning(\"reading single keystrokes not supported on \"\n+\t\t\t\t\"this platform; reading line instead\");\n+\t\t\twarning_displayed = 1;\n+\t\t}\n+\n+\t\treturn strbuf_getline(buf, stdin);\n+\t}\n+\n+\tstrbuf_reset(buf);\n+\tch = getchar();\n+\tif (ch == EOF) {\n+\t\trestore_term();\n+\t\treturn EOF;\n+\t}\n+\n+\tstrbuf_addch(buf, ch);\n+\trestore_term();\n+\treturn 0;\n+}\n+\n #else\n \n char *git_terminal_prompt(const char *prompt, int echo)\n@@ -205,4 +241,23 @@ char *git_terminal_prompt(const char *prompt, int echo)\n \treturn getpass(prompt);\n }\n \n+int read_key_without_echo(struct strbuf *buf)\n+{\n+\tstatic int warning_displayed;\n+\tconst char *res;\n+\n+\tif (!warning_displayed) {\n+\t\twarning(\"reading single keystrokes not supported on this \"\n+\t\t\t\"platform; reading line instead\");\n+\t\twarning_displayed = 1;\n+\t}\n+\n+\tres = getpass(\"\");\n+\tstrbuf_reset(buf);\n+\tif (!res)\n+\t\treturn EOF;\n+\tstrbuf_addstr(buf, res);\n+\treturn 0;\n+}\n+\n #endif\ndiff --git a/compat/terminal.h b/compat/terminal.h\nindex 97db7cd69d..a9d52b8464 100644\n--- a/compat/terminal.h\n+++ b/compat/terminal.h\n@@ -3,4 +3,7 @@\n \n char *git_terminal_prompt(const char *prompt, int echo);\n \n+/* Read a single keystroke, without echoing it to the terminal */\n+int read_key_without_echo(struct strbuf *buf);\n+\n #endif /* COMPAT_TERMINAL_H */\n-- \ngitgitgadget\n\n"},{"id":"388763","messageId":"9719604a1fb6ec4cf1b1297875cae86c076c9cdd.1576968120.git.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.git.1576968120.gitgitgadget@gmail.com","subject":"[PATCH 9/9] ci: include the built-in `git add -i` in the `linux-gcc` job","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-21T22:42:00Z","receivedAt":"2019-12-21T22:42:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThis job runs the test suite twice, once in regular mode, and once with\na whole slew of `GIT_TEST_*` variables set.\n\nNow that the built-in version of `git add --interactive` is\nfeature-complete, let's also throw `GIT_TEST_MULTI_PACK_INDEX` into that\nfray.\n\nSigned-off-by: Johannes Schindelin \n---\n ci/run-build-and-tests.sh | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh\nindex ff0ef7f08e..4df54c4efe 100755\n--- a/ci/run-build-and-tests.sh\n+++ b/ci/run-build-and-tests.sh\n@@ -20,6 +20,7 @@ linux-gcc)\n \texport GIT_TEST_OE_DELTA_SIZE=5\n \texport GIT_TEST_COMMIT_GRAPH=1\n \texport GIT_TEST_MULTI_PACK_INDEX=1\n+\texport GIT_TEST_ADD_I_USE_BUILTIN=1\n \tmake test\n \t;;\n linux-gcc-4.8)\n-- \ngitgitgadget\n"},{"id":"388765","messageId":"20191221225354.GB32750@szeder.dev","threadId":"52505","inReplyTo":"9719604a1fb6ec4cf1b1297875cae86c076c9cdd.1576968120.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 9/9] ci: include the built-in `git add -i` in the `linux-gcc` job","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-12-21T22:53:54Z","receivedAt":"2019-12-21T22:54:11Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Sat, Dec 21, 2019 at 10:42:00PM +0000, Johannes Schindelin via GitGitGadget wrote:\n> From: Johannes Schindelin \n> \n> This job runs the test suite twice, once in regular mode, and once with\n> a whole slew of `GIT_TEST_*` variables set.\n> \n> Now that the built-in version of `git add --interactive` is\n> feature-complete, let's also throw `GIT_TEST_MULTI_PACK_INDEX` into that\n\nGIT_TEST_MULTI_PACK_INDEX? ;)\n\n> fray.\n> \n> Signed-off-by: Johannes Schindelin \n> ---\n> ci/run-build-and-tests.sh | 1 +\n> 1 file changed, 1 insertion(+)\n> \n> diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh\n> index ff0ef7f08e..4df54c4efe 100755\n> --- a/ci/run-build-and-tests.sh\n> +++ b/ci/run-build-and-tests.sh\n> @@ -20,6 +20,7 @@ linux-gcc)\n> \texport GIT_TEST_OE_DELTA_SIZE=5\n> \texport GIT_TEST_COMMIT_GRAPH=1\n> \texport GIT_TEST_MULTI_PACK_INDEX=1\n> +\texport GIT_TEST_ADD_I_USE_BUILTIN=1\n> \tmake test\n> \t;;\n> linux-gcc-4.8)\n> -- \n> gitgitgadget\n"},{"id":"388772","messageId":"xmqq5zi96wxc.fsf@gitster-ct.c.googlers.com","threadId":"52505","inReplyTo":"9719604a1fb6ec4cf1b1297875cae86c076c9cdd.1576968120.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 9/9] ci: include the built-in `git add -i` in the `linux-gcc` job","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-12-22T00:11:59Z","receivedAt":"2019-12-22T00:12:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> From: Johannes Schindelin \n>\n> This job runs the test suite twice, once in regular mode, and once with\n> a whole slew of `GIT_TEST_*` variables set.\n>\n> Now that the built-in version of `git add --interactive` is\n> feature-complete, let's also throw `GIT_TEST_MULTI_PACK_INDEX` into that\n\nHuh?\n\n> fray.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> ci/run-build-and-tests.sh | 1 +\n> 1 file changed, 1 insertion(+)\n>\n> diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh\n> index ff0ef7f08e..4df54c4efe 100755\n> --- a/ci/run-build-and-tests.sh\n> +++ b/ci/run-build-and-tests.sh\n> @@ -20,6 +20,7 @@ linux-gcc)\n> \texport GIT_TEST_OE_DELTA_SIZE=5\n> \texport GIT_TEST_COMMIT_GRAPH=1\n> \texport GIT_TEST_MULTI_PACK_INDEX=1\n> +\texport GIT_TEST_ADD_I_USE_BUILTIN=1\n> \tmake test\n> \t;;\n> linux-gcc-4.8)\n"},{"id":"388869","messageId":"xmqqpngd60rx.fsf@gitster-ct.c.googlers.com","threadId":"52505","inReplyTo":"pull.175.git.1576968120.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/9] built-in add -p: add support for the same config settings as the Perl version","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-12-24T18:23:14Z","receivedAt":"2019-12-24T18:23:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> base-commit: 2d4b85ddc76af3e703e6e3a6a72319b5e79c2d8b\n\nIt is not generally helpful to those who reads this list to use a\ncommit that is not part of history leading to my 'pu' or 'next' as\nthe base.\n\nI am guessing that it will build on top of the \"use add -i/p from\nmore commands\" series, so I'll try to apply these on top there, but\nI wonder if it would help readers if we had some extra comments for\nhuman consumption next to \"base-commit:\" line (similar to the other\nstuff like Published-As added by GGG), perhaps in a format similar\nto the one we use to refer to random commits, e.g.\n\n base-commit: 2d4b85ddc76af3e703e6e3a6a72319b5e79c2d8b\n # commit --interactive: make it work with the built-in `add -i`, 2019-12-17\n\nperhaps?\n\n\n"},{"id":"388870","messageId":"xmqqimm5601h.fsf@gitster-ct.c.googlers.com","threadId":"52505","inReplyTo":"xmqqpngd60rx.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 0/9] built-in add -p: add support for the same config settings as the Perl version","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-12-24T18:39:06Z","receivedAt":"2019-12-24T18:39:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n>\n>> base-commit: 2d4b85ddc76af3e703e6e3a6a72319b5e79c2d8b\n>\n> It is not generally helpful to those who reads this list to use a\n> commit that is not part of history leading to my 'pu' or 'next' as\n> the base.\n\nI think there was only one spot that needed adjusting to the newer\niteration of the js/patch-mode-in-others-in-c series.\n\nThis may have started as \"there are some configuration variables\nthat are ignored in the C version, fix them\" and that may be why\nthe pull-request branch says \"config-settings\", but overall, I think\nthe bulk of the change ends up being a \"how would we implement the\nannoying-to-implement-portably single-key behaviour\".\n\nI think it is a mistake to write the lower-level terminal access\ncode without using established libraries (or write it with a higher\nlevel abstraction offered by scripting languages like Perl and\nPythnon), and I would personally take, given a choice between\naccepting such maintenance/porting liability and dropping of\nsingle-key behaviour, the latter in any second.\n\nI wonder if it makes sense to split this series into two so that the\nearly and easier part for leftover config bits can graduate\nseparately early in the next cycle, instead of letting the parts\nthat tackles the terminal nightmare (note that the problem being\nnightmare is not the fault of this topic) which would inevitably\ntake more time to stabilize take the remainder of the series hostage\nto it.\n\nThanks.\n"},{"id":"388893","messageId":"20191225084632.GA461356@ruderich.org","threadId":"52505","inReplyTo":"xmqqimm5601h.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 0/9] built-in add -p: add support for the same config settings as the Perl version","fromName":"Simon Ruderich","fromEmail":"simon@ruderich.org","sentAt":"2019-12-25T08:46:32Z","receivedAt":"2019-12-25T08:51:42Z","isPatch":true,"sender":{"key":"simon@ruderich.org","avatar":"https://avatars.githubusercontent.com/u/390994?v=4"},"body":"On Tue, Dec 24, 2019 at 10:39:06AM -0800, Junio C Hamano wrote:\n> I think it is a mistake to write the lower-level terminal access\n> code without using established libraries (or write it with a higher\n> level abstraction offered by scripting languages like Perl and\n> Pythnon), and I would personally take, given a choice between\n> accepting such maintenance/porting liability and dropping of\n> single-key behaviour, the latter in any second.\n\nI've no opinion on the implementation or maintenance work.\n\nBut as a heavy user of `git add -p` with interactive.singlekey\nI'd like to keep this feature in Git. It makes using `git add -p`\na lot more comfortable for me.\n\nRegards\nSimon\n-- \n+ privacy is necessary\n+ using gnupg http://gnupg.org\n+ public key id: 0x92FEFDB7E44C32F9\n"},{"id":"388894","messageId":"nycvar.QRO.7.76.6.1912251255160.46@tvgsbejvaqbjf.bet","threadId":"52505","inReplyTo":"20191221225354.GB32750@szeder.dev","subject":"Re: [PATCH 9/9] ci: include the built-in `git add -i` in the `linux-gcc` job","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-12-25T11:56:12Z","receivedAt":"2019-12-25T11:56:38Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Gábor,\n\nOn Sat, 21 Dec 2019, SZEDER Gábor wrote:\n\n> On Sat, Dec 21, 2019 at 10:42:00PM +0000, Johannes Schindelin via GitGitGadget wrote:\n> > From: Johannes Schindelin \n> >\n> > This job runs the test suite twice, once in regular mode, and once with\n> > a whole slew of `GIT_TEST_*` variables set.\n> >\n> > Now that the built-in version of `git add --interactive` is\n> > feature-complete, let's also throw `GIT_TEST_MULTI_PACK_INDEX` into that\n>\n> GIT_TEST_MULTI_PACK_INDEX? ;)\n\nWhoops. Copy/paste fail.\n\nFixed in v2,\nDscho\n\n>\n> > fray.\n> >\n> > Signed-off-by: Johannes Schindelin \n> > ---\n> > ci/run-build-and-tests.sh | 1 +\n> > 1 file changed, 1 insertion(+)\n> >\n> > diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh\n> > index ff0ef7f08e..4df54c4efe 100755\n> > --- a/ci/run-build-and-tests.sh\n> > +++ b/ci/run-build-and-tests.sh\n> > @@ -20,6 +20,7 @@ linux-gcc)\n> > \texport GIT_TEST_OE_DELTA_SIZE=5\n> > \texport GIT_TEST_COMMIT_GRAPH=1\n> > \texport GIT_TEST_MULTI_PACK_INDEX=1\n> > +\texport GIT_TEST_ADD_I_USE_BUILTIN=1\n> > \tmake test\n> > \t;;\n> > linux-gcc-4.8)\n> > --\n> > gitgitgadget\n>\n>\n"},{"id":"388895","messageId":"pull.175.v2.git.1577275020.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.git.1576968120.gitgitgadget@gmail.com","subject":"[PATCH v2 0/9] built-in add -p: add support for the same config settings as the Perl version","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-25T11:56:51Z","receivedAt":"2019-12-25T11:57:06Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"This is the final leg of the journey to a fully built-in git add: the git\nadd -i and git add -p modes were re-implemented in C, but they lacked\nsupport for a couple of config settings.\n\nThe one that sticks out most is the interactive.singleKey setting: it was\nparticularly hard to get to work, especially on Windows.\n\nIt also seems to be the setting that is incomplete already in the Perl\nversion of the interactive add command: while the name of the config setting\nsuggests that it applies to all of the interactive add, including the main\nloop of git add --interactive and to the file selections in that command, it\ndoes not. Only the git add --patch mode respects that setting.\n\nAs it is outside the purpose of the conversion of git-add--interactive.perl \nto C, we will leave that loose end for some future date.\n\nChanges since v1:\n\n * Fixed the commit message where a copy/paste fail made it talk about\n another GIT_TEST_* variable than the GIT_TEST_ADD_I_USE_BUILTIN one.\n\nJohannes Schindelin (9):\n built-in add -p: support interactive.diffFilter\n built-in add -p: handle diff.algorithm\n terminal: make the code of disable_echo() reusable\n terminal: accommodate Git for Windows' default terminal\n terminal: add a new function to read a single keystroke\n built-in add -p: respect the `interactive.singlekey` config setting\n built-in add -p: handle Escape sequences in interactive.singlekey mode\n built-in add -p: handle Escape sequences more efficiently\n ci: include the built-in `git add -i` in the `linux-gcc` job\n\n add-interactive.c | 19 +++\n add-interactive.h | 4 +\n add-patch.c | 57 ++++++++-\n ci/run-build-and-tests.sh | 1 +\n compat/terminal.c | 249 +++++++++++++++++++++++++++++++++++++-\n compat/terminal.h | 3 +\n 6 files changed, 325 insertions(+), 8 deletions(-)\n\n\nbase-commit: c480eeb574e649a19f27dc09a994e45f9b2c2622\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-175%2Fdscho%2Fadd-p-in-c-config-settings-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-175/dscho/add-p-in-c-config-settings-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/175\n\nRange-diff vs v1:\n\n 1: a7355776d6 = 1: f45ff08bd0 built-in add -p: support interactive.diffFilter\n 2: 74958419f6 ! 2: e9c4a13cbf built-in add -p: handle diff.algorithm\n @@ -62,7 +62,7 @@\n @@\n \tint res;\n \n - \targv_array_pushv(&args, s->mode->diff);\n + \targv_array_pushv(&args, s->mode->diff_cmd);\n +\tif (diff_algorithm)\n +\t\targv_array_pushf(&args, \"--diff-algorithm=%s\", diff_algorithm);\n \tif (s->revision) {\n 3: 7631c1ea8c = 3: e643554dba terminal: make the code of disable_echo() reusable\n 4: a77fa914da = 4: bd2306c5d5 terminal: accommodate Git for Windows' default terminal\n 5: 3996d7997a = 5: 190fb4f5e9 terminal: add a new function to read a single keystroke\n 6: 6d6794089d ! 6: 167dfa37dd built-in add -p: respect the `interactive.singlekey` config setting\n @@ -54,7 +54,7 @@\n +#include \"compat/terminal.h\"\n \n enum prompt_mode_type {\n - \tPROMPT_MODE_CHANGE = 0, PROMPT_DELETION, PROMPT_HUNK\n + \tPROMPT_MODE_CHANGE = 0, PROMPT_DELETION, PROMPT_HUNK,\n @@\n \treturn 0;\n }\n 7: fd5a129776 = 7: 32067bebe8 built-in add -p: handle Escape sequences in interactive.singlekey mode\n 8: af9b598738 = 8: 703719ffce built-in add -p: handle Escape sequences more efficiently\n 9: 9719604a1f ! 9: 23a3a47b01 ci: include the built-in `git add -i` in the `linux-gcc` job\n @@ -6,8 +6,8 @@\n a whole slew of `GIT_TEST_*` variables set.\n \n Now that the built-in version of `git add --interactive` is\n - feature-complete, let's also throw `GIT_TEST_MULTI_PACK_INDEX` into that\n - fray.\n + feature-complete, let's also throw `GIT_TEST_ADD_I_USE_BUILTIN` into\n + that fray.\n \n Signed-off-by: Johannes Schindelin \n \n\n-- \ngitgitgadget\n"},{"id":"388896","messageId":"f45ff08bd0a0a2e2aba9ae929b6e5ecb3bdd4e07.1577275020.git.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.v2.git.1577275020.gitgitgadget@gmail.com","subject":"[PATCH v2 1/9] built-in add -p: support interactive.diffFilter","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-25T11:56:52Z","receivedAt":"2019-12-25T11:57:07Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThe Perl version supports post-processing the colored diff (that is\ngenerated in addition to the uncolored diff, intended to offer a\nprettier user experience) by a command configured via that config\nsetting, and now the built-in version does that, too.\n\nSigned-off-by: Johannes Schindelin \n---\n add-interactive.c | 12 ++++++++++++\n add-interactive.h | 3 +++\n add-patch.c | 33 +++++++++++++++++++++++++++++++++\n 3 files changed, 48 insertions(+)\n\ndiff --git a/add-interactive.c b/add-interactive.c\nindex a5bb14f2f4..1786ea29c4 100644\n--- a/add-interactive.c\n+++ b/add-interactive.c\n@@ -52,6 +52,17 @@ void init_add_i_state(struct add_i_state *s, struct repository *r)\n \t\tdiff_get_color(s->use_color, DIFF_FILE_OLD));\n \tinit_color(r, s, \"new\", s->file_new_color,\n \t\tdiff_get_color(s->use_color, DIFF_FILE_NEW));\n+\n+\tFREE_AND_NULL(s->interactive_diff_filter);\n+\tgit_config_get_string(\"interactive.difffilter\",\n+\t\t\t &s->interactive_diff_filter);\n+}\n+\n+void clear_add_i_state(struct add_i_state *s)\n+{\n+\tFREE_AND_NULL(s->interactive_diff_filter);\n+\tmemset(s, 0, sizeof(*s));\n+\ts->use_color = -1;\n }\n \n /*\n@@ -1149,6 +1160,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps)\n \tstrbuf_release(&print_file_item_data.worktree);\n \tstrbuf_release(&header);\n \tprefix_item_list_clear(&commands);\n+\tclear_add_i_state(&s);\n \n \treturn res;\n }\ndiff --git a/add-interactive.h b/add-interactive.h\nindex b2f23479c5..46c73867ad 100644\n--- a/add-interactive.h\n+++ b/add-interactive.h\n@@ -15,9 +15,12 @@ struct add_i_state {\n \tchar context_color[COLOR_MAXLEN];\n \tchar file_old_color[COLOR_MAXLEN];\n \tchar file_new_color[COLOR_MAXLEN];\n+\n+\tchar *interactive_diff_filter;\n };\n \n void init_add_i_state(struct add_i_state *s, struct repository *r);\n+void clear_add_i_state(struct add_i_state *s);\n \n struct repository;\n struct pathspec;\ndiff --git a/add-patch.c b/add-patch.c\nindex 46c6c183d5..78bde41df0 100644\n--- a/add-patch.c\n+++ b/add-patch.c\n@@ -398,6 +398,7 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n \n \tif (want_color_fd(1, -1)) {\n \t\tstruct child_process colored_cp = CHILD_PROCESS_INIT;\n+\t\tconst char *diff_filter = s->s.interactive_diff_filter;\n \n \t\tsetup_child_process(s, &colored_cp, NULL);\n \t\txsnprintf((char *)args.argv[color_arg_index], 8, \"--color\");\n@@ -407,6 +408,24 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n \t\targv_array_clear(&args);\n \t\tif (res)\n \t\t\treturn error(_(\"could not parse colored diff\"));\n+\n+\t\tif (diff_filter) {\n+\t\t\tstruct child_process filter_cp = CHILD_PROCESS_INIT;\n+\n+\t\t\tsetup_child_process(s, &filter_cp,\n+\t\t\t\t\t diff_filter, NULL);\n+\t\t\tfilter_cp.git_cmd = 0;\n+\t\t\tfilter_cp.use_shell = 1;\n+\t\t\tstrbuf_reset(&s->buf);\n+\t\t\tif (pipe_command(&filter_cp,\n+\t\t\t\t\t colored->buf, colored->len,\n+\t\t\t\t\t &s->buf, colored->len,\n+\t\t\t\t\t NULL, 0) < 0)\n+\t\t\t\treturn error(_(\"failed to run '%s'\"),\n+\t\t\t\t\t diff_filter);\n+\t\t\tstrbuf_swap(colored, &s->buf);\n+\t\t}\n+\n \t\tstrbuf_complete_line(colored);\n \t\tcolored_p = colored->buf;\n \t\tcolored_pend = colored_p + colored->len;\n@@ -531,6 +550,9 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n \t\t\t\t\t\t colored_pend - colored_p);\n \t\t\tif (colored_eol)\n \t\t\t\tcolored_p = colored_eol + 1;\n+\t\t\telse if (p != pend)\n+\t\t\t\t/* colored shorter than non-colored? */\n+\t\t\t\tgoto mismatched_output;\n \t\t\telse\n \t\t\t\tcolored_p = colored_pend;\n \n@@ -555,6 +577,15 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n \t\t */\n \t\thunk->splittable_into++;\n \n+\t/* non-colored shorter than colored? */\n+\tif (colored_p != colored_pend) {\n+mismatched_output:\n+\t\terror(_(\"mismatched output from interactive.diffFilter\"));\n+\t\tadvise(_(\"Your filter must maintain a one-to-one correspondence\\n\"\n+\t\t\t \"between its input and output lines.\"));\n+\t\treturn -1;\n+\t}\n+\n \treturn 0;\n }\n \n@@ -1612,6 +1643,7 @@ int run_add_p(struct repository *r, enum add_p_mode mode,\n \t parse_diff(&s, ps) < 0) {\n \t\tstrbuf_release(&s.plain);\n \t\tstrbuf_release(&s.colored);\n+\t\tclear_add_i_state(&s.s);\n \t\treturn -1;\n \t}\n \n@@ -1630,5 +1662,6 @@ int run_add_p(struct repository *r, enum add_p_mode mode,\n \tstrbuf_release(&s.buf);\n \tstrbuf_release(&s.plain);\n \tstrbuf_release(&s.colored);\n+\tclear_add_i_state(&s.s);\n \treturn 0;\n }\n-- \ngitgitgadget\n\n"},{"id":"388897","messageId":"e9c4a13cbfc6921cf7fbf98af9cc914a42bb5e9c.1577275020.git.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.v2.git.1577275020.gitgitgadget@gmail.com","subject":"[PATCH v2 2/9] built-in add -p: handle diff.algorithm","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-25T11:56:53Z","receivedAt":"2019-12-25T11:57:07Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThe Perl version of `git add -p` reads the config setting\n`diff.algorithm` and if set, uses it to generate the diff using the\nspecified algorithm.\n\nThis patch ports that functionality to the C version.\n\nNote: just like `git-add--interactive.perl`, we do _not_ respect this\nconfig setting in `git add -i`'s `diff` command, but _only_ in the\n`patch` command.\n\nSigned-off-by: Johannes Schindelin \n---\n add-interactive.c | 5 +++++\n add-interactive.h | 2 +-\n add-patch.c | 3 +++\n 3 files changed, 9 insertions(+), 1 deletion(-)\n\ndiff --git a/add-interactive.c b/add-interactive.c\nindex 1786ea29c4..9e4bcb382c 100644\n--- a/add-interactive.c\n+++ b/add-interactive.c\n@@ -56,11 +56,16 @@ void init_add_i_state(struct add_i_state *s, struct repository *r)\n \tFREE_AND_NULL(s->interactive_diff_filter);\n \tgit_config_get_string(\"interactive.difffilter\",\n \t\t\t &s->interactive_diff_filter);\n+\n+\tFREE_AND_NULL(s->interactive_diff_algorithm);\n+\tgit_config_get_string(\"diff.algorithm\",\n+\t\t\t &s->interactive_diff_algorithm);\n }\n \n void clear_add_i_state(struct add_i_state *s)\n {\n \tFREE_AND_NULL(s->interactive_diff_filter);\n+\tFREE_AND_NULL(s->interactive_diff_algorithm);\n \tmemset(s, 0, sizeof(*s));\n \ts->use_color = -1;\n }\ndiff --git a/add-interactive.h b/add-interactive.h\nindex 46c73867ad..923efaf527 100644\n--- a/add-interactive.h\n+++ b/add-interactive.h\n@@ -16,7 +16,7 @@ struct add_i_state {\n \tchar file_old_color[COLOR_MAXLEN];\n \tchar file_new_color[COLOR_MAXLEN];\n \n-\tchar *interactive_diff_filter;\n+\tchar *interactive_diff_filter, *interactive_diff_algorithm;\n };\n \n void init_add_i_state(struct add_i_state *s, struct repository *r);\ndiff --git a/add-patch.c b/add-patch.c\nindex 78bde41df0..8f2ee8688b 100644\n--- a/add-patch.c\n+++ b/add-patch.c\n@@ -360,6 +360,7 @@ static int is_octal(const char *p, size_t len)\n static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n {\n \tstruct argv_array args = ARGV_ARRAY_INIT;\n+\tconst char *diff_algorithm = s->s.interactive_diff_algorithm;\n \tstruct strbuf *plain = &s->plain, *colored = NULL;\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n \tchar *p, *pend, *colored_p = NULL, *colored_pend = NULL, marker = '\\0';\n@@ -369,6 +370,8 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n \tint res;\n \n \targv_array_pushv(&args, s->mode->diff_cmd);\n+\tif (diff_algorithm)\n+\t\targv_array_pushf(&args, \"--diff-algorithm=%s\", diff_algorithm);\n \tif (s->revision) {\n \t\tstruct object_id oid;\n \t\targv_array_push(&args,\n-- \ngitgitgadget\n\n"},{"id":"388898","messageId":"e643554dba2b18259d097ee98a9aa1fbbdf8f8c7.1577275020.git.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.v2.git.1577275020.gitgitgadget@gmail.com","subject":"[PATCH v2 3/9] terminal: make the code of disable_echo() reusable","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-25T11:56:54Z","receivedAt":"2019-12-25T11:57:10Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWe are about to introduce the function `enable_non_canonical()`, which\nshares almost the complete code with `disable_echo()`.\n\nLet's prepare for that, by refactoring out that shared code.\n\nSigned-off-by: Johannes Schindelin \n---\n compat/terminal.c | 19 +++++++++++++++----\n 1 file changed, 15 insertions(+), 4 deletions(-)\n\ndiff --git a/compat/terminal.c b/compat/terminal.c\nindex fa13ee672d..1fb40b3a0a 100644\n--- a/compat/terminal.c\n+++ b/compat/terminal.c\n@@ -32,7 +32,7 @@ static void restore_term(void)\n \tterm_fd = -1;\n }\n \n-static int disable_echo(void)\n+static int disable_bits(tcflag_t bits)\n {\n \tstruct termios t;\n \n@@ -43,7 +43,7 @@ static int disable_echo(void)\n \told_term = t;\n \tsigchain_push_common(restore_term_on_signal);\n \n-\tt.c_lflag &= ~ECHO;\n+\tt.c_lflag &= ~bits;\n \tif (!tcsetattr(term_fd, TCSAFLUSH, &t))\n \t\treturn 0;\n \n@@ -53,6 +53,11 @@ static int disable_echo(void)\n \treturn -1;\n }\n \n+static int disable_echo(void)\n+{\n+\treturn disable_bits(ECHO);\n+}\n+\n #elif defined(GIT_WINDOWS_NATIVE)\n \n #define INPUT_PATH \"CONIN$\"\n@@ -72,7 +77,7 @@ static void restore_term(void)\n \thconin = INVALID_HANDLE_VALUE;\n }\n \n-static int disable_echo(void)\n+static int disable_bits(DWORD bits)\n {\n \thconin = CreateFile(\"CONIN$\", GENERIC_READ | GENERIC_WRITE,\n \t FILE_SHARE_READ, NULL, OPEN_EXISTING,\n@@ -82,7 +87,7 @@ static int disable_echo(void)\n \n \tGetConsoleMode(hconin, &cmode);\n \tsigchain_push_common(restore_term_on_signal);\n-\tif (!SetConsoleMode(hconin, cmode & (~ENABLE_ECHO_INPUT))) {\n+\tif (!SetConsoleMode(hconin, cmode & ~bits)) {\n \t\tCloseHandle(hconin);\n \t\thconin = INVALID_HANDLE_VALUE;\n \t\treturn -1;\n@@ -91,6 +96,12 @@ static int disable_echo(void)\n \treturn 0;\n }\n \n+static int disable_echo(void)\n+{\n+\treturn disable_bits(ENABLE_ECHO_INPUT);\n+}\n+\n+\n #endif\n \n #ifndef FORCE_TEXT\n-- \ngitgitgadget\n\n"},{"id":"388899","messageId":"bd2306c5d55986ad991f7a84982f84609f848842.1577275020.git.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.v2.git.1577275020.gitgitgadget@gmail.com","subject":"[PATCH v2 4/9] terminal: accommodate Git for Windows' default terminal","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-25T11:56:55Z","receivedAt":"2019-12-25T11:57:11Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nGit for Windows' Git Bash runs in MinTTY by default, which does not have\na Win32 Console instance, but uses MSYS2 pseudo terminals instead.\n\nThis is a problem, as Git for Windows does not want to use the MSYS2\nemulation layer for Git itself, and therefore has no direct way to\ninteract with that pseudo terminal.\n\nAs a workaround, use the `stty` utility (which is included in Git for\nWindows, and which *is* an MSYS2 program, so it knows how to deal with\nthe pseudo terminal).\n\nNote: If Git runs in a regular CMD or PowerShell window, there *is* a\nregular Win32 Console to work with. This is not a problem for the MSYS2\n`stty`: it copes with this scenario just fine.\n\nAlso note that we introduce support for more bits than would be\nnecessary for a mere `disable_echo()` here, in preparation for the\nupcoming `enable_non_canonical()` function.\n\nSigned-off-by: Johannes Schindelin \n---\n compat/terminal.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 50 insertions(+)\n\ndiff --git a/compat/terminal.c b/compat/terminal.c\nindex 1fb40b3a0a..16e9949da1 100644\n--- a/compat/terminal.c\n+++ b/compat/terminal.c\n@@ -2,6 +2,8 @@\n #include \"compat/terminal.h\"\n #include \"sigchain.h\"\n #include \"strbuf.h\"\n+#include \"run-command.h\"\n+#include \"string-list.h\"\n \n #if defined(HAVE_DEV_TTY) || defined(GIT_WINDOWS_NATIVE)\n \n@@ -64,11 +66,28 @@ static int disable_echo(void)\n #define OUTPUT_PATH \"CONOUT$\"\n #define FORCE_TEXT \"t\"\n \n+static int use_stty = 1;\n+static struct string_list stty_restore = STRING_LIST_INIT_DUP;\n static HANDLE hconin = INVALID_HANDLE_VALUE;\n static DWORD cmode;\n \n static void restore_term(void)\n {\n+\tif (use_stty) {\n+\t\tint i;\n+\t\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\n+\t\tif (stty_restore.nr == 0)\n+\t\t\treturn;\n+\n+\t\targv_array_push(&cp.args, \"stty\");\n+\t\tfor (i = 0; i < stty_restore.nr; i++)\n+\t\t\targv_array_push(&cp.args, stty_restore.items[i].string);\n+\t\trun_command(&cp);\n+\t\tstring_list_clear(&stty_restore, 0);\n+\t\treturn;\n+\t}\n+\n \tif (hconin == INVALID_HANDLE_VALUE)\n \t\treturn;\n \n@@ -79,6 +98,37 @@ static void restore_term(void)\n \n static int disable_bits(DWORD bits)\n {\n+\tif (use_stty) {\n+\t\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\n+\t\targv_array_push(&cp.args, \"stty\");\n+\n+\t\tif (bits & ENABLE_LINE_INPUT) {\n+\t\t\tstring_list_append(&stty_restore, \"icanon\");\n+\t\t\targv_array_push(&cp.args, \"-icanon\");\n+\t\t}\n+\n+\t\tif (bits & ENABLE_ECHO_INPUT) {\n+\t\t\tstring_list_append(&stty_restore, \"echo\");\n+\t\t\targv_array_push(&cp.args, \"-echo\");\n+\t\t}\n+\n+\t\tif (bits & ENABLE_PROCESSED_INPUT) {\n+\t\t\tstring_list_append(&stty_restore, \"-ignbrk\");\n+\t\t\tstring_list_append(&stty_restore, \"intr\");\n+\t\t\tstring_list_append(&stty_restore, \"^c\");\n+\t\t\targv_array_push(&cp.args, \"ignbrk\");\n+\t\t\targv_array_push(&cp.args, \"intr\");\n+\t\t\targv_array_push(&cp.args, \"\");\n+\t\t}\n+\n+\t\tif (run_command(&cp) == 0)\n+\t\t\treturn 0;\n+\n+\t\t/* `stty` could not be executed; access the Console directly */\n+\t\tuse_stty = 0;\n+\t}\n+\n \thconin = CreateFile(\"CONIN$\", GENERIC_READ | GENERIC_WRITE,\n \t FILE_SHARE_READ, NULL, OPEN_EXISTING,\n \t FILE_ATTRIBUTE_NORMAL, NULL);\n-- \ngitgitgadget\n\n"},{"id":"388900","messageId":"167dfa37dde5d04192fff40147f8566d74e96015.1577275020.git.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.v2.git.1577275020.gitgitgadget@gmail.com","subject":"[PATCH v2 6/9] built-in add -p: respect the `interactive.singlekey` config setting","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-25T11:56:57Z","receivedAt":"2019-12-25T11:57:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThe Perl version of `git add -p` supports this config setting to allow\nusers to input commands via single characters (as opposed to having to\npress the key afterwards).\n\nThis is an opt-in feature because it requires Perl packages\n(Term::ReadKey and Term::Cap, where it tries to handle an absence of the\nlatter package gracefully) to work. Note that at least on Ubuntu, that\nPerl package is not installed by default (it needs to be installed via\n`sudo apt-get install libterm-readkey-perl`), so this feature is\nprobably not used a whole lot.\n\nIn C, we obviously do not have these packages available, but we just\nintroduced `read_single_keystroke()` that is similar to what\nTerm::ReadKey provides, and we use that here.\n\nSigned-off-by: Johannes Schindelin \n---\n add-interactive.c | 2 ++\n add-interactive.h | 1 +\n add-patch.c | 21 +++++++++++++++++----\n 3 files changed, 20 insertions(+), 4 deletions(-)\n\ndiff --git a/add-interactive.c b/add-interactive.c\nindex 9e4bcb382c..39c3896494 100644\n--- a/add-interactive.c\n+++ b/add-interactive.c\n@@ -60,6 +60,8 @@ void init_add_i_state(struct add_i_state *s, struct repository *r)\n \tFREE_AND_NULL(s->interactive_diff_algorithm);\n \tgit_config_get_string(\"diff.algorithm\",\n \t\t\t &s->interactive_diff_algorithm);\n+\n+\tgit_config_get_bool(\"interactive.singlekey\", &s->use_single_key);\n }\n \n void clear_add_i_state(struct add_i_state *s)\ndiff --git a/add-interactive.h b/add-interactive.h\nindex 923efaf527..693f125e8e 100644\n--- a/add-interactive.h\n+++ b/add-interactive.h\n@@ -16,6 +16,7 @@ struct add_i_state {\n \tchar file_old_color[COLOR_MAXLEN];\n \tchar file_new_color[COLOR_MAXLEN];\n \n+\tint use_single_key;\n \tchar *interactive_diff_filter, *interactive_diff_algorithm;\n };\n \ndiff --git a/add-patch.c b/add-patch.c\nindex 8f2ee8688b..d8dafa8168 100644\n--- a/add-patch.c\n+++ b/add-patch.c\n@@ -6,6 +6,7 @@\n #include \"pathspec.h\"\n #include \"color.h\"\n #include \"diff.h\"\n+#include \"compat/terminal.h\"\n \n enum prompt_mode_type {\n \tPROMPT_MODE_CHANGE = 0, PROMPT_DELETION, PROMPT_HUNK,\n@@ -1149,14 +1150,27 @@ static int run_apply_check(struct add_p_state *s,\n \treturn 0;\n }\n \n+static int read_single_character(struct add_p_state *s)\n+{\n+\tif (s->s.use_single_key) {\n+\t\tint res = read_key_without_echo(&s->answer);\n+\t\tprintf(\"%s\\n\", res == EOF ? \"\" : s->answer.buf);\n+\t\treturn res;\n+\t}\n+\n+\tif (strbuf_getline(&s->answer, stdin) == EOF)\n+\t\treturn EOF;\n+\tstrbuf_trim_trailing_newline(&s->answer);\n+\treturn 0;\n+}\n+\n static int prompt_yesno(struct add_p_state *s, const char *prompt)\n {\n \tfor (;;) {\n \t\tcolor_fprintf(stdout, s->s.prompt_color, \"%s\", _(prompt));\n \t\tfflush(stdout);\n-\t\tif (strbuf_getline(&s->answer, stdin) == EOF)\n+\t\tif (read_single_character(s) == EOF)\n \t\t\treturn -1;\n-\t\tstrbuf_trim_trailing_newline(&s->answer);\n \t\tswitch (tolower(s->answer.buf[0])) {\n \t\tcase 'n': return 0;\n \t\tcase 'y': return 1;\n@@ -1396,9 +1410,8 @@ static int patch_update_file(struct add_p_state *s,\n \t\t\t _(s->mode->prompt_mode[prompt_mode_type]),\n \t\t\t s->buf.buf);\n \t\tfflush(stdout);\n-\t\tif (strbuf_getline(&s->answer, stdin) == EOF)\n+\t\tif (read_single_character(s) == EOF)\n \t\t\tbreak;\n-\t\tstrbuf_trim_trailing_newline(&s->answer);\n \n \t\tif (!s->answer.len)\n \t\t\tcontinue;\n-- \ngitgitgadget\n\n"},{"id":"388901","messageId":"703719ffce4e69fa1d22fd2b740ddfaa0ef7283d.1577275020.git.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.v2.git.1577275020.gitgitgadget@gmail.com","subject":"[PATCH v2 8/9] built-in add -p: handle Escape sequences more efficiently","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-25T11:56:59Z","receivedAt":"2019-12-25T11:57:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWhen `interactive.singlekey = true`, we react immediately to keystrokes,\neven to Escape sequences (e.g. when pressing a cursor key).\n\nThe problem with Escape sequences is that we do not really know when\nthey are done, and as a heuristic we poll standard input for half a\nsecond to make sure that we got all of it.\n\nWhile waiting half a second is not asking for a whole lot, it can become\nquite annoying over time, therefore with this patch, we read the\nterminal capabilities (if available) and extract known Escape sequences\nfrom there, then stop polling immediately when we detected that the user\npressed a key that generated such a known sequence.\n\nThis recapitulates the remaining part of b5cc003253c8 (add -i: ignore\nterminal escape sequences, 2011-05-17).\n\nNote: We do *not* query the terminal capabilities directly. That would\neither require a lot of platform-specific code, or it would require\nlinking to a library such as ncurses.\n\nLinking to a library in the built-ins is something we try very hard to\navoid (we even kicked the libcurl dependency to a non-built-in remote\nhelper, just to shave off a tiny fraction of a second from Git's startup\ntime). And the platform-specific code would be a maintenance nightmare.\n\nEven worse: in Git for Windows' case, we would need to query MSYS2\npseudo terminals, which `git.exe` simply cannot do (because it is\nintentionally *not* an MSYS2 program).\n\nTo address this, we simply spawn `infocmp -L -1` and parse its output\n(which works even in Git for Windows, because that helper is included in\nthe end-user facing installations).\n\nThis is done only once, as in the Perl version, but it is done only when\nthe first Escape sequence is encountered, not upon startup of `git add\n-i`; This saves on startup time, yet makes reacting to the first Escape\nsequence slightly more sluggish. But it allows us to keep the\nterminal-related code encapsulated in the `compat/terminal.c` file.\n\nSigned-off-by: Johannes Schindelin \n---\n compat/terminal.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++-\n 1 file changed, 72 insertions(+), 1 deletion(-)\n\ndiff --git a/compat/terminal.c b/compat/terminal.c\nindex b7f58d1781..35bca03d14 100644\n--- a/compat/terminal.c\n+++ b/compat/terminal.c\n@@ -4,6 +4,7 @@\n #include \"strbuf.h\"\n #include \"run-command.h\"\n #include \"string-list.h\"\n+#include \"hashmap.h\"\n \n #if defined(HAVE_DEV_TTY) || defined(GIT_WINDOWS_NATIVE)\n \n@@ -238,6 +239,71 @@ char *git_terminal_prompt(const char *prompt, int echo)\n \treturn buf.buf;\n }\n \n+/*\n+ * The `is_known_escape_sequence()` function returns 1 if the passed string\n+ * corresponds to an Escape sequence that the terminal capabilities contains.\n+ *\n+ * To avoid depending on ncurses or other platform-specific libraries, we rely\n+ * on the presence of the `infocmp` executable to do the job for us (failing\n+ * silently if the program is not available or refused to run).\n+ */\n+struct escape_sequence_entry {\n+\tstruct hashmap_entry entry;\n+\tchar sequence[FLEX_ARRAY];\n+};\n+\n+static int sequence_entry_cmp(const void *hashmap_cmp_fn_data,\n+\t\t\t const struct escape_sequence_entry *e1,\n+\t\t\t const struct escape_sequence_entry *e2,\n+\t\t\t const void *keydata)\n+{\n+\treturn strcmp(e1->sequence, keydata ? keydata : e2->sequence);\n+}\n+\n+static int is_known_escape_sequence(const char *sequence)\n+{\n+\tstatic struct hashmap sequences;\n+\tstatic int initialized;\n+\n+\tif (!initialized) {\n+\t\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\t\tstruct strbuf buf = STRBUF_INIT;\n+\t\tchar *p, *eol;\n+\n+\t\thashmap_init(&sequences, (hashmap_cmp_fn)sequence_entry_cmp,\n+\t\t\t NULL, 0);\n+\n+\t\targv_array_pushl(&cp.args, \"infocmp\", \"-L\", \"-1\", NULL);\n+\t\tif (pipe_command(&cp, NULL, 0, &buf, 0, NULL, 0))\n+\t\t\tstrbuf_setlen(&buf, 0);\n+\n+\t\tfor (eol = p = buf.buf; *p; p = eol + 1) {\n+\t\t\tp = strchr(p, '=');\n+\t\t\tif (!p)\n+\t\t\t\tbreak;\n+\t\t\tp++;\n+\t\t\teol = strchrnul(p, '\\n');\n+\n+\t\t\tif (starts_with(p, \"\\\\E\")) {\n+\t\t\t\tchar *comma = memchr(p, ',', eol - p);\n+\t\t\t\tstruct escape_sequence_entry *e;\n+\n+\t\t\t\tp[0] = '^';\n+\t\t\t\tp[1] = '[';\n+\t\t\t\tFLEX_ALLOC_MEM(e, sequence, p, comma - p);\n+\t\t\t\thashmap_entry_init(&e->entry,\n+\t\t\t\t\t\t strhash(e->sequence));\n+\t\t\t\thashmap_add(&sequences, &e->entry);\n+\t\t\t}\n+\t\t\tif (!*eol)\n+\t\t\t\tbreak;\n+\t\t}\n+\t\tinitialized = 1;\n+\t}\n+\n+\treturn !!hashmap_get_from_hash(&sequences, strhash(sequence), sequence);\n+}\n+\n int read_key_without_echo(struct strbuf *buf)\n {\n \tstatic int warning_displayed;\n@@ -271,7 +337,12 @@ int read_key_without_echo(struct strbuf *buf)\n \t\t * Start by replacing the Escape byte with ^[ */\n \t\tstrbuf_splice(buf, buf->len - 1, 1, \"^[\", 2);\n \n-\t\tfor (;;) {\n+\t\t/*\n+\t\t * Query the terminal capabilities once about all the Escape\n+\t\t * sequences it knows about, so that we can avoid waiting for\n+\t\t * half a second when we know that the sequence is complete.\n+\t\t */\n+\t\twhile (!is_known_escape_sequence(buf->buf)) {\n \t\t\tstruct pollfd pfd = { .fd = 0, .events = POLLIN };\n \n \t\t\tif (poll(&pfd, 1, 500) < 1)\n-- \ngitgitgadget\n\n"},{"id":"388902","messageId":"23a3a47b0193395a280f32c01deaae5bdeeaa051.1577275020.git.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.v2.git.1577275020.gitgitgadget@gmail.com","subject":"[PATCH v2 9/9] ci: include the built-in `git add -i` in the `linux-gcc` job","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-25T11:57:00Z","receivedAt":"2019-12-25T11:57:14Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThis job runs the test suite twice, once in regular mode, and once with\na whole slew of `GIT_TEST_*` variables set.\n\nNow that the built-in version of `git add --interactive` is\nfeature-complete, let's also throw `GIT_TEST_ADD_I_USE_BUILTIN` into\nthat fray.\n\nSigned-off-by: Johannes Schindelin \n---\n ci/run-build-and-tests.sh | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh\nindex ff0ef7f08e..4df54c4efe 100755\n--- a/ci/run-build-and-tests.sh\n+++ b/ci/run-build-and-tests.sh\n@@ -20,6 +20,7 @@ linux-gcc)\n \texport GIT_TEST_OE_DELTA_SIZE=5\n \texport GIT_TEST_COMMIT_GRAPH=1\n \texport GIT_TEST_MULTI_PACK_INDEX=1\n+\texport GIT_TEST_ADD_I_USE_BUILTIN=1\n \tmake test\n \t;;\n linux-gcc-4.8)\n-- \ngitgitgadget\n"},{"id":"388903","messageId":"32067bebe87ab5e6ef35be930d722c7e50cf1da6.1577275020.git.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.v2.git.1577275020.gitgitgadget@gmail.com","subject":"[PATCH v2 7/9] built-in add -p: handle Escape sequences in interactive.singlekey mode","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-25T11:56:58Z","receivedAt":"2019-12-25T11:57:16Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThis recapitulates part of b5cc003253c8 (add -i: ignore terminal escape\nsequences, 2011-05-17):\n\n add -i: ignore terminal escape sequences\n\n On the author's terminal, the up-arrow input sequence is ^[[A, and\n thus fat-fingering an up-arrow into 'git checkout -p' is quite\n dangerous: git-add--interactive.perl will ignore the ^[ and [\n characters and happily treat A as \"discard everything\".\n\n As a band-aid fix, use Term::Cap to get all terminal capabilities.\n Then use the heuristic that any capability value that starts with ^[\n (i.e., \\e in perl) must be a key input sequence. Finally, given an\n input that starts with ^[, read more characters until we have read a\n full escape sequence, then return that to the caller. We use a\n timeout of 0.5 seconds on the subsequent reads to avoid getting stuck\n if the user actually input a lone ^[.\n\n Since none of the currently recognized keys start with ^[, the net\n result is that the sequence as a whole will be ignored and the help\n displayed.\n\nNote that we leave part for later which uses \"Term::Cap to get all\nterminal capabilities\", for several reasons:\n\n1. it is actually not really necessary, as the timeout of 0.5 seconds\n should be plenty sufficient to catch Escape sequences,\n\n2. it is cleaner to keep the change to special-case Escape sequences\n separate from the change that reads all terminal capabilities to\n speed things up, and\n\n3. in practice, relying on the terminal capabilities is a bit overrated,\n as the information could be incomplete, or plain wrong. For example,\n in this developer's tmux sessions, the terminal capabilities claim\n that the \"cursor up\" sequence is ^[M, but the actual sequence\n produced by the \"cursor up\" key is ^[[A.\n\nSigned-off-by: Johannes Schindelin \n---\n compat/terminal.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++-\n 1 file changed, 55 insertions(+), 1 deletion(-)\n\ndiff --git a/compat/terminal.c b/compat/terminal.c\nindex 1b2564042a..b7f58d1781 100644\n--- a/compat/terminal.c\n+++ b/compat/terminal.c\n@@ -161,6 +161,37 @@ static int enable_non_canonical(void)\n \treturn disable_bits(ENABLE_ECHO_INPUT | ENABLE_LINE_INPUT | ENABLE_PROCESSED_INPUT);\n }\n \n+/*\n+ * Override `getchar()`, as the default implementation does not use\n+ * `ReadFile()`.\n+ *\n+ * This poses a problem when we want to see whether the standard\n+ * input has more characters, as the default of Git for Windows is to start the\n+ * Bash in a MinTTY, which uses a named pipe to emulate a pty, in which case\n+ * our `poll()` emulation calls `PeekNamedPipe()`, which seems to require\n+ * `ReadFile()` to be called first to work properly (it only reports 0\n+ * available bytes, otherwise).\n+ *\n+ * So let's just override `getchar()` with a version backed by `ReadFile()` and\n+ * go our merry ways from here.\n+ */\n+static int mingw_getchar(void)\n+{\n+\tDWORD read = 0;\n+\tunsigned char ch;\n+\n+\tif (!ReadFile(GetStdHandle(STD_INPUT_HANDLE), &ch, 1, &read, NULL))\n+\t\treturn EOF;\n+\n+\tif (!read) {\n+\t\terror(\"Unexpected 0 read\");\n+\t\treturn EOF;\n+\t}\n+\n+\treturn ch;\n+}\n+#define getchar mingw_getchar\n+\n #endif\n \n #ifndef FORCE_TEXT\n@@ -228,8 +259,31 @@ int read_key_without_echo(struct strbuf *buf)\n \t\trestore_term();\n \t\treturn EOF;\n \t}\n-\n \tstrbuf_addch(buf, ch);\n+\n+\tif (ch == '\\033' /* ESC */) {\n+\t\t/*\n+\t\t * We are most likely looking at an Escape sequence. Let's try\n+\t\t * to read more bytes, waiting at most half a second, assuming\n+\t\t * that the sequence is complete if we did not receive any byte\n+\t\t * within that time.\n+\t\t *\n+\t\t * Start by replacing the Escape byte with ^[ */\n+\t\tstrbuf_splice(buf, buf->len - 1, 1, \"^[\", 2);\n+\n+\t\tfor (;;) {\n+\t\t\tstruct pollfd pfd = { .fd = 0, .events = POLLIN };\n+\n+\t\t\tif (poll(&pfd, 1, 500) < 1)\n+\t\t\t\tbreak;\n+\n+\t\t\tch = getchar();\n+\t\t\tif (ch == EOF)\n+\t\t\t\treturn 0;\n+\t\t\tstrbuf_addch(buf, ch);\n+\t\t}\n+\t}\n+\n \trestore_term();\n \treturn 0;\n }\n-- \ngitgitgadget\n\n"},{"id":"388904","messageId":"190fb4f5e911aca494647c36c9b09ac0cd232170.1577275020.git.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.v2.git.1577275020.gitgitgadget@gmail.com","subject":"[PATCH v2 5/9] terminal: add a new function to read a single keystroke","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-25T11:56:56Z","receivedAt":"2019-12-25T11:57:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nTypically, input on the command-line is line-based. It is actually not\nreally easy to get single characters (or better put: keystrokes).\n\nWe provide two implementations here:\n\n- One that handles `/dev/tty` based systems as well as native Windows.\n The former uses the `tcsetattr()` function to put the terminal into\n \"raw mode\", which allows us to read individual keystrokes, one by one.\n The latter uses `stty.exe` to do the same, falling back to direct\n Win32 Console access.\n\n Thanks to the refactoring leading up to this commit, this is a single\n function, with the platform-specific details hidden away in\n conditionally-compiled code blocks.\n\n- A fall-back which simply punts and reads back an entire line.\n\nNote that the function writes the keystroke into an `strbuf` rather than\na `char`, in preparation for reading Escape sequences (e.g. when the\nuser hit an arrow key). This is also required for UTF-8 sequences in\ncase the keystroke corresponds to a non-ASCII letter.\n\nSigned-off-by: Johannes Schindelin \n---\n compat/terminal.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++\n compat/terminal.h | 3 +++\n 2 files changed, 58 insertions(+)\n\ndiff --git a/compat/terminal.c b/compat/terminal.c\nindex 16e9949da1..1b2564042a 100644\n--- a/compat/terminal.c\n+++ b/compat/terminal.c\n@@ -60,6 +60,11 @@ static int disable_echo(void)\n \treturn disable_bits(ECHO);\n }\n \n+static int enable_non_canonical(void)\n+{\n+\treturn disable_bits(ICANON | ECHO);\n+}\n+\n #elif defined(GIT_WINDOWS_NATIVE)\n \n #define INPUT_PATH \"CONIN$\"\n@@ -151,6 +156,10 @@ static int disable_echo(void)\n \treturn disable_bits(ENABLE_ECHO_INPUT);\n }\n \n+static int enable_non_canonical(void)\n+{\n+\treturn disable_bits(ENABLE_ECHO_INPUT | ENABLE_LINE_INPUT | ENABLE_PROCESSED_INPUT);\n+}\n \n #endif\n \n@@ -198,6 +207,33 @@ char *git_terminal_prompt(const char *prompt, int echo)\n \treturn buf.buf;\n }\n \n+int read_key_without_echo(struct strbuf *buf)\n+{\n+\tstatic int warning_displayed;\n+\tint ch;\n+\n+\tif (warning_displayed || enable_non_canonical() < 0) {\n+\t\tif (!warning_displayed) {\n+\t\t\twarning(\"reading single keystrokes not supported on \"\n+\t\t\t\t\"this platform; reading line instead\");\n+\t\t\twarning_displayed = 1;\n+\t\t}\n+\n+\t\treturn strbuf_getline(buf, stdin);\n+\t}\n+\n+\tstrbuf_reset(buf);\n+\tch = getchar();\n+\tif (ch == EOF) {\n+\t\trestore_term();\n+\t\treturn EOF;\n+\t}\n+\n+\tstrbuf_addch(buf, ch);\n+\trestore_term();\n+\treturn 0;\n+}\n+\n #else\n \n char *git_terminal_prompt(const char *prompt, int echo)\n@@ -205,4 +241,23 @@ char *git_terminal_prompt(const char *prompt, int echo)\n \treturn getpass(prompt);\n }\n \n+int read_key_without_echo(struct strbuf *buf)\n+{\n+\tstatic int warning_displayed;\n+\tconst char *res;\n+\n+\tif (!warning_displayed) {\n+\t\twarning(\"reading single keystrokes not supported on this \"\n+\t\t\t\"platform; reading line instead\");\n+\t\twarning_displayed = 1;\n+\t}\n+\n+\tres = getpass(\"\");\n+\tstrbuf_reset(buf);\n+\tif (!res)\n+\t\treturn EOF;\n+\tstrbuf_addstr(buf, res);\n+\treturn 0;\n+}\n+\n #endif\ndiff --git a/compat/terminal.h b/compat/terminal.h\nindex 97db7cd69d..a9d52b8464 100644\n--- a/compat/terminal.h\n+++ b/compat/terminal.h\n@@ -3,4 +3,7 @@\n \n char *git_terminal_prompt(const char *prompt, int echo);\n \n+/* Read a single keystroke, without echoing it to the terminal */\n+int read_key_without_echo(struct strbuf *buf);\n+\n #endif /* COMPAT_TERMINAL_H */\n-- \ngitgitgadget\n\n"},{"id":"388905","messageId":"nycvar.QRO.7.76.6.1912251256500.46@tvgsbejvaqbjf.bet","threadId":"52505","inReplyTo":"xmqq5zi96wxc.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 9/9] ci: include the built-in `git add -i` in the `linux-gcc` job","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-12-25T11:57:26Z","receivedAt":"2019-12-25T11:57:47Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Sat, 21 Dec 2019, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n>\n> > From: Johannes Schindelin \n> >\n> > This job runs the test suite twice, once in regular mode, and once with\n> > a whole slew of `GIT_TEST_*` variables set.\n> >\n> > Now that the built-in version of `git add --interactive` is\n> > feature-complete, let's also throw `GIT_TEST_MULTI_PACK_INDEX` into that\n>\n> Huh?\n\nRight. This is so obvious a copy/edit fail, but don't you know, I read\nthrough these patches multiple times and still failed to notice...\n\nFixed in v2,\nDscho\n\n>\n> > fray.\n> >\n> > Signed-off-by: Johannes Schindelin \n> > ---\n> > ci/run-build-and-tests.sh | 1 +\n> > 1 file changed, 1 insertion(+)\n> >\n> > diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh\n> > index ff0ef7f08e..4df54c4efe 100755\n> > --- a/ci/run-build-and-tests.sh\n> > +++ b/ci/run-build-and-tests.sh\n> > @@ -20,6 +20,7 @@ linux-gcc)\n> > \texport GIT_TEST_OE_DELTA_SIZE=5\n> > \texport GIT_TEST_COMMIT_GRAPH=1\n> > \texport GIT_TEST_MULTI_PACK_INDEX=1\n> > +\texport GIT_TEST_ADD_I_USE_BUILTIN=1\n> > \tmake test\n> > \t;;\n> > linux-gcc-4.8)\n>\n"},{"id":"388906","messageId":"nycvar.QRO.7.76.6.1912251259540.46@tvgsbejvaqbjf.bet","threadId":"52505","inReplyTo":"xmqqpngd60rx.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 0/9] built-in add -p: add support for the same config settings as the Perl version","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-12-25T12:02:53Z","receivedAt":"2019-12-25T12:03:14Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Tue, 24 Dec 2019, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n>\n> > base-commit: 2d4b85ddc76af3e703e6e3a6a72319b5e79c2d8b\n>\n> It is not generally helpful to those who reads this list to use a\n> commit that is not part of history leading to my 'pu' or 'next' as\n> the base.\n\nRight, but it _was_ part of `pu`. I based it directly on an earlier\nversion of `js/patch-mode-in-others-in-c`.\n\n> I am guessing that it will build on top of the \"use add -i/p from\n> more commands\" series, so I'll try to apply these on top there, but\n> I wonder if it would help readers if we had some extra comments for\n> human consumption next to \"base-commit:\" line (similar to the other\n> stuff like Published-As added by GGG), perhaps in a format similar\n> to the one we use to refer to random commits, e.g.\n>\n> base-commit: 2d4b85ddc76af3e703e6e3a6a72319b5e79c2d8b\n> # commit --interactive: make it work with the built-in `add -i`, 2019-12-17\n>\n> perhaps?\n\nThis is actually the output of `git format-patch --base=...`. I agree that\nyour suggestion makes sense, and just like you suggested in another recent\nmail, it would probably make sense for `git format-patch` to learn that\ntrick and for GitGitGadget to simply be just one of the users.\n\nCiao,\nDscho\n"},{"id":"388907","messageId":"nycvar.QRO.7.76.6.1912251303320.46@tvgsbejvaqbjf.bet","threadId":"52505","inReplyTo":"xmqqimm5601h.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 0/9] built-in add -p: add support for the same config settings as the Perl version","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-12-25T12:09:21Z","receivedAt":"2019-12-25T12:09:46Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Tue, 24 Dec 2019, Junio C Hamano wrote:\n\n> Junio C Hamano writes:\n>\n> > \"Johannes Schindelin via GitGitGadget\" \n> > writes:\n> >\n> >> base-commit: 2d4b85ddc76af3e703e6e3a6a72319b5e79c2d8b\n> >\n> > It is not generally helpful to those who reads this list to use a\n> > commit that is not part of history leading to my 'pu' or 'next' as\n> > the base.\n>\n> I think there was only one spot that needed adjusting to the newer\n> iteration of the js/patch-mode-in-others-in-c series.\n>\n> This may have started as \"there are some configuration variables\n> that are ignored in the C version, fix them\" and that may be why\n> the pull-request branch says \"config-settings\", but overall, I think\n> the bulk of the change ends up being a \"how would we implement the\n> annoying-to-implement-portably single-key behaviour\".\n>\n> I think it is a mistake to write the lower-level terminal access\n> code without using established libraries (or write it with a higher\n> level abstraction offered by scripting languages like Perl and\n> Pythnon), and I would personally take, given a choice between\n> accepting such maintenance/porting liability and dropping of\n> single-key behaviour, the latter in any second.\n>\n> I wonder if it makes sense to split this series into two so that the\n> early and easier part for leftover config bits can graduate\n> separately early in the next cycle, instead of letting the parts\n> that tackles the terminal nightmare (note that the problem being\n> nightmare is not the fault of this topic) which would inevitably\n> take more time to stabilize take the remainder of the series hostage\n> to it.\n\nI wondered about the same two things: whether to use an established\nlibrary, and whether to split off the patches for the config settings.\n\nAlas, I did not find any established library that I could use on Windows,\nso there is _already_ a precedent for doing it the way my patches do it.\nAnd if we already have to e.g. spawn `infocmp` and poll to catch Escape\nsequences for Windows, my reasoning went: why not just do it for all\nplatforms? This simplifies the overall complexity of the patches, as we do\nnot have to do _too_ different things for Windows vs non-Windows.\n\nAbout the config settings, sure, they could be split off, but for me this\npatch series really is about getting the built-in to reach parity with the\nPerl script version of `git add -i`/`git add -p`.\n\nIn short, I really would like to keep the overall direction of this patch\nseries intact.\n\nCiao,\nDscho\n"},{"id":"388950","messageId":"xmqqsgl6ke7t.fsf@gitster-ct.c.googlers.com","threadId":"52505","inReplyTo":"pull.175.v2.git.1577275020.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 0/9] built-in add -p: add support for the same config settings as the Perl version","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-12-26T20:45:58Z","receivedAt":"2019-12-26T20:46:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> * Fixed the commit message where a copy/paste fail made it talk about\n> another GIT_TEST_* variable than the GIT_TEST_ADD_I_USE_BUILTIN one.\n\nThis matches what is queued (with a hand-fix while queuing), so\nI'll keep it, together with the older author dates.\n\nThanks.\n"},{"id":"388951","messageId":"600bb1f7-b54a-3bf7-40ae-af656768a752@gmail.com","threadId":"52505","inReplyTo":"23a3a47b0193395a280f32c01deaae5bdeeaa051.1577275020.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 9/9] ci: include the built-in `git add -i` in the `linux-gcc` job","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2019-12-26T20:48:45Z","receivedAt":"2019-12-26T20:48:48Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 12/25/2019 6:57 AM, Johannes Schindelin via GitGitGadget wrote:\n> From: Johannes Schindelin \n> \n> This job runs the test suite twice, once in regular mode, and once with\n> a whole slew of `GIT_TEST_*` variables set.\n> \n> Now that the built-in version of `git add --interactive` is\n> feature-complete, let's also throw `GIT_TEST_ADD_I_USE_BUILTIN` into\n> that fray.\n> \n> Signed-off-by: Johannes Schindelin \n> ---\n> ci/run-build-and-tests.sh | 1 +\n> 1 file changed, 1 insertion(+)\n> \n> diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh\n> index ff0ef7f08e..4df54c4efe 100755\n> --- a/ci/run-build-and-tests.sh\n> +++ b/ci/run-build-and-tests.sh\n> @@ -20,6 +20,7 @@ linux-gcc)\n> \texport GIT_TEST_OE_DELTA_SIZE=5\n> \texport GIT_TEST_COMMIT_GRAPH=1\n> \texport GIT_TEST_MULTI_PACK_INDEX=1\n> +\texport GIT_TEST_ADD_I_USE_BUILTIN=1\n> \tmake test\n\nI see that I need to add this to the test-coverage builds.\n\nWill do.\n-Stolee\n\n"},{"id":"389155","messageId":"nycvar.QRO.7.76.6.2001012309390.46@tvgsbejvaqbjf.bet","threadId":"52505","inReplyTo":"600bb1f7-b54a-3bf7-40ae-af656768a752@gmail.com","subject":"Re: [PATCH v2 9/9] ci: include the built-in `git add -i` in the `linux-gcc` job","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-01-01T22:10:35Z","receivedAt":"2020-01-01T22:10:55Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Stolee,\n\nOn Thu, 26 Dec 2019, Derrick Stolee wrote:\n\n> On 12/25/2019 6:57 AM, Johannes Schindelin via GitGitGadget wrote:\n> > From: Johannes Schindelin \n> >\n> > This job runs the test suite twice, once in regular mode, and once with\n> > a whole slew of `GIT_TEST_*` variables set.\n> >\n> > Now that the built-in version of `git add --interactive` is\n> > feature-complete, let's also throw `GIT_TEST_ADD_I_USE_BUILTIN` into\n> > that fray.\n> >\n> > Signed-off-by: Johannes Schindelin \n> > ---\n> > ci/run-build-and-tests.sh | 1 +\n> > 1 file changed, 1 insertion(+)\n> >\n> > diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh\n> > index ff0ef7f08e..4df54c4efe 100755\n> > --- a/ci/run-build-and-tests.sh\n> > +++ b/ci/run-build-and-tests.sh\n> > @@ -20,6 +20,7 @@ linux-gcc)\n> > \texport GIT_TEST_OE_DELTA_SIZE=5\n> > \texport GIT_TEST_COMMIT_GRAPH=1\n> > \texport GIT_TEST_MULTI_PACK_INDEX=1\n> > +\texport GIT_TEST_ADD_I_USE_BUILTIN=1\n> > \tmake test\n>\n> I see that I need to add this to the test-coverage builds.\n\nThank you for catching this!\n\nIt makes me wonder whether the test-coverage builds should use some `sed`\ninvocation on the `ci/run-build-and-tests.sh` script, though, so that you\ndo not have to edit the Azure Pipelines definition manually all the time?\n\nCiao,\nDscho\n"},{"id":"389432","messageId":"20200107225749.GD32750@szeder.dev","threadId":"52505","inReplyTo":"f45ff08bd0a0a2e2aba9ae929b6e5ecb3bdd4e07.1577275020.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 1/9] built-in add -p: support interactive.diffFilter","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2020-01-07T22:57:49Z","receivedAt":"2020-01-07T22:57:56Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Wed, Dec 25, 2019 at 11:56:52AM +0000, Johannes Schindelin via GitGitGadget wrote:\n> The Perl version supports post-processing the colored diff (that is\n> generated in addition to the uncolored diff, intended to offer a\n> prettier user experience) by a command configured via that config\n> setting, and now the built-in version does that, too.\n\nSo this patch makes the test 'detect bogus diffFilter output' in\n't3701-add-interactive.sh' succeed with the builtin interactive add,\nbut I stumbled upon a test failure caused by SIGPIPE in an\nexperimental Travis CI s390x build:\n\n expecting success of 3701.49 'detect bogus diffFilter output': \n git reset --hard &&\n \n echo content >test &&\n test_config interactive.diffFilter \"echo too-short\" &&\n printf y >y &&\n test_must_fail force_color git add -p Signed-off-by: Johannes Schindelin \n> ---\n> add-interactive.c | 12 ++++++++++++\n> add-interactive.h | 3 +++\n> add-patch.c | 33 +++++++++++++++++++++++++++++++++\n> 3 files changed, 48 insertions(+)\n> \n> diff --git a/add-interactive.c b/add-interactive.c\n> index a5bb14f2f4..1786ea29c4 100644\n> --- a/add-interactive.c\n> +++ b/add-interactive.c\n> @@ -52,6 +52,17 @@ void init_add_i_state(struct add_i_state *s, struct repository *r)\n> \t\tdiff_get_color(s->use_color, DIFF_FILE_OLD));\n> \tinit_color(r, s, \"new\", s->file_new_color,\n> \t\tdiff_get_color(s->use_color, DIFF_FILE_NEW));\n> +\n> +\tFREE_AND_NULL(s->interactive_diff_filter);\n> +\tgit_config_get_string(\"interactive.difffilter\",\n> +\t\t\t &s->interactive_diff_filter);\n> +}\n> +\n> +void clear_add_i_state(struct add_i_state *s)\n> +{\n> +\tFREE_AND_NULL(s->interactive_diff_filter);\n> +\tmemset(s, 0, sizeof(*s));\n> +\ts->use_color = -1;\n> }\n> \n> /*\n> @@ -1149,6 +1160,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps)\n> \tstrbuf_release(&print_file_item_data.worktree);\n> \tstrbuf_release(&header);\n> \tprefix_item_list_clear(&commands);\n> +\tclear_add_i_state(&s);\n> \n> \treturn res;\n> }\n> diff --git a/add-interactive.h b/add-interactive.h\n> index b2f23479c5..46c73867ad 100644\n> --- a/add-interactive.h\n> +++ b/add-interactive.h\n> @@ -15,9 +15,12 @@ struct add_i_state {\n> \tchar context_color[COLOR_MAXLEN];\n> \tchar file_old_color[COLOR_MAXLEN];\n> \tchar file_new_color[COLOR_MAXLEN];\n> +\n> +\tchar *interactive_diff_filter;\n> };\n> \n> void init_add_i_state(struct add_i_state *s, struct repository *r);\n> +void clear_add_i_state(struct add_i_state *s);\n> \n> struct repository;\n> struct pathspec;\n> diff --git a/add-patch.c b/add-patch.c\n> index 46c6c183d5..78bde41df0 100644\n> --- a/add-patch.c\n> +++ b/add-patch.c\n> @@ -398,6 +398,7 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n> \n> \tif (want_color_fd(1, -1)) {\n> \t\tstruct child_process colored_cp = CHILD_PROCESS_INIT;\n> +\t\tconst char *diff_filter = s->s.interactive_diff_filter;\n> \n> \t\tsetup_child_process(s, &colored_cp, NULL);\n> \t\txsnprintf((char *)args.argv[color_arg_index], 8, \"--color\");\n> @@ -407,6 +408,24 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n> \t\targv_array_clear(&args);\n> \t\tif (res)\n> \t\t\treturn error(_(\"could not parse colored diff\"));\n> +\n> +\t\tif (diff_filter) {\n> +\t\t\tstruct child_process filter_cp = CHILD_PROCESS_INIT;\n> +\n> +\t\t\tsetup_child_process(s, &filter_cp,\n> +\t\t\t\t\t diff_filter, NULL);\n> +\t\t\tfilter_cp.git_cmd = 0;\n> +\t\t\tfilter_cp.use_shell = 1;\n> +\t\t\tstrbuf_reset(&s->buf);\n> +\t\t\tif (pipe_command(&filter_cp,\n> +\t\t\t\t\t colored->buf, colored->len,\n> +\t\t\t\t\t &s->buf, colored->len,\n> +\t\t\t\t\t NULL, 0) < 0)\n> +\t\t\t\treturn error(_(\"failed to run '%s'\"),\n> +\t\t\t\t\t diff_filter);\n> +\t\t\tstrbuf_swap(colored, &s->buf);\n> +\t\t}\n> +\n> \t\tstrbuf_complete_line(colored);\n> \t\tcolored_p = colored->buf;\n> \t\tcolored_pend = colored_p + colored->len;\n> @@ -531,6 +550,9 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n> \t\t\t\t\t\t colored_pend - colored_p);\n> \t\t\tif (colored_eol)\n> \t\t\t\tcolored_p = colored_eol + 1;\n> +\t\t\telse if (p != pend)\n> +\t\t\t\t/* colored shorter than non-colored? */\n> +\t\t\t\tgoto mismatched_output;\n> \t\t\telse\n> \t\t\t\tcolored_p = colored_pend;\n> \n> @@ -555,6 +577,15 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n> \t\t */\n> \t\thunk->splittable_into++;\n> \n> +\t/* non-colored shorter than colored? */\n> +\tif (colored_p != colored_pend) {\n> +mismatched_output:\n> +\t\terror(_(\"mismatched output from interactive.diffFilter\"));\n> +\t\tadvise(_(\"Your filter must maintain a one-to-one correspondence\\n\"\n> +\t\t\t \"between its input and output lines.\"));\n> +\t\treturn -1;\n> +\t}\n> +\n> \treturn 0;\n> }\n> \n> @@ -1612,6 +1643,7 @@ int run_add_p(struct repository *r, enum add_p_mode mode,\n> \t parse_diff(&s, ps) < 0) {\n> \t\tstrbuf_release(&s.plain);\n> \t\tstrbuf_release(&s.colored);\n> +\t\tclear_add_i_state(&s.s);\n> \t\treturn -1;\n> \t}\n> \n> @@ -1630,5 +1662,6 @@ int run_add_p(struct repository *r, enum add_p_mode mode,\n> \tstrbuf_release(&s.buf);\n> \tstrbuf_release(&s.plain);\n> \tstrbuf_release(&s.colored);\n> +\tclear_add_i_state(&s.s);\n> \treturn 0;\n> }\n> -- \n> gitgitgadget\n> \n"},{"id":"389650","messageId":"nycvar.QRO.7.76.6.2001130740240.46@tvgsbejvaqbjf.bet","threadId":"52505","inReplyTo":"20200107225749.GD32750@szeder.dev","subject":"Re: [PATCH v2 1/9] built-in add -p: support interactive.diffFilter","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-01-13T06:47:23Z","receivedAt":"2020-01-13T06:47:32Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Gábor,\n\nOn Tue, 7 Jan 2020, SZEDER Gábor wrote:\n\n> On Wed, Dec 25, 2019 at 11:56:52AM +0000, Johannes Schindelin via GitGitGadget wrote:\n> > The Perl version supports post-processing the colored diff (that is\n> > generated in addition to the uncolored diff, intended to offer a\n> > prettier user experience) by a command configured via that config\n> > setting, and now the built-in version does that, too.\n>\n> So this patch makes the test 'detect bogus diffFilter output' in\n> 't3701-add-interactive.sh' succeed with the builtin interactive add,\n> but I stumbled upon a test failure caused by SIGPIPE in an\n> experimental Travis CI s390x build:\n>\n> expecting success of 3701.49 'detect bogus diffFilter output':\n> git reset --hard &&\n>\n> echo content >test &&\n> test_config interactive.diffFilter \"echo too-short\" &&\n> printf y >y &&\n> test_must_fail force_color git add -p \n> + git reset --hard\n> HEAD is now at 6ee5ee5 test\n> + echo content\n> + test_config interactive.diffFilter echo too-short\n> + printf y\n> + test_must_fail force_color git add -p\n> test_must_fail: died by signal 13: force_color git add -p\n> error: last command exited with $?=1\n>\n> Turns out it's a general issue, and\n>\n> GIT_TEST_ADD_I_USE_BUILTIN=1 ./t3701-add-interactive.sh -r 39,49 --stress\n>\n> fails within 10 seconds on my Linux box, whereas the scripted 'add -p'\n> managed to survive a couple hundred repetitions.\n\nYou're right, of course. And I had let that slip for too long, as I saw it\nsporadically happen in the Azure Pipeline, too.\n\nThis took quite a while to figure out, and I won't claim that I understand\n_all_ the details: I _think_ that `stdin` being so short \"breaks the pipe\"\nand interferes with `add -p`'s normal operation, so I needed to explicitly\nuse the `sigchain` feature to ignore `SIGPIPE` during `add -p`'s main\nloop.\n\nThanks,\nDscho\n"},{"id":"389651","messageId":"5e258a8d2bb271433902b2e44c3a30a988bbf512.1578904171.git.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.v3.git.1578904171.gitgitgadget@gmail.com","subject":"[PATCH v3 01/10] built-in add -i/-p: treat SIGPIPE as EOF","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-01-13T08:29:22Z","receivedAt":"2020-01-13T08:29:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nAs noticed by Gábor Szeder, if we want to run `git add -p` with\nredirected input through `test_must_fail` in the test suite, we must\nexpect that a SIGPIPE can happen due to `stdin` coming to its end.\n\nThe appropriate action here is to ignore that signal and treat it as a\nregular end-of-file, otherwise the test will fail. In preparation for\nsuch a test, introduce precisely this handling of SIGPIPE into the\nbuilt-in version of `git add -p`.\n\nFor good measure, teach the built-in `git add -i` the same trick: it\n_also_ runs a loop waiting for input, and can receive a SIGPIPE just the\nsame (and wants to treat it as end-of-file, too).\n\nSigned-off-by: Johannes Schindelin \n---\n add-interactive.c | 3 +++\n add-patch.c | 4 ++++\n 2 files changed, 7 insertions(+)\n\ndiff --git a/add-interactive.c b/add-interactive.c\nindex a5bb14f2f4..3ff8400ea4 100644\n--- a/add-interactive.c\n+++ b/add-interactive.c\n@@ -9,6 +9,7 @@\n #include \"lockfile.h\"\n #include \"dir.h\"\n #include \"run-command.h\"\n+#include \"sigchain.h\"\n \n static void init_color(struct repository *r, struct add_i_state *s,\n \t\t const char *slot_name, char *dst,\n@@ -1097,6 +1098,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps)\n \t\t\t->util = util;\n \t}\n \n+\tsigchain_push(SIGPIPE, SIG_IGN);\n \tinit_add_i_state(&s, r);\n \n \t/*\n@@ -1149,6 +1151,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps)\n \tstrbuf_release(&print_file_item_data.worktree);\n \tstrbuf_release(&header);\n \tprefix_item_list_clear(&commands);\n+\tsigchain_pop(SIGPIPE);\n \n \treturn res;\n }\ndiff --git a/add-patch.c b/add-patch.c\nindex 46c6c183d5..9a3beed72e 100644\n--- a/add-patch.c\n+++ b/add-patch.c\n@@ -6,6 +6,7 @@\n #include \"pathspec.h\"\n #include \"color.h\"\n #include \"diff.h\"\n+#include \"sigchain.h\"\n \n enum prompt_mode_type {\n \tPROMPT_MODE_CHANGE = 0, PROMPT_DELETION, PROMPT_HUNK,\n@@ -1578,6 +1579,7 @@ int run_add_p(struct repository *r, enum add_p_mode mode,\n \t};\n \tsize_t i, binary_count = 0;\n \n+\tsigchain_push(SIGPIPE, SIG_IGN);\n \tinit_add_i_state(&s.s, r);\n \n \tif (mode == ADD_P_STASH)\n@@ -1612,6 +1614,7 @@ int run_add_p(struct repository *r, enum add_p_mode mode,\n \t parse_diff(&s, ps) < 0) {\n \t\tstrbuf_release(&s.plain);\n \t\tstrbuf_release(&s.colored);\n+\t\tsigchain_pop(SIGPIPE);\n \t\treturn -1;\n \t}\n \n@@ -1630,5 +1633,6 @@ int run_add_p(struct repository *r, enum add_p_mode mode,\n \tstrbuf_release(&s.buf);\n \tstrbuf_release(&s.plain);\n \tstrbuf_release(&s.colored);\n+\tsigchain_pop(SIGPIPE);\n \treturn 0;\n }\n-- \ngitgitgadget\n\n"},{"id":"389652","messageId":"2a5951ecfef7a5d93fd6f5a36c4b5df75ec910e1.1578904171.git.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.v3.git.1578904171.gitgitgadget@gmail.com","subject":"[PATCH v3 02/10] built-in add -p: support interactive.diffFilter","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-01-13T08:29:23Z","receivedAt":"2020-01-13T08:29:37Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThe Perl version supports post-processing the colored diff (that is\ngenerated in addition to the uncolored diff, intended to offer a\nprettier user experience) by a command configured via that config\nsetting, and now the built-in version does that, too.\n\nSigned-off-by: Johannes Schindelin \n---\n add-interactive.c | 12 ++++++++++++\n add-interactive.h | 3 +++\n add-patch.c | 33 +++++++++++++++++++++++++++++++++\n 3 files changed, 48 insertions(+)\n\ndiff --git a/add-interactive.c b/add-interactive.c\nindex 3ff8400ea4..b36e5d97d8 100644\n--- a/add-interactive.c\n+++ b/add-interactive.c\n@@ -53,6 +53,17 @@ void init_add_i_state(struct add_i_state *s, struct repository *r)\n \t\tdiff_get_color(s->use_color, DIFF_FILE_OLD));\n \tinit_color(r, s, \"new\", s->file_new_color,\n \t\tdiff_get_color(s->use_color, DIFF_FILE_NEW));\n+\n+\tFREE_AND_NULL(s->interactive_diff_filter);\n+\tgit_config_get_string(\"interactive.difffilter\",\n+\t\t\t &s->interactive_diff_filter);\n+}\n+\n+void clear_add_i_state(struct add_i_state *s)\n+{\n+\tFREE_AND_NULL(s->interactive_diff_filter);\n+\tmemset(s, 0, sizeof(*s));\n+\ts->use_color = -1;\n }\n \n /*\n@@ -1151,6 +1162,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps)\n \tstrbuf_release(&print_file_item_data.worktree);\n \tstrbuf_release(&header);\n \tprefix_item_list_clear(&commands);\n+\tclear_add_i_state(&s);\n \tsigchain_pop(SIGPIPE);\n \n \treturn res;\ndiff --git a/add-interactive.h b/add-interactive.h\nindex b2f23479c5..46c73867ad 100644\n--- a/add-interactive.h\n+++ b/add-interactive.h\n@@ -15,9 +15,12 @@ struct add_i_state {\n \tchar context_color[COLOR_MAXLEN];\n \tchar file_old_color[COLOR_MAXLEN];\n \tchar file_new_color[COLOR_MAXLEN];\n+\n+\tchar *interactive_diff_filter;\n };\n \n void init_add_i_state(struct add_i_state *s, struct repository *r);\n+void clear_add_i_state(struct add_i_state *s);\n \n struct repository;\n struct pathspec;\ndiff --git a/add-patch.c b/add-patch.c\nindex 9a3beed72e..7d6015229c 100644\n--- a/add-patch.c\n+++ b/add-patch.c\n@@ -399,6 +399,7 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n \n \tif (want_color_fd(1, -1)) {\n \t\tstruct child_process colored_cp = CHILD_PROCESS_INIT;\n+\t\tconst char *diff_filter = s->s.interactive_diff_filter;\n \n \t\tsetup_child_process(s, &colored_cp, NULL);\n \t\txsnprintf((char *)args.argv[color_arg_index], 8, \"--color\");\n@@ -408,6 +409,24 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n \t\targv_array_clear(&args);\n \t\tif (res)\n \t\t\treturn error(_(\"could not parse colored diff\"));\n+\n+\t\tif (diff_filter) {\n+\t\t\tstruct child_process filter_cp = CHILD_PROCESS_INIT;\n+\n+\t\t\tsetup_child_process(s, &filter_cp,\n+\t\t\t\t\t diff_filter, NULL);\n+\t\t\tfilter_cp.git_cmd = 0;\n+\t\t\tfilter_cp.use_shell = 1;\n+\t\t\tstrbuf_reset(&s->buf);\n+\t\t\tif (pipe_command(&filter_cp,\n+\t\t\t\t\t colored->buf, colored->len,\n+\t\t\t\t\t &s->buf, colored->len,\n+\t\t\t\t\t NULL, 0) < 0)\n+\t\t\t\treturn error(_(\"failed to run '%s'\"),\n+\t\t\t\t\t diff_filter);\n+\t\t\tstrbuf_swap(colored, &s->buf);\n+\t\t}\n+\n \t\tstrbuf_complete_line(colored);\n \t\tcolored_p = colored->buf;\n \t\tcolored_pend = colored_p + colored->len;\n@@ -532,6 +551,9 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n \t\t\t\t\t\t colored_pend - colored_p);\n \t\t\tif (colored_eol)\n \t\t\t\tcolored_p = colored_eol + 1;\n+\t\t\telse if (p != pend)\n+\t\t\t\t/* colored shorter than non-colored? */\n+\t\t\t\tgoto mismatched_output;\n \t\t\telse\n \t\t\t\tcolored_p = colored_pend;\n \n@@ -556,6 +578,15 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n \t\t */\n \t\thunk->splittable_into++;\n \n+\t/* non-colored shorter than colored? */\n+\tif (colored_p != colored_pend) {\n+mismatched_output:\n+\t\terror(_(\"mismatched output from interactive.diffFilter\"));\n+\t\tadvise(_(\"Your filter must maintain a one-to-one correspondence\\n\"\n+\t\t\t \"between its input and output lines.\"));\n+\t\treturn -1;\n+\t}\n+\n \treturn 0;\n }\n \n@@ -1614,6 +1645,7 @@ int run_add_p(struct repository *r, enum add_p_mode mode,\n \t parse_diff(&s, ps) < 0) {\n \t\tstrbuf_release(&s.plain);\n \t\tstrbuf_release(&s.colored);\n+\t\tclear_add_i_state(&s.s);\n \t\tsigchain_pop(SIGPIPE);\n \t\treturn -1;\n \t}\n@@ -1633,6 +1665,7 @@ int run_add_p(struct repository *r, enum add_p_mode mode,\n \tstrbuf_release(&s.buf);\n \tstrbuf_release(&s.plain);\n \tstrbuf_release(&s.colored);\n+\tclear_add_i_state(&s.s);\n \tsigchain_pop(SIGPIPE);\n \treturn 0;\n }\n-- \ngitgitgadget\n\n"},{"id":"389653","messageId":"a2bce01818b6ab0374c19f82acf91b7353f90ef8.1578904171.git.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.v3.git.1578904171.gitgitgadget@gmail.com","subject":"[PATCH v3 03/10] built-in add -p: handle diff.algorithm","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-01-13T08:29:24Z","receivedAt":"2020-01-13T08:29:38Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThe Perl version of `git add -p` reads the config setting\n`diff.algorithm` and if set, uses it to generate the diff using the\nspecified algorithm.\n\nThis patch ports that functionality to the C version.\n\nNote: just like `git-add--interactive.perl`, we do _not_ respect this\nconfig setting in `git add -i`'s `diff` command, but _only_ in the\n`patch` command.\n\nSigned-off-by: Johannes Schindelin \n---\n add-interactive.c | 5 +++++\n add-interactive.h | 2 +-\n add-patch.c | 3 +++\n 3 files changed, 9 insertions(+), 1 deletion(-)\n\ndiff --git a/add-interactive.c b/add-interactive.c\nindex b36e5d97d8..e3cc30ad24 100644\n--- a/add-interactive.c\n+++ b/add-interactive.c\n@@ -57,11 +57,16 @@ void init_add_i_state(struct add_i_state *s, struct repository *r)\n \tFREE_AND_NULL(s->interactive_diff_filter);\n \tgit_config_get_string(\"interactive.difffilter\",\n \t\t\t &s->interactive_diff_filter);\n+\n+\tFREE_AND_NULL(s->interactive_diff_algorithm);\n+\tgit_config_get_string(\"diff.algorithm\",\n+\t\t\t &s->interactive_diff_algorithm);\n }\n \n void clear_add_i_state(struct add_i_state *s)\n {\n \tFREE_AND_NULL(s->interactive_diff_filter);\n+\tFREE_AND_NULL(s->interactive_diff_algorithm);\n \tmemset(s, 0, sizeof(*s));\n \ts->use_color = -1;\n }\ndiff --git a/add-interactive.h b/add-interactive.h\nindex 46c73867ad..923efaf527 100644\n--- a/add-interactive.h\n+++ b/add-interactive.h\n@@ -16,7 +16,7 @@ struct add_i_state {\n \tchar file_old_color[COLOR_MAXLEN];\n \tchar file_new_color[COLOR_MAXLEN];\n \n-\tchar *interactive_diff_filter;\n+\tchar *interactive_diff_filter, *interactive_diff_algorithm;\n };\n \n void init_add_i_state(struct add_i_state *s, struct repository *r);\ndiff --git a/add-patch.c b/add-patch.c\nindex 7d6015229c..736bcb4aa7 100644\n--- a/add-patch.c\n+++ b/add-patch.c\n@@ -361,6 +361,7 @@ static int is_octal(const char *p, size_t len)\n static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n {\n \tstruct argv_array args = ARGV_ARRAY_INIT;\n+\tconst char *diff_algorithm = s->s.interactive_diff_algorithm;\n \tstruct strbuf *plain = &s->plain, *colored = NULL;\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n \tchar *p, *pend, *colored_p = NULL, *colored_pend = NULL, marker = '\\0';\n@@ -370,6 +371,8 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n \tint res;\n \n \targv_array_pushv(&args, s->mode->diff_cmd);\n+\tif (diff_algorithm)\n+\t\targv_array_pushf(&args, \"--diff-algorithm=%s\", diff_algorithm);\n \tif (s->revision) {\n \t\tstruct object_id oid;\n \t\targv_array_push(&args,\n-- \ngitgitgadget\n\n"},{"id":"389654","messageId":"pull.175.v3.git.1578904171.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.v2.git.1577275020.gitgitgadget@gmail.com","subject":"[PATCH v3 00/10] built-in add -p: add support for the same config settings as the Perl version","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-01-13T08:29:21Z","receivedAt":"2020-01-13T08:29:39Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"This is the final leg of the journey to a fully built-in git add: the git\nadd -i and git add -p modes were re-implemented in C, but they lacked\nsupport for a couple of config settings.\n\nThe one that sticks out most is the interactive.singleKey setting: it was\nparticularly hard to get to work, especially on Windows.\n\nIt also seems to be the setting that is incomplete already in the Perl\nversion of the interactive add command: while the name of the config setting\nsuggests that it applies to all of the interactive add, including the main\nloop of git add --interactive and to the file selections in that command, it\ndoes not. Only the git add --patch mode respects that setting.\n\nAs it is outside the purpose of the conversion of git-add--interactive.perl \nto C, we will leave that loose end for some future date.\n\nChanges since v2:\n\n * Fixed the SIGPIPE issue pointed out by Gábor Szeder.\n\nChanges since v1:\n\n * Fixed the commit message where a copy/paste fail made it talk about\n another GIT_TEST_* variable than the GIT_TEST_ADD_I_USE_BUILTIN one.\n\nJohannes Schindelin (10):\n built-in add -i/-p: treat SIGPIPE as EOF\n built-in add -p: support interactive.diffFilter\n built-in add -p: handle diff.algorithm\n terminal: make the code of disable_echo() reusable\n terminal: accommodate Git for Windows' default terminal\n terminal: add a new function to read a single keystroke\n built-in add -p: respect the `interactive.singlekey` config setting\n built-in add -p: handle Escape sequences in interactive.singlekey mode\n built-in add -p: handle Escape sequences more efficiently\n ci: include the built-in `git add -i` in the `linux-gcc` job\n\n add-interactive.c | 22 ++++\n add-interactive.h | 4 +\n add-patch.c | 61 +++++++++-\n ci/run-build-and-tests.sh | 1 +\n compat/terminal.c | 249 +++++++++++++++++++++++++++++++++++++-\n compat/terminal.h | 3 +\n 6 files changed, 332 insertions(+), 8 deletions(-)\n\n\nbase-commit: c480eeb574e649a19f27dc09a994e45f9b2c2622\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-175%2Fdscho%2Fadd-p-in-c-config-settings-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-175/dscho/add-p-in-c-config-settings-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/175\n\nRange-diff vs v2:\n\n -: ---------- > 1: 5e258a8d2b built-in add -i/-p: treat SIGPIPE as EOF\n 1: f45ff08bd0 ! 2: 2a5951ecfe built-in add -p: support interactive.diffFilter\n @@ -35,9 +35,9 @@\n \tstrbuf_release(&header);\n \tprefix_item_list_clear(&commands);\n +\tclear_add_i_state(&s);\n + \tsigchain_pop(SIGPIPE);\n \n \treturn res;\n - }\n \n diff --git a/add-interactive.h b/add-interactive.h\n --- a/add-interactive.h\n @@ -123,13 +123,14 @@\n \t\tstrbuf_release(&s.plain);\n \t\tstrbuf_release(&s.colored);\n +\t\tclear_add_i_state(&s.s);\n + \t\tsigchain_pop(SIGPIPE);\n \t\treturn -1;\n \t}\n - \n @@\n \tstrbuf_release(&s.buf);\n \tstrbuf_release(&s.plain);\n \tstrbuf_release(&s.colored);\n +\tclear_add_i_state(&s.s);\n + \tsigchain_pop(SIGPIPE);\n \treturn 0;\n }\n 2: e9c4a13cbf = 3: a2bce01818 built-in add -p: handle diff.algorithm\n 3: e643554dba = 4: be40a37c0c terminal: make the code of disable_echo() reusable\n 4: bd2306c5d5 = 5: 233f23791c terminal: accommodate Git for Windows' default terminal\n 5: 190fb4f5e9 = 6: 74593b5115 terminal: add a new function to read a single keystroke\n 6: 167dfa37dd ! 7: 197fe1e14a built-in add -p: respect the `interactive.singlekey` config setting\n @@ -48,9 +48,9 @@\n --- a/add-patch.c\n +++ b/add-patch.c\n @@\n - #include \"pathspec.h\"\n #include \"color.h\"\n #include \"diff.h\"\n + #include \"sigchain.h\"\n +#include \"compat/terminal.h\"\n \n enum prompt_mode_type {\n 7: 32067bebe8 = 8: 9ab381d539 built-in add -p: handle Escape sequences in interactive.singlekey mode\n 8: 703719ffce = 9: bdb6268b8b built-in add -p: handle Escape sequences more efficiently\n 9: 23a3a47b01 = 10: c4195969a6 ci: include the built-in `git add -i` in the `linux-gcc` job\n\n-- \ngitgitgadget\n"},{"id":"389655","messageId":"233f23791caaca5dee6b227520560f53977c8ea7.1578904171.git.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.v3.git.1578904171.gitgitgadget@gmail.com","subject":"[PATCH v3 05/10] terminal: accommodate Git for Windows' default terminal","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-01-13T08:29:26Z","receivedAt":"2020-01-13T08:29:40Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nGit for Windows' Git Bash runs in MinTTY by default, which does not have\na Win32 Console instance, but uses MSYS2 pseudo terminals instead.\n\nThis is a problem, as Git for Windows does not want to use the MSYS2\nemulation layer for Git itself, and therefore has no direct way to\ninteract with that pseudo terminal.\n\nAs a workaround, use the `stty` utility (which is included in Git for\nWindows, and which *is* an MSYS2 program, so it knows how to deal with\nthe pseudo terminal).\n\nNote: If Git runs in a regular CMD or PowerShell window, there *is* a\nregular Win32 Console to work with. This is not a problem for the MSYS2\n`stty`: it copes with this scenario just fine.\n\nAlso note that we introduce support for more bits than would be\nnecessary for a mere `disable_echo()` here, in preparation for the\nupcoming `enable_non_canonical()` function.\n\nSigned-off-by: Johannes Schindelin \n---\n compat/terminal.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 50 insertions(+)\n\ndiff --git a/compat/terminal.c b/compat/terminal.c\nindex 1fb40b3a0a..16e9949da1 100644\n--- a/compat/terminal.c\n+++ b/compat/terminal.c\n@@ -2,6 +2,8 @@\n #include \"compat/terminal.h\"\n #include \"sigchain.h\"\n #include \"strbuf.h\"\n+#include \"run-command.h\"\n+#include \"string-list.h\"\n \n #if defined(HAVE_DEV_TTY) || defined(GIT_WINDOWS_NATIVE)\n \n@@ -64,11 +66,28 @@ static int disable_echo(void)\n #define OUTPUT_PATH \"CONOUT$\"\n #define FORCE_TEXT \"t\"\n \n+static int use_stty = 1;\n+static struct string_list stty_restore = STRING_LIST_INIT_DUP;\n static HANDLE hconin = INVALID_HANDLE_VALUE;\n static DWORD cmode;\n \n static void restore_term(void)\n {\n+\tif (use_stty) {\n+\t\tint i;\n+\t\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\n+\t\tif (stty_restore.nr == 0)\n+\t\t\treturn;\n+\n+\t\targv_array_push(&cp.args, \"stty\");\n+\t\tfor (i = 0; i < stty_restore.nr; i++)\n+\t\t\targv_array_push(&cp.args, stty_restore.items[i].string);\n+\t\trun_command(&cp);\n+\t\tstring_list_clear(&stty_restore, 0);\n+\t\treturn;\n+\t}\n+\n \tif (hconin == INVALID_HANDLE_VALUE)\n \t\treturn;\n \n@@ -79,6 +98,37 @@ static void restore_term(void)\n \n static int disable_bits(DWORD bits)\n {\n+\tif (use_stty) {\n+\t\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\n+\t\targv_array_push(&cp.args, \"stty\");\n+\n+\t\tif (bits & ENABLE_LINE_INPUT) {\n+\t\t\tstring_list_append(&stty_restore, \"icanon\");\n+\t\t\targv_array_push(&cp.args, \"-icanon\");\n+\t\t}\n+\n+\t\tif (bits & ENABLE_ECHO_INPUT) {\n+\t\t\tstring_list_append(&stty_restore, \"echo\");\n+\t\t\targv_array_push(&cp.args, \"-echo\");\n+\t\t}\n+\n+\t\tif (bits & ENABLE_PROCESSED_INPUT) {\n+\t\t\tstring_list_append(&stty_restore, \"-ignbrk\");\n+\t\t\tstring_list_append(&stty_restore, \"intr\");\n+\t\t\tstring_list_append(&stty_restore, \"^c\");\n+\t\t\targv_array_push(&cp.args, \"ignbrk\");\n+\t\t\targv_array_push(&cp.args, \"intr\");\n+\t\t\targv_array_push(&cp.args, \"\");\n+\t\t}\n+\n+\t\tif (run_command(&cp) == 0)\n+\t\t\treturn 0;\n+\n+\t\t/* `stty` could not be executed; access the Console directly */\n+\t\tuse_stty = 0;\n+\t}\n+\n \thconin = CreateFile(\"CONIN$\", GENERIC_READ | GENERIC_WRITE,\n \t FILE_SHARE_READ, NULL, OPEN_EXISTING,\n \t FILE_ATTRIBUTE_NORMAL, NULL);\n-- \ngitgitgadget\n\n"},{"id":"389656","messageId":"be40a37c0c3b47e12ecb4423ea03f4592b8ea4ec.1578904171.git.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.v3.git.1578904171.gitgitgadget@gmail.com","subject":"[PATCH v3 04/10] terminal: make the code of disable_echo() reusable","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-01-13T08:29:25Z","receivedAt":"2020-01-13T08:29:42Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWe are about to introduce the function `enable_non_canonical()`, which\nshares almost the complete code with `disable_echo()`.\n\nLet's prepare for that, by refactoring out that shared code.\n\nSigned-off-by: Johannes Schindelin \n---\n compat/terminal.c | 19 +++++++++++++++----\n 1 file changed, 15 insertions(+), 4 deletions(-)\n\ndiff --git a/compat/terminal.c b/compat/terminal.c\nindex fa13ee672d..1fb40b3a0a 100644\n--- a/compat/terminal.c\n+++ b/compat/terminal.c\n@@ -32,7 +32,7 @@ static void restore_term(void)\n \tterm_fd = -1;\n }\n \n-static int disable_echo(void)\n+static int disable_bits(tcflag_t bits)\n {\n \tstruct termios t;\n \n@@ -43,7 +43,7 @@ static int disable_echo(void)\n \told_term = t;\n \tsigchain_push_common(restore_term_on_signal);\n \n-\tt.c_lflag &= ~ECHO;\n+\tt.c_lflag &= ~bits;\n \tif (!tcsetattr(term_fd, TCSAFLUSH, &t))\n \t\treturn 0;\n \n@@ -53,6 +53,11 @@ static int disable_echo(void)\n \treturn -1;\n }\n \n+static int disable_echo(void)\n+{\n+\treturn disable_bits(ECHO);\n+}\n+\n #elif defined(GIT_WINDOWS_NATIVE)\n \n #define INPUT_PATH \"CONIN$\"\n@@ -72,7 +77,7 @@ static void restore_term(void)\n \thconin = INVALID_HANDLE_VALUE;\n }\n \n-static int disable_echo(void)\n+static int disable_bits(DWORD bits)\n {\n \thconin = CreateFile(\"CONIN$\", GENERIC_READ | GENERIC_WRITE,\n \t FILE_SHARE_READ, NULL, OPEN_EXISTING,\n@@ -82,7 +87,7 @@ static int disable_echo(void)\n \n \tGetConsoleMode(hconin, &cmode);\n \tsigchain_push_common(restore_term_on_signal);\n-\tif (!SetConsoleMode(hconin, cmode & (~ENABLE_ECHO_INPUT))) {\n+\tif (!SetConsoleMode(hconin, cmode & ~bits)) {\n \t\tCloseHandle(hconin);\n \t\thconin = INVALID_HANDLE_VALUE;\n \t\treturn -1;\n@@ -91,6 +96,12 @@ static int disable_echo(void)\n \treturn 0;\n }\n \n+static int disable_echo(void)\n+{\n+\treturn disable_bits(ENABLE_ECHO_INPUT);\n+}\n+\n+\n #endif\n \n #ifndef FORCE_TEXT\n-- \ngitgitgadget\n\n"},{"id":"389657","messageId":"74593b51157c49f1785e217446f3af0889b0fdf2.1578904171.git.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.v3.git.1578904171.gitgitgadget@gmail.com","subject":"[PATCH v3 06/10] terminal: add a new function to read a single keystroke","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-01-13T08:29:27Z","receivedAt":"2020-01-13T08:29:43Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nTypically, input on the command-line is line-based. It is actually not\nreally easy to get single characters (or better put: keystrokes).\n\nWe provide two implementations here:\n\n- One that handles `/dev/tty` based systems as well as native Windows.\n The former uses the `tcsetattr()` function to put the terminal into\n \"raw mode\", which allows us to read individual keystrokes, one by one.\n The latter uses `stty.exe` to do the same, falling back to direct\n Win32 Console access.\n\n Thanks to the refactoring leading up to this commit, this is a single\n function, with the platform-specific details hidden away in\n conditionally-compiled code blocks.\n\n- A fall-back which simply punts and reads back an entire line.\n\nNote that the function writes the keystroke into an `strbuf` rather than\na `char`, in preparation for reading Escape sequences (e.g. when the\nuser hit an arrow key). This is also required for UTF-8 sequences in\ncase the keystroke corresponds to a non-ASCII letter.\n\nSigned-off-by: Johannes Schindelin \n---\n compat/terminal.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++\n compat/terminal.h | 3 +++\n 2 files changed, 58 insertions(+)\n\ndiff --git a/compat/terminal.c b/compat/terminal.c\nindex 16e9949da1..1b2564042a 100644\n--- a/compat/terminal.c\n+++ b/compat/terminal.c\n@@ -60,6 +60,11 @@ static int disable_echo(void)\n \treturn disable_bits(ECHO);\n }\n \n+static int enable_non_canonical(void)\n+{\n+\treturn disable_bits(ICANON | ECHO);\n+}\n+\n #elif defined(GIT_WINDOWS_NATIVE)\n \n #define INPUT_PATH \"CONIN$\"\n@@ -151,6 +156,10 @@ static int disable_echo(void)\n \treturn disable_bits(ENABLE_ECHO_INPUT);\n }\n \n+static int enable_non_canonical(void)\n+{\n+\treturn disable_bits(ENABLE_ECHO_INPUT | ENABLE_LINE_INPUT | ENABLE_PROCESSED_INPUT);\n+}\n \n #endif\n \n@@ -198,6 +207,33 @@ char *git_terminal_prompt(const char *prompt, int echo)\n \treturn buf.buf;\n }\n \n+int read_key_without_echo(struct strbuf *buf)\n+{\n+\tstatic int warning_displayed;\n+\tint ch;\n+\n+\tif (warning_displayed || enable_non_canonical() < 0) {\n+\t\tif (!warning_displayed) {\n+\t\t\twarning(\"reading single keystrokes not supported on \"\n+\t\t\t\t\"this platform; reading line instead\");\n+\t\t\twarning_displayed = 1;\n+\t\t}\n+\n+\t\treturn strbuf_getline(buf, stdin);\n+\t}\n+\n+\tstrbuf_reset(buf);\n+\tch = getchar();\n+\tif (ch == EOF) {\n+\t\trestore_term();\n+\t\treturn EOF;\n+\t}\n+\n+\tstrbuf_addch(buf, ch);\n+\trestore_term();\n+\treturn 0;\n+}\n+\n #else\n \n char *git_terminal_prompt(const char *prompt, int echo)\n@@ -205,4 +241,23 @@ char *git_terminal_prompt(const char *prompt, int echo)\n \treturn getpass(prompt);\n }\n \n+int read_key_without_echo(struct strbuf *buf)\n+{\n+\tstatic int warning_displayed;\n+\tconst char *res;\n+\n+\tif (!warning_displayed) {\n+\t\twarning(\"reading single keystrokes not supported on this \"\n+\t\t\t\"platform; reading line instead\");\n+\t\twarning_displayed = 1;\n+\t}\n+\n+\tres = getpass(\"\");\n+\tstrbuf_reset(buf);\n+\tif (!res)\n+\t\treturn EOF;\n+\tstrbuf_addstr(buf, res);\n+\treturn 0;\n+}\n+\n #endif\ndiff --git a/compat/terminal.h b/compat/terminal.h\nindex 97db7cd69d..a9d52b8464 100644\n--- a/compat/terminal.h\n+++ b/compat/terminal.h\n@@ -3,4 +3,7 @@\n \n char *git_terminal_prompt(const char *prompt, int echo);\n \n+/* Read a single keystroke, without echoing it to the terminal */\n+int read_key_without_echo(struct strbuf *buf);\n+\n #endif /* COMPAT_TERMINAL_H */\n-- \ngitgitgadget\n\n"},{"id":"389658","messageId":"197fe1e14adfd8142fa9f4f6c93937d9349d0bdc.1578904171.git.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.v3.git.1578904171.gitgitgadget@gmail.com","subject":"[PATCH v3 07/10] built-in add -p: respect the `interactive.singlekey` config setting","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-01-13T08:29:28Z","receivedAt":"2020-01-13T08:29:45Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThe Perl version of `git add -p` supports this config setting to allow\nusers to input commands via single characters (as opposed to having to\npress the key afterwards).\n\nThis is an opt-in feature because it requires Perl packages\n(Term::ReadKey and Term::Cap, where it tries to handle an absence of the\nlatter package gracefully) to work. Note that at least on Ubuntu, that\nPerl package is not installed by default (it needs to be installed via\n`sudo apt-get install libterm-readkey-perl`), so this feature is\nprobably not used a whole lot.\n\nIn C, we obviously do not have these packages available, but we just\nintroduced `read_single_keystroke()` that is similar to what\nTerm::ReadKey provides, and we use that here.\n\nSigned-off-by: Johannes Schindelin \n---\n add-interactive.c | 2 ++\n add-interactive.h | 1 +\n add-patch.c | 21 +++++++++++++++++----\n 3 files changed, 20 insertions(+), 4 deletions(-)\n\ndiff --git a/add-interactive.c b/add-interactive.c\nindex e3cc30ad24..bb6acf5ef6 100644\n--- a/add-interactive.c\n+++ b/add-interactive.c\n@@ -61,6 +61,8 @@ void init_add_i_state(struct add_i_state *s, struct repository *r)\n \tFREE_AND_NULL(s->interactive_diff_algorithm);\n \tgit_config_get_string(\"diff.algorithm\",\n \t\t\t &s->interactive_diff_algorithm);\n+\n+\tgit_config_get_bool(\"interactive.singlekey\", &s->use_single_key);\n }\n \n void clear_add_i_state(struct add_i_state *s)\ndiff --git a/add-interactive.h b/add-interactive.h\nindex 923efaf527..693f125e8e 100644\n--- a/add-interactive.h\n+++ b/add-interactive.h\n@@ -16,6 +16,7 @@ struct add_i_state {\n \tchar file_old_color[COLOR_MAXLEN];\n \tchar file_new_color[COLOR_MAXLEN];\n \n+\tint use_single_key;\n \tchar *interactive_diff_filter, *interactive_diff_algorithm;\n };\n \ndiff --git a/add-patch.c b/add-patch.c\nindex 736bcb4aa7..67741128a8 100644\n--- a/add-patch.c\n+++ b/add-patch.c\n@@ -7,6 +7,7 @@\n #include \"color.h\"\n #include \"diff.h\"\n #include \"sigchain.h\"\n+#include \"compat/terminal.h\"\n \n enum prompt_mode_type {\n \tPROMPT_MODE_CHANGE = 0, PROMPT_DELETION, PROMPT_HUNK,\n@@ -1150,14 +1151,27 @@ static int run_apply_check(struct add_p_state *s,\n \treturn 0;\n }\n \n+static int read_single_character(struct add_p_state *s)\n+{\n+\tif (s->s.use_single_key) {\n+\t\tint res = read_key_without_echo(&s->answer);\n+\t\tprintf(\"%s\\n\", res == EOF ? \"\" : s->answer.buf);\n+\t\treturn res;\n+\t}\n+\n+\tif (strbuf_getline(&s->answer, stdin) == EOF)\n+\t\treturn EOF;\n+\tstrbuf_trim_trailing_newline(&s->answer);\n+\treturn 0;\n+}\n+\n static int prompt_yesno(struct add_p_state *s, const char *prompt)\n {\n \tfor (;;) {\n \t\tcolor_fprintf(stdout, s->s.prompt_color, \"%s\", _(prompt));\n \t\tfflush(stdout);\n-\t\tif (strbuf_getline(&s->answer, stdin) == EOF)\n+\t\tif (read_single_character(s) == EOF)\n \t\t\treturn -1;\n-\t\tstrbuf_trim_trailing_newline(&s->answer);\n \t\tswitch (tolower(s->answer.buf[0])) {\n \t\tcase 'n': return 0;\n \t\tcase 'y': return 1;\n@@ -1397,9 +1411,8 @@ static int patch_update_file(struct add_p_state *s,\n \t\t\t _(s->mode->prompt_mode[prompt_mode_type]),\n \t\t\t s->buf.buf);\n \t\tfflush(stdout);\n-\t\tif (strbuf_getline(&s->answer, stdin) == EOF)\n+\t\tif (read_single_character(s) == EOF)\n \t\t\tbreak;\n-\t\tstrbuf_trim_trailing_newline(&s->answer);\n \n \t\tif (!s->answer.len)\n \t\t\tcontinue;\n-- \ngitgitgadget\n\n"},{"id":"389659","messageId":"c4195969a6d3beb3cb91a608910d6e4f7ee9a4e1.1578904171.git.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.v3.git.1578904171.gitgitgadget@gmail.com","subject":"[PATCH v3 10/10] ci: include the built-in `git add -i` in the `linux-gcc` job","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-01-13T08:29:31Z","receivedAt":"2020-01-13T08:29:46Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThis job runs the test suite twice, once in regular mode, and once with\na whole slew of `GIT_TEST_*` variables set.\n\nNow that the built-in version of `git add --interactive` is\nfeature-complete, let's also throw `GIT_TEST_ADD_I_USE_BUILTIN` into\nthat fray.\n\nSigned-off-by: Johannes Schindelin \n---\n ci/run-build-and-tests.sh | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh\nindex ff0ef7f08e..4df54c4efe 100755\n--- a/ci/run-build-and-tests.sh\n+++ b/ci/run-build-and-tests.sh\n@@ -20,6 +20,7 @@ linux-gcc)\n \texport GIT_TEST_OE_DELTA_SIZE=5\n \texport GIT_TEST_COMMIT_GRAPH=1\n \texport GIT_TEST_MULTI_PACK_INDEX=1\n+\texport GIT_TEST_ADD_I_USE_BUILTIN=1\n \tmake test\n \t;;\n linux-gcc-4.8)\n-- \ngitgitgadget\n"},{"id":"389660","messageId":"bdb6268b8b5830311d0e7c8528eaa2e7a0664ab8.1578904171.git.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.v3.git.1578904171.gitgitgadget@gmail.com","subject":"[PATCH v3 09/10] built-in add -p: handle Escape sequences more efficiently","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-01-13T08:29:30Z","receivedAt":"2020-01-13T08:29:47Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWhen `interactive.singlekey = true`, we react immediately to keystrokes,\neven to Escape sequences (e.g. when pressing a cursor key).\n\nThe problem with Escape sequences is that we do not really know when\nthey are done, and as a heuristic we poll standard input for half a\nsecond to make sure that we got all of it.\n\nWhile waiting half a second is not asking for a whole lot, it can become\nquite annoying over time, therefore with this patch, we read the\nterminal capabilities (if available) and extract known Escape sequences\nfrom there, then stop polling immediately when we detected that the user\npressed a key that generated such a known sequence.\n\nThis recapitulates the remaining part of b5cc003253c8 (add -i: ignore\nterminal escape sequences, 2011-05-17).\n\nNote: We do *not* query the terminal capabilities directly. That would\neither require a lot of platform-specific code, or it would require\nlinking to a library such as ncurses.\n\nLinking to a library in the built-ins is something we try very hard to\navoid (we even kicked the libcurl dependency to a non-built-in remote\nhelper, just to shave off a tiny fraction of a second from Git's startup\ntime). And the platform-specific code would be a maintenance nightmare.\n\nEven worse: in Git for Windows' case, we would need to query MSYS2\npseudo terminals, which `git.exe` simply cannot do (because it is\nintentionally *not* an MSYS2 program).\n\nTo address this, we simply spawn `infocmp -L -1` and parse its output\n(which works even in Git for Windows, because that helper is included in\nthe end-user facing installations).\n\nThis is done only once, as in the Perl version, but it is done only when\nthe first Escape sequence is encountered, not upon startup of `git add\n-i`; This saves on startup time, yet makes reacting to the first Escape\nsequence slightly more sluggish. But it allows us to keep the\nterminal-related code encapsulated in the `compat/terminal.c` file.\n\nSigned-off-by: Johannes Schindelin \n---\n compat/terminal.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++-\n 1 file changed, 72 insertions(+), 1 deletion(-)\n\ndiff --git a/compat/terminal.c b/compat/terminal.c\nindex b7f58d1781..35bca03d14 100644\n--- a/compat/terminal.c\n+++ b/compat/terminal.c\n@@ -4,6 +4,7 @@\n #include \"strbuf.h\"\n #include \"run-command.h\"\n #include \"string-list.h\"\n+#include \"hashmap.h\"\n \n #if defined(HAVE_DEV_TTY) || defined(GIT_WINDOWS_NATIVE)\n \n@@ -238,6 +239,71 @@ char *git_terminal_prompt(const char *prompt, int echo)\n \treturn buf.buf;\n }\n \n+/*\n+ * The `is_known_escape_sequence()` function returns 1 if the passed string\n+ * corresponds to an Escape sequence that the terminal capabilities contains.\n+ *\n+ * To avoid depending on ncurses or other platform-specific libraries, we rely\n+ * on the presence of the `infocmp` executable to do the job for us (failing\n+ * silently if the program is not available or refused to run).\n+ */\n+struct escape_sequence_entry {\n+\tstruct hashmap_entry entry;\n+\tchar sequence[FLEX_ARRAY];\n+};\n+\n+static int sequence_entry_cmp(const void *hashmap_cmp_fn_data,\n+\t\t\t const struct escape_sequence_entry *e1,\n+\t\t\t const struct escape_sequence_entry *e2,\n+\t\t\t const void *keydata)\n+{\n+\treturn strcmp(e1->sequence, keydata ? keydata : e2->sequence);\n+}\n+\n+static int is_known_escape_sequence(const char *sequence)\n+{\n+\tstatic struct hashmap sequences;\n+\tstatic int initialized;\n+\n+\tif (!initialized) {\n+\t\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\t\tstruct strbuf buf = STRBUF_INIT;\n+\t\tchar *p, *eol;\n+\n+\t\thashmap_init(&sequences, (hashmap_cmp_fn)sequence_entry_cmp,\n+\t\t\t NULL, 0);\n+\n+\t\targv_array_pushl(&cp.args, \"infocmp\", \"-L\", \"-1\", NULL);\n+\t\tif (pipe_command(&cp, NULL, 0, &buf, 0, NULL, 0))\n+\t\t\tstrbuf_setlen(&buf, 0);\n+\n+\t\tfor (eol = p = buf.buf; *p; p = eol + 1) {\n+\t\t\tp = strchr(p, '=');\n+\t\t\tif (!p)\n+\t\t\t\tbreak;\n+\t\t\tp++;\n+\t\t\teol = strchrnul(p, '\\n');\n+\n+\t\t\tif (starts_with(p, \"\\\\E\")) {\n+\t\t\t\tchar *comma = memchr(p, ',', eol - p);\n+\t\t\t\tstruct escape_sequence_entry *e;\n+\n+\t\t\t\tp[0] = '^';\n+\t\t\t\tp[1] = '[';\n+\t\t\t\tFLEX_ALLOC_MEM(e, sequence, p, comma - p);\n+\t\t\t\thashmap_entry_init(&e->entry,\n+\t\t\t\t\t\t strhash(e->sequence));\n+\t\t\t\thashmap_add(&sequences, &e->entry);\n+\t\t\t}\n+\t\t\tif (!*eol)\n+\t\t\t\tbreak;\n+\t\t}\n+\t\tinitialized = 1;\n+\t}\n+\n+\treturn !!hashmap_get_from_hash(&sequences, strhash(sequence), sequence);\n+}\n+\n int read_key_without_echo(struct strbuf *buf)\n {\n \tstatic int warning_displayed;\n@@ -271,7 +337,12 @@ int read_key_without_echo(struct strbuf *buf)\n \t\t * Start by replacing the Escape byte with ^[ */\n \t\tstrbuf_splice(buf, buf->len - 1, 1, \"^[\", 2);\n \n-\t\tfor (;;) {\n+\t\t/*\n+\t\t * Query the terminal capabilities once about all the Escape\n+\t\t * sequences it knows about, so that we can avoid waiting for\n+\t\t * half a second when we know that the sequence is complete.\n+\t\t */\n+\t\twhile (!is_known_escape_sequence(buf->buf)) {\n \t\t\tstruct pollfd pfd = { .fd = 0, .events = POLLIN };\n \n \t\t\tif (poll(&pfd, 1, 500) < 1)\n-- \ngitgitgadget\n\n"},{"id":"389661","messageId":"9ab381d539584d4ddb854e6a7054cab90b5fdf16.1578904171.git.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.v3.git.1578904171.gitgitgadget@gmail.com","subject":"[PATCH v3 08/10] built-in add -p: handle Escape sequences in interactive.singlekey mode","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-01-13T08:29:29Z","receivedAt":"2020-01-13T08:29:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThis recapitulates part of b5cc003253c8 (add -i: ignore terminal escape\nsequences, 2011-05-17):\n\n add -i: ignore terminal escape sequences\n\n On the author's terminal, the up-arrow input sequence is ^[[A, and\n thus fat-fingering an up-arrow into 'git checkout -p' is quite\n dangerous: git-add--interactive.perl will ignore the ^[ and [\n characters and happily treat A as \"discard everything\".\n\n As a band-aid fix, use Term::Cap to get all terminal capabilities.\n Then use the heuristic that any capability value that starts with ^[\n (i.e., \\e in perl) must be a key input sequence. Finally, given an\n input that starts with ^[, read more characters until we have read a\n full escape sequence, then return that to the caller. We use a\n timeout of 0.5 seconds on the subsequent reads to avoid getting stuck\n if the user actually input a lone ^[.\n\n Since none of the currently recognized keys start with ^[, the net\n result is that the sequence as a whole will be ignored and the help\n displayed.\n\nNote that we leave part for later which uses \"Term::Cap to get all\nterminal capabilities\", for several reasons:\n\n1. it is actually not really necessary, as the timeout of 0.5 seconds\n should be plenty sufficient to catch Escape sequences,\n\n2. it is cleaner to keep the change to special-case Escape sequences\n separate from the change that reads all terminal capabilities to\n speed things up, and\n\n3. in practice, relying on the terminal capabilities is a bit overrated,\n as the information could be incomplete, or plain wrong. For example,\n in this developer's tmux sessions, the terminal capabilities claim\n that the \"cursor up\" sequence is ^[M, but the actual sequence\n produced by the \"cursor up\" key is ^[[A.\n\nSigned-off-by: Johannes Schindelin \n---\n compat/terminal.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++-\n 1 file changed, 55 insertions(+), 1 deletion(-)\n\ndiff --git a/compat/terminal.c b/compat/terminal.c\nindex 1b2564042a..b7f58d1781 100644\n--- a/compat/terminal.c\n+++ b/compat/terminal.c\n@@ -161,6 +161,37 @@ static int enable_non_canonical(void)\n \treturn disable_bits(ENABLE_ECHO_INPUT | ENABLE_LINE_INPUT | ENABLE_PROCESSED_INPUT);\n }\n \n+/*\n+ * Override `getchar()`, as the default implementation does not use\n+ * `ReadFile()`.\n+ *\n+ * This poses a problem when we want to see whether the standard\n+ * input has more characters, as the default of Git for Windows is to start the\n+ * Bash in a MinTTY, which uses a named pipe to emulate a pty, in which case\n+ * our `poll()` emulation calls `PeekNamedPipe()`, which seems to require\n+ * `ReadFile()` to be called first to work properly (it only reports 0\n+ * available bytes, otherwise).\n+ *\n+ * So let's just override `getchar()` with a version backed by `ReadFile()` and\n+ * go our merry ways from here.\n+ */\n+static int mingw_getchar(void)\n+{\n+\tDWORD read = 0;\n+\tunsigned char ch;\n+\n+\tif (!ReadFile(GetStdHandle(STD_INPUT_HANDLE), &ch, 1, &read, NULL))\n+\t\treturn EOF;\n+\n+\tif (!read) {\n+\t\terror(\"Unexpected 0 read\");\n+\t\treturn EOF;\n+\t}\n+\n+\treturn ch;\n+}\n+#define getchar mingw_getchar\n+\n #endif\n \n #ifndef FORCE_TEXT\n@@ -228,8 +259,31 @@ int read_key_without_echo(struct strbuf *buf)\n \t\trestore_term();\n \t\treturn EOF;\n \t}\n-\n \tstrbuf_addch(buf, ch);\n+\n+\tif (ch == '\\033' /* ESC */) {\n+\t\t/*\n+\t\t * We are most likely looking at an Escape sequence. Let's try\n+\t\t * to read more bytes, waiting at most half a second, assuming\n+\t\t * that the sequence is complete if we did not receive any byte\n+\t\t * within that time.\n+\t\t *\n+\t\t * Start by replacing the Escape byte with ^[ */\n+\t\tstrbuf_splice(buf, buf->len - 1, 1, \"^[\", 2);\n+\n+\t\tfor (;;) {\n+\t\t\tstruct pollfd pfd = { .fd = 0, .events = POLLIN };\n+\n+\t\t\tif (poll(&pfd, 1, 500) < 1)\n+\t\t\t\tbreak;\n+\n+\t\t\tch = getchar();\n+\t\t\tif (ch == EOF)\n+\t\t\t\treturn 0;\n+\t\t\tstrbuf_addch(buf, ch);\n+\t\t}\n+\t}\n+\n \trestore_term();\n \treturn 0;\n }\n-- \ngitgitgadget\n\n"},{"id":"389717","messageId":"20200113170417.GK32750@szeder.dev","threadId":"52505","inReplyTo":"5e258a8d2bb271433902b2e44c3a30a988bbf512.1578904171.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 01/10] built-in add -i/-p: treat SIGPIPE as EOF","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2020-01-13T17:04:17Z","receivedAt":"2020-01-13T17:04:25Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Mon, Jan 13, 2020 at 08:29:22AM +0000, Johannes Schindelin via GitGitGadget wrote:\n> From: Johannes Schindelin \n> \n> As noticed by Gábor Szeder, if we want to run `git add -p` with\n> redirected input through `test_must_fail` in the test suite, we must\n> expect that a SIGPIPE can happen due to `stdin` coming to its end.\n\nI don't think this issue is related to the redirected input: I\nmodified that flaky test to send \"unlimited\" data to 'git add's stdin,\ni.e.:\n\n /usr/bin/yes | test_must_fail force_color git add -p\n\nand the test with --stress still failed with SIGPIPE all the same and\njust as fast.\n\nAfter looking into it, the issue seems to be sending data to the\nbroken diffFilter process. So in that test the diff is \"filtered\"\nthrough 'echo too-short', which exits real fast, and doesn't read its\nstandard input at all (well, apart from e.g. the usual kernel\nbuffering that might happen on a pipe between the two processes).\nMaking sure that the diffFilter process reads all the data before\nexiting, i.e. changing it to:\n\n test_config interactive.diffFilter \"cat >/dev/null ; echo too-short\" &&\n\nmade the test reliable, with over 2000 --stress repetitions, and that\nwith only a single \"y\" on 'git add's stdin.\n\nNow, merely tweaking the test is clearly insufficient, because we not\nonly want the test to be realiable, but we want 'git add' to die\ngracefully when users out there mess up their configuration.\n\nIgnoring SIGPIPE can surely accomplish that, but I'm not sure about\nthe scope. I mean your patch seems to ignore SIGPIPE basically for\nalmost the whole 'git add -(i|p)' process, but perhaps it should be\nlimited only to the surroundings of the pipe_command() call running\nthe diffFilter, and be done as part of the next patch adding the 'if\n(diff_filter)' block.\n\nFurthermore, I'm worried that by simply ignoring SIGPIPE we might just\nignore a more fundamental issue in pipe_command(): shouldn't that\nfunction be smart enough not to write() to a fd that has no one on the\nother side to read it in the first place?!\n\nSo, when the diffFilter process exits unexpectedly early, then the\npoll() call in pipe_command() -> pump_io() -> pump_io_round() returns\nwith success and usually sets 'revents' for the child process' stdin\nto 12 (i.e. 'POLLOUT | POLLERR'; gah, how I hate unnamed constants :).\nUnfortunately, at that point we don't take any special action on\nPOLLERR, but call xwrite() to try to write to the dead fd anyway,\nwhich then promptly triggers SIGPIPE. (This is what usually happens\nwhen stepping through the statements of those functions in a debugger,\nand the diffFilter process has all the time in the world to exit.)\n\nWe could handle POLLERR with a patch like this:\n\n --- >8 ---\n\nSubject: run-command: handle POLLERR in pump_io_round() to reduce risk of SIGPIPE\n\ndiff --git a/run-command.c b/run-command.c\nindex 3449db319b..57093f0acc 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -1416,25 +1416,31 @@ static int pump_io_round(struct io_pump *slots, int nr, struct pollfd *pfd)\n \tif (poll(pfd, pollsize, -1) < 0) {\n \t\tif (errno == EINTR)\n \t\t\treturn 1;\n \t\tdie_errno(\"poll failed\");\n \t}\n \n \tfor (i = 0; i < nr; i++) {\n \t\tstruct io_pump *io = &slots[i];\n \n \t\tif (io->fd < 0)\n \t\t\tcontinue;\n \n-\t\tif (!(io->pfd->revents & (POLLOUT|POLLIN|POLLHUP|POLLERR|POLLNVAL)))\n+\t\tif (io->pfd->revents & POLLERR) {\n+\t\t\tio->error = ECONNRESET; /* What should we report to the caller? */\n+\t\t\tclose(io->fd);\n+\t\t\tio->fd = -1;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (!(io->pfd->revents & (POLLOUT|POLLIN|POLLHUP|POLLNVAL)))\n \t\t\tcontinue;\n \n \t\tif (io->type == POLLOUT) {\n \t\t\tssize_t len = xwrite(io->fd,\n \t\t\t\t\t io->u.out.buf, io->u.out.len);\n \t\t\tif (len < 0) {\n \t\t\t\tio->error = errno;\n \t\t\t\tclose(io->fd);\n \t\t\t\tio->fd = -1;\n \t\t\t} else {\n \t\t\t\tio->u.out.buf += len;\n \t\t\t\tio->u.out.len -= len;\n\n --- >8 ---\n\nUnfortunately #1, this changes the error 'git add -p' dies with from:\n\n error: mismatched output from interactive.diffFilter\n\nto:\n\n error: failed to run 'echo too-short'\n\nIt might affect other commands as well, but FWIW the test suite\ndoesn't catch any.\n\n\nUnfortunately #2, the above patch doesn't completely eliminates the\nSIGPIPE, but only (greatly) reduces its probability. It is possible\nthat:\n\n - poll() returns with success and indicating a writable fd without\n any error, i.e. 'revents = 4'.\n\n - the bogus diffFilter exits, closing its stdin.\n\n - 'git add' attempts to xwrite() to the now closed fd, and triggers\n a SIGPIPE right away.\n\nThis happens much rarer, 'GIT_TEST_ADD_I_USE_BUILTIN=1\n./t3701-add-interactive.sh -r 39,49 --stress-jobs=<4*nr-of-cores>\n--stress' tends to take over 200 repetitions. The patch below\nreproduces it fairly reliably by adding two strategically-placed\nsleep()s, with a bit of extra debug output:\n\n --- >8 ---\n\ndiff --git a/add-patch.c b/add-patch.c\nindex d8dafa8168..0fd017bbd3 100644\n--- a/add-patch.c\n+++ b/add-patch.c\n@@ -421,6 +421,7 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n \t\t\tfilter_cp.git_cmd = 0;\n \t\t\tfilter_cp.use_shell = 1;\n \t\t\tstrbuf_reset(&s->buf);\n+\t\t\tfprintf(stderr, \"about to run diffFilter\\n\");\n \t\t\tif (pipe_command(&filter_cp,\n \t\t\t\t\t colored->buf, colored->len,\n \t\t\t\t\t &s->buf, colored->len,\ndiff --git a/run-command.c b/run-command.c\nindex 57093f0acc..49ae88a922 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -1419,6 +1419,7 @@ static int pump_io_round(struct io_pump *slots, int nr, struct pollfd *pfd)\n \t\tdie_errno(\"poll failed\");\n \t}\n \n+\tsleep(2);\n \tfor (i = 0; i < nr; i++) {\n \t\tstruct io_pump *io = &slots[i];\n \n@@ -1435,8 +1436,11 @@ static int pump_io_round(struct io_pump *slots, int nr, struct pollfd *pfd)\n \t\t\tcontinue;\n \n \t\tif (io->type == POLLOUT) {\n-\t\t\tssize_t len = xwrite(io->fd,\n+\t\t\tssize_t len;\n+\t\t\tfprintf(stderr, \"attempting to xwrite() %lu bytes to a fd with revents flags 0x%hx\\n\", io->u.out.len, io->pfd->revents);\n+\t\t\tlen = xwrite(io->fd,\n \t\t\t\t\t io->u.out.buf, io->u.out.len);\n+\t\t\tfprintf(stderr, \"after xwrite()\\n\");\n \t\t\tif (len < 0) {\n \t\t\t\tio->error = errno;\n \t\t\t\tclose(io->fd);\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 12ee321707..acffc9af37 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -561,7 +561,7 @@ test_expect_success 'detect bogus diffFilter output' '\n \tgit reset --hard &&\n \n \techo content >test &&\n-\ttest_config interactive.diffFilter \"echo too-short\" &&\n+\ttest_config interactive.diffFilter \"sleep 1 ; echo too-short\" &&\n \tprintf y >y &&\n \ttest_must_fail force_color git add -p 8 ---\n\nand 'GIT_TEST_ADD_I_USE_BUILTIN=1 ./t3701-add-interactive.sh -r 39,49'\nfails with:\n\n + test_must_fail force_color git add -p\n about to run diffFilter\n attempting to xwrite() 224 bytes to a fd with revents flags 0x4\n test_must_fail: died by signal 13: force_color git add -p\n\nI don't understand why we get SIGPIPE right away instead of some error\nthat we can act upon (ECONNRESET?). FWIW, it fails the same way not\nonly on my box, but on Travis CI's Linux and OSX images as well.\n\n https://travis-ci.org/szeder/git/jobs/636446843#L2937\n\n\nCc'ing Peff for all things SIGPIPE :) who also happens to be the\nauthor of both pipe_command() and that now flaky test.\n\n\n> The appropriate action here is to ignore that signal and treat it as a\n> regular end-of-file, otherwise the test will fail. In preparation for\n> such a test, introduce precisely this handling of SIGPIPE into the\n> built-in version of `git add -p`.\n> \n> For good measure, teach the built-in `git add -i` the same trick: it\n> _also_ runs a loop waiting for input, and can receive a SIGPIPE just the\n> same (and wants to treat it as end-of-file, too).\n> \n> Signed-off-by: Johannes Schindelin \n> ---\n> add-interactive.c | 3 +++\n> add-patch.c | 4 ++++\n> 2 files changed, 7 insertions(+)\n> \n> diff --git a/add-interactive.c b/add-interactive.c\n> index a5bb14f2f4..3ff8400ea4 100644\n> --- a/add-interactive.c\n> +++ b/add-interactive.c\n> @@ -9,6 +9,7 @@\n> #include \"lockfile.h\"\n> #include \"dir.h\"\n> #include \"run-command.h\"\n> +#include \"sigchain.h\"\n> \n> static void init_color(struct repository *r, struct add_i_state *s,\n> \t\t const char *slot_name, char *dst,\n> @@ -1097,6 +1098,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps)\n> \t\t\t->util = util;\n> \t}\n> \n> +\tsigchain_push(SIGPIPE, SIG_IGN);\n> \tinit_add_i_state(&s, r);\n> \n> \t/*\n> @@ -1149,6 +1151,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps)\n> \tstrbuf_release(&print_file_item_data.worktree);\n> \tstrbuf_release(&header);\n> \tprefix_item_list_clear(&commands);\n> +\tsigchain_pop(SIGPIPE);\n> \n> \treturn res;\n> }\n> diff --git a/add-patch.c b/add-patch.c\n> index 46c6c183d5..9a3beed72e 100644\n> --- a/add-patch.c\n> +++ b/add-patch.c\n> @@ -6,6 +6,7 @@\n> #include \"pathspec.h\"\n> #include \"color.h\"\n> #include \"diff.h\"\n> +#include \"sigchain.h\"\n> \n> enum prompt_mode_type {\n> \tPROMPT_MODE_CHANGE = 0, PROMPT_DELETION, PROMPT_HUNK,\n> @@ -1578,6 +1579,7 @@ int run_add_p(struct repository *r, enum add_p_mode mode,\n> \t};\n> \tsize_t i, binary_count = 0;\n> \n> +\tsigchain_push(SIGPIPE, SIG_IGN);\n> \tinit_add_i_state(&s.s, r);\n> \n> \tif (mode == ADD_P_STASH)\n> @@ -1612,6 +1614,7 @@ int run_add_p(struct repository *r, enum add_p_mode mode,\n> \t parse_diff(&s, ps) < 0) {\n> \t\tstrbuf_release(&s.plain);\n> \t\tstrbuf_release(&s.colored);\n> +\t\tsigchain_pop(SIGPIPE);\n> \t\treturn -1;\n> \t}\n> \n> @@ -1630,5 +1633,6 @@ int run_add_p(struct repository *r, enum add_p_mode mode,\n> \tstrbuf_release(&s.buf);\n> \tstrbuf_release(&s.plain);\n> \tstrbuf_release(&s.colored);\n> +\tsigchain_pop(SIGPIPE);\n> \treturn 0;\n> }\n> -- \n> gitgitgadget\n> \n"},{"id":"389721","messageId":"20200113183313.GA2087@coredump.intra.peff.net","threadId":"52505","inReplyTo":"20200113170417.GK32750@szeder.dev","subject":"Re: [PATCH v3 01/10] built-in add -i/-p: treat SIGPIPE as EOF","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-01-13T18:33:13Z","receivedAt":"2020-01-13T18:33:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jan 13, 2020 at 06:04:17PM +0100, SZEDER Gábor wrote:\n\n> After looking into it, the issue seems to be sending data to the\n> broken diffFilter process. So in that test the diff is \"filtered\"\n> through 'echo too-short', which exits real fast, and doesn't read its\n> standard input at all (well, apart from e.g. the usual kernel\n> buffering that might happen on a pipe between the two processes).\n> Making sure that the diffFilter process reads all the data before\n> exiting, i.e. changing it to:\n> \n> test_config interactive.diffFilter \"cat >/dev/null ; echo too-short\" &&\n> \n> made the test reliable, with over 2000 --stress repetitions, and that\n> with only a single \"y\" on 'git add's stdin.\n\nYeah, I agree the test should be changed. What you wrote above was my\nfirst thought, too, but I think \"sed 1d\" is actually a more realistic\ntest (and is shorter and one fewer process).\n\n> Now, merely tweaking the test is clearly insufficient, because we not\n> only want the test to be realiable, but we want 'git add' to die\n> gracefully when users out there mess up their configuration.\n\nI also agree that it would be nice to deal with this for real-world\ncases. I suspect it's not something that would come up a lot, though.\n\n> Ignoring SIGPIPE can surely accomplish that, but I'm not sure about\n> the scope. I mean your patch seems to ignore SIGPIPE basically for\n> almost the whole 'git add -(i|p)' process, but perhaps it should be\n> limited only to the surroundings of the pipe_command() call running\n> the diffFilter, and be done as part of the next patch adding the 'if\n> (diff_filter)' block.\n\nThe scope there is probably OK in practice. In my opinion SIGPIPE is\nusually _not_ what the behavior we want. If we're carefully checking our\nwrite() return values, then we'd get EPIPE in such an instance and\nbehave appropriately. And if we're not checking our write() return\nvalues, that's generally a bug that ought to be fixed.\n\nThe big exception is when we are writing copious output to stdout (or\nthe pager) via printf() or similar, and want to die rather than continue\nwriting output nobody will see. But I don't think git-add really counts\nas generating a lot of output, where EPIPE could prevent us from doing\nuseless work (unlike, say, git-log).\n\n> Furthermore, I'm worried that by simply ignoring SIGPIPE we might just\n> ignore a more fundamental issue in pipe_command(): shouldn't that\n> function be smart enough not to write() to a fd that has no one on the\n> other side to read it in the first place?!\n\nMaybe. As you noted below, checking for POLLERR is racy. Seeing that we\n\"can\" write to an fd and doing it to discover what write() returns\n(whether error or not) doesn't seem like the worst strategy. If the\ncaller cares about pipe death, then it needs to be handling SIGPIPE\nanyway.\n\nI really wish there was a way to set a handler for SIGPIPE that tells\n_which_ descriptor caused it. Because I think logic like \"die if it was\nfd 1, ignore and let write() return EPIPE otherwise\" is the behavior\nwe'd like. But I don't think there's a portable way to do so.\n\nI've been tempted to say that we should just ignore SIGPIPE everywhere,\nand convert even copious-output programs like git-log to just check for\nerrors (they could probably even just check ferror(stdout) for each\ncommit we output, if we didn't want to touch every printf call).\n\n-Peff\n"},{"id":"389742","messageId":"nycvar.QRO.7.76.6.2001141301310.46@tvgsbejvaqbjf.bet","threadId":"52505","inReplyTo":"20200113170417.GK32750@szeder.dev","subject":"Re: [PATCH v3 01/10] built-in add -i/-p: treat SIGPIPE as EOF","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-01-14T12:47:43Z","receivedAt":"2020-01-14T12:47:57Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Gábor,\n\nOn Mon, 13 Jan 2020, SZEDER Gábor wrote:\n\n> On Mon, Jan 13, 2020 at 08:29:22AM +0000, Johannes Schindelin via GitGitGadget wrote:\n> > From: Johannes Schindelin \n> >\n> > As noticed by Gábor Szeder, if we want to run `git add -p` with\n> > redirected input through `test_must_fail` in the test suite, we must\n> > expect that a SIGPIPE can happen due to `stdin` coming to its end.\n>\n> I don't think this issue is related to the redirected input: I\n> modified that flaky test to send \"unlimited\" data to 'git add's stdin,\n> i.e.:\n>\n> /usr/bin/yes | test_must_fail force_color git add -p\n>\n> and the test with --stress still failed with SIGPIPE all the same and\n> just as fast.\n>\n> After looking into it, the issue seems to be sending data to the\n> broken diffFilter process.\n\nOuch. Thank you for investigating. For my education, how did you debug\nthis? I could not find a way to identify *what* caused that SIGPIPE...\n\n> So in that test the diff is \"filtered\" through 'echo too-short', which\n> exits real fast, and doesn't read its standard input at all (well, apart\n> from e.g. the usual kernel buffering that might happen on a pipe between\n> the two processes). Making sure that the diffFilter process reads all\n> the data before exiting, i.e. changing it to:\n>\n> test_config interactive.diffFilter \"cat >/dev/null ; echo too-short\" &&\n>\n> made the test reliable, with over 2000 --stress repetitions, and that\n> with only a single \"y\" on 'git add's stdin.\n\nAh, my diff filter simply ignores the `stdin`... That's easy enough to\nfix, and since real-world diff filters probably won't just blatantly\nignore the input, I think it is legitimate to change the test.\n\n> Now, merely tweaking the test is clearly insufficient, because we not\n> only want the test to be realiable, but we want 'git add' to die\n> gracefully when users out there mess up their configuration.\n\nI think it is sufficient to tweak the test, but I agree that a better\nerror message might be good when users out there mess up their\nconfiguration.\n\n> Ignoring SIGPIPE can surely accomplish that, but I'm not sure about\n> the scope. I mean your patch seems to ignore SIGPIPE basically for\n> almost the whole 'git add -(i|p)' process, but perhaps it should be\n> limited only to the surroundings of the pipe_command() call running\n> the diffFilter, and be done as part of the next patch adding the 'if\n> (diff_filter)' block.\n\nRight. Very heavy-handed, and probably inviting unwanted side effects.\n\n> Furthermore, I'm worried that by simply ignoring SIGPIPE we might just\n> ignore a more fundamental issue in pipe_command(): shouldn't that\n> function be smart enough not to write() to a fd that has no one on the\n> other side to read it in the first place?!\n>\n> So, when the diffFilter process exits unexpectedly early, then the\n> poll() call in pipe_command() -> pump_io() -> pump_io_round() returns\n> with success and usually sets 'revents' for the child process' stdin\n> to 12 (i.e. 'POLLOUT | POLLERR'; gah, how I hate unnamed constants :).\n> Unfortunately, at that point we don't take any special action on\n> POLLERR, but call xwrite() to try to write to the dead fd anyway,\n> which then promptly triggers SIGPIPE. (This is what usually happens\n> when stepping through the statements of those functions in a debugger,\n> and the diffFilter process has all the time in the world to exit.)\n>\n> We could handle POLLERR with a patch like this:\n>\n> --- >8 ---\n>\n> Subject: run-command: handle POLLERR in pump_io_round() to reduce risk of SIGPIPE\n>\n> diff --git a/run-command.c b/run-command.c\n> index 3449db319b..57093f0acc 100644\n> --- a/run-command.c\n> +++ b/run-command.c\n> @@ -1416,25 +1416,31 @@ static int pump_io_round(struct io_pump *slots, int nr, struct pollfd *pfd)\n> \tif (poll(pfd, pollsize, -1) < 0) {\n> \t\tif (errno == EINTR)\n> \t\t\treturn 1;\n> \t\tdie_errno(\"poll failed\");\n> \t}\n>\n> \tfor (i = 0; i < nr; i++) {\n> \t\tstruct io_pump *io = &slots[i];\n>\n> \t\tif (io->fd < 0)\n> \t\t\tcontinue;\n>\n> -\t\tif (!(io->pfd->revents & (POLLOUT|POLLIN|POLLHUP|POLLERR|POLLNVAL)))\n> +\t\tif (io->pfd->revents & POLLERR) {\n> +\t\t\tio->error = ECONNRESET; /* What should we report to the caller? */\n> +\t\t\tclose(io->fd);\n> +\t\t\tio->fd = -1;\n> +\t\t\tcontinue;\n> +\t\t}\n> +\t\tif (!(io->pfd->revents & (POLLOUT|POLLIN|POLLHUP|POLLNVAL)))\n> \t\t\tcontinue;\n>\n> \t\tif (io->type == POLLOUT) {\n> \t\t\tssize_t len = xwrite(io->fd,\n> \t\t\t\t\t io->u.out.buf, io->u.out.len);\n> \t\t\tif (len < 0) {\n> \t\t\t\tio->error = errno;\n> \t\t\t\tclose(io->fd);\n> \t\t\t\tio->fd = -1;\n> \t\t\t} else {\n> \t\t\t\tio->u.out.buf += len;\n> \t\t\t\tio->u.out.len -= len;\n>\n> --- >8 ---\n>\n> Unfortunately #1, this changes the error 'git add -p' dies with from:\n>\n> error: mismatched output from interactive.diffFilter\n>\n> to:\n>\n> error: failed to run 'echo too-short'\n>\n> It might affect other commands as well, but FWIW the test suite\n> doesn't catch any.\n\nHmm. My first impression is that the error message could be a bit better,\nbut that it is probably a good thing to have. It would have helped _me_\nunderstand the issue at hand.\n\n> Unfortunately #2, the above patch doesn't completely eliminates the\n> SIGPIPE, but only (greatly) reduces its probability. It is possible\n> that:\n>\n> - poll() returns with success and indicating a writable fd without\n> any error, i.e. 'revents = 4'.\n>\n> - the bogus diffFilter exits, closing its stdin.\n>\n> - 'git add' attempts to xwrite() to the now closed fd, and triggers\n> a SIGPIPE right away.\n>\n> This happens much rarer, 'GIT_TEST_ADD_I_USE_BUILTIN=1\n> ./t3701-add-interactive.sh -r 39,49 --stress-jobs=<4*nr-of-cores>\n> --stress' tends to take over 200 repetitions. The patch below\n> reproduces it fairly reliably by adding two strategically-placed\n> sleep()s, with a bit of extra debug output:\n>\n> --- >8 ---\n>\n> diff --git a/add-patch.c b/add-patch.c\n> index d8dafa8168..0fd017bbd3 100644\n> --- a/add-patch.c\n> +++ b/add-patch.c\n> @@ -421,6 +421,7 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n> \t\t\tfilter_cp.git_cmd = 0;\n> \t\t\tfilter_cp.use_shell = 1;\n> \t\t\tstrbuf_reset(&s->buf);\n> +\t\t\tfprintf(stderr, \"about to run diffFilter\\n\");\n> \t\t\tif (pipe_command(&filter_cp,\n> \t\t\t\t\t colored->buf, colored->len,\n> \t\t\t\t\t &s->buf, colored->len,\n> diff --git a/run-command.c b/run-command.c\n> index 57093f0acc..49ae88a922 100644\n> --- a/run-command.c\n> +++ b/run-command.c\n> @@ -1419,6 +1419,7 @@ static int pump_io_round(struct io_pump *slots, int nr, struct pollfd *pfd)\n> \t\tdie_errno(\"poll failed\");\n> \t}\n>\n> +\tsleep(2);\n> \tfor (i = 0; i < nr; i++) {\n> \t\tstruct io_pump *io = &slots[i];\n>\n> @@ -1435,8 +1436,11 @@ static int pump_io_round(struct io_pump *slots, int nr, struct pollfd *pfd)\n> \t\t\tcontinue;\n>\n> \t\tif (io->type == POLLOUT) {\n> -\t\t\tssize_t len = xwrite(io->fd,\n> +\t\t\tssize_t len;\n> +\t\t\tfprintf(stderr, \"attempting to xwrite() %lu bytes to a fd with revents flags 0x%hx\\n\", io->u.out.len, io->pfd->revents);\n> +\t\t\tlen = xwrite(io->fd,\n> \t\t\t\t\t io->u.out.buf, io->u.out.len);\n> +\t\t\tfprintf(stderr, \"after xwrite()\\n\");\n> \t\t\tif (len < 0) {\n> \t\t\t\tio->error = errno;\n> \t\t\t\tclose(io->fd);\n> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\n> index 12ee321707..acffc9af37 100755\n> --- a/t/t3701-add-interactive.sh\n> +++ b/t/t3701-add-interactive.sh\n> @@ -561,7 +561,7 @@ test_expect_success 'detect bogus diffFilter output' '\n> \tgit reset --hard &&\n>\n> \techo content >test &&\n> -\ttest_config interactive.diffFilter \"echo too-short\" &&\n> +\ttest_config interactive.diffFilter \"sleep 1 ; echo too-short\" &&\n> \tprintf y >y &&\n> \ttest_must_fail force_color git add -p '\n>\n> --- >8 ---\n>\n> and 'GIT_TEST_ADD_I_USE_BUILTIN=1 ./t3701-add-interactive.sh -r 39,49'\n> fails with:\n>\n> + test_must_fail force_color git add -p\n> about to run diffFilter\n> attempting to xwrite() 224 bytes to a fd with revents flags 0x4\n> test_must_fail: died by signal 13: force_color git add -p\n>\n> I don't understand why we get SIGPIPE right away instead of some error\n> that we can act upon (ECONNRESET?).\n\nIsn't it buffered?\n\nIn any case, I would take the above-mentioned patch, even if it makes it\n\"only\" less likely to hit `SIGPIPE`.\n\n> FWIW, it fails the same way not only on my box, but on Travis CI's Linux\n> and OSX images as well.\n>\n> https://travis-ci.org/szeder/git/jobs/636446843#L2937\n>\n>\n> Cc'ing Peff for all things SIGPIPE :) who also happens to be the\n> author of both pipe_command() and that now flaky test.\n\nI'll go with Peff's suggestion to use `sed 1d` instead of `echo\ntoo-short`.\n\nThanks,\nDscho\n"},{"id":"389751","messageId":"pull.175.v4.git.1579027433.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.v3.git.1578904171.gitgitgadget@gmail.com","subject":"[PATCH v4 00/10] built-in add -p: add support for the same config settings as the Perl version","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-01-14T18:43:43Z","receivedAt":"2020-01-14T18:44:00Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"This is the final leg of the journey to a fully built-in git add: the git\nadd -i and git add -p modes were re-implemented in C, but they lacked\nsupport for a couple of config settings.\n\nThe one that sticks out most is the interactive.singleKey setting: it was\nparticularly hard to get to work, especially on Windows.\n\nIt also seems to be the setting that is incomplete already in the Perl\nversion of the interactive add command: while the name of the config setting\nsuggests that it applies to all of the interactive add, including the main\nloop of git add --interactive and to the file selections in that command, it\ndoes not. Only the git add --patch mode respects that setting.\n\nAs it is outside the purpose of the conversion of git-add--interactive.perl \nto C, we will leave that loose end for some future date.\n\nChanges since v3:\n\n * Reverted that heavy-handed SIGPIPE handling.\n * Instead, changed the diffFilter test case to process the standard input\n instead of ignoring it.\n\n(The range diff between v2 and v4 actually only shows the new patch 1/10\n\"t3701: adjust difffilter test\".)\n\nChanges since v2:\n\n * Fixed the SIGPIPE issue pointed out by Gábor Szeder.\n\nChanges since v1:\n\n * Fixed the commit message where a copy/paste fail made it talk about\n another GIT_TEST_* variable than the GIT_TEST_ADD_I_USE_BUILTIN one.\n\nJohannes Schindelin (10):\n t3701: adjust difffilter test\n built-in add -p: support interactive.diffFilter\n built-in add -p: handle diff.algorithm\n terminal: make the code of disable_echo() reusable\n terminal: accommodate Git for Windows' default terminal\n terminal: add a new function to read a single keystroke\n built-in add -p: respect the `interactive.singlekey` config setting\n built-in add -p: handle Escape sequences in interactive.singlekey mode\n built-in add -p: handle Escape sequences more efficiently\n ci: include the built-in `git add -i` in the `linux-gcc` job\n\n add-interactive.c | 19 +++\n add-interactive.h | 4 +\n add-patch.c | 57 ++++++++-\n ci/run-build-and-tests.sh | 1 +\n compat/terminal.c | 249 ++++++++++++++++++++++++++++++++++++-\n compat/terminal.h | 3 +\n t/t3701-add-interactive.sh | 2 +-\n 7 files changed, 326 insertions(+), 9 deletions(-)\n\n\nbase-commit: c480eeb574e649a19f27dc09a994e45f9b2c2622\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-175%2Fdscho%2Fadd-p-in-c-config-settings-v4\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-175/dscho/add-p-in-c-config-settings-v4\nPull-Request: https://github.com/gitgitgadget/git/pull/175\n\nRange-diff vs v3:\n\n 1: 5e258a8d2b < -: ---------- built-in add -i/-p: treat SIGPIPE as EOF\n -: ---------- > 1: e12df77e8a t3701: adjust difffilter test\n 2: 2a5951ecfe ! 2: 413a87bd79 built-in add -p: support interactive.diffFilter\n @@ -35,9 +35,9 @@\n \tstrbuf_release(&header);\n \tprefix_item_list_clear(&commands);\n +\tclear_add_i_state(&s);\n - \tsigchain_pop(SIGPIPE);\n \n \treturn res;\n + }\n \n diff --git a/add-interactive.h b/add-interactive.h\n --- a/add-interactive.h\n @@ -123,14 +123,13 @@\n \t\tstrbuf_release(&s.plain);\n \t\tstrbuf_release(&s.colored);\n +\t\tclear_add_i_state(&s.s);\n - \t\tsigchain_pop(SIGPIPE);\n \t\treturn -1;\n \t}\n + \n @@\n \tstrbuf_release(&s.buf);\n \tstrbuf_release(&s.plain);\n \tstrbuf_release(&s.colored);\n +\tclear_add_i_state(&s.s);\n - \tsigchain_pop(SIGPIPE);\n \treturn 0;\n }\n 3: a2bce01818 = 3: 062c624547 built-in add -p: handle diff.algorithm\n 4: be40a37c0c = 4: 09a8946303 terminal: make the code of disable_echo() reusable\n 5: 233f23791c = 5: a81304cb76 terminal: accommodate Git for Windows' default terminal\n 6: 74593b5115 = 6: 8d9c703f3b terminal: add a new function to read a single keystroke\n 7: 197fe1e14a ! 7: 8ed4487ae4 built-in add -p: respect the `interactive.singlekey` config setting\n @@ -48,9 +48,9 @@\n --- a/add-patch.c\n +++ b/add-patch.c\n @@\n + #include \"pathspec.h\"\n #include \"color.h\"\n #include \"diff.h\"\n - #include \"sigchain.h\"\n +#include \"compat/terminal.h\"\n \n enum prompt_mode_type {\n 8: 9ab381d539 = 8: cdc609f8fa built-in add -p: handle Escape sequences in interactive.singlekey mode\n 9: bdb6268b8b = 9: 80b0f2528d built-in add -p: handle Escape sequences more efficiently\n 10: c4195969a6 = 10: 7ab7ec62d0 ci: include the built-in `git add -i` in the `linux-gcc` job\n\n-- \ngitgitgadget\n"},{"id":"389752","messageId":"e12df77e8aa9f6105ab54b41c570df71fd43698d.1579027433.git.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.v4.git.1579027433.gitgitgadget@gmail.com","subject":"[PATCH v4 01/10] t3701: adjust difffilter test","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-01-14T18:43:44Z","receivedAt":"2020-01-14T18:44:00Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIn 42f7d45428e (add--interactive: detect bogus diffFilter output,\n2018-03-03), we added a test case that verifies that the diffFilter\nfeature complains appropriately when the output is too short.\n\nIn preparation for the upcoming change where the built-in `add -p` is\ntaught to respect that setting, let's adjust that test a little. The\nproblem is that `echo too-short` is configured as diffFilter, and it\ndoes not read the `stdin`. When calling it through `pipe_command()`, it\nis therefore possible that we try to feed the `diff` to it while it is\nno longer listening, and we receive a `SIGPIPE`.\n\nThe Perl code apparently handles this in a way similar to an\nend-of-file, but taking a step back, we realize that a diffFilter that\ndoes not even _look_ at its standard input is very unrealistic. The\nentire point of this feature is to transform the diff, not to ignore it\naltogether.\n\nSo let's modify the test case to reflect that insight: instead of\nprinting some bogus text, let's use a diffFilter that deletes the first\nline of the diff instead.\n\nThis still tests for the same thing, but it does not confuse the\nbuilt-in `add -p` with that `SIGPIPE`.\n\nHelped-by: SZEDER Gábor \nHelped-by: Jeff King \nSigned-off-by: Johannes Schindelin \n---\n t/t3701-add-interactive.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 12ee321707..ac43f835a5 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -561,7 +561,7 @@ test_expect_success 'detect bogus diffFilter output' '\n \tgit reset --hard &&\n \n \techo content >test &&\n-\ttest_config interactive.diffFilter \"echo too-short\" &&\n+\ttest_config interactive.diffFilter \"sed 1d\" &&\n \tprintf y >y &&\n \ttest_must_fail force_color git add -p \n\nThe Perl version supports post-processing the colored diff (that is\ngenerated in addition to the uncolored diff, intended to offer a\nprettier user experience) by a command configured via that config\nsetting, and now the built-in version does that, too.\n\nSigned-off-by: Johannes Schindelin \n---\n add-interactive.c | 12 ++++++++++++\n add-interactive.h | 3 +++\n add-patch.c | 33 +++++++++++++++++++++++++++++++++\n 3 files changed, 48 insertions(+)\n\ndiff --git a/add-interactive.c b/add-interactive.c\nindex a5bb14f2f4..1786ea29c4 100644\n--- a/add-interactive.c\n+++ b/add-interactive.c\n@@ -52,6 +52,17 @@ void init_add_i_state(struct add_i_state *s, struct repository *r)\n \t\tdiff_get_color(s->use_color, DIFF_FILE_OLD));\n \tinit_color(r, s, \"new\", s->file_new_color,\n \t\tdiff_get_color(s->use_color, DIFF_FILE_NEW));\n+\n+\tFREE_AND_NULL(s->interactive_diff_filter);\n+\tgit_config_get_string(\"interactive.difffilter\",\n+\t\t\t &s->interactive_diff_filter);\n+}\n+\n+void clear_add_i_state(struct add_i_state *s)\n+{\n+\tFREE_AND_NULL(s->interactive_diff_filter);\n+\tmemset(s, 0, sizeof(*s));\n+\ts->use_color = -1;\n }\n \n /*\n@@ -1149,6 +1160,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps)\n \tstrbuf_release(&print_file_item_data.worktree);\n \tstrbuf_release(&header);\n \tprefix_item_list_clear(&commands);\n+\tclear_add_i_state(&s);\n \n \treturn res;\n }\ndiff --git a/add-interactive.h b/add-interactive.h\nindex b2f23479c5..46c73867ad 100644\n--- a/add-interactive.h\n+++ b/add-interactive.h\n@@ -15,9 +15,12 @@ struct add_i_state {\n \tchar context_color[COLOR_MAXLEN];\n \tchar file_old_color[COLOR_MAXLEN];\n \tchar file_new_color[COLOR_MAXLEN];\n+\n+\tchar *interactive_diff_filter;\n };\n \n void init_add_i_state(struct add_i_state *s, struct repository *r);\n+void clear_add_i_state(struct add_i_state *s);\n \n struct repository;\n struct pathspec;\ndiff --git a/add-patch.c b/add-patch.c\nindex 46c6c183d5..78bde41df0 100644\n--- a/add-patch.c\n+++ b/add-patch.c\n@@ -398,6 +398,7 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n \n \tif (want_color_fd(1, -1)) {\n \t\tstruct child_process colored_cp = CHILD_PROCESS_INIT;\n+\t\tconst char *diff_filter = s->s.interactive_diff_filter;\n \n \t\tsetup_child_process(s, &colored_cp, NULL);\n \t\txsnprintf((char *)args.argv[color_arg_index], 8, \"--color\");\n@@ -407,6 +408,24 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n \t\targv_array_clear(&args);\n \t\tif (res)\n \t\t\treturn error(_(\"could not parse colored diff\"));\n+\n+\t\tif (diff_filter) {\n+\t\t\tstruct child_process filter_cp = CHILD_PROCESS_INIT;\n+\n+\t\t\tsetup_child_process(s, &filter_cp,\n+\t\t\t\t\t diff_filter, NULL);\n+\t\t\tfilter_cp.git_cmd = 0;\n+\t\t\tfilter_cp.use_shell = 1;\n+\t\t\tstrbuf_reset(&s->buf);\n+\t\t\tif (pipe_command(&filter_cp,\n+\t\t\t\t\t colored->buf, colored->len,\n+\t\t\t\t\t &s->buf, colored->len,\n+\t\t\t\t\t NULL, 0) < 0)\n+\t\t\t\treturn error(_(\"failed to run '%s'\"),\n+\t\t\t\t\t diff_filter);\n+\t\t\tstrbuf_swap(colored, &s->buf);\n+\t\t}\n+\n \t\tstrbuf_complete_line(colored);\n \t\tcolored_p = colored->buf;\n \t\tcolored_pend = colored_p + colored->len;\n@@ -531,6 +550,9 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n \t\t\t\t\t\t colored_pend - colored_p);\n \t\t\tif (colored_eol)\n \t\t\t\tcolored_p = colored_eol + 1;\n+\t\t\telse if (p != pend)\n+\t\t\t\t/* colored shorter than non-colored? */\n+\t\t\t\tgoto mismatched_output;\n \t\t\telse\n \t\t\t\tcolored_p = colored_pend;\n \n@@ -555,6 +577,15 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n \t\t */\n \t\thunk->splittable_into++;\n \n+\t/* non-colored shorter than colored? */\n+\tif (colored_p != colored_pend) {\n+mismatched_output:\n+\t\terror(_(\"mismatched output from interactive.diffFilter\"));\n+\t\tadvise(_(\"Your filter must maintain a one-to-one correspondence\\n\"\n+\t\t\t \"between its input and output lines.\"));\n+\t\treturn -1;\n+\t}\n+\n \treturn 0;\n }\n \n@@ -1612,6 +1643,7 @@ int run_add_p(struct repository *r, enum add_p_mode mode,\n \t parse_diff(&s, ps) < 0) {\n \t\tstrbuf_release(&s.plain);\n \t\tstrbuf_release(&s.colored);\n+\t\tclear_add_i_state(&s.s);\n \t\treturn -1;\n \t}\n \n@@ -1630,5 +1662,6 @@ int run_add_p(struct repository *r, enum add_p_mode mode,\n \tstrbuf_release(&s.buf);\n \tstrbuf_release(&s.plain);\n \tstrbuf_release(&s.colored);\n+\tclear_add_i_state(&s.s);\n \treturn 0;\n }\n-- \ngitgitgadget\n\n"},{"id":"389754","messageId":"a81304cb76e127c78c0d6e69520477e6862df7f5.1579027433.git.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.v4.git.1579027433.gitgitgadget@gmail.com","subject":"[PATCH v4 05/10] terminal: accommodate Git for Windows' default terminal","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-01-14T18:43:48Z","receivedAt":"2020-01-14T18:44:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nGit for Windows' Git Bash runs in MinTTY by default, which does not have\na Win32 Console instance, but uses MSYS2 pseudo terminals instead.\n\nThis is a problem, as Git for Windows does not want to use the MSYS2\nemulation layer for Git itself, and therefore has no direct way to\ninteract with that pseudo terminal.\n\nAs a workaround, use the `stty` utility (which is included in Git for\nWindows, and which *is* an MSYS2 program, so it knows how to deal with\nthe pseudo terminal).\n\nNote: If Git runs in a regular CMD or PowerShell window, there *is* a\nregular Win32 Console to work with. This is not a problem for the MSYS2\n`stty`: it copes with this scenario just fine.\n\nAlso note that we introduce support for more bits than would be\nnecessary for a mere `disable_echo()` here, in preparation for the\nupcoming `enable_non_canonical()` function.\n\nSigned-off-by: Johannes Schindelin \n---\n compat/terminal.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 50 insertions(+)\n\ndiff --git a/compat/terminal.c b/compat/terminal.c\nindex 1fb40b3a0a..16e9949da1 100644\n--- a/compat/terminal.c\n+++ b/compat/terminal.c\n@@ -2,6 +2,8 @@\n #include \"compat/terminal.h\"\n #include \"sigchain.h\"\n #include \"strbuf.h\"\n+#include \"run-command.h\"\n+#include \"string-list.h\"\n \n #if defined(HAVE_DEV_TTY) || defined(GIT_WINDOWS_NATIVE)\n \n@@ -64,11 +66,28 @@ static int disable_echo(void)\n #define OUTPUT_PATH \"CONOUT$\"\n #define FORCE_TEXT \"t\"\n \n+static int use_stty = 1;\n+static struct string_list stty_restore = STRING_LIST_INIT_DUP;\n static HANDLE hconin = INVALID_HANDLE_VALUE;\n static DWORD cmode;\n \n static void restore_term(void)\n {\n+\tif (use_stty) {\n+\t\tint i;\n+\t\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\n+\t\tif (stty_restore.nr == 0)\n+\t\t\treturn;\n+\n+\t\targv_array_push(&cp.args, \"stty\");\n+\t\tfor (i = 0; i < stty_restore.nr; i++)\n+\t\t\targv_array_push(&cp.args, stty_restore.items[i].string);\n+\t\trun_command(&cp);\n+\t\tstring_list_clear(&stty_restore, 0);\n+\t\treturn;\n+\t}\n+\n \tif (hconin == INVALID_HANDLE_VALUE)\n \t\treturn;\n \n@@ -79,6 +98,37 @@ static void restore_term(void)\n \n static int disable_bits(DWORD bits)\n {\n+\tif (use_stty) {\n+\t\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\n+\t\targv_array_push(&cp.args, \"stty\");\n+\n+\t\tif (bits & ENABLE_LINE_INPUT) {\n+\t\t\tstring_list_append(&stty_restore, \"icanon\");\n+\t\t\targv_array_push(&cp.args, \"-icanon\");\n+\t\t}\n+\n+\t\tif (bits & ENABLE_ECHO_INPUT) {\n+\t\t\tstring_list_append(&stty_restore, \"echo\");\n+\t\t\targv_array_push(&cp.args, \"-echo\");\n+\t\t}\n+\n+\t\tif (bits & ENABLE_PROCESSED_INPUT) {\n+\t\t\tstring_list_append(&stty_restore, \"-ignbrk\");\n+\t\t\tstring_list_append(&stty_restore, \"intr\");\n+\t\t\tstring_list_append(&stty_restore, \"^c\");\n+\t\t\targv_array_push(&cp.args, \"ignbrk\");\n+\t\t\targv_array_push(&cp.args, \"intr\");\n+\t\t\targv_array_push(&cp.args, \"\");\n+\t\t}\n+\n+\t\tif (run_command(&cp) == 0)\n+\t\t\treturn 0;\n+\n+\t\t/* `stty` could not be executed; access the Console directly */\n+\t\tuse_stty = 0;\n+\t}\n+\n \thconin = CreateFile(\"CONIN$\", GENERIC_READ | GENERIC_WRITE,\n \t FILE_SHARE_READ, NULL, OPEN_EXISTING,\n \t FILE_ATTRIBUTE_NORMAL, NULL);\n-- \ngitgitgadget\n\n"},{"id":"389755","messageId":"cdc609f8fa349b9ffada68dfd6fd183365222ba7.1579027433.git.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.v4.git.1579027433.gitgitgadget@gmail.com","subject":"[PATCH v4 08/10] built-in add -p: handle Escape sequences in interactive.singlekey mode","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-01-14T18:43:51Z","receivedAt":"2020-01-14T18:44:07Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThis recapitulates part of b5cc003253c8 (add -i: ignore terminal escape\nsequences, 2011-05-17):\n\n add -i: ignore terminal escape sequences\n\n On the author's terminal, the up-arrow input sequence is ^[[A, and\n thus fat-fingering an up-arrow into 'git checkout -p' is quite\n dangerous: git-add--interactive.perl will ignore the ^[ and [\n characters and happily treat A as \"discard everything\".\n\n As a band-aid fix, use Term::Cap to get all terminal capabilities.\n Then use the heuristic that any capability value that starts with ^[\n (i.e., \\e in perl) must be a key input sequence. Finally, given an\n input that starts with ^[, read more characters until we have read a\n full escape sequence, then return that to the caller. We use a\n timeout of 0.5 seconds on the subsequent reads to avoid getting stuck\n if the user actually input a lone ^[.\n\n Since none of the currently recognized keys start with ^[, the net\n result is that the sequence as a whole will be ignored and the help\n displayed.\n\nNote that we leave part for later which uses \"Term::Cap to get all\nterminal capabilities\", for several reasons:\n\n1. it is actually not really necessary, as the timeout of 0.5 seconds\n should be plenty sufficient to catch Escape sequences,\n\n2. it is cleaner to keep the change to special-case Escape sequences\n separate from the change that reads all terminal capabilities to\n speed things up, and\n\n3. in practice, relying on the terminal capabilities is a bit overrated,\n as the information could be incomplete, or plain wrong. For example,\n in this developer's tmux sessions, the terminal capabilities claim\n that the \"cursor up\" sequence is ^[M, but the actual sequence\n produced by the \"cursor up\" key is ^[[A.\n\nSigned-off-by: Johannes Schindelin \n---\n compat/terminal.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++-\n 1 file changed, 55 insertions(+), 1 deletion(-)\n\ndiff --git a/compat/terminal.c b/compat/terminal.c\nindex 1b2564042a..b7f58d1781 100644\n--- a/compat/terminal.c\n+++ b/compat/terminal.c\n@@ -161,6 +161,37 @@ static int enable_non_canonical(void)\n \treturn disable_bits(ENABLE_ECHO_INPUT | ENABLE_LINE_INPUT | ENABLE_PROCESSED_INPUT);\n }\n \n+/*\n+ * Override `getchar()`, as the default implementation does not use\n+ * `ReadFile()`.\n+ *\n+ * This poses a problem when we want to see whether the standard\n+ * input has more characters, as the default of Git for Windows is to start the\n+ * Bash in a MinTTY, which uses a named pipe to emulate a pty, in which case\n+ * our `poll()` emulation calls `PeekNamedPipe()`, which seems to require\n+ * `ReadFile()` to be called first to work properly (it only reports 0\n+ * available bytes, otherwise).\n+ *\n+ * So let's just override `getchar()` with a version backed by `ReadFile()` and\n+ * go our merry ways from here.\n+ */\n+static int mingw_getchar(void)\n+{\n+\tDWORD read = 0;\n+\tunsigned char ch;\n+\n+\tif (!ReadFile(GetStdHandle(STD_INPUT_HANDLE), &ch, 1, &read, NULL))\n+\t\treturn EOF;\n+\n+\tif (!read) {\n+\t\terror(\"Unexpected 0 read\");\n+\t\treturn EOF;\n+\t}\n+\n+\treturn ch;\n+}\n+#define getchar mingw_getchar\n+\n #endif\n \n #ifndef FORCE_TEXT\n@@ -228,8 +259,31 @@ int read_key_without_echo(struct strbuf *buf)\n \t\trestore_term();\n \t\treturn EOF;\n \t}\n-\n \tstrbuf_addch(buf, ch);\n+\n+\tif (ch == '\\033' /* ESC */) {\n+\t\t/*\n+\t\t * We are most likely looking at an Escape sequence. Let's try\n+\t\t * to read more bytes, waiting at most half a second, assuming\n+\t\t * that the sequence is complete if we did not receive any byte\n+\t\t * within that time.\n+\t\t *\n+\t\t * Start by replacing the Escape byte with ^[ */\n+\t\tstrbuf_splice(buf, buf->len - 1, 1, \"^[\", 2);\n+\n+\t\tfor (;;) {\n+\t\t\tstruct pollfd pfd = { .fd = 0, .events = POLLIN };\n+\n+\t\t\tif (poll(&pfd, 1, 500) < 1)\n+\t\t\t\tbreak;\n+\n+\t\t\tch = getchar();\n+\t\t\tif (ch == EOF)\n+\t\t\t\treturn 0;\n+\t\t\tstrbuf_addch(buf, ch);\n+\t\t}\n+\t}\n+\n \trestore_term();\n \treturn 0;\n }\n-- \ngitgitgadget\n\n"},{"id":"389756","messageId":"7ab7ec62d0d67c0adbef54d2a363c77a12d689bc.1579027433.git.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.v4.git.1579027433.gitgitgadget@gmail.com","subject":"[PATCH v4 10/10] ci: include the built-in `git add -i` in the `linux-gcc` job","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-01-14T18:43:53Z","receivedAt":"2020-01-14T18:44:08Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThis job runs the test suite twice, once in regular mode, and once with\na whole slew of `GIT_TEST_*` variables set.\n\nNow that the built-in version of `git add --interactive` is\nfeature-complete, let's also throw `GIT_TEST_ADD_I_USE_BUILTIN` into\nthat fray.\n\nSigned-off-by: Johannes Schindelin \n---\n ci/run-build-and-tests.sh | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh\nindex ff0ef7f08e..4df54c4efe 100755\n--- a/ci/run-build-and-tests.sh\n+++ b/ci/run-build-and-tests.sh\n@@ -20,6 +20,7 @@ linux-gcc)\n \texport GIT_TEST_OE_DELTA_SIZE=5\n \texport GIT_TEST_COMMIT_GRAPH=1\n \texport GIT_TEST_MULTI_PACK_INDEX=1\n+\texport GIT_TEST_ADD_I_USE_BUILTIN=1\n \tmake test\n \t;;\n linux-gcc-4.8)\n-- \ngitgitgadget\n"},{"id":"389757","messageId":"8ed4487ae49f5ff416d0dbcfdb7292056c7e3b85.1579027433.git.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.v4.git.1579027433.gitgitgadget@gmail.com","subject":"[PATCH v4 07/10] built-in add -p: respect the `interactive.singlekey` config setting","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-01-14T18:43:50Z","receivedAt":"2020-01-14T18:44:09Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThe Perl version of `git add -p` supports this config setting to allow\nusers to input commands via single characters (as opposed to having to\npress the key afterwards).\n\nThis is an opt-in feature because it requires Perl packages\n(Term::ReadKey and Term::Cap, where it tries to handle an absence of the\nlatter package gracefully) to work. Note that at least on Ubuntu, that\nPerl package is not installed by default (it needs to be installed via\n`sudo apt-get install libterm-readkey-perl`), so this feature is\nprobably not used a whole lot.\n\nIn C, we obviously do not have these packages available, but we just\nintroduced `read_single_keystroke()` that is similar to what\nTerm::ReadKey provides, and we use that here.\n\nSigned-off-by: Johannes Schindelin \n---\n add-interactive.c | 2 ++\n add-interactive.h | 1 +\n add-patch.c | 21 +++++++++++++++++----\n 3 files changed, 20 insertions(+), 4 deletions(-)\n\ndiff --git a/add-interactive.c b/add-interactive.c\nindex 9e4bcb382c..39c3896494 100644\n--- a/add-interactive.c\n+++ b/add-interactive.c\n@@ -60,6 +60,8 @@ void init_add_i_state(struct add_i_state *s, struct repository *r)\n \tFREE_AND_NULL(s->interactive_diff_algorithm);\n \tgit_config_get_string(\"diff.algorithm\",\n \t\t\t &s->interactive_diff_algorithm);\n+\n+\tgit_config_get_bool(\"interactive.singlekey\", &s->use_single_key);\n }\n \n void clear_add_i_state(struct add_i_state *s)\ndiff --git a/add-interactive.h b/add-interactive.h\nindex 923efaf527..693f125e8e 100644\n--- a/add-interactive.h\n+++ b/add-interactive.h\n@@ -16,6 +16,7 @@ struct add_i_state {\n \tchar file_old_color[COLOR_MAXLEN];\n \tchar file_new_color[COLOR_MAXLEN];\n \n+\tint use_single_key;\n \tchar *interactive_diff_filter, *interactive_diff_algorithm;\n };\n \ndiff --git a/add-patch.c b/add-patch.c\nindex 8f2ee8688b..d8dafa8168 100644\n--- a/add-patch.c\n+++ b/add-patch.c\n@@ -6,6 +6,7 @@\n #include \"pathspec.h\"\n #include \"color.h\"\n #include \"diff.h\"\n+#include \"compat/terminal.h\"\n \n enum prompt_mode_type {\n \tPROMPT_MODE_CHANGE = 0, PROMPT_DELETION, PROMPT_HUNK,\n@@ -1149,14 +1150,27 @@ static int run_apply_check(struct add_p_state *s,\n \treturn 0;\n }\n \n+static int read_single_character(struct add_p_state *s)\n+{\n+\tif (s->s.use_single_key) {\n+\t\tint res = read_key_without_echo(&s->answer);\n+\t\tprintf(\"%s\\n\", res == EOF ? \"\" : s->answer.buf);\n+\t\treturn res;\n+\t}\n+\n+\tif (strbuf_getline(&s->answer, stdin) == EOF)\n+\t\treturn EOF;\n+\tstrbuf_trim_trailing_newline(&s->answer);\n+\treturn 0;\n+}\n+\n static int prompt_yesno(struct add_p_state *s, const char *prompt)\n {\n \tfor (;;) {\n \t\tcolor_fprintf(stdout, s->s.prompt_color, \"%s\", _(prompt));\n \t\tfflush(stdout);\n-\t\tif (strbuf_getline(&s->answer, stdin) == EOF)\n+\t\tif (read_single_character(s) == EOF)\n \t\t\treturn -1;\n-\t\tstrbuf_trim_trailing_newline(&s->answer);\n \t\tswitch (tolower(s->answer.buf[0])) {\n \t\tcase 'n': return 0;\n \t\tcase 'y': return 1;\n@@ -1396,9 +1410,8 @@ static int patch_update_file(struct add_p_state *s,\n \t\t\t _(s->mode->prompt_mode[prompt_mode_type]),\n \t\t\t s->buf.buf);\n \t\tfflush(stdout);\n-\t\tif (strbuf_getline(&s->answer, stdin) == EOF)\n+\t\tif (read_single_character(s) == EOF)\n \t\t\tbreak;\n-\t\tstrbuf_trim_trailing_newline(&s->answer);\n \n \t\tif (!s->answer.len)\n \t\t\tcontinue;\n-- \ngitgitgadget\n\n"},{"id":"389758","messageId":"062c6245477b23b9c13a6324e677e7a2be62dd65.1579027433.git.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.v4.git.1579027433.gitgitgadget@gmail.com","subject":"[PATCH v4 03/10] built-in add -p: handle diff.algorithm","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-01-14T18:43:46Z","receivedAt":"2020-01-14T18:44:09Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThe Perl version of `git add -p` reads the config setting\n`diff.algorithm` and if set, uses it to generate the diff using the\nspecified algorithm.\n\nThis patch ports that functionality to the C version.\n\nNote: just like `git-add--interactive.perl`, we do _not_ respect this\nconfig setting in `git add -i`'s `diff` command, but _only_ in the\n`patch` command.\n\nSigned-off-by: Johannes Schindelin \n---\n add-interactive.c | 5 +++++\n add-interactive.h | 2 +-\n add-patch.c | 3 +++\n 3 files changed, 9 insertions(+), 1 deletion(-)\n\ndiff --git a/add-interactive.c b/add-interactive.c\nindex 1786ea29c4..9e4bcb382c 100644\n--- a/add-interactive.c\n+++ b/add-interactive.c\n@@ -56,11 +56,16 @@ void init_add_i_state(struct add_i_state *s, struct repository *r)\n \tFREE_AND_NULL(s->interactive_diff_filter);\n \tgit_config_get_string(\"interactive.difffilter\",\n \t\t\t &s->interactive_diff_filter);\n+\n+\tFREE_AND_NULL(s->interactive_diff_algorithm);\n+\tgit_config_get_string(\"diff.algorithm\",\n+\t\t\t &s->interactive_diff_algorithm);\n }\n \n void clear_add_i_state(struct add_i_state *s)\n {\n \tFREE_AND_NULL(s->interactive_diff_filter);\n+\tFREE_AND_NULL(s->interactive_diff_algorithm);\n \tmemset(s, 0, sizeof(*s));\n \ts->use_color = -1;\n }\ndiff --git a/add-interactive.h b/add-interactive.h\nindex 46c73867ad..923efaf527 100644\n--- a/add-interactive.h\n+++ b/add-interactive.h\n@@ -16,7 +16,7 @@ struct add_i_state {\n \tchar file_old_color[COLOR_MAXLEN];\n \tchar file_new_color[COLOR_MAXLEN];\n \n-\tchar *interactive_diff_filter;\n+\tchar *interactive_diff_filter, *interactive_diff_algorithm;\n };\n \n void init_add_i_state(struct add_i_state *s, struct repository *r);\ndiff --git a/add-patch.c b/add-patch.c\nindex 78bde41df0..8f2ee8688b 100644\n--- a/add-patch.c\n+++ b/add-patch.c\n@@ -360,6 +360,7 @@ static int is_octal(const char *p, size_t len)\n static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n {\n \tstruct argv_array args = ARGV_ARRAY_INIT;\n+\tconst char *diff_algorithm = s->s.interactive_diff_algorithm;\n \tstruct strbuf *plain = &s->plain, *colored = NULL;\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n \tchar *p, *pend, *colored_p = NULL, *colored_pend = NULL, marker = '\\0';\n@@ -369,6 +370,8 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n \tint res;\n \n \targv_array_pushv(&args, s->mode->diff_cmd);\n+\tif (diff_algorithm)\n+\t\targv_array_pushf(&args, \"--diff-algorithm=%s\", diff_algorithm);\n \tif (s->revision) {\n \t\tstruct object_id oid;\n \t\targv_array_push(&args,\n-- \ngitgitgadget\n\n"},{"id":"389759","messageId":"8d9c703f3b02cd784025dec5c0be682ea25811de.1579027433.git.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.v4.git.1579027433.gitgitgadget@gmail.com","subject":"[PATCH v4 06/10] terminal: add a new function to read a single keystroke","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-01-14T18:43:49Z","receivedAt":"2020-01-14T18:44:11Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nTypically, input on the command-line is line-based. It is actually not\nreally easy to get single characters (or better put: keystrokes).\n\nWe provide two implementations here:\n\n- One that handles `/dev/tty` based systems as well as native Windows.\n The former uses the `tcsetattr()` function to put the terminal into\n \"raw mode\", which allows us to read individual keystrokes, one by one.\n The latter uses `stty.exe` to do the same, falling back to direct\n Win32 Console access.\n\n Thanks to the refactoring leading up to this commit, this is a single\n function, with the platform-specific details hidden away in\n conditionally-compiled code blocks.\n\n- A fall-back which simply punts and reads back an entire line.\n\nNote that the function writes the keystroke into an `strbuf` rather than\na `char`, in preparation for reading Escape sequences (e.g. when the\nuser hit an arrow key). This is also required for UTF-8 sequences in\ncase the keystroke corresponds to a non-ASCII letter.\n\nSigned-off-by: Johannes Schindelin \n---\n compat/terminal.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++\n compat/terminal.h | 3 +++\n 2 files changed, 58 insertions(+)\n\ndiff --git a/compat/terminal.c b/compat/terminal.c\nindex 16e9949da1..1b2564042a 100644\n--- a/compat/terminal.c\n+++ b/compat/terminal.c\n@@ -60,6 +60,11 @@ static int disable_echo(void)\n \treturn disable_bits(ECHO);\n }\n \n+static int enable_non_canonical(void)\n+{\n+\treturn disable_bits(ICANON | ECHO);\n+}\n+\n #elif defined(GIT_WINDOWS_NATIVE)\n \n #define INPUT_PATH \"CONIN$\"\n@@ -151,6 +156,10 @@ static int disable_echo(void)\n \treturn disable_bits(ENABLE_ECHO_INPUT);\n }\n \n+static int enable_non_canonical(void)\n+{\n+\treturn disable_bits(ENABLE_ECHO_INPUT | ENABLE_LINE_INPUT | ENABLE_PROCESSED_INPUT);\n+}\n \n #endif\n \n@@ -198,6 +207,33 @@ char *git_terminal_prompt(const char *prompt, int echo)\n \treturn buf.buf;\n }\n \n+int read_key_without_echo(struct strbuf *buf)\n+{\n+\tstatic int warning_displayed;\n+\tint ch;\n+\n+\tif (warning_displayed || enable_non_canonical() < 0) {\n+\t\tif (!warning_displayed) {\n+\t\t\twarning(\"reading single keystrokes not supported on \"\n+\t\t\t\t\"this platform; reading line instead\");\n+\t\t\twarning_displayed = 1;\n+\t\t}\n+\n+\t\treturn strbuf_getline(buf, stdin);\n+\t}\n+\n+\tstrbuf_reset(buf);\n+\tch = getchar();\n+\tif (ch == EOF) {\n+\t\trestore_term();\n+\t\treturn EOF;\n+\t}\n+\n+\tstrbuf_addch(buf, ch);\n+\trestore_term();\n+\treturn 0;\n+}\n+\n #else\n \n char *git_terminal_prompt(const char *prompt, int echo)\n@@ -205,4 +241,23 @@ char *git_terminal_prompt(const char *prompt, int echo)\n \treturn getpass(prompt);\n }\n \n+int read_key_without_echo(struct strbuf *buf)\n+{\n+\tstatic int warning_displayed;\n+\tconst char *res;\n+\n+\tif (!warning_displayed) {\n+\t\twarning(\"reading single keystrokes not supported on this \"\n+\t\t\t\"platform; reading line instead\");\n+\t\twarning_displayed = 1;\n+\t}\n+\n+\tres = getpass(\"\");\n+\tstrbuf_reset(buf);\n+\tif (!res)\n+\t\treturn EOF;\n+\tstrbuf_addstr(buf, res);\n+\treturn 0;\n+}\n+\n #endif\ndiff --git a/compat/terminal.h b/compat/terminal.h\nindex 97db7cd69d..a9d52b8464 100644\n--- a/compat/terminal.h\n+++ b/compat/terminal.h\n@@ -3,4 +3,7 @@\n \n char *git_terminal_prompt(const char *prompt, int echo);\n \n+/* Read a single keystroke, without echoing it to the terminal */\n+int read_key_without_echo(struct strbuf *buf);\n+\n #endif /* COMPAT_TERMINAL_H */\n-- \ngitgitgadget\n\n"},{"id":"389760","messageId":"09a8946303720f8abd168d0590d421c1dbbbd71a.1579027433.git.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.v4.git.1579027433.gitgitgadget@gmail.com","subject":"[PATCH v4 04/10] terminal: make the code of disable_echo() reusable","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-01-14T18:43:47Z","receivedAt":"2020-01-14T18:44:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWe are about to introduce the function `enable_non_canonical()`, which\nshares almost the complete code with `disable_echo()`.\n\nLet's prepare for that, by refactoring out that shared code.\n\nSigned-off-by: Johannes Schindelin \n---\n compat/terminal.c | 19 +++++++++++++++----\n 1 file changed, 15 insertions(+), 4 deletions(-)\n\ndiff --git a/compat/terminal.c b/compat/terminal.c\nindex fa13ee672d..1fb40b3a0a 100644\n--- a/compat/terminal.c\n+++ b/compat/terminal.c\n@@ -32,7 +32,7 @@ static void restore_term(void)\n \tterm_fd = -1;\n }\n \n-static int disable_echo(void)\n+static int disable_bits(tcflag_t bits)\n {\n \tstruct termios t;\n \n@@ -43,7 +43,7 @@ static int disable_echo(void)\n \told_term = t;\n \tsigchain_push_common(restore_term_on_signal);\n \n-\tt.c_lflag &= ~ECHO;\n+\tt.c_lflag &= ~bits;\n \tif (!tcsetattr(term_fd, TCSAFLUSH, &t))\n \t\treturn 0;\n \n@@ -53,6 +53,11 @@ static int disable_echo(void)\n \treturn -1;\n }\n \n+static int disable_echo(void)\n+{\n+\treturn disable_bits(ECHO);\n+}\n+\n #elif defined(GIT_WINDOWS_NATIVE)\n \n #define INPUT_PATH \"CONIN$\"\n@@ -72,7 +77,7 @@ static void restore_term(void)\n \thconin = INVALID_HANDLE_VALUE;\n }\n \n-static int disable_echo(void)\n+static int disable_bits(DWORD bits)\n {\n \thconin = CreateFile(\"CONIN$\", GENERIC_READ | GENERIC_WRITE,\n \t FILE_SHARE_READ, NULL, OPEN_EXISTING,\n@@ -82,7 +87,7 @@ static int disable_echo(void)\n \n \tGetConsoleMode(hconin, &cmode);\n \tsigchain_push_common(restore_term_on_signal);\n-\tif (!SetConsoleMode(hconin, cmode & (~ENABLE_ECHO_INPUT))) {\n+\tif (!SetConsoleMode(hconin, cmode & ~bits)) {\n \t\tCloseHandle(hconin);\n \t\thconin = INVALID_HANDLE_VALUE;\n \t\treturn -1;\n@@ -91,6 +96,12 @@ static int disable_echo(void)\n \treturn 0;\n }\n \n+static int disable_echo(void)\n+{\n+\treturn disable_bits(ENABLE_ECHO_INPUT);\n+}\n+\n+\n #endif\n \n #ifndef FORCE_TEXT\n-- \ngitgitgadget\n\n"},{"id":"389761","messageId":"80b0f2528d360e11f8eea4ca9ee69d1ed570414b.1579027433.git.gitgitgadget@gmail.com","threadId":"52505","inReplyTo":"pull.175.v4.git.1579027433.gitgitgadget@gmail.com","subject":"[PATCH v4 09/10] built-in add -p: handle Escape sequences more efficiently","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-01-14T18:43:52Z","receivedAt":"2020-01-14T18:44:14Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWhen `interactive.singlekey = true`, we react immediately to keystrokes,\neven to Escape sequences (e.g. when pressing a cursor key).\n\nThe problem with Escape sequences is that we do not really know when\nthey are done, and as a heuristic we poll standard input for half a\nsecond to make sure that we got all of it.\n\nWhile waiting half a second is not asking for a whole lot, it can become\nquite annoying over time, therefore with this patch, we read the\nterminal capabilities (if available) and extract known Escape sequences\nfrom there, then stop polling immediately when we detected that the user\npressed a key that generated such a known sequence.\n\nThis recapitulates the remaining part of b5cc003253c8 (add -i: ignore\nterminal escape sequences, 2011-05-17).\n\nNote: We do *not* query the terminal capabilities directly. That would\neither require a lot of platform-specific code, or it would require\nlinking to a library such as ncurses.\n\nLinking to a library in the built-ins is something we try very hard to\navoid (we even kicked the libcurl dependency to a non-built-in remote\nhelper, just to shave off a tiny fraction of a second from Git's startup\ntime). And the platform-specific code would be a maintenance nightmare.\n\nEven worse: in Git for Windows' case, we would need to query MSYS2\npseudo terminals, which `git.exe` simply cannot do (because it is\nintentionally *not* an MSYS2 program).\n\nTo address this, we simply spawn `infocmp -L -1` and parse its output\n(which works even in Git for Windows, because that helper is included in\nthe end-user facing installations).\n\nThis is done only once, as in the Perl version, but it is done only when\nthe first Escape sequence is encountered, not upon startup of `git add\n-i`; This saves on startup time, yet makes reacting to the first Escape\nsequence slightly more sluggish. But it allows us to keep the\nterminal-related code encapsulated in the `compat/terminal.c` file.\n\nSigned-off-by: Johannes Schindelin \n---\n compat/terminal.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++-\n 1 file changed, 72 insertions(+), 1 deletion(-)\n\ndiff --git a/compat/terminal.c b/compat/terminal.c\nindex b7f58d1781..35bca03d14 100644\n--- a/compat/terminal.c\n+++ b/compat/terminal.c\n@@ -4,6 +4,7 @@\n #include \"strbuf.h\"\n #include \"run-command.h\"\n #include \"string-list.h\"\n+#include \"hashmap.h\"\n \n #if defined(HAVE_DEV_TTY) || defined(GIT_WINDOWS_NATIVE)\n \n@@ -238,6 +239,71 @@ char *git_terminal_prompt(const char *prompt, int echo)\n \treturn buf.buf;\n }\n \n+/*\n+ * The `is_known_escape_sequence()` function returns 1 if the passed string\n+ * corresponds to an Escape sequence that the terminal capabilities contains.\n+ *\n+ * To avoid depending on ncurses or other platform-specific libraries, we rely\n+ * on the presence of the `infocmp` executable to do the job for us (failing\n+ * silently if the program is not available or refused to run).\n+ */\n+struct escape_sequence_entry {\n+\tstruct hashmap_entry entry;\n+\tchar sequence[FLEX_ARRAY];\n+};\n+\n+static int sequence_entry_cmp(const void *hashmap_cmp_fn_data,\n+\t\t\t const struct escape_sequence_entry *e1,\n+\t\t\t const struct escape_sequence_entry *e2,\n+\t\t\t const void *keydata)\n+{\n+\treturn strcmp(e1->sequence, keydata ? keydata : e2->sequence);\n+}\n+\n+static int is_known_escape_sequence(const char *sequence)\n+{\n+\tstatic struct hashmap sequences;\n+\tstatic int initialized;\n+\n+\tif (!initialized) {\n+\t\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\t\tstruct strbuf buf = STRBUF_INIT;\n+\t\tchar *p, *eol;\n+\n+\t\thashmap_init(&sequences, (hashmap_cmp_fn)sequence_entry_cmp,\n+\t\t\t NULL, 0);\n+\n+\t\targv_array_pushl(&cp.args, \"infocmp\", \"-L\", \"-1\", NULL);\n+\t\tif (pipe_command(&cp, NULL, 0, &buf, 0, NULL, 0))\n+\t\t\tstrbuf_setlen(&buf, 0);\n+\n+\t\tfor (eol = p = buf.buf; *p; p = eol + 1) {\n+\t\t\tp = strchr(p, '=');\n+\t\t\tif (!p)\n+\t\t\t\tbreak;\n+\t\t\tp++;\n+\t\t\teol = strchrnul(p, '\\n');\n+\n+\t\t\tif (starts_with(p, \"\\\\E\")) {\n+\t\t\t\tchar *comma = memchr(p, ',', eol - p);\n+\t\t\t\tstruct escape_sequence_entry *e;\n+\n+\t\t\t\tp[0] = '^';\n+\t\t\t\tp[1] = '[';\n+\t\t\t\tFLEX_ALLOC_MEM(e, sequence, p, comma - p);\n+\t\t\t\thashmap_entry_init(&e->entry,\n+\t\t\t\t\t\t strhash(e->sequence));\n+\t\t\t\thashmap_add(&sequences, &e->entry);\n+\t\t\t}\n+\t\t\tif (!*eol)\n+\t\t\t\tbreak;\n+\t\t}\n+\t\tinitialized = 1;\n+\t}\n+\n+\treturn !!hashmap_get_from_hash(&sequences, strhash(sequence), sequence);\n+}\n+\n int read_key_without_echo(struct strbuf *buf)\n {\n \tstatic int warning_displayed;\n@@ -271,7 +337,12 @@ int read_key_without_echo(struct strbuf *buf)\n \t\t * Start by replacing the Escape byte with ^[ */\n \t\tstrbuf_splice(buf, buf->len - 1, 1, \"^[\", 2);\n \n-\t\tfor (;;) {\n+\t\t/*\n+\t\t * Query the terminal capabilities once about all the Escape\n+\t\t * sequences it knows about, so that we can avoid waiting for\n+\t\t * half a second when we know that the sequence is complete.\n+\t\t */\n+\t\twhile (!is_known_escape_sequence(buf->buf)) {\n \t\t\tstruct pollfd pfd = { .fd = 0, .events = POLLIN };\n \n \t\t\tif (poll(&pfd, 1, 500) < 1)\n-- \ngitgitgadget\n\n"},{"id":"389810","messageId":"xmqqblr4k1t0.fsf@gitster-ct.c.googlers.com","threadId":"52505","inReplyTo":"20200113183313.GA2087@coredump.intra.peff.net","subject":"Re: [PATCH v3 01/10] built-in add -i/-p: treat SIGPIPE as EOF","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-15T18:32:59Z","receivedAt":"2020-01-15T18:33:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Mon, Jan 13, 2020 at 06:04:17PM +0100, SZEDER Gábor wrote:\n>\n>> After looking into it, the issue seems to be sending data to the\n>> broken diffFilter process. So in that test the diff is \"filtered\"\n>> through 'echo too-short', which exits real fast, and doesn't read its\n>> standard input at all (well, apart from e.g. the usual kernel\n>> buffering that might happen on a pipe between the two processes).\n>> Making sure that the diffFilter process reads all the data before\n>> exiting, i.e. changing it to:\n>> \n>> test_config interactive.diffFilter \"cat >/dev/null ; echo too-short\" &&\n>> \n>> made the test reliable, with over 2000 --stress repetitions, and that\n>> with only a single \"y\" on 'git add's stdin.\n>\n> Yeah, I agree the test should be changed. What you wrote above was my\n> first thought, too, but I think \"sed 1d\" is actually a more realistic\n> test (and is shorter and one fewer process).\n\nI am not sure what we are aiming for. Are we making sure the\ncommand behaves well in the hands of end users, who may write a\nscript that consumes only early parts of the input that is needed\nfor its use and stops reading, or are we just aiming to claim \"all\nour tests pass\"? I was hoping that we would be doing the former,\nand I would understand if the suggestion were \"sed 1q\" for that\nexact reason.\n\nIOW, shouldn't we be fixing the part that drives the external\nprocess, so that the test \"passes\" even with such a \"broken\" filter?\n\n>> Now, merely tweaking the test is clearly insufficient, because we not\n>> only want the test to be realiable, but we want 'git add' to die\n>> gracefully when users out there mess up their configuration.\n\nYes, and I was hoping that we do not have to touch the test if we\ndid the latter.\n\n> I really wish there was a way to set a handler for SIGPIPE that tells\n> _which_ descriptor caused it. Because I think logic like \"die if it was\n> fd 1, ignore and let write() return EPIPE otherwise\" is the behavior\n> we'd like. But I don't think there's a portable way to do so.\n>\n> I've been tempted to say that we should just ignore SIGPIPE everywhere,\n> and convert even copious-output programs like git-log to just check for\n> errors (they could probably even just check ferror(stdout) for each\n> commit we output, if we didn't want to touch every printf call).\n\nYeah, I share that temptation.\n"},{"id":"389812","messageId":"20200115190322.GA4087422@coredump.intra.peff.net","threadId":"52505","inReplyTo":"xmqqblr4k1t0.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3 01/10] built-in add -i/-p: treat SIGPIPE as EOF","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-01-15T19:03:22Z","receivedAt":"2020-01-15T19:03:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 15, 2020 at 10:32:59AM -0800, Junio C Hamano wrote:\n\n> >> test_config interactive.diffFilter \"cat >/dev/null ; echo too-short\" &&\n> >> \n> >> made the test reliable, with over 2000 --stress repetitions, and that\n> >> with only a single \"y\" on 'git add's stdin.\n> >\n> > Yeah, I agree the test should be changed. What you wrote above was my\n> > first thought, too, but I think \"sed 1d\" is actually a more realistic\n> > test (and is shorter and one fewer process).\n> \n> I am not sure what we are aiming for. Are we making sure the\n> command behaves well in the hands of end users, who may write a\n> script that consumes only early parts of the input that is needed\n> for its use and stops reading, or are we just aiming to claim \"all\n> our tests pass\"? I was hoping that we would be doing the former,\n> and I would understand if the suggestion were \"sed 1q\" for that\n> exact reason.\n> \n> IOW, shouldn't we be fixing the part that drives the external\n> process, so that the test \"passes\" even with such a \"broken\" filter?\n\nThe original motivation for this test (and the code that fixes it) was\ndiff-so-fancy, which read all of the input but didn't have a 1:1 line\ncorrespondence in the output (IIRC it condensed some particular lines,\nlike rename from/to into a single line).\n\nAnd I think most sane filters would end up reading all of the content.\nOr a misconfiguration would cause them to read nothing at all.\n\nSo something like \"sed 1d\" is more representative of a real filter. If\nwe want to test SIGPIPE, then the current one that reads _nothing_ is\nthe most torturous. But \"sed 1q\" is neither realistic (if that's what\nwe're going for) nor the hardest thing we can throw at the code (if\nthat's what we want).\n\n> > I've been tempted to say that we should just ignore SIGPIPE everywhere,\n> > and convert even copious-output programs like git-log to just check for\n> > errors (they could probably even just check ferror(stdout) for each\n> > commit we output, if we didn't want to touch every printf call).\n> \n> Yeah, I share that temptation.\n\nHmm. My recollection was that you were more of a fan of SIGPIPE than I\nam. But if you agree, then maybe the time has come for action. :)\n\n-Peff\n"},{"id":"389986","messageId":"20200117143236.GA11737@szeder.dev","threadId":"52505","inReplyTo":"20200113170417.GK32750@szeder.dev","subject":"Re: [PATCH v3 01/10] built-in add -i/-p: treat SIGPIPE as EOF","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2020-01-17T14:32:36Z","receivedAt":"2020-01-17T14:32:45Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Mon, Jan 13, 2020 at 06:04:17PM +0100, SZEDER Gábor wrote:\n> and 'GIT_TEST_ADD_I_USE_BUILTIN=1 ./t3701-add-interactive.sh -r 39,49'\n> fails with:\n> \n> + test_must_fail force_color git add -p\n> about to run diffFilter\n> attempting to xwrite() 224 bytes to a fd with revents flags 0x4\n> test_must_fail: died by signal 13: force_color git add -p\n> \n> I don't understand why we get SIGPIPE right away instead of some error\n> that we can act upon (ECONNRESET?).\n\nDoh', because it's a pipe, not a socket, that's why. pipe(7):\n\n \"If all file descriptors referring to the read end of a pipe have\n been closed, then a write(2) will cause a SIGPIPE signal to be\n generated for the calling process.\"\n\nSo ECONNRESET is definitely not the right error to set on POLLERR,\nthough I'm still not sure what the right one would be (perhaps\nEPIPE?).\n\n\n"},{"id":"390006","messageId":"20200117185836.GA11358@coredump.intra.peff.net","threadId":"52505","inReplyTo":"20200117143236.GA11737@szeder.dev","subject":"Re: [PATCH v3 01/10] built-in add -i/-p: treat SIGPIPE as EOF","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-01-17T18:58:36Z","receivedAt":"2020-01-17T18:58:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 17, 2020 at 03:32:36PM +0100, SZEDER Gábor wrote:\n\n> On Mon, Jan 13, 2020 at 06:04:17PM +0100, SZEDER Gábor wrote:\n> > and 'GIT_TEST_ADD_I_USE_BUILTIN=1 ./t3701-add-interactive.sh -r 39,49'\n> > fails with:\n> > \n> > + test_must_fail force_color git add -p\n> > about to run diffFilter\n> > attempting to xwrite() 224 bytes to a fd with revents flags 0x4\n> > test_must_fail: died by signal 13: force_color git add -p\n> > \n> > I don't understand why we get SIGPIPE right away instead of some error\n> > that we can act upon (ECONNRESET?).\n> \n> Doh', because it's a pipe, not a socket, that's why. pipe(7):\n> \n> \"If all file descriptors referring to the read end of a pipe have\n> been closed, then a write(2) will cause a SIGPIPE signal to be\n> generated for the calling process.\"\n> \n> So ECONNRESET is definitely not the right error to set on POLLERR,\n> though I'm still not sure what the right one would be (perhaps\n> EPIPE?).\n\nYes, if SIGPIPE is ignored, then that write() would produce EPIPE. So if\nyou're trying to emulate it via POLLERR, that would be accurate. Of\ncourse it could fail for _other_ reasons, and I don't think we'd know\nwhat those are without actually calling write(). Practically speaking,\nthough, if we know it's a pipe with a valid descriptor then any error is\nbasically equivalent to EPIPE (we don't care how, but for whatever\nreason we couldn't write to the other end).\n\n-Peff\n"}]}