Fix ENTER on macos #89
Conversation
There was a problem hiding this comment.
First of all, thanks for contributing, but your PR is not well formated.
Please keep your commits atomic (meaning each one should containe a single standalone change), so avoid commit that just fix a small mistake in a previos commit (like your last commit that just adds a missing \n). Please create a first commit for your refractoring of the if statements and than a second commit with your macos-fix.
Also your commit messages contain some random text. keep the first line short and descriptive and use the second line to give further explanations if necessary (see herer for more info). And only link other issues / PR's on commits that specifically fix them, not when the fix is mixed with other stuff.
Please remove #70 from this PR. I will take care of it once I have some time for it, but it is to complex and I don't know enought about macos to just handle it quick and dirty. For now this will have to stay a limitation (which is fine as MacOS is not supported anyway)
| or platform == "darwin" | ||
| or platform.startswith("freebsd") | ||
| ): | ||
| if platform.startswith(("linux", "darwin", "freebsd")): |
There was a problem hiding this comment.
thats a cool idea. very nice.
| @@ -0,0 +1,4 @@ | |||
| # common | |||
|
|
|||
There was a problem hiding this comment.
please import exsiting definitions from other files here
| from ._posix_key import * |
| @@ -0,0 +1,4 @@ | |||
| # common | |||
|
|
|||
| CR = "\x0d" | |||
There was a problem hiding this comment.
this is alread defined. no need to redefine it
| if sys.platform == "darwin": | ||
| ch = sys.stdin.readline(1) | ||
| else: | ||
| ch = sys.stdin.read(1) |
There was a problem hiding this comment.
this is defnetly to complex for me to just sign of on. Please remove #70 from this PR. I will decide if it makes sense to include it once I finde the time to get around to it.
| from ._posix_key import * | ||
| elif platform.startswith("darwin"): | ||
| from ._posix_key import * |
There was a problem hiding this comment.
like I said above, this should be part of the _mac_key.py file
| from ._posix_key import * |
|
I am not jet sure if I prefere the "proper" way of having a seperate file for mac, or if I am fine with a dirty override in the |
|
also please rebase to current master, as some stuff changed in 77c1ac5 |
|
and do you have access to a mac? can you provide screenshot of the |
|
Now my Mac uses \n, but I remember exactly what didn't work and was saved only by changing to \r. I think now you can close the MP, and if I reproduce there will already be additional data in what situation such a problem appears. |
|
OK, thats weird 🤦🏼♂️ |
|
you can still open a PR for your refactoring of the |
After merge #79 in macos was broken enter. It's fix.
And i include #70