Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bash-on-Windows input handling doesn't account for alternate terminal modes #90

Closed
rprichard opened this issue Aug 14, 2016 · 2 comments

Comments

@rprichard
Copy link
Owner

A terminal can be placed into alternate modes where some keypresses generate alternate input escape sequences. For example, xterm/mintty typically generates
\e[A for Up, but if I first output \e[?1h, then Up instead generates \eOA. Outputting \e[?1l restores the normal mode.

The Windows console (as of 14393 at least), implements this mode-switching behavior. When I tested, the console modes reported by GetConsoleMode (input and output) did not change as the terminal arrow key mode was toggled.

This issue affects Ubuntu 14.04's Midnight Commander, which apparently expects the "O" versions of the arrow key escapes. The arrow keys have no effect, other than to suppress all keyboard input for a moment while mc discards the input buffer.

Aside: While it looks like mc should also be OK with the normal escapes, at least in Ubuntu 14.04, the program's config file handling is buggy, so it doesn't work. (See here and here). When it iterates over the profile keys, it does so by string value, so it sees up twice, but looks up the \\eOA value both times. I tried to expose the bug more easily by swapping the two lines around in a normal, working configuration, but that didn't expose the failure. I had noticed that mc also reads a terminal database somehow (ncurses / SLang?), where it also finds the "O" escapes. I assume that explains it.

Maybe it's possible to fix this issue by sending some keys to the console HWND rather than calling WriteConsoleInput. I think winpty would have to send the keys synchronously. It already uses the synchronous SendMessage for freezing/unfreezing. I don't think it can do that for all keys, because the console will interpret some of them. I think it's OK to send a key even if the console input buffer is non-empty; the console should append the new key.

@rprichard
Copy link
Owner Author

It looks like the \e[?1h sequence is known as DECCKM. It's documented here: http://man7.org/linux/man-pages/man4/console_codes.4.html. DECPAM and DECPNM are the other mode-switching escapes I'm aware of, but maybe there are others.

@rprichard
Copy link
Owner Author

SendMessage seemed to fix the cursor key issue. I need to review it more closely, though. Here's my patch:

diff --git a/src/agent/Agent.cc b/src/agent/Agent.cc
index 748c540..5d85fe3 100644
--- a/src/agent/Agent.cc
+++ b/src/agent/Agent.cc
@@ -208,7 +208,8 @@ Agent::Agent(LPCWSTR controlPipeName,
     m_console.setTitle(m_currentTitle);

     const HANDLE conin = GetStdHandle(STD_INPUT_HANDLE);
-    m_consoleInput.reset(new ConsoleInput(conin, m_mouseMode, *this));
+    m_consoleInput.reset(
+        new ConsoleInput(conin, m_mouseMode, *this, m_console));

     // Setup Ctrl-C handling.  First restore default handling of Ctrl-C.  This
     // attribute is inherited by child processes.  Then register a custom
diff --git a/src/agent/ConsoleInput.cc b/src/agent/ConsoleInput.cc
index 0f4950f..145a983 100644
--- a/src/agent/ConsoleInput.cc
+++ b/src/agent/ConsoleInput.cc
@@ -191,7 +191,9 @@ static int matchMouseRecord(const char *input, int inputSize, MouseRecord &out)

 } // anonymous namespace

-ConsoleInput::ConsoleInput(HANDLE conin, int mouseMode, DsrSender &dsrSender) :
+ConsoleInput::ConsoleInput(HANDLE conin, int mouseMode, DsrSender &dsrSender,
+                           Win32Console &console) :
+    m_console(console),
     m_conin(conin),
     m_mouseMode(mouseMode),
     m_dsrSender(dsrSender)
@@ -592,15 +604,36 @@ void ConsoleInput::appendKeyPress(std::vector<INPUT_RECORD> &records,
     const bool ctrl = keyState & LEFT_CTRL_PRESSED;
     const bool alt = keyState & LEFT_ALT_PRESSED;
     const bool shift = keyState & SHIFT_PRESSED;
+    bool hasDebugInput = false;

     if (isTracingEnabled()) {
         static bool debugInput = hasDebugFlag("input");
         if (debugInput) {
+            hasDebugInput = true;
             InputMap::Key key = { virtualKey, codePoint, keyState };
             trace("keypress: %s", key.toString().c_str());
         }
     }

+    if (m_escapeInputEnabled &&
+            (virtualKey == VK_UP ||
+                virtualKey == VK_DOWN ||
+                virtualKey == VK_LEFT ||
+                virtualKey == VK_RIGHT)) {
+        if (hasDebugInput) {
+            trace("sending keypress to console HWND");
+        }
+        uint32_t scanCode = MapVirtualKey(virtualKey, MAPVK_VK_TO_VSC);
+        if (scanCode > 255) {
+            scanCode = 0;
+        }
+        SendMessage(m_console.hwnd(), WM_KEYDOWN, virtualKey,
+            (scanCode << 16) | 1u);
+        SendMessage(m_console.hwnd(), WM_KEYUP, virtualKey,
+            (scanCode << 16) | (1u | (1u << 30) | (1u << 31)));
+        return;
+    }
+
     uint16_t stepKeyState = 0;
     if (ctrl) {
         stepKeyState |= LEFT_CTRL_PRESSED;
diff --git a/src/agent/ConsoleInput.h b/src/agent/ConsoleInput.h
index 31a9295..e71ebbb 100644
--- a/src/agent/ConsoleInput.h
+++ b/src/agent/ConsoleInput.h
@@ -38,7 +38,8 @@ class DsrSender;
 class ConsoleInput
 {
 public:
-    ConsoleInput(HANDLE conin, int mouseMode, DsrSender &dsrSender);
+    ConsoleInput(HANDLE conin, int mouseMode, DsrSender &dsrSender,
+                 Win32Console &console);
     void writeInput(const std::string &input);
     void flushIncompleteEscapeCode();
     void setMouseWindowRect(SmallRect val) { m_mouseWindowRect = val; }
@@ -79,6 +80,7 @@ private:
     DWORD inputConsoleMode();

 private:
+    Win32Console &m_console;
     HANDLE m_conin = nullptr;
     int m_mouseMode = 0;
     DsrSender &m_dsrSender;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant