tui: stabilize shortcut overlay snapshots on WSL #9359
Conversation
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Fixes #9058 ## Summary When the transcript backtrack preview is armed (press `Esc`), allow navigating to newer user messages with the `→` arrow, in addition to navigating backwards with `Esc`/`←`, before confirming with `Enter`. ## Changes - Backtrack preview navigation: `Esc`/`←` steps to older user messages, `→` steps to newer ones, `Enter` edits the selected message (clamped at bounds, no wrap-around). - Transcript overlay footer hints updated to advertise `esc/←`, `→`, and `enter` when a message is highlighted. ## Related - WSL shortcut-overlay snapshot determinism: #9359 ## Testing - `just fmt` - `just fix -p codex-tui` - `just fix -p codex-tui2` - `cargo test -p codex-tui app_backtrack::` - `cargo test -p codex-tui pager_overlay::` - `cargo test -p codex-tui2 app_backtrack::` - `cargo test -p codex-tui2 pager_overlay::` --------- Co-authored-by: Josh McKinney <joshka@openai.com>
| //! hint. The owning widgets schedule redraws so time-based hints can expire even if the UI is | ||
| //! otherwise idle. | ||
| #[cfg(target_os = "linux")] | ||
| #[cfg(all(target_os = "linux", not(test)))] |
There was a problem hiding this comment.
I don't like having test related cfg checks in non-test code like this. It makes it difficult to reason about how the behavior works as different behavior under tests is usually not something that most people will anticipate. Likely this instead should be separate test behavior that triggers based on the test environment.
This probably means that we need to investing in actually setting up our CI to have a WSL specific test in the matrix.
Fixes openai#9058 ## Summary When the transcript backtrack preview is armed (press `Esc`), allow navigating to newer user messages with the `→` arrow, in addition to navigating backwards with `Esc`/`←`, before confirming with `Enter`. ## Changes - Backtrack preview navigation: `Esc`/`←` steps to older user messages, `→` steps to newer ones, `Enter` edits the selected message (clamped at bounds, no wrap-around). - Transcript overlay footer hints updated to advertise `esc/←`, `→`, and `enter` when a message is highlighted. ## Related - WSL shortcut-overlay snapshot determinism: openai#9359 ## Testing - `just fmt` - `just fix -p codex-tui` - `just fix -p codex-tui2` - `cargo test -p codex-tui app_backtrack::` - `cargo test -p codex-tui pager_overlay::` - `cargo test -p codex-tui2 app_backtrack::` - `cargo test -p codex-tui2 pager_overlay::` --------- Co-authored-by: Josh McKinney <joshka@openai.com>
joshka-oai
force-pushed
the
fix/tui-wsl-shortcut-overlay-snapshots
branch
from
4e35d38 to
1f92b9e
Compare
|
I resolved the I made the WSL-specific shortcut overlay behavior deterministic for snapshots, while keeping a unit test that still asserts the WSL paste-image binding (Ctrl+Alt+V vs Ctrl+V). I also ran clippy ( Updated commit: c5c847d I’ve pushed the updated branch to your fork ( |
Gate WSL detection behind cfg(not(test)) so shortcut-overlay snapshots stay deterministic across environments. Add a focused unit test that still asserts the WSL-specific paste-image binding.
joshka-oai
force-pushed
the
fix/tui-wsl-shortcut-overlay-snapshots
branch
from
1f92b9e to
c5c847d
Compare
3 participants
Fixes #9361
Context
Split out from #9059 per review: #9059 (comment)
Summary
The shortcut overlay renders different paste-image bindings on WSL (Ctrl+Alt+V) vs non-WSL (Ctrl+V), which makes snapshot tests non-deterministic when run under WSL.
Changes
cfg(not(test))so snapshot tests are deterministic across environments.Testing
just fmtjust fix -p codex-tuijust fix -p codex-tui2cargo test -p codex-tuicargo test -p codex-tui2