Repository navigation
fix(rawmode): use the right termios ioctls on the given file - #841
Open
dwisiswant0 wants to merge 1 commit into
Open
dwisiswant0 wants to merge 1 commit into
dwisiswant0 wants to merge 1 commit into
Conversation
GetMode wiped the terminal attributes (iflag, oflag, cflag, lflag and the control characters, so echo, canonical mode and signals) and returned a non-nil "errno 0" error on darwin and linux. The exported TCGETS and TCSETS values were assigned crosswise in values_darwin.go and values_linux.go, so getTermios issued the "set" ioctl with a zeroed Termios and setTermios issued the "get" ioctl. On top of that, the raw syscall's Errno was returned as error without checking for zero, which makes every success a non-nil error. The posix helpers and the windows ones also ignored their *os.File argument and always operated on os.Stdin. Assign each constant its matching request, return nil when the errno is zero, and use the file the caller passes. On posix, GetMode now returns a nil interface on failure so a failed call cannot be passed on to SetRawMode as a valid mode; the windows implementation still returns the zero mode with its error. Because the mode is nil after a failed GetMode, KeyPressReader would have reported the opaque "invalid syscall.Termios" for a stdin that is not a terminal, where it used to surface the errno. Start in raw mode now returns GetMode's error, and Stop does nothing when Start never obtained a mode, since it changed nothing to restore. The posix ioctl and read errors, and the GetMode error returned by Start, are now wrapped with %w and a short description of the failed operation, so errors.Is and errors.As still match the underlying errno. The tests drive a pseudo-terminal pair and swap os.Stdin for a second one with different attributes, so they need no controlling terminal and notice any call that goes to stdin instead of the given file. Fixes: #831 Signed-off-by: Dwi Siswanto <git@dw1.io>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
GetMode wiped the terminal attributes (iflag, oflag, cflag, lflag and
the control characters, so echo, canonical mode and signals) and
returned a non-nil "errno 0" error on darwin and linux.
The exported TCGETS and TCSETS values were assigned crosswise in
values_darwin.go and values_linux.go, so getTermios issued the "set"
ioctl with a zeroed Termios and setTermios issued the "get" ioctl. On
top of that, the raw syscall's Errno was returned as error without
checking for zero, which makes every success a non-nil error. The
posix helpers and the windows ones also ignored their *os.File argument
and always operated on os.Stdin.
Assign each constant its matching request, return nil when the errno is
zero, and use the file the caller passes. On posix, GetMode now returns
a nil interface on failure so a failed call cannot be passed on to
SetRawMode as a valid mode; the windows implementation still returns
the zero mode with its error.
Because the mode is nil after a failed GetMode, KeyPressReader would
have reported the opaque "invalid syscall.Termios" for a stdin that is
not a terminal, where it used to surface the errno. Start in raw mode
now returns GetMode's error, and Stop does nothing when Start never
obtained a mode, since it changed nothing to restore.
The posix ioctl and read errors, and the GetMode error returned by
Start, are now wrapped with %w and a short description of the failed
operation, so errors.Is and errors.As still match the underlying errno.
The tests drive a pseudo-terminal pair and swap os.Stdin for a second
one with different attributes, so they need no controlling terminal and
notice any call that goes to stdin instead of the given file.
Fixes: #831