[codex] fix Windows PowerShell flicker#1591
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces detection for legacy Windows console hosts (ConHost) to improve TUI rendering stability. When a legacy host is detected via the absence of modern terminal environment variables, the application now automatically enables low_motion, disables fancy_animations, and sets synchronized_output to off. These changes are reflected in the changelog, diagnostic output, and TUI initialization logic. Feedback was provided regarding the unit tests for terminal marker detection, suggesting a refactor into a data-driven loop to improve maintainability and reduce repetition.
| #[test] | ||
| fn legacy_windows_console_host_excludes_modern_terminal_markers() { | ||
| use std::ffi::OsStr; | ||
|
|
||
| let marker = Some(OsStr::new("1")); | ||
| assert!(!legacy_windows_console_host_env( | ||
| marker, None, None, None, None, None, None, None | ||
| )); | ||
| assert!(!legacy_windows_console_host_env( | ||
| None, marker, None, None, None, None, None, None | ||
| )); | ||
| assert!(!legacy_windows_console_host_env( | ||
| None, None, Some(OsStr::new("vscode")), None, None, None, None, None | ||
| )); | ||
| assert!(!legacy_windows_console_host_env( | ||
| None, None, None, marker, None, None, None, None | ||
| )); | ||
| assert!(!legacy_windows_console_host_env( | ||
| None, None, None, None, marker, None, None, None | ||
| )); | ||
| assert!(!legacy_windows_console_host_env( | ||
| None, None, None, None, None, marker, None, None | ||
| )); | ||
| assert!(!legacy_windows_console_host_env( | ||
| None, None, None, None, None, None, marker, None | ||
| )); | ||
| assert!(!legacy_windows_console_host_env( | ||
| None, | ||
| None, | ||
| None, | ||
| None, | ||
| None, | ||
| None, | ||
| None, | ||
| Some(OsStr::new("xterm-256color")) | ||
| )); | ||
| } |
There was a problem hiding this comment.
This test is quite repetitive and could be hard to maintain if the signature of legacy_windows_console_host_env changes. Consider refactoring it into a data-driven test using a loop to improve readability and maintainability.
#[test]
fn legacy_windows_console_host_excludes_modern_terminal_markers() {
use std::ffi::OsStr;
let marker = Some(OsStr::new("1"));
let vscode_marker = Some(OsStr::new("vscode"));
let xterm_marker = Some(OsStr::new("xterm-256color"));
for i in 0..8 {
let mut args: [Option<&OsStr>; 8] = [None; 8];
args[i] = match i {
2 => vscode_marker,
7 => xterm_marker,
_ => marker,
};
assert!(!legacy_windows_console_host_env(
args[0], args[1], args[2], args[3], args[4], args[5], args[6], args[7]
), "failed at index {i}");
}
}|
The narrow Windows PowerShell/ConHost flicker fix was harvested into #1655 and merged to main for v0.8.38, with changelog credit to @asdfg314284230 / WuMing.\n\nThat landed the legacy-console detection, calmer rendering defaults, synchronized-output auto-disable, startup reset guard, and doctor note without taking the broader dirty branch as-is. Closing this PR as superseded by the merged release fix. Thank you for the root-cause work. |
Closes #1590
Summary
Validation