-
Notifications
You must be signed in to change notification settings - Fork 47
Fixed most issues on Windows/Python 3.5 + Moved the linux loop in the linux code file #13
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
Conversation
P.S. I'm not able to install/run the test suite on my pc, the setup just crashes... and looking at the Travis build, I can't really tell what went wrong. So I'd be glad to fix the build errors, but I'll need a hand in identifying them. |
Hello, I've just fixed the dependency problem, so travis should work right now. Could you kindly merge with master? thank you. |
Moved the linux loop in the linux code file
Hi, I just rebased as requested. Travis now works but all build fail even though the code works well in practice both under linux and windows. I suspect this is because:
Again, I would suggest making the Linux code work like the new Windows code instead of the opposite. The way it's done for Windows, it is easy to both compare a key, and print it:
but this is not so easy on Linux since the special characters are not translated to a readable string. |
I like this PR but it also has the un-needed "return buffer" statement addressed in my PR #21. However, it is now in a different file on this PR (the un-needed return is found at the end of May I also suggest not using 'buffer' as as variable name and use charbuffer (or similar) instead? Buffer shadows built-in name 'buffer'. I am closing my PR; I hope this one gets pulled back to the branch... |
- Removed unneeded return statement - Improved redebility by adding else blocs - Renaed bufffer to char_buffer for python 2.x compatibility
@suresttexas00 I just pushed a new commit with the improvements you mentioned. Would you mind testing it to make sure I didn't break anything? My linux machine is not working at the moment. |
In this:
is it really necessary to have a "space" item? Shouldn't these items simply be replacement for non-ASCII, multicharacter keypresses? That could play havok on someone's program if it did not check for a space explicity... a user might type "hello world" and the program sees "hellospaceworld". This was not an issue for the linux handler because SPACE was just \x20 (which is still just a space). |
At the end of keys.py:
Most ctrl key and shift combos don't work on my Linux, so that seems to be all of them. Space is not there for which reasons I indicated. I also do not think the special key is getting intercepted on Linux. |
gives characters a name versus codes
Hello. I've reverted some changes that were problematic in failed release 1.1.1, so I released 0.8. Please, fix the conflicts and travis and I will merge this. I'm afraid you used 1.1.1 as base and it was wrong, so maybe you have to change something. Thank you |
Just FYI, the travis problems are due to this code:
The Travis test code substitutes a 'mock" get_char function in place of your (or mine) readchar()... so it does not every return more than a single character and it never uses the logic for parsing escape keys because we moved that to the readchar (or in my PR, a new get_char function). I wasted much time trying to "fix" my code before I realized the test was not testing our code properly. |
I have, for the time being, adopted my own single module version that goes back primarily to the original Danny Yoo thread version with some of these elements we have experimented with here thrown in. I have included Danny Yoo's thread and your Readchar in the attributions/credits: https://github.com/suresttexas00/readkey I think most everything there has been tried here too at one point or another, but I borrowed a nice Windows 10 unit and updated a lot of key mappings; please have a look and see if this can be fixed. I would really like to see the non-blocking versions that can read ESC come back! |
Update readchar_linux.py
New keys.py for new readchar_linux (coming separately)
807f406
to
1bc1529
Compare
# Conflicts: # readchar/readchar.py # readchar/readchar_linux.py # readchar/readchar_windows.py
You are not going to be able to get it to work. The testing suite is not actually testing "real-world" conditions... look carefully at the travis test code and how the factory functions are doing the testing. It is too much for me to explain, but I have tried elsewhere in various comments. |
@suresttexas00 Indeed, I did see your other comments and I haven't tried to make it pass the Travis build. I'm hoping @magmax will eventually adapt the Travis test suite. The issue I was referring to happens when using the package in my own code (you can run example.py to test a quick implementation). The code in example.py worked on Linux and Windows before the merge, but now it only works as intended on Windows. |
The example.py is really the best way to test for now. Unfortunately, I don't have the time to test it at the moment. when I get the opportunity, I'll revisit it. |
This pull request is an improvement over pull request #9, which hasn't moved in some time.
Almost all the special characters are now working (including the arrows). I moved the Linux character read loop to the Linux file, since it's not needed on Windows, cleaned up some code sections to better align with PEP8 and prevented a crash for characters that cannot be decoded.
This doesn't resolve the fact that two checks need to be made for cross-platform scripts (e.g.
if (inp == key.DOWN or inp == "down")
)However, I would suggest making the Linux code work like the Windows code instead of the opposite. The way it's done for Windows, it is easy to both compare a key, and print it:
but this is not so easy on Linux since the special characters are not translated to a readable string. Let me know what you think.