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

Async job results only show up after the next keypress #402

Closed
archseer opened this issue Jul 3, 2021 · 5 comments
Closed

Async job results only show up after the next keypress #402

archseer opened this issue Jul 3, 2021 · 5 comments

Comments

@archseer
Copy link
Member

archseer commented Jul 3, 2021

@jneem after #285, pressing gd or K will appear as if nothing happened, then once the next input is entered j/k etc. the view will refresh and the popup appears. It seems like a rendering issue, but it might be next_job returning None too early.

@archseer
Copy link
Member Author

archseer commented Jul 3, 2021

Yeah I think it's an issue with futures::select. The callback only appears in the event loop after a second input.

@jneem
Copy link
Contributor

jneem commented Jul 3, 2021

Hm, sounds like this might be annoying to debug. Maybe I can check when the job's future is getting polled? Or maybe I should be spawning a task instead of just relying on the select macro? There is something in the select docs about that, but I have to admit that I don't fully understand the implications...

Unfortunately I can't play with this tonight, but I'll try to do it sometime this weekend.

@archseer
Copy link
Member Author

archseer commented Jul 3, 2021

I'm debugging it right now, the problem is that the stream future immediately returns None. tokio::select avoids this if you poll the stream directly, but it re-surfaced now that it's wrapped in a method.

select_next_some is what we want to use but I'm still trying to get it to work with tokio::select
https://docs.rs/futures/0.3.15/futures/prelude/stream/trait.StreamExt.html#method.select_next_some

archseer added a commit that referenced this issue Jul 3, 2021
@archseer
Copy link
Member Author

archseer commented Jul 3, 2021

I pushed a temporary fix: 83e7dd8

Need to document this behavior and make sure to audit the event loop if any changes are made.

jneem added a commit to jneem/helix that referenced this issue Jul 5, 2021
jneem added a commit to jneem/helix that referenced this issue Jul 6, 2021
@archseer
Copy link
Member Author

I'll mark this as resolved for now.

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