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

_stdin_hook_default incorrectly uses msg_ready #813

Closed
BrenBarn opened this issue Jul 8, 2022 · 2 comments · Fixed by #814
Closed

_stdin_hook_default incorrectly uses msg_ready #813

BrenBarn opened this issue Jul 8, 2022 · 2 comments · Fixed by #814

Comments

@BrenBarn
Copy link

BrenBarn commented Jul 8, 2022

See this line. msg_ready is now async, so its return value can't be treated as a boolean anymore. I suppose it needs to be wrapped in run_sync.

@davidbrochart
Copy link
Member

Thanks for reporting @BrenBarn.
Indeed, this cannot work. Rather than wrapping in run_sync, we could make _stdin_hook_default an async function, and support both async and sync stdin_hook. What do you think?

@BrenBarn
Copy link
Author

BrenBarn commented Jul 8, 2022

Having support for either sync or async stdin_hook would be great. I don't have much of an opinion on whether _stdin_hook_default should be sync or async, although I suppose making it async makes sense as it is called by an async function and calls async functions itself.

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 a pull request may close this issue.

2 participants