magmax / python-readchar

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

Feature: developing a context handler #92

Open Cube707 opened 1 year ago

Cube707 commented 1 year ago

Due to the way readchar currently handles terminal setup, we cannot control terminal behaviour outside of the two readchar functions. This leads to issues #62 and #73

We could solve this by externalising terminal setup and teardown and have it run at the beginning and end, ensuring consistent behaviour in between. Under python this calls for a Context Manager

Requirements would be:

Here is an example of how it could work:

With ReadChar() as read:
    if read.key_iswaiting():
        print(read.key())

    #other stuff in parallel #
    sleep(0.5)

If this is working an properly set up, I could replace the existing functions and they could be simply wrappers for this.

Cube707 commented 1 year ago

@petereon i opened a new issue to discuss this. Feel free to drop more questions

For more context read:
https://github.com/magmax/python-readchar/issues/62#issuecomment-1190213664 And everything in #73

petereon commented 1 year ago

Perfect, thank you very much for an exhaustive description. I wonder if it doesn't sound like a good opportunity to utilize Queues and reactive paradigm.

That would give us a very good way to check if there is a character waiting for processing with a FIFO Queue's get method. With asyncio, it should also deal with the back pressure, if someone typed faster than it is able to process mentioned in #73 . For some reference, consider this article and specifically this code snippet. Does that sound like something that could fit into the current architecture?

Cube707 commented 1 year ago

While this would defnetly be a reasonable approach where we to rewrite all of he input gathering yourselfs.

But lukily the operating system already handles most of the heavy lifting. We just need to configure it to match our needs. So don't overcomplicate it.

petereon commented 1 year ago

Yeah, that sounds like a pretty sane approach. I am just unsure how to handle the none-blocking read then. Sounds like there is either going to have to be an async event-loop or separate process/thread in that case.

Cube707 commented 1 year ago

No, the terminal is already a queue and you can query it for content. On windows its super simple as there is a function for it. msvcrt.kbhit() returns true if there are keypresses waiting to be read.

On linux use this https://gist.github.com/michelbl/efda48b19d3e587685e3441a74457024#file-kbhit-py-L103-L111

Cube707 commented 1 year ago

Also check out this development version that I made but didn't use

https://github.com/Cube707/python-readchar/blob/c3081ec51581ead9b4db2f60a8f1567ed809d06a/readchar/posix_read.py

https://github.com/Cube707/python-readchar/blob/c3081ec51581ead9b4db2f60a8f1567ed809d06a/readchar/win_read.py

petereon commented 1 year ago

Great, thank you for the information and apologies for how very uninformed I am about tty. I'll take a deep dive in this stuff in coming days 😃

Cube707 commented 1 year ago

No worry. The reason I never used the development version is that I didn't know enough and thought it would work just like that. Turned out its more complicated 😂

Cube707 commented 1 year ago

Also keep in mind that on the windows side a lot of stuff doesn't work, so this will mostlikley be the limiting factor. So don't spend to much time (unless you want to 😆)

petereon commented 1 year ago

I will have a proper read into this. Good opportunity to learn how OS's handle this stuff :smile:. It might take a bit of time.

Cube707 commented 1 year ago

take your time. there is no rush.

I will also have more time in the coming weeks, as my thesis is due tomorrow and will no longer eat up my schedule

petereon commented 1 year ago

Good luck with your thesis, mate. So glad I am done with that stuff already 😃

On Sun, Sep 11, 2022, 13:53 Jan Wille @.***> wrote:

take your time. there is no rush.

I will also have more time in the coming weeks, as my thesis is due tomorrow and no longer eat up my schedule

— Reply to this email directly, view it on GitHub https://github.com/magmax/python-readchar/issues/92#issuecomment-1242948146, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALGZGPORQAA37NXTURFU3MLV5XB4LANCNFSM6AAAAAAQJMRZIA . You are receiving this because you were assigned.Message ID: @.***>

Cube707 commented 1 year ago

I spend some time on this and prepeard the function definitions without content. See: https://github.com/magmax/python-readchar/commit/aa55140219032449be29482b5b3727038987d77b

petereon commented 1 year ago

Thanks, let me have a look.

Cube707 commented 1 year ago

did some more work, check https://github.com/magmax/python-readchar/tree/contexmanager

petereon commented 1 year ago

That's very cool, also I can see you are already done by the time I got to it. Impressive. Apologies for my inactivity, I was really hard pressed for time with work related stuff.

Cube707 commented 1 year ago

I would still be happy about feedback. Especially about the intuitivenes of names and the general usability

petereon commented 1 year ago

I have played around with it and had one issue in terms of usability: When there are multiple keys waiting at the same time, once the first is read it behaves as if no more keys were waiting. To replicate this, try the following code, press multiple keys while sleeping and wait for while loop to start running. Only printed the first pressed keys for me. After I pressed another as the while loop was running it dumped all the other ones.

import time
from readchar import ReadChar

with ReadChar() as stream:
    time.sleep(5)
    while True:
        if stream.key_waiting:
            char = stream.key()
            print(char)

In terms of naming conventions, I wondered if this isn't more of a CharQueue or CharStream rather than ReadChar.

Cube707 commented 1 year ago

I will look into the issue. I assume you tested on Linux, not Windows?

For the name, my initial idea was something like KeyReader, but I chose this as it is how other libary tend to name their context managers that have a normal function a counterpart. I also figured that it would be among the first thing people guess, alog the lines of "what was the CM from readchar? Probably just a capital R". But I might be totally off here...

petereon commented 1 year ago

I tested on macOS. But that's still POSIX I guess.

As for the naming, that does sound like a good way to approach it. Would be my first guess also.