search

magmax / python-readchar

Python library to read characters and key strokes
MIT License
143 stars 45 forks source link

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

Closed qianyun210603 closed 2 years ago

qianyun210603 commented 3 years ago

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().

  2. 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 #53 and (s)he provided a fix in pull request #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 commented 3 years ago

tests passed image

Cube707 commented 2 years ago

most of this should be solved by #79

qianyun210603 commented 2 years ago

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