fix(tui): suppress verbose CLI logging on Windows alt-screen to prevent TUI leak#1905
fix(tui): suppress verbose CLI logging on Windows alt-screen to prevent TUI leak#1905aboimpinto wants to merge 1 commit into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces Windows-specific logic to suppress verbose CLI logging when the TUI alternate screen is active, preventing log output from corrupting the TUI buffer. Review feedback suggests refactoring this logic into recover_terminal_modes to eliminate duplication and address redundancy, as verbosity is currently not re-enabled when the TUI is paused. Additionally, the reviewer noted the accidental removal of a trailing newline at the end of crates/tui/src/tui/ui.rs.
| #[cfg(windows)] | ||
| crate::logging::set_verbose(false); |
There was a problem hiding this comment.
This logic is duplicated in resume_terminal (line 6428). To improve maintainability and follow the project's established patterns, consider moving this Windows-specific verbosity suppression into recover_terminal_modes (line 6562). That function is already documented as the canonical location for terminal-mode setup and is called by both run_tui and resume_terminal.
| // Re-entering alt-screen after mode recovery — suppress verbose | ||
| // CLI logging again so eprintln! doesn't leak into the TUI. | ||
| #[cfg(windows)] | ||
| crate::logging::set_verbose(false); |
There was a problem hiding this comment.
This call to set_verbose(false) is redundant because verbosity is already disabled at the start of run_tui (line 283) and is never re-enabled within the TUI lifecycle (including in pause_terminal). If the intention is to allow verbose logging while the TUI is paused (e.g., during a shell command), then pause_terminal should restore the state. If not, this call can be safely removed or refactored into recover_terminal_modes as suggested in the previous comment.
|
|
||
| #[cfg(test)] | ||
| mod tests; | ||
| mod tests; No newline at end of file |
There was a problem hiding this comment.
The trailing newline was removed from the end of the file. It is a standard convention in Rust (and enforced by rustfmt) to end files with a newline to ensure compatibility with various Unix tools and to avoid unnecessary diff noise in future changes.
mod tests;
|
Superseded by PR #1910 which includes the verbosity-restore on cleanup and UTF-8 encoding fix. |
1 participant
On Windows stderr cannot be redirected to the log file (no `dup2`), so any `eprintln!` calls from `crate::logging` emit directly into the terminal even inside the alt-screen buffer, corrupting the display.