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

Implement grace period for process selection #48

Merged
merged 8 commits into from
Jun 5, 2020

Conversation

alexmaco
Copy link
Contributor

@alexmaco alexmaco commented Jun 1, 2020

Fixes #47

This mostly refactors the event handling to use pattern matching and fewer indent levels, and adds a selection grace period.

Right now it's hardcoded to 2000ms, so the actual observed interval by the user will be [2000ms, 4000ms]. This feels ok to me, but you may also want to test it.

@bvaisvil
Copy link
Owner

bvaisvil commented Jun 3, 2020

This looks great! I really like the event processing refactor. Eventually, I'd like to move all event processes into their own file (maybe events.rs).

In my testing the 3000 ms time works as expected. I'm happy if we leave it as is.

However, I did find one issue. After filtering the process table with f, then typing a string, I can't move the selection.

I think what happens is when self.show_find is true all the key presses get swallowed at that point. Perhaps, we could move the up/down arrow processing to their own functions, then that could be called when processing input for find. What do you think?

Other than that one issue, looks very good and thank you for the contribution!

@alexmaco
Copy link
Contributor Author

alexmaco commented Jun 4, 2020

I can see the problem, thanks for the testing to find it! Tbh, i don't exactly understand why the up/down keys are not swallowed in the original version. It would probably be worth adding some unittests to this area when event processing gets moved to event.rs.

@alexmaco
Copy link
Contributor Author

alexmaco commented Jun 4, 2020

I think what happens is when self.show_find is true all the key presses get swallowed at that point. Perhaps, we could move the up/down arrow processing to their own functions, then that could be called when processing input for find. What do you think?

I was preparing another refactoring and cleanup PR after this one. For now, i just split apart the inner match. Moving different sections of input handling to their own function is definitely something to do, but i would rather do it by input "mode" first, so there could be handling for keys when filtering, handling for keys during non-filtering, and handling for keys that work for both.

@bvaisvil
Copy link
Owner

bvaisvil commented Jun 5, 2020

I was preparing another refactoring and cleanup PR after this one.

That would be great. Looking forward to that.

but i would rather do it by input "mode" first, so there could be handling for keys when filtering, handling for keys during non-filtering, and handling for keys that work for both.

Great. I tried your latest updates and everything works as expected.

Thanks!

@bvaisvil bvaisvil merged commit d022282 into bvaisvil:master Jun 5, 2020
@alexmaco alexmaco deleted the grace branch June 6, 2020 20:02
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.

Add grace period for process selection after input
2 participants