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

Windows: Added ReadConsoleInput functionality #98

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

scrouthtv
Copy link

Hello.
I recently opened an issue on the main repository: golang/go#44373, where I was wondering whether there was a way to read special keys on Windows. After some investigation I came up with this change.

Motivation

  • Go, especially the x/sys/windows, provides wrappers for many C functions that are exposed by the windows console api. The ReadConsole() function for example can read single characters from the console.
  • The Windows Console API also provides a C function for reading special keys from the console (think arrow keys, e.g.). There is currently no wrapper function for this one in x/sys/windows.
  • As of today, there are two big Go libraries that need this exact functionality: reading special keys from the console - termbox-go* (4k stars) and tcell (2.6k stars).
    Both libraries currently use the syscall package (which has been deprecated for ages now) for reading from the console on Windows.

* DISCLAIMER: I am a maintainer of termbox-go.

Background

On Linux, any key can be read from the console using only x/sys/unix by setting the termios to raw mode and reading using unix.Read(Handle, []byte).

On Windows, the x/sys/windows package also provides means to get and set the console mode (comparable to termios).
However, the provided ReadConsole() functionality does only read characters, it does not read special keys (arrow keys, F1 through F12, Home/End, PgUp/PgDn, Delete/Erase).

If a Windows C developer wants to read special keys, they can use the ReadConsoleInput() instead of ReadConsole(). ReadConsole() only reads characters from the console, while ReadConsoleInput() reads almost every event type (key press, key release, special keys, window size change, ...).

Implementation

ReadConsoleInput

I implemented the ReadConsoleInput function which works like the original ReadConsole() function. The implementation simply calls Windows's ReadConsoleInputW function.

The difference in parameters passed is taken care of:

ReadConsole takes a Handle, pointer to a buffer, number of characters to read, pointer to storage for the amount of total read characters and pointer to an InputControl while ReadConsoleinput takes a Handle, pointer to a buffer, buffer length, pointer to a storage for the amount of total read characters.

The first four parameters are comparable, except for the buffer type. The last (optional) parameter does not apply to ReadConsoleInput (nargs argument of the syscall is 4 instead of 5).

InputRecord

However, the ReadConsoleInput() function reads one or multiple INPUT_RECORDs, see https://docs.microsoft.com/en-us/windows/console/input-record-str.
So I added a new type, called InputRecord which represents Windows's original type.
ReadConsoleInput() reads into a pointer to an InputRecord.

Problems

There were some inconsistencies during my testing:

  • I couldn't get ReadConsoleInput() to read multiple records. I tried setting the toread to more than 1 and
    • passing a pointer to a []InputRecord to the syscall
    • passing a pointer to the first value of a []InputRecord to the syscall
      both times, only the first entry was written
  • ReadConsoleInput() would very rarely segfault if they key was held down after all data was read.
    The error was something along of unknown pc reported, though I can't reproduce it after I've changed some internals (changed the nargs parameter to 4 instead of 5).

Here's a testing file I used during development if anyone wants to investigate the implementation details further:
input_test.txt

I won't add it to the tree since it's not a Unit test but an interactive test.

ReadConsole InputControl

I also took a look at the ReadConsole() functionality. The Windows version optionally takes as a last argument a pointer to a CONSOLE_READCONSOLE_CONTROL struct. You can see that I recreated this struct in my testing file.

I tried to use it - the Windows API marks it as optional input. I would expect

  • setting the nInitialChars member to a value greater than 0 would retain the first value(s) in the supplied array. It did not.
  • the dwControlKeyState to be set after the call to ReadConsole(). It was not.

Any usage of ReadConsole() I could find out there did simply use 0 as an inputControl value so I sticked with that.

ReadConsole() returns an array of TCHAR which can be represented in Go using a []byte which in turn can be converted into a []rune, so that parameter to the already existing ReadConsole() function makes sense as-is.

Auto-generated code

A disclaimer at the top of zsyscall_windows.go says it is auto-generated. I couldn't find any documentation how that generation works, and no template file either, so I added my code manually.
Maybe it has to be included in some template file.

Documentation

As the code is auto-generated it does not feature any documentation.
However, both ReadConsole() and ReadConsoleInput() are not self-explanatory (especially the difference between the two).
I added documentation for both functions as well as my newly added struct.

Copyright

The content of the documentation is orginally provided by Microsoft, licensed under CC Attribution 4.0 International.

I don't know anything about licensing, I just want people to understand my code. I added Copyright disclaimers to a reasonable extent, but maybe it's better to strip all documentation and only link to Microsoft's original documentation?

The same problem applies to the definition of the InputRecord struct - does my definition infringe Microsoft's intellectual property? It is originally defined in WinConTypes.h.

In case this (partly) gets merged I really want to advise squashing the whole changeset because there are many testing iterations in between.

Thanks for your time.

@google-cla google-cla bot added the cla: yes label Feb 19, 2021
@gopherbot
Copy link

This PR (HEAD: cd77da5) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/sys/+/294049 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link

Message from Go Bot:

Patch Set 1:

Congratulations on opening your first change. Thank you for your contribution!

Next steps:
A maintainer will review your change and provide feedback. See
https://golang.org/doc/contribute.html#review for more info and tips to get your
patch through code review.

Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.

During May-July and Nov-Jan the Go project is in a code freeze, during which
little code gets reviewed or merged. If a reviewer responds with a comment like
R=go1.11 or adds a tag like "wait-release", it means that this CL will be
reviewed as part of the next development cycle. See https://golang.org/s/release
for more details.


Please don’t reply on this GitHub thread. Visit golang.org/cl/294049.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Lenni:

Patch Set 1: Code-Review-1

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/294049.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Lenni:

Patch Set 1: -Code-Review

(2 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/294049.
After addressing review feedback, remember to publish your drafts!

aymanbagabas added a commit to aymanbagabas/sys that referenced this pull request Jun 6, 2024
Add Console API input handling syscall wrappers. This is an effort to
upstream the amazing coninput library by @erikgeiser
https://github.com/erikgeiser/coninput

Related golang#98
aymanbagabas added a commit to aymanbagabas/sys that referenced this pull request Jun 6, 2024
Add Console API input handling syscall wrappers. This is an effort to
upstream the amazing coninput library by @erikgeiser
https://github.com/erikgeiser/coninput

Related golang#98
aymanbagabas added a commit to aymanbagabas/sys that referenced this pull request Jul 18, 2024
Add Console input API types and syscalls. Since InputRecord contains a
`union` type, we need some way to access the union members. This is done
here by extending InputRecord to return the respective type based on the
function called.

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

Successfully merging this pull request may close these issues.

2 participants