fix: restore the terminal on every exit path - #9
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Raw mode and the alternate screen were undone only at the end of the happy
path in
run, so a panic left the shell with no echo and no line editing.The panic message was printed into the alternate screen while raw mode was
still on, so it staircased down the screen and then scrolled away with the
alternate screen, leaving the user a broken terminal and no explanation.
An early
?fromTerminal::newdid the same thing silently.Move setup and teardown into
src/terminal.rsbehind aTerminalGuardwhose
Droprestores, so it runs on a normal return, an early?and anunwind alike.
enterconstructs the guard before entering the alternatescreen, so a failure there still switches raw mode back off.
The panic hook restores before delegating to the previous hook, which puts
the message on the normal screen and keeps
RUST_BACKTRACEworking. Itrestores only when the thread that owns the terminal panics: a panic on
the loading or detail worker kills only that thread and leaves the TUI
running, where restoring would turn a background failure into a corrupted
screen.
Teardown is best effort rather than a chain of
?, which previously meanta failing
disable_raw_modeskippedLeaveAlternateScreenentirely.Related Issues
Closes #3
Related PR's & Commits
7c6584f of #7 which replaced
fn main() -> Result<..>withfn main() -> ExitCode: errors now print with Display viaeprintln!("gitloom: {err}")instead ofprintln!("{:?}", err), and the error path returnsExitCode::FAILUREinstead of falling through toOk(())and exiting0.