Skip to content

Feature: calloop EventSource implementation #987

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

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

ogios
Copy link

@ogios ogios commented Apr 25, 2025

I have read the issue #608 which refers to EventStream, and i've also looked into ratatui-calloop, they both creates another thread for the events reading.

This implementation wraps the fd of tty_fd and UnixStream for SIGWINCH with calloop::generic::Generic, and implements calloop::EventSource.
It's a runtime specific impl so i created a separate mod for it.

Others to be done(under consideration):

  • disable INTERNAL_EVENT_READER and global functions related to it, read, poll, position, query_keyboard_enhancement_flags.
  • handle InternalEvent only public to crate, like position(), query_keyboard_enhancement_flags().
  • libc support.

currently i have InternalEvent public for testing with cl_demo.rs

@ogios
Copy link
Author

ogios commented Apr 26, 2025

I cannot find a proper way to deal with INTERNAL_EVENT_READER, read, poll, position, query_keyboard_enhancement_flags, etc.
I would want to disable these global access and static variables while i can handle the event reader and the internal events in the runtime, but it's too much of a change, and not a call for me.

@ogios ogios marked this pull request as ready for review April 26, 2025 09:22
@ogios ogios requested a review from TimonPost as a code owner April 26, 2025 09:22
@ogios ogios changed the title calloop EventSource implementation Feature: calloop EventSource implementation Apr 26, 2025
@joshka
Copy link
Collaborator

joshka commented Apr 26, 2025

Hey there, it would be useful to take a step back from the solution and talk a bit more about the context of the problem that's being solved here. I'm familiar only a little with calloop. I wrote ratatui-calloop as an experiment to play with ideas around how event loops should be structured in apps, but haven't really progressed it beyond that point. I'd expect others are probably even less familiar with calloop than me, so it's worth giving some background about why this is useful to have in crossterm.

I expect there probably to be a bit of push back on implementing this that's related to keeping things in crossterm simple and not increasing the number of dependencies, so it's worth considering alternatives to this solution.

I wonder if it would be possible to introduce this outside of crossterm rather than bringing it into the library. I haven't yet started looking at the code to understand whether this is a reasonable question though, so I'm guessing that probably not. But if there is, then I'd suggest making a secondary library that adds this functionality and exposing the necessary internals of crossterm if needed.

@ogios
Copy link
Author

ogios commented Apr 27, 2025

yeah doing this inside crossterm may not be ideal, but there's reason for this under current situation.

I'm attempting to implement single-threaded async event reading with calloop. This requires registering both the TTY file descriptor (tty_fd) and SIGWINCH signal as event sources in the event loop, then process InternalEvents. But these are only accessible within the crate.

If we could find a proper way to expose fd and internal events, it would be easy to make the implementation outside crossterm library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants