Skip to content

WIP: autorun with delay#41

Open
pianohacker wants to merge 1 commit into
akavel:masterfrom
pianohacker:autorun-delay-spike
Open

WIP: autorun with delay#41
pianohacker wants to merge 1 commit into
akavel:masterfrom
pianohacker:autorun-delay-spike

Conversation

@pianohacker

Copy link
Copy Markdown

No description provided.

@akavel

akavel commented Dec 26, 2018

Copy link
Copy Markdown
Owner

❤️ 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.

Comment thread up.go

event := tui.PollEvent()
if autoRunTimer != nil {
autoRunTimer.Stop()

Copy link
Copy Markdown
Owner

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: ?

Comment thread up.go
if command != lastCommand && autoRunDelay > 0 {
autoRunTimer = time.AfterFunc(autoRunDelay, func() {
log.Println("Firing interrupt - poll timed out")
tui.PostEvent(tcell.NewEventInterrupt("autoRunTimer"))

Copy link
Copy Markdown
Owner

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!

@akavel

akavel commented Jan 3, 2019

Copy link
Copy Markdown
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants