From c16475f6e1097a729b5f3886ca4c32a6796c33be Mon Sep 17 00:00:00 2001 From: Anton Lindqvist Date: Wed, 7 Jun 2017 15:18:53 +0200 Subject: [PATCH 1/4] Define keys using a macro I find this a easier to read and less error-prone due to usage of sizeof() instead of explicitly stating the length of the sequence. --- pick.c | 59 +++++++++++++++++++++++++++++----------------------------- 1 file changed, 30 insertions(+), 29 deletions(-) diff --git a/pick.c b/pick.c index 40b7dc2b..590a0274 100644 --- a/pick.c +++ b/pick.c @@ -788,39 +788,40 @@ print_choices(int offset, int selection) int get_key(char *buf, size_t size, size_t *nread) { +#define KEY(k, s) { k, s, sizeof(s) - 1 } static struct { + int key; const char *s; size_t length; - int key; } keys[] = { - { "\n", 1, ENTER }, - { "\r", 1, ENTER }, - { "\177", 1, BACKSPACE }, - { "\001", 1, CTRL_A }, - { "\002", 1, LEFT }, - { "\004", 1, DEL }, - { "\005", 1, CTRL_E }, - { "\006", 1, RIGHT }, - { "\013", 1, CTRL_K }, - { "\016", 1, DOWN }, - { "\020", 1, UP }, - { "\025", 1, CTRL_U }, - { "\027", 1, CTRL_W }, - { "\033\n", 2, ALT_ENTER }, - { "\033\r", 2, ALT_ENTER }, - { "\033[A", 3, UP }, - { "\033OA", 3, UP }, - { "\033[B", 3, DOWN }, - { "\033OB", 3, DOWN }, - { "\033[C", 3, RIGHT }, - { "\033OC", 3, RIGHT }, - { "\033[D", 3, LEFT }, - { "\033OD", 3, LEFT }, - { "\033[3~", 4, DEL }, - { "\033O3~", 4, DEL }, - { "\033[6~", 4, PAGE_DOWN }, - { "\033[5~", 4, PAGE_UP }, - { NULL, 0, 0 }, + KEY(ALT_ENTER, "\033\n"), + KEY(ALT_ENTER, "\033\r"), + KEY(BACKSPACE, "\177"), + KEY(CTRL_A, "\001"), + KEY(CTRL_E, "\005"), + KEY(CTRL_K, "\013"), + KEY(CTRL_U, "\025"), + KEY(CTRL_W, "\027"), + KEY(DEL, "\004"), + KEY(DEL, "\033O3~"), + KEY(DEL, "\033[3~"), + KEY(DOWN, "\016"), + KEY(DOWN, "\033OB"), + KEY(DOWN, "\033[B"), + KEY(ENTER, "\n"), + KEY(ENTER, "\r"), + KEY(LEFT, "\002"), + KEY(LEFT, "\033OD"), + KEY(LEFT, "\033[D"), + KEY(PAGE_DOWN, "\033[6~"), + KEY(PAGE_UP, "\033[5~"), + KEY(RIGHT, "\006"), + KEY(RIGHT, "\033OC"), + KEY(RIGHT, "\033[C"), + KEY(UP, "\020"), + KEY(UP, "\033OA"), + KEY(UP, "\033[A"), + { 0, NULL, 0 }, }; int c, i; From bfe26ae454a3ae2bc875da6492cf7057b71dc9ff Mon Sep 17 00:00:00 2001 From: Anton Lindqvist Date: Wed, 7 Jun 2017 15:26:00 +0200 Subject: [PATCH 2/4] Replace goto with a nested loop --- pick.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/pick.c b/pick.c index 590a0274..89455dc8 100644 --- a/pick.c +++ b/pick.c @@ -826,20 +826,22 @@ get_key(char *buf, size_t size, size_t *nread) int c, i; *nread = 0; -getc: - buf[(*nread)++] = tty_getc(); - size--; - for (i = 0; keys[i].s != NULL; i++) { - if (*nread > keys[i].length - || strncmp(buf, keys[i].s, *nread) != 0) - continue; + for (; size > 0; size--) { + buf[(*nread)++] = tty_getc(); + + for (i = 0; keys[i].s != NULL; i++) { + if (*nread > keys[i].length + || strncmp(buf, keys[i].s, *nread) != 0) + continue; - if (*nread == keys[i].length) - return keys[i].key; + if (*nread == keys[i].length) + return keys[i].key; - /* Partial match found, continue reading. */ - if (size > 0) - goto getc; + /* Partial match found, continue reading. */ + break; + } + if (keys[i].s == NULL) + break; } if (*nread > 1 && buf[0] == '\033' && (buf[1] == '[' || buf[1] == 'O')) { From 5b237d3418264434e6f4a85b869488c4fe881b09 Mon Sep 17 00:00:00 2001 From: Anton Lindqvist Date: Wed, 7 Jun 2017 15:27:43 +0200 Subject: [PATCH 3/4] Rework handling of unrecognized escape sequences Make sure to check the last read character prior reading a new one. --- pick.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/pick.c b/pick.c index 89455dc8..b2fbce05 100644 --- a/pick.c +++ b/pick.c @@ -846,14 +846,12 @@ get_key(char *buf, size_t size, size_t *nread) if (*nread > 1 && buf[0] == '\033' && (buf[1] == '[' || buf[1] == 'O')) { /* - * A escape sequence which is not a supported key is being read. - * Discard the rest of the sequence. + * An escape sequence which is not a supported key is being + * read. Discard the rest of the sequence. */ - for (;;) { + c = buf[(*nread) - 1]; + while (c < '@' || c > '~') c = tty_getc(); - if (c >= '@' && c <= '~') - break; - } return UNKNOWN; } From 74aa733b8f111e4238ec129fb29331c1dd0a4f72 Mon Sep 17 00:00:00 2001 From: Anton Lindqvist Date: Wed, 7 Jun 2017 15:35:36 +0200 Subject: [PATCH 4/4] Make get_key() return PRINTABLE - Removes the need to validate the input buffer in get_choices() - Returning an enum gives us nice compiler warnings if a new key type where to be added --- pick.c | 40 +++++++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/pick.c b/pick.c index b2fbce05..ac985d83 100644 --- a/pick.c +++ b/pick.c @@ -30,7 +30,7 @@ errx(1, #capability ": unknown terminfo capability"); \ } while (0) -enum { +enum key { UNKNOWN, ALT_ENTER, BACKSPACE, @@ -46,7 +46,8 @@ enum { DOWN, LEFT, PAGE_DOWN, - PAGE_UP + PAGE_UP, + PRINTABLE }; struct choice { @@ -63,7 +64,7 @@ static void delete_between(char *, size_t, size_t, size_t); static char *eager_strpbrk(const char *, const char *); static void filter_choices(void); static char *get_choices(void); -static int get_key(char *, size_t, size_t *); +static enum key get_key(char *, size_t, size_t *); static void handle_sigint(int); static int isu8cont(unsigned char); static int isu8start(unsigned char); @@ -275,7 +276,7 @@ selected_choice(void) size_t cursor_position, i, j, length; size_t xscroll = 0; char buf[6]; - int choices_count, key, word_position; + int choices_count, word_position; int selection = 0; int yscroll = 0; @@ -327,8 +328,7 @@ selected_choice(void) tty_putp(cursor_normal, 0); fflush(tty_out); - key = get_key(buf, sizeof(buf), &length); - switch (key) { + switch (get_key(buf, sizeof(buf), &length)) { case ENTER: if (choices_count > 0) return &choices.v[selection]; @@ -456,10 +456,7 @@ selected_choice(void) else yscroll = selection = 0; break; - default: - if (!isu8start(buf[0]) && !isprint(buf[0])) - continue; - + case PRINTABLE: if (query_length + length >= query_size) { query_size = 2*query_length + length; if ((query = reallocarray(query, query_size, @@ -478,6 +475,8 @@ selected_choice(void) query[query_length] = '\0'; filter_choices(); selection = yscroll = 0; + case UNKNOWN: + break; } } } @@ -785,12 +784,12 @@ print_choices(int offset, int selection) return i; } -int +enum key get_key(char *buf, size_t size, size_t *nread) { #define KEY(k, s) { k, s, sizeof(s) - 1 } static struct { - int key; + enum key key; const char *s; size_t length; } keys[] = { @@ -856,8 +855,12 @@ get_key(char *buf, size_t size, size_t *nread) return UNKNOWN; } - if (!isu8start(buf[0])) + if (!isu8start(buf[0])) { + if (isprint(buf[0])) + return PRINTABLE; + return UNKNOWN; + } /* * Ensure a whole Unicode character is read. The number of MSBs in the @@ -865,10 +868,17 @@ get_key(char *buf, size_t size, size_t *nread) * the character consists of, followed by a zero. Therefore, as long as * the MSB is not zero there is still bytes left to read. */ - while ((((unsigned int)buf[0] << *nread) & 0x80) == 0x80 && size-- > 0) + for (;;) { + if ((((unsigned int)buf[0] << *nread) & 0x80) == 0) + break; + if (size == 0) + return UNKNOWN; + buf[(*nread)++] = tty_getc(); + size--; + } - return UNKNOWN; + return PRINTABLE; } int