Closed Cube707 closed 2 years ago
Hi there!
I like the approach and the idea to launch a version 4.
Please, rebase master. I've updated some of the tooling and workflows.
I will rebase and add your suggstions and update the PR as soon as possible. Thanks for taking a look at it
@magmax I updated the PR. Take a look when you have the time.
Through brief testing, the changes made do resolve the issues I was experiencing with version 3.0.5.
I second that this should be Pulled into Main and bundled with a new release.
To fix the linux tests you can use this conftest.py:
import pytest
import sys
import select
import termios
import tty
import readchar.read_linux as read_linux
# ignore all tests in this folder if not on linux
def pytest_ignore_collect(path, config):
if not sys.platform.startswith("linux"):
return True
@pytest.fixture
def patched_stdin():
class mocked_stdin:
buffer = []
def push(self, string):
for c in string:
self.buffer.append(c)
def read(self, n):
string = ""
for i in range(n):
string += self.buffer.pop(0)
return string
def mock_tcgetattr(fd):
return None
def mock_tcsetattr(fd, TCSADRAIN, old_settings):
return None
def mock_setraw(fd):
return None
def mock_kbhit():
return True
mock = mocked_stdin()
with pytest.MonkeyPatch.context() as mp:
mp.setattr(sys.stdin, "read", mock.read)
mp.setattr(termios, "tcgetattr", mock_tcgetattr)
mp.setattr(termios, "tcsetattr", mock_tcsetattr)
mp.setattr(tty, "setraw", mock_setraw)
mp.setattr(read_linux, "kbhit", mock_kbhit)
yield mock
We have only 3 failing tests then (I removed all skips)
FAILED tests/linux/test_readkey.py::test_specialKeys[\x1b[2~-\x1b[2~] - AssertionError: assert '\x1b[2~' == '\x1b[2'
FAILED tests/linux/test_readkey.py::test_specialKeys[\x1b[5~-\x1b[5~] - AssertionError: assert '\x1b[5~' == '\x1b[5'
FAILED tests/linux/test_readkey.py::test_specialKeys[\x1b[6~-\x1b[6~] - AssertionError: assert '\x1b[6~' == '\x1b[6'
Ah all the test_functionKeys
fail too:
very good. I will incorperate this into this PR in the next few days.
Most of the Linux code I just copied over from the current version and tried my best to make it a little more dynamic (while having mostly no idea if it is acually a good aproach). So if you have suggestions on how to make this better, feel free to leave them here as well.
this read_linux.py fixes all tests: I changed line 44, 48, 52 to scan longer scan codes and added 55-57 so we can scan the codes for F5-F10 (they have 5 parts)
# -*- coding: utf-8 -*-
import sys
import termios
import tty
import select
# idea from:
# https://repolinux.wordpress.com/2012/10/09/non-blocking-read-from-stdin-in-python/
# Thanks to REPOLINUX
def kbhit():
return sys.stdin in select.select([sys.stdin], [], [], 0)[0]
# Initially taken from:
# http://code.activestate.com/recipes/134892/
# Thanks to Danny Yoo
def readchar(blocking=False):
if not blocking and not kbhit():
return None
fd = sys.stdin.fileno()
old_settings = termios.tcgetattr(fd)
try:
tty.setraw(sys.stdin.fileno())
ch = sys.stdin.read(1)
finally:
termios.tcsetattr(fd, termios.TCSADRAIN, old_settings)
return ch
def readkey():
"""Get a single character on Linux. If an escaped key is pressed, the
following characters are read as well (see key_linux.py)."""
c1 = readchar(blocking=True)
if c1 == "\x03":
raise KeyboardInterrupt
if c1 != "\x1B":
return c1
c2 = readchar(blocking=True)
if c2 not in ["\x4F", "\x5B"]:
return c1 + c2
c3 = readchar(blocking=True)
if c3 not in ["\x31", "\x32", "\x33", "\x35", "\x36"]:
return c1 + c2 + c3
c4 = readchar(blocking=True)
if c2 != "\x4F" or c4 not in ["\x30", "\x31", "\x33", "\x34", "\x35", "\x37", "\x38", "\x39"]:
return c1 + c2 + c3 + c4
c5 = readchar(blocking=True)
return c1 + c2 + c3 + c4 + c5
pytest
================================================ test session starts ================================================
platform linux -- Python 3.10.4, pytest-7.1.1, pluggy-1.0.0
rootdir: /home/daniel/Desktop/other_repos/python-readchar, configfile: setup.cfg, testpaths: tests
plugins: cov-3.0.0, Faker-12.1.0, typeguard-2.13.3
collected 134 items
tests/linux/test_keys.py ....
tests/linux/test_readchar.py ...........................................................................................................
tests/linux/test_readkey.py .......................
---------- coverage: platform linux, python 3.10.4-final-0 -----------
Name Stmts Miss Cover
----------------------------------------------
readchar/__init__.py 9 4 56%
readchar/key_linux.py 60 0 100%
readchar/key_windows.py 31 31 0%
readchar/read_linux.py 33 4 88%
readchar/read_windows.py 13 13 0%
----------------------------------------------
TOTAL 146 52 64%
================================================ 134 passed in 0.53s ================================================
I also tested the codes via manual input with a short script :D they work now ^^ It's funny that they never worked before :DD but I also only used the Arrow keys and Enter/Space Keys till now.
Backspace is on linux 0x7f see http://www.macfreek.nl/memory/Backspace_and_Delete_key_reversed this can also be chagned
CTRL_A to CTRL_Z codes are the same on windows as in linux.
That sounds very good. If you want to you can open a PR on my fork and I can push to this PR from there. That we can also keep diskussing there and stop kluttering this PR :)
EDIT: I jsut created a branch on my fork for development, use that because everything I put on master gets directly pushed to this PR
in __init__.py
__all__
should be a list of strings
__all__ = ["readchar", "readkey", "key"]
I think you did the work on this and so it is only fair that you are listed as the commits author :) But if you don't care I will incorperate the changes in the comming days.
PR is send to your master :D sorry I was stupid xD I could have done that in first place
@magmax This now also contains working and extensive tests for the Linux side, so I would consider it ready.
I hope you find the time soon to review and merge this. Here is a changelog to make it easyser to rewiev:
readkey
now raises KeyboardInterrupt
You should add the check for darwin
elif sys.platform == "darwin":
macos should also use the linux readchar
good idea and it all the tests seam to pass: https://github.com/Cube707/python-readchar/runs/6272490957?check_suite_focus=true
But I would also like some to test it manually a little. Do you have a Mac? I don't and I would like to not have to set up a VM for that...
Nope I also have no Mac :D I only became aware of it because one of my users complained: https://github.com/C0D3D3V/Moodle-Downloader-2/issues/143 but i'm still waiting to see if it works for him :D though i only use a small subset of the keys in my project.
I just realised that my flatmate has a Mac. I will ask him and test it a bit later.
Notes for Mac:
Most stuff seems to work fine, espacially the arrow keys work.
But I found a few keys that don't work:
import readchar
want_exit = True
while True:
readed_key = readchar.readkey()
found_key_name = None
for known_key_name in vars(readchar.key):
known_key = getattr(readchar.key, known_key_name)
if known_key == readed_key:
found_key_name = known_key_name
break
hex_rep = ":".join("{:02x}".format(c) for c in readed_key.encode())
if found_key_name is None:
print(f"Key not in Keylist: '{readed_key}'")
print(f"In Hex: {hex_rep}")
else:
print(f"You pressed: {found_key_name}")
print(f"In Hex: {hex_rep}")
if readed_key == readchar.key.CTRL_C and want_exit:
exit(0)
elif readed_key == readchar.key.CTRL_C and not want_exit:
want_exit = True
print(f"Press {found_key_name} again to exit")
elif want_exit:
want_exit = False
I made this little script for testing. We could maybe ship a clean version of this for manual testing.
F5 to F12 also do not work on linux, but Enter works
Also Ctr+C raises a KeyboardInterrupt, the old version did not.
The Hex codes for f5 to f12 are wrong, they should be:
F5 = "\x1b\x5b\x31\x35\x7e"
F6 = "\x1b\x5b\x31\x37\x7e"
F7 = "\x1b\x5b\x31\x38\x7e"
F8 = "\x1b\x5b\x31\x39\x7e"
F9 = "\x1b\x5b\x32\x30\x7e"
F10 = "\x1b\x5b\x32\x31\x7e"
F11 = "\x1b\x5b\x32\x33\x7e"
F12 = "\x1b\x5b\x32\x34\x7e"
then you also have to change line 52 in read_linux
if c4 not in "\x30\x31\x33\x34\x35\x37\x38\x39":
I just tested it:
python ./manual.py
You pressed: F5
In Hex: 1b:5b:31:35:7e
You pressed: F6
In Hex: 1b:5b:31:37:7e
You pressed: F7
In Hex: 1b:5b:31:38:7e
You pressed: F8
In Hex: 1b:5b:31:39:7e
You pressed: F9
In Hex: 1b:5b:32:30:7e
You pressed: F10
In Hex: 1b:5b:32:31:7e
You pressed: F11
In Hex: 1b:5b:32:33:7e
You pressed: F12
In Hex: 1b:5b:32:34:7e
I can make a PR to your master xD
On linux it depends on what Enter you press, the Numpad Enter is LF, the normal Enter is CR
We probably should also remove ESCAPE_SEQUENCES, I have no idea what use that has. Since they overlap with the Function keys, I think they are wrong
I also figured that the codes for f5... are probably wrong, but my Linux VM decided to explode yesterday, so I didn't get to testing.
I don't have time today and also probably not tomorrow, so if you hsve the timd go ahead and make a PR.
already done
I made this little script for testing. We could maybe ship a clean version of this for manual testing.
I actually removed the testing script, but by now I am considering putting it back. I will at least put it on the dev branch...
Also Ctr+C raises a KeyboardInterrupt, the old version did not.
That is also deliberate. I feel it is more pythonic that way. When using something like:
while True:
c = readkey()
if c == key.UP:
break
You have to make sure the user gets not trapped and having to catche the CTRL_C
manually feels unnececary and will be forgotten often. (Also only readkey raises the error, readchar still returnes the CTRL_C as is).
But this is of course open for debate and was just what I felt made the most sense.
On linux it depends on what Enter you press, the Numpad Enter is LF, the normal Enter is CR
interesting and anoying, We will have to wait for userfeedback to see if anyone has problems with that...
We probably should also remove ESCAPE_SEQUENCES, I have no idea what use that has. Since they overlap with the Function keys, I think they are wrong.
I guesse this was an attemped to simplify the readingprocess by looping and comparing against teh list od escape_secenses to now how many bytes to read. But it is no longer needed an I will remove it.
I actually removed the testing script, but by now I am considering putting it back. I will at least put it on the dev branch...
what was there already a test script xD then I would not have had to make the effort ^^ I would find it handy if you would add something like this in the test folder, you can also quite quickly determine the keycodes for combinations that we do not have in the list, such as Alt+G so all Alt combinations.
for now I added it under : tests/manual-test.py
for final decitions we will have to wait for @magmax anyway...
I will also reorder and compress the commits once I get some feedback from him. Then I will probably not delete the testing script and jut update it a little
btw: I have been thinking about the filestructure and the importprocess a little. Whats your opinion on this:
import readchar.key
is not longer possible, only from readchar import key
. Should I re-add support for that, even if its a little hacky?
when typing import readchar.read...
or from readchar import read...
my autocompletion will suggest the read_windows/linux
subfiles. This is anoying and potentioally confusing. My first idea was to prefix the files with an underscoore (to mark them as internal), this puts them at the botom of the sugestions, but they still come up. Second idea was to change the naminsceam to platform first, than function, so renaming read_linux.py
->linux_read.py
avoiding the similar names and therefore the problem. Maybe even implement both at the same time?
This than also brings up the question on how to implement mac support. Should it get its own files? even if it just reuses pretty much all of the linux code? Or would be a seperate if line in the init file be clear enough? (maybe also rename linux->unix to make it more clear that its generally used)
I find my version of the test script better, since it can output all keys from the key definitions. Maybe you can adapt it according to your preferences.
You could just make an extra file key.py which will include key_linux or key_python depending on the os. I personally don't need it, but then we're backwards compatible. I would not hide the OS_dependent versions and also not show them further down in the list. Python itself does not do that with OS dependent modules (e.g. PurePosixPath) I find it as suffix also good, but would also be good as prefix.
you can rename linux to posix or unix, i think in python core posix is used more often.
I find my version of the test script better, since it can output all keys from the key definitions. Maybe you can adapt it according to your preferences.
good idea, I didn't realise you did that. Stolen and incorporated ;)
closing in favor of #79
Hello there, I really like your project ant wanted to contribute. This is more of a Disskussion PR that should probaply target a development branch, but since I can't create new branches here we go.
addidtions
I cleand up the windows side of the code and made it work consistently with the behavior on the linux side. For that I also had to chage a few small things in the Linux functions.
I than went ahead and upgraded the testing system. For the windows side I creaded a number of testcases covering all printable character and all currently supported special keys (Arrow-keys, F-keys, etc.).
On the Linux side I hit a wall and can only get the tests to work localy, not on GitHub. Maybe someone else can fix that?
I also cleand out the repo, removing (hopefully) not needed files and improving the makefile.
drawbacks
The Linuxside ist still mostly untested and not automated. While I copyed most of the code from the current version, I am not that versed with Linux.
Because windows uses some scancodes that can't be utf-8 encoded directly, I have to do the byte->str conversion manually. This however uses function not present in python 2.7, so I doped that for now. It is probably possible to make this work with 2.7 as well, but I didn't want to waste time on it until I have feedback if this is desirable.
While the libary should be mostly compatible with the current version, there are a few bigger changes that my break compatibilyty with some older projects.