-
Notifications
You must be signed in to change notification settings - Fork 6.7k
feat(tui2): add feature-flagged tui2 frontend #7793
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
Conversation
ee9c68a to
96d9202
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
codex-rs/cli/src/main.rs
Outdated
| fn is_tui2_enabled(cli: &TuiCli) -> bool { | ||
| cli.config_overrides | ||
| .raw_overrides | ||
| .iter() | ||
| .any(|raw| raw.trim() == "features.tui2=true") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honor config-based tui2 feature selection
The frontend switch only checks for a literal features.tui2=true in the CLI overrides (is_tui2_enabled walks config_overrides.raw_overrides), so enabling tui2 in a config/profile file—as documented in the features table and supported by codex_core::features—never routes to codex_tui2::run_main. Users who persist the flag in config will still get the legacy TUI, meaning the new feature flag is non-functional unless passed on the command line. Consider evaluating the merged config (e.g., via Config/feature set) instead of only CLI overrides.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree: users should be able to set this in config.toml so they don't have to pass the flag every time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added in 0914195
a5731c6 to
718afdf
Compare
codex-rs/cli/src/main.rs
Outdated
| fn is_tui2_enabled(cli: &TuiCli) -> bool { | ||
| cli.config_overrides | ||
| .raw_overrides | ||
| .iter() | ||
| .any(|raw| raw.trim() == "features.tui2=true") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree: users should be able to set this in config.toml so they don't have to pass the flag every time.
ba73f0d to
0914195
Compare
Introduce a new codex-tui2 crate that re-exports the existing interactive TUI surface and delegates run_main directly to codex-tui. This keeps behavior identical while giving tui2 its own crate for future viewport work. Wire the codex CLI to select the frontend via the tui2 feature flag. When the merged CLI overrides include features.tui2=true (e.g. via --enable tui2), interactive runs are routed through codex_tui2::run_main; otherwise they continue to use the original codex_tui::run_main. Register Feature::Tui2 in the core feature registry and add the tui2 crate and dependency entries so the new frontend builds alongside the existing TUI.
0914195 to
b80820b
Compare
Introduce a new codex-tui2 crate that re-exports the existing interactive TUI surface and delegates run_main directly to codex-tui. This keeps behavior identical while giving tui2 its own crate for future viewport work.
Wire the codex CLI to select the frontend via the tui2 feature flag. When the merged CLI overrides include features.tui2=true (e.g. via --enable tui2), interactive runs are routed through codex_tui2::run_main; otherwise they continue to use the original codex_tui::run_main.
Register Feature::Tui2 in the core feature registry and add the tui2 crate and dependency entries so the new frontend builds alongside the existing TUI.
This is a stub that only wires up the feature flag for this.