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

Bring back Console.ReadKey() #14354

Closed
MattGal opened this issue Mar 16, 2015 · 20 comments
Closed

Bring back Console.ReadKey() #14354

MattGal opened this issue Mar 16, 2015 · 20 comments
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation area-System.Console
Milestone

Comments

@MattGal
Copy link
Member

MattGal commented Mar 16, 2015

For reference, see issue #13945 and the related aspnet/Hosting#140. Console.ReadKey() can solve both issues (thought explicit support for CancelKeyPress is nice as that can handle different keyboard configurations etc).

ReadKey() is a helpful API for many console app scenarios and is quite difficult for users to implement in a non-Windows-specific way if they want to get the same functionality. Please consider bringing it back.

@ellismg ellismg self-assigned this Mar 17, 2015
@txdv
Copy link
Contributor

txdv commented Mar 22, 2015

What do you mean with ' is quite difficult for users to implement in a non-Windows-specific way'? Is it hard to implement on other platforms?

@ghost
Copy link

ghost commented Apr 30, 2015

BTW, both tagged issues are resolved.

@ellismg
Copy link
Contributor

ellismg commented May 20, 2015

@MattGal Do you know if we still need this?

@mspeterhon
Copy link

yes, this would be super helpful, especially to get modifier keys and keys like escape and tab

@MattGal
Copy link
Member Author

MattGal commented Jun 24, 2015

@ellismg I believe the scenario of Ctrl-C being the specific key combination to kill hosting an app is something that hasn't been addressed yet, so it'd be nice to have. In my experience, I don't find it to be in any way blocking so I am OK if it doesn't happen.

@NickCraver
Copy link
Member

I'm currently hitting a wall without ReadKey() because as far as I'm aware, there's no other way to read and mask console input as it arrives, e.g. when entering a password. Unfortunately, I'll likely have to abandon DNX for a string of projects if I can't do something so simple.

Can we please bring this back? It is rather important.

There are other use cases as well, for example hitting y/n/a for continue without needing to hit enter as well, which is counter-intuitive since with most existing programs and prompts (including system utilities on all platforms) enter is not required. The need in that use case is getting input back to the program without needing enter/return.

@akoeplinger
Copy link
Member

@NickCraver if you're fine with a Windows-only implementation as a temporary workaround you could leverage the implementation in ReferenceSource. I agree this is an important API that should be brought back in corefx.

@weshaggard
Copy link
Member

We definitely need to have a more holistic look at our Console support. It is on our list but we haven't focused on it yet. I suspect this is one we will likely need to include.

@weshaggard weshaggard assigned pallavit and unassigned ellismg Aug 29, 2015
@NickCraver
Copy link
Member

To add to the use cases here. On most programs on most platforms where a prompt is involved you'll see something to the effect of:

Continue? (y/n):

This currently isn't possible, you have to do a much less intuitive "(y/n then enter)" without .ReadKey() which allows processing per-character. It's unintuitive because of violating the principle least-surprise - matching the 99% behavior.

In my use case I've (very hackily) re-implemented the bare bones of .ReadKey(bool interrupt) to get past the lack of things here, and using the reference source @akoeplinger refers to above. I needed it for both the prompts to continue as well as (more importantly) password masking on entry. However, it locks me down to Windows (and only NT-based versions at that, since CharSet.Auto isn't present), and eliminates my ability to run on OS X.

To get an idea of what I currently resorted to, here's the very minimal implementation that pretty much only supports my one-off use case:

Note to others: please do not use this, it is not suitable for security sensitive multi-tenant environments and is just a minimum viable implementation

public struct TempKeyInfo
{
    public char KeyChar;
    public short KeyCode;
}

public static class TempConsoleKey
{
    public static short Backspace = 8;
    public static short Enter = 13;
}

// Temporary implementation until corefx gets ReadKey
public class TempConsole
{
    [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Unicode)]
    internal struct InputRecord
    {
        internal short eventType;
        internal KeyEventRecord keyEvent;
    }

    [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Unicode)]
    internal struct KeyEventRecord
    {
        internal bool keyDown;
        internal short repeatCount;
        internal short virtualKeyCode;
        internal short virtualScanCode;
        internal char uChar;
        internal int controlKeyState;
    }

    [DllImport("kernel32.dll", SetLastError = true)]
    internal static extern IntPtr GetStdHandle(int nStdHandle);

    [DllImport("kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true)]
    internal static extern bool ReadConsoleInput(IntPtr hConsoleInput, out InputRecord buffer, int numInputRecords_UseOne, out int numEventsRead);

    private static InputRecord _cachedInputRecord;
    private static volatile IntPtr _consoleInputHandle;
    private static IntPtr ConsoleInputHandle
    {
        get
        {
            if (_consoleInputHandle == IntPtr.Zero)
                _consoleInputHandle = GetStdHandle(-10);
            return _consoleInputHandle;
        }
    }

    public static TempKeyInfo ReadKey()
    {
        InputRecord ir;
        int numEventsRead = -1;
        bool r;

        if (_cachedInputRecord.eventType == 1)
        {
            ir = _cachedInputRecord;
            if (_cachedInputRecord.keyEvent.repeatCount == 0)
                _cachedInputRecord.eventType = -1;
            else
                _cachedInputRecord.keyEvent.repeatCount--;
        }
        else
        {
            while (true)
            {
                r = ReadConsoleInput(ConsoleInputHandle, out ir, 1, out numEventsRead);
                if (!r || numEventsRead == 0)
                {
                    throw new InvalidOperationException("Cannot read keys when either application does not have a console or when console input has been redirected from a file. Try Console.Read.");
                }

                short keyCode = ir.keyEvent.virtualKeyCode;
                if (!(ir.eventType == 1 && ir.keyEvent.keyDown) && keyCode != 0x12) continue;

                char ch = (char)ir.keyEvent.uChar;
                if (ch == 0 && ((keyCode >= 0x10 && keyCode <= 0x12) || keyCode == 0x14 || keyCode == 0x90 || keyCode == 0x91)) continue;

                if (ir.keyEvent.repeatCount > 1)
                {
                    ir.keyEvent.repeatCount--;
                    _cachedInputRecord = ir;
                }
                break;
            }
        }

        return new TempKeyInfo
        {
            KeyChar = (char)ir.keyEvent.uChar,
            KeyCode = ir.keyEvent.virtualKeyCode
        };
    }
}

Please, please, bring back the essentials of Console, and let me delete the abomination above :)

@anuchandy
Copy link

+1, having Console.ReadKey in CoreCLR is really required functionality for the project we are building on CoreCLR.

@andyleejordan
Copy link
Member

👍 likewise, this is required functionality we need.

@pallavit
Copy link
Contributor

I will start looking at the windows implementation soon. From the cross-plat perspective, I could not find a way to capture virtual key codes. One alternative would be to leverage Console.Read() itself to get the ASCII code however that would leave a whole set of keys as unimplemented - modiifier keys (Shift, Alt), function keys etc. Thoughts?

@andyleejordan
Copy link
Member

If we don't have the modifier keys, we would be unable to implement commonly expected shortcuts such as Ctrl-A and Ctrl-E for home and end in our custom consoles.

@pallavit
Copy link
Contributor

I don't think Ctrl key will be the problem. Looking at the https://en.wikipedia.org/wiki/ASCII all the ctrl keys can be correctly intercepted, however, I am not sure, how would we intercept Alt, Shift or function keys.

@pallavit
Copy link
Contributor

@stephentoub Do you have any insights into its implementation cross-plat?

@pallavit
Copy link
Contributor

I think we can implement Console.KeyAvailable by using poll() API.

@stephentoub
Copy link
Member

To my knowledge, there's no direct equivalent to "what key is currently pressed" as you have on Windows. Rather, various character sequences are written to the console to represent the various keys being pressed. For the most part, these sequences are discernable by reading the terminfo file. It contains the descriptions for the given terminal of what character sequence will be sent for what key. If you look at "man 5 terminfo", you'll see all of the capabilities enumerated, including ones that, for example, provide the sequences used for each of the function keys. The only capabilities we're currently reading are the strings for foreground/background color, so I was lazy and only extracted those two values in the code, but we could certainly augment the code to have the names/values for all of the relevant capabilities we care about (all of the terminfo parsing support is there), and modify the console code to read those out (and cache them) and then use them in ReadKey to understand what's being sent. I believe alt generally sends an escape (27). Not sure about shift... might need to approximate that based on the case of letters, though it's also possible there's some terminfo sequence for it.

@pallavit
Copy link
Contributor

@stephentoub Thanks for the direction. I will take a look at it :)

pallavit referenced this issue in pallavit/corefx Oct 17, 2015
This change tries to enable #1153.

On Windows the implementation is pretty straight-forward. We have APIs to capture the standard input and APIs that tell us the kind of key and modifier is pressed.

On Unix, there is no concept of virtual key codes and so, the implementation is mostly best case effort.
 The current implementation works as follows.
 1. Capture stdin in the raw mode and reads as much of buffer as possible (max = 255 bytes).
 2. Try to match the buffer with any of the special keys which it maintains in the db from the static string capabilities from TermInfo.
 3. If not, it simply tries to use the ASCII key chart and map it to an equivalent ConsoleKey.

It also tries to second guess the modifier keys as follows
1. Alt - This is captured as esc key, so, we try to check if at the time of processing we can a combination of ESC + known key and if so, we assume alt was pressed.
 2. Ctrl - Other than the known keys like \b \t and \n, the rest of the ASCII key charts from 1 to 26 are considered typed with Ctrl + the alphabet.
 3. Shift - Any capital A to Z are assumed to be pressed with Shift key.

Not all keys are captured by this logic. For example, on my keyboard, NumLock + No. key results in a byte sequence, that can't be mapped to the no. itself. So, Unix implementation can never return ConsoleKey.NumPadX, Similarly combinations of Alt+FunctionKeys can't be interpreted correctly and are interpreted as Esc + the individual keys. Since not all key presses can be mapped to the correct ConsoleKey, the implementation tries to ignore 1 byte in case it can't be mapped to anything and continue to interpret the next byte. This is done because (ConsoleKey)0 is invalid. So, even if we can get the int representation of the byte being processed, we can't really create a valid ConsoleKeyInfo from it.
pallavit referenced this issue in pallavit/corefx Oct 17, 2015
This change tries to enable #1153.

On Windows the implementation is pretty straight-forward. We have APIs to capture the standard input and APIs that tell us the kind of key and modifier is pressed.

On Unix, there is no concept of virtual key codes and so, the implementation is mostly best case effort.
 The current implementation works as follows.
 1. Capture stdin in the raw mode and reads as much of buffer as possible (max = 255 bytes).
 2. Try to match the buffer with any of the special keys which it maintains in the db from the static string capabilities from TermInfo.
 3. If not, it simply tries to use the ASCII key chart and map it to an equivalent ConsoleKey.

It also tries to second guess the modifier keys as follows
1. Alt - This is captured as esc key, so, we try to check if at the time of processing we can a combination of ESC + known key and if so, we assume alt was pressed.
 2. Ctrl - Other than the known keys like \b \t and \n, the rest of the ASCII key charts from 1 to 26 are considered typed with Ctrl + the alphabet.
 3. Shift - Any capital A to Z are assumed to be pressed with Shift key.

Not all keys are captured by this logic. For example, on my keyboard, NumLock + No. key results in a byte sequence, that can't be mapped to the no. itself. So, Unix implementation can never return ConsoleKey.NumPadX, Similarly combinations of Alt+FunctionKeys can't be interpreted correctly and are interpreted as Esc + the individual keys. Since not all key presses can be mapped to the correct ConsoleKey, the implementation tries to ignore 1 byte in case it can't be mapped to anything and continue to interpret the next byte. This is done because (ConsoleKey)0 is invalid. So, even if we can get the int representation of the byte being processed, we can't really create a valid ConsoleKeyInfo from it.
@pallavit
Copy link
Contributor

Merges with dotnet/corefx#3939

@andyleejordan
Copy link
Member

Awesome, thanks guys!

@pallavit pallavit removed their assignment Oct 28, 2015
@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 1.0.0-rc1 milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Jan 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation area-System.Console
Projects
None yet
Development

No branches or pull requests