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

Simplify runner::Runner::run #292

Open
sayanarijit opened this issue Jul 1, 2021 · 3 comments
Open

Simplify runner::Runner::run #292

sayanarijit opened this issue Jul 1, 2021 · 3 comments

Comments

@sayanarijit
Copy link
Owner

The runner::Runner::run method is quite complicated and deals with a lot of variables.
This can be simplified by consolidating the variables into one or multiple structs.

@sayanarijit sayanarijit added good first issue Good for newcomers help wanted Extra attention is needed labels Jul 5, 2021
@dtomvan
Copy link
Contributor

dtomvan commented Sep 18, 2021

The current implementation is also not very flexible (in the code). For example, I am trying to write a test for keybindings. I don't want the whole UI to show up, so I use an App and call handle_task on it. There is one problem, though. The App doesn't actually "handle" most tasks. It just enqueues it, for a Runner to use. Why not move the big match statement in app.rs (or use something like ExternalMsg::handle and let it match on self)? Runner handles the messages in a blocking fashion anyways.

@sayanarijit
Copy link
Owner Author

sayanarijit commented Sep 18, 2021

I wanted to keep App free of references. App should be an owned, serializable object. On the other hand, we have non-serializable objects like the terminal, channels, Lua VM etc. Runner::run is the place where they all interact with each other, without getting mixed up.

All I can think of is how to identify different objects in the Runner::run scope, and try to group them together in structs or functions, defined somewhere outside the run function, so that run doesn't take that much space. Unfortunately, I don't see how we can reduce complexity by moving the match statement somewhere else. But of course, if you have something in mind, you can raise a PR. Or maybe a stripped down version of xplr as demo.

@sayanarijit
Copy link
Owner Author

sayanarijit commented Sep 18, 2021

As for the keybindings, I agree it's a bit of a challenge to test. Need to steal ideas from other tui projects.

@sayanarijit sayanarijit removed the good first issue Good for newcomers label Oct 4, 2021
@sayanarijit sayanarijit pinned this issue Oct 24, 2021
@sayanarijit sayanarijit unpinned this issue Oct 27, 2021
@sayanarijit sayanarijit removed the help wanted Extra attention is needed label Oct 24, 2022
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

No branches or pull requests

2 participants