fix handling input in the C locale

In the C/POSIX locale EOF on the tty wasn't handled correctly due to a change a few months ago to fix an unrelated problem with that locale. What is surprising is that the core fish code doesn't explicitly depend on input_common_readch returning WEOF if a character isn't seen within `wait_on_escape_ms` after an escape. Fixes fish-shell#3214
nomaed · Jul 17, 2017 · a643cd4 · a643cd4
1 parent 5e72b0d
commit a643cd4
Show file tree

Hide file tree

Showing 11 changed files with 35 additions and 35 deletions.
diff --git a/po/de.po b/po/de.po
@@ -1291,8 +1291,8 @@ msgstr ""
 
 #: reader.cpp:4115
 #, c-format
-msgid "Unknown keybinding %d"
-msgstr "Unbekannte Tastenkombination %d"
+msgid "Unknown key binding 0x%X"
+msgstr "Unbekannte Tastenkombination 0x%X"
 
 #: reader.cpp:4226
 #, fuzzy

diff --git a/po/en.po b/po/en.po
@@ -1284,8 +1284,8 @@ msgstr ""
 
 #: reader.cpp:4115
 #, c-format
-msgid "Unknown keybinding %d"
-msgstr "Unknown keybinding %d"
+msgid "Unknown key binding 0x%X"
+msgstr "Unknown key binding 0x%X"
 
 #: reader.cpp:4226
 msgid "Error while reading from file descriptor"

diff --git a/po/fr.po b/po/fr.po
@@ -1306,8 +1306,8 @@ msgstr ""
 
 #: reader.cpp:4115
 #, c-format
-msgid "Unknown keybinding %d"
-msgstr "Raccourci clavier inconnu %d"
+msgid "Unknown key binding 0x%X"
+msgstr "Raccourci clavier inconnu 0x%X"
 
 #: reader.cpp:4226
 #, fuzzy

diff --git a/po/pt_BR.po b/po/pt_BR.po
@@ -1306,8 +1306,8 @@ msgstr ""
 
 #: reader.cpp:4115
 #, c-format
-msgid "Unknown keybinding %d"
-msgstr "Atalho desconhecido %d"
+msgid "Unknown key binding 0x%X"
+msgstr "Atalho desconhecido 0x%X"
 
 #: reader.cpp:4226
 msgid "Error while reading from file descriptor"

diff --git a/po/sv.po b/po/sv.po
@@ -1295,8 +1295,8 @@ msgstr ""
 
 #: reader.cpp:4115
 #, c-format
-msgid "Unknown keybinding %d"
-msgstr "Okänd tangentbordsbindning %d"
+msgid "Unknown key binding 0x%X"
+msgstr "Okänd tangentbords binding 0x%X"
 
 #: reader.cpp:4226
 msgid "Error while reading from file descriptor"

diff --git a/po/zh_CN.po b/po/zh_CN.po
@@ -1273,8 +1273,8 @@ msgstr ""
 
 #: reader.cpp:4115
 #, c-format
-msgid "Unknown keybinding %d"
-msgstr "Unknown keybinding %d"
+msgid "Unknown key binding 0x%X"
+msgstr "Unknown key binding 0x%X"
 
 #: reader.cpp:4226
 msgid "Error while reading from file descriptor"

diff --git a/src/fish_key_reader.cpp b/src/fish_key_reader.cpp
@@ -16,7 +16,6 @@
 #include <string.h>
 #include <unistd.h>
 #include <wchar.h>
-#include <wctype.h>
 #include <memory>
 #include <string>
 #include <vector>
@@ -203,7 +202,7 @@ static void process_input(bool continuous_mode) {
     fprintf(stderr, "Press a key\n\n");
     while (keep_running) {
         wchar_t wc = input_common_readch(true);
-        if (wc == WEOF) {
+        if (wc == R_TIMEOUT || wc == R_EOF) {
             output_bind_command(bind_chars);
             if (first_char_seen && !continuous_mode) {
                 return;

diff --git a/src/input.cpp b/src/input.cpp
@@ -419,7 +419,7 @@ void input_function_push_args(int code) {
         wchar_t arg;
 
         // Skip and queue up any function codes. See issue #2357.
-        while (((arg = input_common_readch(0)) >= R_MIN) && (arg <= R_MAX)) {
+        while ((arg = input_common_readch(0)) >= R_MIN && arg <= R_MAX) {
             skipped.push_back(arg);
         }
 
@@ -497,7 +497,7 @@ static bool input_mapping_is_match(const input_mapping_t &m) {
     wint_t c = 0;
     int j;
 
-    // debug(0, L"trying mapping %ls\n", escape(m.seq.c_str(), ESCAPE_ALL).c_str());
+    debug(2, L"trying to match mapping %ls", escape(m.seq.c_str(), ESCAPE_ALL).c_str());
     const wchar_t *str = m.seq.c_str();
     for (j = 0; str[j] != L'\0'; j++) {
         bool timed = (j > 0 && iswcntrl(str[0]));
@@ -515,7 +515,8 @@ static bool input_mapping_is_match(const input_mapping_t &m) {
         return true;
     }
 
-    // Return the read characters.
+    // Reinsert the chars we read to be read again since we didn't match the bind sequence (i.e.,
+    // the input mapping).
     input_common_next_ch(c);
     for (int k = j - 1; k >= 0; k--) {
         input_common_next_ch(m.seq[k]);
@@ -554,9 +555,11 @@ static void input_mapping_execute_matching_or_generic(bool allow_commands) {
     if (generic) {
         input_mapping_execute(*generic, allow_commands);
     } else {
-        // debug(0, L"no generic found, ignoring...");
+        debug(2, L"no generic found, ignoring char...");
         wchar_t c = input_common_readch(0);
-        if (c == R_EOF) input_common_next_ch(c);
+        if (c == R_EOF) {
+            input_common_next_ch(c);
+        }
     }
 }
 

diff --git a/src/input_common.cpp b/src/input_common.cpp
@@ -16,8 +16,6 @@
 #include <sys/time.h>
 #include <sys/types.h>
 #include <wchar.h>
-#include <wctype.h>
-#include <cwctype>
 #include <memory>
 
 #include "common.h"
@@ -35,7 +33,7 @@
 static int wait_on_escape_ms = WAIT_ON_ESCAPE_DEFAULT;
 
 /// Characters that have been read and returned by the sequence matching code.
-static std::deque<wint_t> lookahead_list;
+static std::deque<wchar_t> lookahead_list;
 
 // Queue of pairs of (function pointer, argument) to be invoked. Expected to be mostly empty.
 typedef std::pair<void (*)(void *), void *> callback_info_t;
@@ -203,7 +201,7 @@ wchar_t input_common_readch(int timed) {
             struct timeval tm = {wait_on_escape_ms / 1000, 1000 * (wait_on_escape_ms % 1000)};
             int count = select(1, &fds, 0, 0, &tm);
             if (count <= 0) {
-                return WEOF;
+                return R_TIMEOUT;
             }
         }
 
@@ -213,12 +211,12 @@ wchar_t input_common_readch(int timed) {
         while (1) {
             wint_t b = readb();
 
-            if (MB_CUR_MAX == 1)  // single-byte locale, all values are legal
-            {
-                return (unsigned char)b;
-            }
+            if (b >= R_NULL && b <= R_MAX) return b;
 
-            if ((b >= R_NULL) && (b < R_NULL + 1000)) return b;
+            if (MB_CUR_MAX == 1) {
+                // return (unsigned char)b;  // single-byte locale, all values are legal
+                return b;  // single-byte locale, all values are legal
+            }
 
             char bb = b;
             size_t sz = mbrtowc(&res, &bb, 1, &state);
@@ -240,7 +238,7 @@ wchar_t input_common_readch(int timed) {
         }
     } else {
         if (!timed) {
-            while (has_lookahead() && lookahead_front() == WEOF) lookahead_pop();
+            while (has_lookahead() && lookahead_front() == R_TIMEOUT) lookahead_pop();
             if (!has_lookahead()) return input_common_readch(0);
         }
 

diff --git a/src/input_common.h b/src/input_common.h
@@ -70,6 +70,7 @@ enum {
     R_BACKWARD_JUMP,
     R_AND,
     R_CANCEL,
+    R_TIMEOUT,  // we didn't get interactive input within wait_on_escape_ms
     R_MAX = R_CANCEL,
     // This is a special psuedo-char that is not used other than to mark the end of the the special
     // characters so we can sanity check the enum range.
@@ -92,7 +93,7 @@ void set_wait_on_escape_ms(int ms);
 /// convert them to a wchar_t. Conversion is done using mbrtowc. If a character has previously been
 /// read and then 'unread' using \c input_common_unreadch, that character is returned. If timed is
 /// true, readch2 will wait at most WAIT_ON_ESCAPE milliseconds for a character to be available for
-/// reading before returning with the value WEOF.
+/// reading before returning with the value R_EOF.
 wchar_t input_common_readch(int timed);
 
 /// Enqueue a character or a readline function to the queue of unread characters that input_readch

diff --git a/src/reader.cpp b/src/reader.cpp
@@ -3250,7 +3250,7 @@ const wchar_t *reader_readline(int nchars) {
             }
             default: {
                 // Other, if a normal character, we add it to the command.
-                if ((!wchar_private(c)) && (((c > 31) || (c == L'\n')) && (c != 127))) {
+                if (!wchar_private(c) && (c >= L' ' || c == L'\n' || c == L'\r') && c != 0x7F) {
                     bool allow_expand_abbreviations = false;
                     if (data->is_navigating_pager_contents()) {
                         data->pager.set_search_field_shown(true);
@@ -3266,11 +3266,10 @@ const wchar_t *reader_readline(int nchars) {
                     if (el == &data->command_line) {
                         clear_pager();
                     }
-
                 } else {
-                    // Low priority debug message. These can happen if the user presses an unefined
-                    // control sequnece. No reason to report.
-                    debug(2, _(L"Unknown keybinding %d"), c);
+                    // This can happen if the user presses a control char we don't recognize. No
+                    // reason to report this to the user unless they've enabled debugging output.
+                    debug(2, _(L"Unknown key binding 0x%X"), c);
                 }
                 break;
             }