-
Notifications
You must be signed in to change notification settings - Fork 580
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
base: master
Are you sure you want to change the base?
Conversation
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 |
Message from Go Bot: Patch Set 1: Congratulations on opening your first change. Thank you for your contribution! Next steps: Most changes in the Go project go through a few rounds of revision. This can be During May-July and Nov-Jan the Go project is in a code freeze, during which Please don’t reply on this GitHub thread. Visit golang.org/cl/294049. |
Message from Lenni: Patch Set 1: Code-Review-1 (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/294049. |
Message from Lenni: Patch Set 1: -Code-Review (2 comments) Please don’t reply on this GitHub thread. Visit golang.org/cl/294049. |
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
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
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
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
x/sys/windows
, provides wrappers for many C functions that are exposed by the windows console api. TheReadConsole()
function for example can read single characters from the console.x/sys/windows
.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 usingunix.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 ofReadConsole()
.ReadConsole()
only reads characters from the console, whileReadConsoleInput()
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 originalReadConsole()
function. The implementation simply calls Windows'sReadConsoleInputW
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 multipleINPUT_RECORD
s, 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 anInputRecord
.Problems
There were some inconsistencies during my testing:
ReadConsoleInput()
to read multiple records. I tried setting thetoread
to more than 1 and[]InputRecord
to the syscall[]InputRecord
to the syscallboth 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 thenargs
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
nInitialChars
member to a value greater than 0 would retain the first value(s) in the supplied array. It did not.dwControlKeyState
to be set after the call toReadConsole()
. 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 ofTCHAR
which can be represented in Go using a[]byte
which in turn can be converted into a[]rune
, so that parameter to the already existingReadConsole()
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()
andReadConsoleInput()
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 inWinConTypes.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.