refactor TUI event loop to enable dropping + recreating crossterm event stream #7961
Conversation
|
@codex review this |
|
Codex Review: Didn't find any major issues. Keep them coming! ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
joshka-oai
left a comment
joshka-oai
left a comment
There was a problem hiding this comment.
General approach LGTM - would suggest having @nornagon-openai also weigh in on this.
Doc suggestions are the main things I'd fix on this. The other suggestions are less important. Async (or adjacently async) code is often difficult to maintain. Capturing the context for why it's there / intended behavior in various situations is pretty important.
codex-rs/tui/src/tui/event_stream.rs
Outdated
| /// Per-call event stream wrapper. Each handle has its own draw subscription but | ||
| /// pulls crossterm events from the shared broker so nested/sequential streams | ||
| /// can coexist (only one should be polled at a time to avoid stealing). |
There was a problem hiding this comment.
I'm not sure I understand this comment at all as it assumes that the reader has a lot of context into a bunch of concepts that aren't explained / anchored.
- what's a call?
- what's a handle?
- why is a draw subscription relevant?
- why would there multiple copies of this?
- what is stealing and why is it bad?
There was a problem hiding this comment.
Fair. I tried to clarify, let me know if it's still unclear.
|
@codex review this |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review this |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
Introduces an
EventBrokerbetween the crosstermEventStreamsource and the consumers in the TUI. This enables dropping + recreating thecrossterm_eventswithout invalidating the consumer.Dropping and recreating the crossterm event stream enables us to fully relinquish
stdinwhile the app keeps running. If the stream is not dropped, it will continue to read fromstdineven when it is not actively being polled, potentially stealing input from other processes. See here and here for details.Tests
Added tests for new
EventBrokersetup, existing tests pass, tested locally.