fix: End synchronized update on StdTerminal drop

[barsoosayque]

What It Does

Add EndSynchronizedUpdate to the list of executed commands for impl Drop for StdTerminal.

In the general terminal_render_loop, terminal executes an enter to synchronized update, but the render may panic mid-update (for example, #103), which usually causes StdTerminal to drop. But it never ends synchronized update which can cause issues.

In my case, I was trying to run something akin to #114 in alacritty + zellij, panic mid-update would freeze the terminal rendering (but it would accept input still); just alacritty worked as expected though (so I think it's zellij related, but I have no idea actually how). Ending synchronized update fixed the issue !

Related Issues

• Originally discovered while trying to panic safely in a fullscreen app: Show panic output even when running in fullscreen mode #103

[ccbrown]

Thanks for the PR! This looks good, but I think there are some more error cases that would be covered if we did something like this:

diff --git a/packages/iocraft/src/render.rs b/packages/iocraft/src/render.rs
index ec7acb1..b6efc5e 100644
--- a/packages/iocraft/src/render.rs
+++ b/packages/iocraft/src/render.rs
@@ -13,7 +13,6 @@ use core::{
     pin::Pin,
     task::{self, Poll},
 };
-use crossterm::{execute, terminal};
 use futures::{
     future::{select, FutureExt, LocalBoxFuture},
     stream::{Stream, StreamExt},
@@ -461,16 +460,18 @@ impl<'a> Tree<'a> {
         loop {
             term.refresh_size();
             let terminal_size = term.size();
-            execute!(term, terminal::BeginSynchronizedUpdate,)?;
-            let output = self.render(terminal_size.map(|(w, _)| w as usize), Some(&mut term));
-            if output.did_clear_terminal_output || prev_canvas.as_ref() != Some(&output.canvas) {
-                if !output.did_clear_terminal_output {
-                    term.clear_canvas()?;
+            term.synchronized_update(|mut term| {
+                let output = self.render(terminal_size.map(|(w, _)| w as usize), Some(&mut term));
+                if output.did_clear_terminal_output || prev_canvas.as_ref() != Some(&output.canvas)
+                {
+                    if !output.did_clear_terminal_output {
+                        term.clear_canvas()?;
+                    }
+                    term.write_canvas(&output.canvas)?;
                 }
-                term.write_canvas(&output.canvas)?;
-            }
-            prev_canvas = Some(output.canvas);
-            execute!(term, terminal::EndSynchronizedUpdate)?;
+                prev_canvas = Some(output.canvas);
+                Ok(())
+            })?;
             if self.system_context.should_exit() || term.received_ctrl_c() {
                 break;
             }
diff --git a/packages/iocraft/src/terminal.rs b/packages/iocraft/src/terminal.rs
index 3d92bcf..e65a819 100644
--- a/packages/iocraft/src/terminal.rs
+++ b/packages/iocraft/src/terminal.rs
@@ -282,7 +282,6 @@ impl Drop for StdTerminal {
         if self.fullscreen {
             let _ = queue!(self.dest, terminal::LeaveAlternateScreen);
         }
-        let _ = execute!(self.dest, cursor::Show);
     }
 }
 
@@ -381,6 +380,17 @@ pub(crate) struct Terminal {
     received_ctrl_c: bool,
 }
 
+/// Ends a synchronized update when dropped.
+struct SynchronizedTerminal<'a> {
+    inner: &'a mut Terminal,
+}
+
+impl Drop for SynchronizedTerminal<'_> {
+    fn drop(&mut self) {
+        let _ = execute!(self.inner, terminal::EndSynchronizedUpdate);
+    }
+}
+
 impl Terminal {
     pub fn new() -> io::Result<Self> {
         Ok(Self::new_with_impl(StdTerminal::new(false)?))
@@ -428,6 +438,17 @@ impl Terminal {
         self.received_ctrl_c
     }
 
+    /// Wraps a series of terminal updates in a synchronized update block, making sure to end the
+    /// synchronized update even if there is an error or panic.
+    pub fn synchronized_update<F>(&mut self, f: F) -> io::Result<()>
+    where
+        F: FnOnce(&mut Self) -> io::Result<()>,
+    {
+        execute!(self, terminal::BeginSynchronizedUpdate)?;
+        let t = SynchronizedTerminal { inner: self };
+        f(t.inner)
+    }
+
     pub async fn wait(&mut self) {
         match &mut self.event_stream {
             Some(event_stream) => {

That should provide stronger guarantee that the synchronized update will end as soon as it's intended to. Thoughts?

[barsoosayque]

Looks good ! I think it would make sense to begin synchronized update when creating SynchronizedTerminal, e.g.:

/// Synchronized update terminal guard.
/// Enters synchronized update on creation, exits when dropped.
struct SynchronizedUpdate<'a> {
    inner: &'a mut Terminal,
}

impl<'a> SynchronizedUpdate<'a> {
    fn begin(terminal: &'a mut Terminal) -> io::Result<Self> {
        execute!(terminal, terminal::BeginSynchronizedUpdate)?;
        Ok(Self { inner: terminal })
    }
}

Sadly none of that seems

[ccbrown]

Sadly none of that seems

Did this comment get truncated?

I think your suggestion looks good. If you want to update the PR, I'll be glad to merge it!

[barsoosayque]

Just pushed SynchronizedUpdate::begin code. Works as expected.

Did this comment get truncated?

Yeah, turns out I was too sleep deprived to finish the sentence. I was trying to say that sadly none of that seems to prevent terminal poisoning on std::process::abort or std::process::exit, but that's out of the scope for this PR

[codecov]

Codecov Report

❌ Patch coverage is 75.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.57%. Comparing base (6b7fa17) to head (dac602e).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
packages/iocraft/src/render.rs	60.00%	0 Missing and 4 partials ⚠️
packages/iocraft/src/terminal.rs	85.71%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #140      +/-   ##
==========================================
+ Coverage   89.56%   89.57%   +0.01%     
==========================================
  Files          31       31              
  Lines        5039     5054      +15     
  Branches     5039     5054      +15     
==========================================
+ Hits         4513     4527      +14     
  Misses        428      428              
- Partials       98       99       +1     

Files with missing lines	Coverage Δ
packages/iocraft/src/terminal.rs	69.15% <85.71%> (+0.82%)	⬆️
packages/iocraft/src/render.rs	96.34% <60.00%> (+0.22%)	⬆️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[ccbrown]

Thank you! 🚀