| Message ID | 20220309110325.36917-4-phillip.wood123@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | builtin add -p: hopefully final readkey fixes | expand |
On Wed, Mar 09 2022, Phillip Wood wrote: > From: Phillip Wood <phillip.wood@dunelm.org.uk> > > On macos the builtin "add -p" does not handle keys that generate > escape sequences because poll() does not work with terminals > there. Switch to using select() on non-windows platforms to work > around this. > > Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> > --- > compat/terminal.c | 43 +++++++++++++++++++++++++++++++++++++------ > 1 file changed, 37 insertions(+), 6 deletions(-) > > diff --git a/compat/terminal.c b/compat/terminal.c > index cb653419c3..4189cbb12c 100644 > --- a/compat/terminal.c > +++ b/compat/terminal.c > @@ -82,6 +82,32 @@ static int enable_non_canonical(enum save_term_flags flags) > return disable_bits(flags, ICANON | ECHO); > } > > +/* > + * On macos it is not possible to use poll() with a terminal so use select > + * instead. > + */ > +#include <sys/select.h> I don't think this breaks anything in practice due to the platforms we define HAVE_DEV_TTY and NO_SYS_SELECT_H on, but it does look redundant & confusing, and will break if the current users of HAVE_DEV_TTY and NO_SYS_SELECT_H change. I.e. isn't sys/select.h already pulled in by the relevant include in git-compat-util.h? Why is it needed again here?
Hi Ævar On 10/03/2022 13:35, Ævar Arnfjörð Bjarmason wrote: > [...] >> >> +/* >> + * On macos it is not possible to use poll() with a terminal so use select >> + * instead. >> + */ >> +#include <sys/select.h> > > I don't think this breaks anything in practice due to the platforms we > define HAVE_DEV_TTY and NO_SYS_SELECT_H on, but it does look redundant & > confusing, and will break if the current users of HAVE_DEV_TTY and > NO_SYS_SELECT_H change. > > I.e. isn't sys/select.h already pulled in by the relevant include in > git-compat-util.h? Why is it needed again here? I didn't realize that git-compat-util.h already included that header, I'll remove it. Thanks Phillip
Phillip Wood <phillip.wood123@gmail.com> writes: > Hi Ævar > > On 10/03/2022 13:35, Ævar Arnfjörð Bjarmason wrote: >> [...] >>> +/* >>> + * On macos it is not possible to use poll() with a terminal so use select >>> + * instead. >>> + */ >>> +#include <sys/select.h> >> I don't think this breaks anything in practice due to the platforms >> we >> define HAVE_DEV_TTY and NO_SYS_SELECT_H on, but it does look redundant & >> confusing, and will break if the current users of HAVE_DEV_TTY and >> NO_SYS_SELECT_H change. >> I.e. isn't sys/select.h already pulled in by the relevant include in >> git-compat-util.h? Why is it needed again here? > > I didn't realize that git-compat-util.h already included that header, > I'll remove it. Thanks, both. The removal of that line is very much appreciated.
diff --git a/compat/terminal.c b/compat/terminal.c index cb653419c3..4189cbb12c 100644 --- a/compat/terminal.c +++ b/compat/terminal.c @@ -82,6 +82,32 @@ static int enable_non_canonical(enum save_term_flags flags) return disable_bits(flags, ICANON | ECHO); } +/* + * On macos it is not possible to use poll() with a terminal so use select + * instead. + */ +#include <sys/select.h> +static int getchar_with_timeout(int timeout) +{ + struct timeval tv, *tvp = NULL; + fd_set readfds; + int res; + + if (timeout >= 0) { + tv.tv_sec = timeout / 1000; + tv.tv_usec = (timeout % 1000) * 1000; + tvp = &tv; + } + + FD_ZERO(&readfds); + FD_SET(0, &readfds); + res = select(1, &readfds, NULL, NULL, tvp); + if (res < 0) + return EOF; + + return getchar(); +} + #elif defined(GIT_WINDOWS_NATIVE) #define INPUT_PATH "CONIN$" @@ -247,6 +273,16 @@ static int mingw_getchar(void) } #define getchar mingw_getchar +static int getchar_with_timeout(int timeout) +{ + struct pollfd pfd = { .fd = 0, .events = POLLIN }; + + if (poll(&pfd, 1, timeout) < 1) + return EOF; + + return getchar(); +} + #endif #ifndef FORCE_TEXT @@ -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);