From: Phillip Wood Date: Thu, 10 Mar 2022 13:28:05 GMT Subject: Re: [PATCH v2 0/4] builtin add -p: hopefully final readkey fixes Message-ID: <93d197db-c52c-101b-bdb0-3b4c9b073705@gmail.com> In-Reply-To: Hi Junio On 09/03/2022 23:37, Junio C Hamano wrote: > Junio C Hamano writes: > >>> These patches are based on 'pw/single-key-interactive' >> >> Is it still true, or does the base only apply to v1? The base is unchanged but does not seem to match pw/single-key-interactive. I'm not sure what happened there. They are based on 300db53b37 ("add -p: disable stdin buffering when interactive.singlekey is set", 2022-02-16) which is the second parent of e53fb7aa3f ("Merge branch 'pw/single-key-interactive' into seen", 2022-02-20) >> $ git checkout --detach pw/single-key-interactive >> HEAD is now at ac618c418e add -p: disable stdin buffering when interactive.singlekey is set >> $ git am -s ./+pw4-v2-add-p-single >> Applying: terminal: use flags for save_term() >> Applying: terminal: don't assume stdin is /dev/tty >> Applying: terminal: work around macos poll() bug >> error: patch failed: compat/terminal.c:397 >> error: compat/terminal.c: patch does not apply >> Patch failed at 0003 terminal: work around macos poll() bug >> hint: Use 'git am --show-current-patch=diff' to see the failed patch >> When you have resolved this problem, run "git am --continue". >> If you prefer to skip this patch, run "git am --skip" instead. >> To restore the original branch and stop patching, run "git am --abort". >> $ exit > > I think I figured it out. A merge of pw/single-key-interactive into > a recent tip of 'master' wants the "return 0" in the preimage below > to be "break" in compat/terminal.c > > >> @@ -397,12 +433,7 @@ int read_key_without_echo(struct strbuf *buf) >> * half a second when we know that the sequence is complete. >> */ >> while (!is_known_escape_sequence(buf->buf)) { >> - struct pollfd pfd = { .fd = 0, .events = POLLIN }; >> - >> - if (poll(&pfd, 1, 500) < 1) >> - break; >> - >> - ch = getchar(); >> + ch = getchar_with_timeout(500); >> if (ch == EOF) >> return 0; >> strbuf_addch(buf, ch); That looks good to me. However unfortunately there are some semantic conflicts as well. The patch below is based on 6b1f77214c ("Merge branch 'pw/add-p-single-key' into seen", 2022-03-09), hopefully Thunderbird wont mangle it. Whilst preparing the fixup I realized I need to reroll to fix a closing stdin in patch 2 and resetting the job signals on error in patch 4. What's the best base to use when rerolling? Best Wishes Phillip ---- >8 ---- From: Phillip Wood Date: Thu, 10 Mar 2022 11:05:26 +0000 Subject: [PATCH] fixup! Merge branch 'pw/add-p-single-key' into seen --- compat/terminal.c | 3 ++- editor.c | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/compat/terminal.c b/compat/terminal.c index 3172f4f408..d1ed5c07dc 100644 --- a/compat/terminal.c +++ b/compat/terminal.c @@ -141,7 +141,8 @@ int save_term(enum save_term_flags flags) if (term_fd < 0) return -1; if ((flags & SAVE_TERM_DUPLEX) && !is_controlling_terminal(term_fd)) { - close(term_fd); + if (term_fd) /* avoid closing stdin */ + close(term_fd); term_fd = -1; return -1; } diff --git a/editor.c b/editor.c index 6c5c95e6a2..192d6ea75d 100644 --- a/editor.c +++ b/editor.c @@ -55,7 +55,7 @@ static int prepare_term(const char *editor) git_config_get_bool("editor.stty", &need_saverestore); if (need_saverestore) - return save_term(1); + return save_term(SAVE_TERM_DUPLEX); return 0; } -- 2.33.0.342.g580a6b0edd