From 0db9ff66830cad226aa447ab51a548006e642ae5 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 16 Mar 2022 18:02:20 +0100 Subject: [PATCH] Console.ReadKey should print a char, not an integer (#66609) Co-authored-by: Jan Kotas --- .../Kernel32/Interop.PeekConsoleInput.cs | 2 +- .../Kernel32/Interop.ReadConsoleInput.cs | 26 ++++++------ .../src/System/ConsolePal.Windows.cs | 40 +++++++++---------- .../tests/ManualTests/ManualTests.cs | 11 +++++ 4 files changed, 46 insertions(+), 33 deletions(-) diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.PeekConsoleInput.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.PeekConsoleInput.cs index 23b9e80be3a71..d3d65f504cd7d 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.PeekConsoleInput.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.PeekConsoleInput.cs @@ -10,6 +10,6 @@ internal static partial class Kernel32 { [LibraryImport(Libraries.Kernel32, EntryPoint = "PeekConsoleInputW", SetLastError = true, StringMarshalling = StringMarshalling.Utf16)] [return: MarshalAs(UnmanagedType.Bool)] - internal static partial bool PeekConsoleInput(IntPtr hConsoleInput, out InputRecord buffer, int numInputRecords_UseOne, out int numEventsRead); + internal static partial bool PeekConsoleInput(IntPtr hConsoleInput, out INPUT_RECORD buffer, int numInputRecords_UseOne, out int numEventsRead); } } diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.ReadConsoleInput.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.ReadConsoleInput.cs index 50367574e2edd..a8c6cf1cc872a 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.ReadConsoleInput.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.ReadConsoleInput.cs @@ -8,24 +8,26 @@ internal static partial class Interop { internal const short KEY_EVENT = 1; - // Windows's KEY_EVENT_RECORD [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Unicode)] - internal struct KeyEventRecord + internal struct KEY_EVENT_RECORD { - internal BOOL keyDown; - internal short repeatCount; - internal short virtualKeyCode; - internal short virtualScanCode; - internal ushort uChar; // Union between WCHAR and ASCII char - internal int controlKeyState; + internal BOOL bKeyDown; + internal ushort wRepeatCount; + internal ushort wVirtualKeyCode; + internal ushort wVirtualScanCode; + private ushort _uChar; // Union between WCHAR and ASCII char + internal uint dwControlKeyState; + + // _uChar is stored as short to avoid any ambiguity for interop marshaling + internal char uChar => (char)_uChar; } // Really, this is a union of KeyEventRecords and other types. [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Unicode)] - internal struct InputRecord + internal struct INPUT_RECORD { - internal short eventType; - internal KeyEventRecord keyEvent; + internal ushort EventType; + internal KEY_EVENT_RECORD keyEvent; // This struct is a union! Word alignment should take care of padding! } @@ -34,6 +36,6 @@ internal static partial class Kernel32 { [LibraryImport(Libraries.Kernel32, EntryPoint = "ReadConsoleInputW", SetLastError = true, StringMarshalling = StringMarshalling.Utf16)] [return: MarshalAs(UnmanagedType.Bool)] - internal static partial bool ReadConsoleInput(IntPtr hConsoleInput, out InputRecord buffer, int numInputRecords_UseOne, out int numEventsRead); + internal static partial bool ReadConsoleInput(IntPtr hConsoleInput, out INPUT_RECORD buffer, int numInputRecords_UseOne, out int numEventsRead); } } diff --git a/src/libraries/System.Console/src/System/ConsolePal.Windows.cs b/src/libraries/System.Console/src/System/ConsolePal.Windows.cs index 32cb7659d5193..028dec358a854 100644 --- a/src/libraries/System.Console/src/System/ConsolePal.Windows.cs +++ b/src/libraries/System.Console/src/System/ConsolePal.Windows.cs @@ -177,24 +177,24 @@ internal static TextReader GetOrCreateReader() // we will lose repeated keystrokes when someone switches from // calling ReadKey to calling Read or ReadLine. Those methods should // ideally flush this cache as well. - private static Interop.InputRecord _cachedInputRecord; + private static Interop.INPUT_RECORD _cachedInputRecord; // Skip non key events. Generally we want to surface only KeyDown event // and suppress KeyUp event from the same Key press but there are cases // where the assumption of KeyDown-KeyUp pairing for a given key press // is invalid. For example in IME Unicode keyboard input, we often see // only KeyUp until the key is released. - private static bool IsKeyDownEvent(Interop.InputRecord ir) + private static bool IsKeyDownEvent(Interop.INPUT_RECORD ir) { - return (ir.eventType == Interop.KEY_EVENT && ir.keyEvent.keyDown != Interop.BOOL.FALSE); + return (ir.EventType == Interop.KEY_EVENT && ir.keyEvent.bKeyDown != Interop.BOOL.FALSE); } - private static bool IsModKey(Interop.InputRecord ir) + private static bool IsModKey(Interop.INPUT_RECORD ir) { // We should also skip over Shift, Control, and Alt, as well as caps lock. // Apparently we don't need to check for 0xA0 through 0xA5, which are keys like // Left Control & Right Control. See the ConsoleKey enum for these values. - short keyCode = ir.keyEvent.virtualKeyCode; + ushort keyCode = ir.keyEvent.wVirtualKeyCode; return ((keyCode >= 0x10 && keyCode <= 0x12) || keyCode == 0x14 || keyCode == 0x90 || keyCode == 0x91); } @@ -218,9 +218,9 @@ internal enum ControlKeyState // desired effect is to translate the sequence into one Unicode KeyPress. // We need to keep track of the Alt+NumPad sequence and surface the final // unicode char alone when the Alt key is released. - private static bool IsAltKeyDown(Interop.InputRecord ir) + private static bool IsAltKeyDown(Interop.INPUT_RECORD ir) { - return (((ControlKeyState)ir.keyEvent.controlKeyState) + return (((ControlKeyState)ir.keyEvent.dwControlKeyState) & (ControlKeyState.LeftAltPressed | ControlKeyState.RightAltPressed)) != 0; } @@ -269,12 +269,12 @@ public static bool KeyAvailable { get { - if (_cachedInputRecord.eventType == Interop.KEY_EVENT) + if (_cachedInputRecord.EventType == Interop.KEY_EVENT) return true; while (true) { - bool r = Interop.Kernel32.PeekConsoleInput(InputHandle, out Interop.InputRecord ir, 1, out int numEventsRead); + bool r = Interop.Kernel32.PeekConsoleInput(InputHandle, out Interop.INPUT_RECORD ir, 1, out int numEventsRead); if (!r) { int errorCode = Marshal.GetLastPInvokeError(); @@ -306,20 +306,20 @@ public static bool KeyAvailable public static ConsoleKeyInfo ReadKey(bool intercept) { - Interop.InputRecord ir; + Interop.INPUT_RECORD ir; bool r; lock (s_readKeySyncObject) { - if (_cachedInputRecord.eventType == Interop.KEY_EVENT) + if (_cachedInputRecord.EventType == Interop.KEY_EVENT) { // We had a previous keystroke with repeated characters. ir = _cachedInputRecord; - if (_cachedInputRecord.keyEvent.repeatCount == 0) - _cachedInputRecord.eventType = -1; + if (_cachedInputRecord.keyEvent.wRepeatCount == 0) + _cachedInputRecord.EventType = 0; else { - _cachedInputRecord.keyEvent.repeatCount--; + _cachedInputRecord.keyEvent.wRepeatCount--; } // We will return one key from this method, so we decrement the // repeatCount here, leaving the cachedInputRecord in the "queue". @@ -339,7 +339,7 @@ public static ConsoleKeyInfo ReadKey(bool intercept) throw new InvalidOperationException(SR.InvalidOperation_ConsoleReadKeyOnFile); } - short keyCode = ir.keyEvent.virtualKeyCode; + ushort keyCode = ir.keyEvent.wVirtualKeyCode; // First check for non-keyboard events & discard them. Generally we tap into only KeyDown events and ignore the KeyUp events // but it is possible that we are dealing with a Alt+NumPad unicode key sequence, the final unicode char is revealed only when @@ -353,7 +353,7 @@ public static ConsoleKeyInfo ReadKey(bool intercept) continue; } - char ch = (char)ir.keyEvent.uChar; + char ch = ir.keyEvent.uChar; // In a Alt+NumPad unicode sequence, when the alt key is released uChar will represent the final unicode character, we need to // surface this. VirtualKeyCode for this event will be Alt from the Alt-Up key event. This is probably not the right code, @@ -377,9 +377,9 @@ public static ConsoleKeyInfo ReadKey(bool intercept) continue; } - if (ir.keyEvent.repeatCount > 1) + if (ir.keyEvent.wRepeatCount > 1) { - ir.keyEvent.repeatCount--; + ir.keyEvent.wRepeatCount--; _cachedInputRecord = ir; } break; @@ -387,12 +387,12 @@ public static ConsoleKeyInfo ReadKey(bool intercept) } // we did NOT have a previous keystroke with repeated characters. } - ControlKeyState state = (ControlKeyState)ir.keyEvent.controlKeyState; + ControlKeyState state = (ControlKeyState)ir.keyEvent.dwControlKeyState; bool shift = (state & ControlKeyState.ShiftPressed) != 0; bool alt = (state & (ControlKeyState.LeftAltPressed | ControlKeyState.RightAltPressed)) != 0; bool control = (state & (ControlKeyState.LeftCtrlPressed | ControlKeyState.RightCtrlPressed)) != 0; - ConsoleKeyInfo info = new ConsoleKeyInfo((char)ir.keyEvent.uChar, (ConsoleKey)ir.keyEvent.virtualKeyCode, shift, alt, control); + ConsoleKeyInfo info = new ConsoleKeyInfo(ir.keyEvent.uChar, (ConsoleKey)ir.keyEvent.wVirtualKeyCode, shift, alt, control); if (!intercept) Console.Write(ir.keyEvent.uChar); diff --git a/src/libraries/System.Console/tests/ManualTests/ManualTests.cs b/src/libraries/System.Console/tests/ManualTests/ManualTests.cs index 322322d083633..367b711128b23 100644 --- a/src/libraries/System.Console/tests/ManualTests/ManualTests.cs +++ b/src/libraries/System.Console/tests/ManualTests/ManualTests.cs @@ -121,6 +121,17 @@ public static void ReadKey() AssertUserExpectedResults("\"console\" correctly not echoed as you typed it"); } + [ConditionalFact(nameof(ManualTestsEnabled))] + public static void ReadKeyNoIntercept() + { + Console.WriteLine("Please type \"console\" (without the quotes). You should see it as you type:"); + foreach (ConsoleKey k in new[] { ConsoleKey.C, ConsoleKey.O, ConsoleKey.N, ConsoleKey.S, ConsoleKey.O, ConsoleKey.L, ConsoleKey.E }) + { + Assert.Equal(k, Console.ReadKey(intercept: false).Key); + } + AssertUserExpectedResults("\"console\" correctly echoed as you typed it"); + } + [ConditionalFact(nameof(ManualTestsEnabled))] public static void EnterKeyIsEnterAfterKeyAvailableCheck() {