-
Notifications
You must be signed in to change notification settings - Fork 129
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
WIP: autorun with delay #41
base: master
Are you sure you want to change the base?
Conversation
❤️ Thanks a lot for the PR! I will try to find some time to test and review it, though I could not manage to do that yet. |
|
||
event := tui.PollEvent() | ||
if autoRunTimer != nil { | ||
autoRunTimer.Stop() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm; I think as is now, this will get the autorun cancelled when triggerRefresh
is called, no? I think this results in weird semantics, where slow input can delay the autorun; maybe the timer should be stopped only in case *tcell.Key:
?
if command != lastCommand && autoRunDelay > 0 { | ||
autoRunTimer = time.AfterFunc(autoRunDelay, func() { | ||
log.Println("Firing interrupt - poll timed out") | ||
tui.PostEvent(tcell.NewEventInterrupt("autoRunTimer")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mwahaha! I just realized we can probably do a sneaky hack here, by replacing NewEventInterrupt with NewEventKey(tcell.KeyEnter, ...)
; this should let us remove the whole l.201-207 block!
I just compiled and run it for a test drive. I think this is pure awesome!!! 😄 I'm totally for adding this feature. Let's go with a regular review. Apart from the comments I already added, one thing I'm somewhat worried about is a possible race between the autoRun "interrupt" and a "real" key pressed by user. Such that we Poll and start processing a real key, but before we managed to Stop the timer, it's already fired and enqueued a new "interrupt". It's not 100% clear to me if and how this can be avoided; but I am starting to have some initial intuition that it might be possible. Though it might require backing off from the "NewEventKey(KeyEnter,...)" hack. edit: Or maybe not? There could be some counter of KeyEnters to ignore... ehh, anyway, currently I think this "avoidance" will be a tad bit ugly anyway. |
No description provided.