bug: handle terminal cleanup if main.rs panics from an Err (#1660)

* bug: handle terminal cleanup if main.rs panics from an Err

* add comment

* changelog
This commit is contained in:
Clement Tsang 2025-01-17 21:43:58 -05:00 committed by GitHub
parent 873434b4b7
commit c970037546
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 28 additions and 20 deletions

View File

@ -36,6 +36,7 @@ That said, these are more guidelines rather than hardset rules, though the proje
- [#1593](https://github.com/ClementTsang/bottom/pull/1593): Fix using `"none"` for chart legend position in configs. - [#1593](https://github.com/ClementTsang/bottom/pull/1593): Fix using `"none"` for chart legend position in configs.
- [#1594](https://github.com/ClementTsang/bottom/pull/1594): Fix incorrect default config definitions for chart legends. - [#1594](https://github.com/ClementTsang/bottom/pull/1594): Fix incorrect default config definitions for chart legends.
- [#1596](https://github.com/ClementTsang/bottom/pull/1596): Fix support for nilfs2 file system. - [#1596](https://github.com/ClementTsang/bottom/pull/1596): Fix support for nilfs2 file system.
- [#1660](https://github.com/ClementTsang/bottom/pull/1660): Handle terminal cleanup if the program is terminated due to an `Err` bubbling to the top.
### Changes ### Changes

View File

@ -1,5 +1,7 @@
use bottom::start_bottom; use bottom::{reset_stdout, start_bottom};
fn main() -> anyhow::Result<()> { fn main() -> anyhow::Result<()> {
start_bottom() start_bottom().inspect_err(|_| {
reset_stdout();
})
} }

View File

@ -26,7 +26,7 @@ pub mod widgets;
use std::{ use std::{
boxed::Box, boxed::Box,
io::{stderr, stdout, Write}, io::{stderr, stdout, Stdout, Write},
panic::{self, PanicHookInfo}, panic::{self, PanicHookInfo},
sync::{ sync::{
mpsc::{self, Receiver, Sender}, mpsc::{self, Receiver, Sender},
@ -38,12 +38,12 @@ use std::{
use app::{layout_manager::UsedWidgets, App, AppConfigFields, DataFilters}; use app::{layout_manager::UsedWidgets, App, AppConfigFields, DataFilters};
use crossterm::{ use crossterm::{
cursor::{Hide, Show},
event::{ event::{
poll, read, DisableBracketedPaste, DisableMouseCapture, EnableBracketedPaste, poll, read, DisableBracketedPaste, DisableMouseCapture, EnableBracketedPaste,
EnableMouseCapture, Event, KeyEventKind, MouseEventKind, EnableMouseCapture, Event, KeyEventKind, MouseEventKind,
}, },
execute, execute,
style::Print,
terminal::{disable_raw_mode, enable_raw_mode, EnterAlternateScreen, LeaveAlternateScreen}, terminal::{disable_raw_mode, enable_raw_mode, EnterAlternateScreen, LeaveAlternateScreen},
}; };
use data_conversion::*; use data_conversion::*;
@ -80,7 +80,8 @@ fn cleanup_terminal(
terminal.backend_mut(), terminal.backend_mut(),
DisableBracketedPaste, DisableBracketedPaste,
DisableMouseCapture, DisableMouseCapture,
LeaveAlternateScreen LeaveAlternateScreen,
Show,
)?; )?;
terminal.show_cursor()?; terminal.show_cursor()?;
@ -101,11 +102,24 @@ fn check_if_terminal() {
} }
} }
/// This manually resets stdout back to normal state.
pub fn reset_stdout() -> Stdout {
let mut stdout = stdout();
let _ = disable_raw_mode();
let _ = execute!(
stdout,
DisableBracketedPaste,
DisableMouseCapture,
LeaveAlternateScreen,
Show,
);
stdout
}
/// A panic hook to properly restore the terminal in the case of a panic. /// A panic hook to properly restore the terminal in the case of a panic.
/// Originally based on [spotify-tui's implementation](https://github.com/Rigellute/spotify-tui/blob/master/src/main.rs). /// Originally based on [spotify-tui's implementation](https://github.com/Rigellute/spotify-tui/blob/master/src/main.rs).
fn panic_hook(panic_info: &PanicHookInfo<'_>) { fn panic_hook(panic_info: &PanicHookInfo<'_>) {
let mut stdout = stdout();
let msg = match panic_info.payload().downcast_ref::<&'static str>() { let msg = match panic_info.payload().downcast_ref::<&'static str>() {
Some(s) => *s, Some(s) => *s,
None => match panic_info.payload().downcast_ref::<String>() { None => match panic_info.payload().downcast_ref::<String>() {
@ -116,22 +130,11 @@ fn panic_hook(panic_info: &PanicHookInfo<'_>) {
let backtrace = format!("{:?}", backtrace::Backtrace::new()); let backtrace = format!("{:?}", backtrace::Backtrace::new());
let _ = disable_raw_mode(); reset_stdout();
let _ = execute!(
stdout,
DisableBracketedPaste,
DisableMouseCapture,
LeaveAlternateScreen
);
// Print stack trace. Must be done after! // Print stack trace. Must be done after!
if let Some(panic_info) = panic_info.location() { if let Some(panic_info) = panic_info.location() {
let _ = execute!( println!("thread '<unnamed>' panicked at '{msg}', {panic_info}\n\r{backtrace}")
stdout,
Print(format!(
"thread '<unnamed>' panicked at '{msg}', {panic_info}\n\r{backtrace}",
)),
);
} }
// TODO: Might be cleaner in the future to use a cancellation token, but that causes some fun issues with // TODO: Might be cleaner in the future to use a cancellation token, but that causes some fun issues with
@ -339,6 +342,7 @@ pub fn start_bottom() -> anyhow::Result<()> {
let mut stdout_val = stdout(); let mut stdout_val = stdout();
execute!( execute!(
stdout_val, stdout_val,
Hide,
EnterAlternateScreen, EnterAlternateScreen,
EnableMouseCapture, EnableMouseCapture,
EnableBracketedPaste EnableBracketedPaste
@ -365,6 +369,7 @@ pub fn start_bottom() -> anyhow::Result<()> {
panic::set_hook(Box::new(panic_hook)); panic::set_hook(Box::new(panic_hook));
// Set termination hook // Set termination hook
// TODO: On UNIX, use signal-hook to handle cleanup as well.
ctrlc::set_handler(move || { ctrlc::set_handler(move || {
let _ = sender.send(BottomEvent::Terminate); let _ = sender.send(BottomEvent::Terminate);
})?; })?;