Skip to content
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

Feature: developing a context handler #92

Open
4 of 6 tasks
Cube707 opened this issue Sep 10, 2022 · 20 comments · May be fixed by #96
Open
4 of 6 tasks

Feature: developing a context handler #92

Cube707 opened this issue Sep 10, 2022 · 20 comments · May be fixed by #96
Labels
enhancement great new idea

Comments

@Cube707
Copy link
Collaborator

Cube707 commented Sep 10, 2022

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:

  • setting up the terminal for readchars needs, this means:
    • no echoing or other handeling of typed keys. They need to be waiting untill readchar calls read the next time
    • no interruptions, readchar will read CTRL+C etc. As normal keystrokesand handle them
    • whatever else is needet to ensure readchar works consistently
  • giving a way to check if a key is waiting to be read
  • reading chars and keys as with the current behavior
  • ensuring consistent behaviour above both platform's (even if that means limiting one)
  • decide on good, intuitive names
  • document the new feature

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
Copy link
Collaborator Author

Cube707 commented Sep 10, 2022

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

For more context read:
#62 (comment)
And everything in #73

@petereon
Copy link

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
Copy link
Collaborator Author

Cube707 commented Sep 10, 2022

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
Copy link

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
Copy link
Collaborator Author

Cube707 commented Sep 10, 2022

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

@petereon
Copy link

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
Copy link
Collaborator Author

Cube707 commented Sep 10, 2022

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
Copy link
Collaborator Author

Cube707 commented Sep 10, 2022

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
Copy link

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

@Cube707
Copy link
Collaborator Author

Cube707 commented Sep 11, 2022

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
Copy link

petereon commented Sep 11, 2022 via email

@Cube707
Copy link
Collaborator Author

Cube707 commented Sep 17, 2022

I spend some time on this and prepeard the function definitions without content. See: aa55140

@petereon
Copy link

Thanks, let me have a look.

@Cube707
Copy link
Collaborator Author

Cube707 commented Sep 19, 2022

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

@petereon
Copy link

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.

@petereon petereon removed their assignment Sep 21, 2022
@Cube707
Copy link
Collaborator Author

Cube707 commented Sep 22, 2022

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

@petereon
Copy link

petereon commented Sep 22, 2022

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
Copy link
Collaborator Author

Cube707 commented Sep 23, 2022

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
Copy link

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.

@Cube707 Cube707 linked a pull request Oct 30, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement great new idea
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants