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

merge request: on windows return str and linux use cbreaker and overwrite '\n' with '\r' #64

Closed
wants to merge 5 commits into from

Conversation

qianyun210603
Copy link

I'd like to introduce this pull request to make below fix/improve:

  1. Inconsistent return of readchar function on windows 10 and linux platform
  • on windows it returns 'bytes' type
  • on linux it returns 'str' type.
    So even on win 10 I think there should be a decode operation on the return from msvcrt.getch().
  1. use setcbreak instead of setraw on linux platform to avoid unwanted, mysteriously-generated blank characters, then overwrite the result by the desired code of "ENTER", instead of using setraw and bear the blank characters.
  • This was originally raised by @insidewhy in while readchar is running there is no linefeed after each print #53 and (s)he provided a fix in pull request use setcbreak instead of setraw, fixes #53 #54 by changing setraw to setcbreak. And this was released in v3.0.1.
  • However, from some follow-up discussion this changes the returns of readchar function to \x0a (Line feed, '\n') from \x0d (Carriage Return, '\r'). Where the latter is desired by some other lib (like inquirer) from @C0D3D3V 's test.
  • I think it is some legacy issue inherited from mechanical typewriter to have separate Line feed and Carriage Return. To the limit of my knowledge there is no modern keyboard which have separate key to generate both \x0a (Line feed, '\n') and \x0d (Carriage Return, '\r'). Also on windows since the line break is defined as '\r\n', but on linux it is defined as '\n' only. I think this should be why setcbreak returns \x0a (Line feed, '\n') on linux.
  • In this case I think it should be better to use setcbreak instead of setraw to avoid the white spaces issue, and then overwrite the the return to the desired "ENTER KEY" code afterwards in the function (as mentioned before, there should be no modern keyboard which have separate key to generate both \x0a (Line feed, '\n') and \x0d (Carriage Return, '\r')).

@magmax would you give this some consideration please.

@qianyun210603
Copy link
Author

tests passed
image

@Cube707
Copy link
Collaborator

Cube707 commented Jul 7, 2022

most of this should be solved by #79

@qianyun210603
Copy link
Author

Issue to be solve here has been included in a more comprehensive refactoring by Cube707. Close this and any further discussion should be directed to https://github.com/Cube707/python-readchar/pull/5

Cube707 added a commit that referenced this pull request Jul 25, 2022
fixes #64, as described there, the raw terminal can produce a lot of
unneeded whitespace. Using the new configuration it seem to behave as
expected.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants