chiark / gitweb /
Implement a visible 'busy' indicator.
authorSimon Tatham <anakin@pobox.com>
Sun, 4 Feb 2024 14:23:48 +0000 (14:23 +0000)
committerSimon Tatham <anakin@pobox.com>
Sun, 4 Feb 2024 15:02:40 +0000 (15:02 +0000)
This is implemented by giving Client a reference to the TuiOutput,
which it can call to notify that a network operation has started. So
whenever any keystroke results in network activity, we immediately
update the screen to indicate that something is happening that takes
time, reducing the risk that the user will believe their keystroke was
ignored.

The Client's reference is wrapped in a new 'TuiBusyIndicator' struct,
in case we need to change the notification mechanism. (For example, if
this business ever starts to sprawl across multiple threads, it might
be more convenient to make TuiBusyIndicator contain the sending end of
a sync_channel, so that the display thread would receive the busy
indication from wherever it was sent.)

All client activity goes via the just-moved execute_and_log_request
method, so that's the only place I need to insert a call to set_busy.
So this causes no extra boilerplate per Client method.

TODO.md
src/client.rs
src/tui.rs

diff --git a/TODO.md b/TODO.md
index 59f33249443ae4ea25745d786b4c592b2633f1f5..deed1e173fd9750886fe0a4ab87bf9f32795be47 100644 (file)
--- a/TODO.md
+++ b/TODO.md
@@ -20,17 +20,6 @@ Again, this will need some persistent inter-activity state in a
 `RefCell` in the `TuiLogicalState`. Also some UI thought about what
 history should be shared with what.
 
-## General UI improvements
-
-When a keystroke requires a network transaction, the transaction often
-takes place in a `handle_keypress()` method, which means the display
-isn't updated until that method returns, because only `Tui` can do
-display updates. But some network transactions take noticeable time,
-so it would be nice to do an immediate display update to indicate that
-the keystroke was received and something is being done. This may
-involve actual code restructuring, since in Rust it's not trivial to
-just casually call back to the top-level thing.
-
 ## Error handling improvements
 
 Error Log entries could usefully include various extra detail:
index fed32029e941d789ad5c4b8083035f354f3dd88f..208124be9109c1ba4be711e3fd8d37f41656876f 100644 (file)
@@ -7,6 +7,7 @@ use std::io::{IoSlice, Read, Write};
 
 use super::auth::AuthConfig;
 use super::posting::Post;
+use super::tui::TuiBusyIndicator;
 use super::types::*;
 
 #[derive(Hash, Debug, PartialEq, Eq, Clone, Copy)]
@@ -157,6 +158,7 @@ pub struct Client {
     permit_write: bool,
     logfile: Option<File>,
     error_log: ErrorLog,
+    busy: Option<TuiBusyIndicator>,
 }
 
 #[derive(Debug, PartialEq, Eq, Clone)]
@@ -542,6 +544,7 @@ impl Client {
             permit_write: false,
             logfile: None,
             error_log: ErrorLog::new(),
+            busy: None,
         };
 
         client.feeds.insert(
@@ -569,6 +572,16 @@ impl Client {
         self.logfile = file;
     }
 
+    pub fn set_busy_indicator(&mut self, busy: TuiBusyIndicator) {
+        self.busy = Some(busy);
+    }
+
+    fn set_busy(&self) {
+        if let Some(busy) = &self.busy {
+            busy.set_busy();
+        }
+    }
+
     pub fn clear_caches(&mut self) {
         self.accounts.clear();
         self.statuses.clear();
@@ -650,6 +663,7 @@ impl Client {
             req_headers.push(TransactionLogEntry::translate_header(h));
         }
 
+        self.set_busy();
         let rsp = self.client.execute(req)?;
         let status = rsp.status();
         let mut rsp_headers = Vec::new();
index e427d4ed97d60bce8b214f013b669bc5e8c39875..282cffdec8f7126dba2dae94ffc5a9941f2071f2 100644 (file)
@@ -32,6 +32,7 @@ use super::login::{finish_account_setup, login_menu, LoginError};
 use super::menu::*;
 use super::options::*;
 use super::posting::*;
+use super::text::{CentredInfoLine, DefaultDisplayStyle, TextFragmentOneLine};
 
 fn ratatui_style_from_colour(colour: char) -> Style {
     match colour {
@@ -335,6 +336,10 @@ impl Tui {
 
         let output = Rc::new(RefCell::new(TuiOutput::new(terminal)));
 
+        client.set_busy_indicator(TuiBusyIndicator {
+            output: Rc::clone(&output),
+        });
+
         let mut tui = Tui {
             output,
             subthread_sender: sender,
@@ -569,12 +574,21 @@ impl Tui {
 
 struct TuiOutput {
     terminal: Terminal<CrosstermBackend<Stdout>>,
+    prev_buffer: Option<Buffer>,
+    pending_io_error: Result<(), std::io::Error>,
+    busy_msg: CentredInfoLine,
 }
 
 impl TuiOutput {
     fn new(terminal: Terminal<CrosstermBackend<Stdout>>) -> Self {
         TuiOutput {
             terminal,
+            prev_buffer: None,
+            pending_io_error: Ok(()),
+            busy_msg: CentredInfoLine::new(ColouredString::general(
+                "<+- please wait, accessing network -+>",
+                "HHHHKKKKKKKKKKKKKKKKKKKKKKKKKKKKKKHHHH",
+            )),
         }
     }
 
@@ -582,10 +596,15 @@ impl TuiOutput {
         &mut self,
         state: &mut TuiLogicalState,
     ) -> Result<(), std::io::Error> {
+        // In case a previous set_busy reported an I/O error, return it now
+        std::mem::replace(&mut self.pending_io_error, Ok(()))?;
+
         self.terminal.draw(|frame| {
             let area = frame.size();
             let buf = frame.buffer_mut();
-            if let Some((x, y)) = state.draw_frame(area, buf) {
+            let cursor = state.draw_frame(area, buf);
+            self.prev_buffer = Some(buf.clone());
+            if let Some((x, y)) = cursor {
                 if let (Ok(x), Ok(y)) = (x.try_into(), y.try_into()) {
                     frame.set_cursor(x, y);
                 }
@@ -598,6 +617,50 @@ impl TuiOutput {
     fn clear(&mut self) -> Result<(), std::io::Error> {
         self.terminal.clear()
     }
+
+    fn set_busy(&mut self) {
+        let result = self.terminal.draw(|frame| {
+            let area = frame.size();
+            let buf = frame.buffer_mut();
+            buf.reset();
+            if let Some(prev) = &self.prev_buffer {
+                buf.merge(prev);
+            }
+            let width = area.width as usize;
+            let height = area.height as usize;
+            let bottom_line = self.busy_msg.render_oneline(
+                width,
+                None,
+                &DefaultDisplayStyle,
+            );
+            let rpad = width.saturating_sub(bottom_line.width());
+            let bottom_line =
+                bottom_line + ColouredString::plain(" ").repeat(rpad);
+            ratatui_set_string(buf, 0, height.saturating_sub(1), bottom_line);
+        });
+
+        // This function can't conveniently return its I/O error, so
+        // instead we keep it until the network operation has
+        // finished, and report it in the next draw()
+        match result {
+            Ok(_) => (),
+            Err(e) => {
+                if self.pending_io_error.is_ok() {
+                    self.pending_io_error = Err(e);
+                }
+            }
+        }
+    }
+}
+
+pub struct TuiBusyIndicator {
+    output: Rc<RefCell<TuiOutput>>,
+}
+
+impl TuiBusyIndicator {
+    pub fn set_busy(&self) {
+        self.output.borrow_mut().set_busy();
+    }
 }
 
 #[derive(Debug)]