From 729eb72aa3ec750751b56a5d898afef2736cf364 Mon Sep 17 00:00:00 2001 From: Lucas Pickering Date: Sat, 4 Jan 2025 11:20:22 -0500 Subject: [PATCH] Redraw after local tasks complete This eliminates the 250ms delay between a local task completing and the next redraw. Redraws are only triggered by messages in the queue (and not events), so we can send an empty message to trigger a redraw. --- crates/tui/src/lib.rs | 10 ++--- crates/tui/src/message.rs | 8 ++-- crates/tui/src/util.rs | 26 +++++++++++-- crates/tui/src/view/common/text_box.rs | 6 +-- .../tui/src/view/component/queryable_body.rs | 6 +-- .../tui/src/view/component/response_view.rs | 2 + crates/tui/src/view/context.rs | 2 +- crates/tui/src/view/util.rs | 39 +++++++++---------- 8 files changed, 58 insertions(+), 41 deletions(-) diff --git a/crates/tui/src/lib.rs b/crates/tui/src/lib.rs index d4c8e6da..255fbf18 100644 --- a/crates/tui/src/lib.rs +++ b/crates/tui/src/lib.rs @@ -22,7 +22,7 @@ use crate::{ message::{Callback, Message, MessageSender, RequestConfig}, util::{ clear_event_buffer, delete_temp_file, get_editor_command, - get_pager_command, save_file, signals, ResultReported, + get_pager_command, save_file, signals, spawn_local, ResultReported, }, view::{PreviewPrompter, UpdateContext, View}, }; @@ -356,8 +356,9 @@ impl Tui { on_complete, )?; } + // This message exists just to trigger a draw - Message::TemplatePreviewComplete => {} + Message::Tick => {} Message::Quit => self.quit(), } @@ -657,13 +658,10 @@ impl Tui { on_complete: Callback>, ) -> anyhow::Result<()> { let context = self.template_context(profile_id, true)?; - let messages_tx = self.messages_tx(); - tokio::spawn(async move { + spawn_local(async move { // Render chunks, then write them to the output destination let chunks = template.render_chunks(&context).await; on_complete(chunks); - // Trigger a draw - messages_tx.send(Message::TemplatePreviewComplete); }); Ok(()) } diff --git a/crates/tui/src/message.rs b/crates/tui/src/message.rs index 0563dc79..52dafd17 100644 --- a/crates/tui/src/message.rs +++ b/crates/tui/src/message.rs @@ -154,10 +154,10 @@ pub enum Message { #[debug(skip)] on_complete: Callback>, }, - /// An empty event to trigger a draw when a template preview is done being - /// rendered. This is a bit hacky, but it's an explicit way to tell the TUI - /// "we know something in the view has changed asynchronously". - TemplatePreviewComplete, + + /// Trigger a redraw. This should be called whenever we have reason to + /// believe the UI may have changed due to a background task + Tick, } /// A static callback included in a message diff --git a/crates/tui/src/util.rs b/crates/tui/src/util.rs index b499f5c7..49bc5208 100644 --- a/crates/tui/src/util.rs +++ b/crates/tui/src/util.rs @@ -1,7 +1,7 @@ use crate::{ context::TuiContext, message::{Message, MessageSender}, - view::Confirm, + view::{Confirm, ViewContext}, }; use anyhow::{bail, Context}; use bytes::Bytes; @@ -14,13 +14,20 @@ use slumber_core::{ util::{doc_link, paths::expand_home, ResultTraced}, }; use std::{ - env, io, + env, + future::Future, + io, ops::Deref, path::{Path, PathBuf}, process::{Command, Stdio}, time::Duration, }; -use tokio::{fs::OpenOptions, io::AsyncWriteExt, sync::oneshot}; +use tokio::{ + fs::OpenOptions, + io::AsyncWriteExt, + sync::oneshot, + task::{self, JoinHandle}, +}; use tracing::{debug, debug_span, error, info, warn}; use uuid::Uuid; @@ -126,6 +133,19 @@ pub fn delete_temp_file(path: &Path) { .traced(); } +/// Spawn a task on the main thread. Most tasks can use this because the app is +/// generally I/O bound, so we can handle all async stuff on a single thread +pub fn spawn_local( + future: impl 'static + Future, +) -> JoinHandle<()> { + task::spawn_local(async move { + future.await; + // Assume the task updated _something_ visible to the user, so trigger + // a redraw here + ViewContext::messages_tx().send(Message::Tick); + }) +} + /// Save some data to disk. This will: /// - Ask the user for a path /// - Attempt to save a *new* file diff --git a/crates/tui/src/view/common/text_box.rs b/crates/tui/src/view/common/text_box.rs index 77c59a12..238acf29 100644 --- a/crates/tui/src/view/common/text_box.rs +++ b/crates/tui/src/view/common/text_box.rs @@ -177,10 +177,10 @@ impl TextBox { /// Emit a change event. Should be called whenever text _content_ is changed fn change(&mut self) { let is_valid = self.is_valid(); - if let Some(debounce) = &self.on_change_debounce { + let emitter = self.handle(); + if let Some(debounce) = &mut self.on_change_debounce { if is_valid { // Defer the change event until after the debounce period - let emitter = self.handle(); debounce.start(move || emitter.emit(TextBoxEvent::Change)); } else { debounce.cancel(); @@ -201,7 +201,7 @@ impl TextBox { /// Cancel any pending debounce. Should be called on submit or cancel, when /// the user is no longer making changes fn cancel_debounce(&mut self) { - if let Some(debounce) = &self.on_change_debounce { + if let Some(debounce) = &mut self.on_change_debounce { debounce.cancel(); } } diff --git a/crates/tui/src/view/component/queryable_body.rs b/crates/tui/src/view/component/queryable_body.rs index 97278043..3f4e7357 100644 --- a/crates/tui/src/view/component/queryable_body.rs +++ b/crates/tui/src/view/component/queryable_body.rs @@ -2,7 +2,7 @@ use crate::{ context::TuiContext, - util::run_command, + util::{run_command, spawn_local}, view::{ common::{ text_box::{TextBox, TextBoxEvent, TextBoxProps}, @@ -30,7 +30,7 @@ use slumber_core::{ util::MaybeStr, }; use std::{borrow::Cow, mem, sync::Arc}; -use tokio::task::{self, AbortHandle}; +use tokio::task::AbortHandle; /// Display response body as text, with a query box to run commands on the body. /// The query state can be persisted by persisting this entire container. @@ -206,7 +206,7 @@ impl QueryableBody { body: Bytes, on_complete: impl 'static + FnOnce(String, anyhow::Result>), ) -> AbortHandle { - task::spawn_local(async move { + spawn_local(async move { let shell = &TuiContext::get().config.commands.shell; let result = run_command(shell, &command, Some(&body)) .await diff --git a/crates/tui/src/view/component/response_view.rs b/crates/tui/src/view/component/response_view.rs index 07aa7abb..bad31a35 100644 --- a/crates/tui/src/view/component/response_view.rs +++ b/crates/tui/src/view/component/response_view.rs @@ -302,6 +302,8 @@ mod tests { // component }) .await; + // Background task sends a message to redraw + assert_matches!(harness.pop_message_now(), Message::Tick); component.drain_draw().assert_empty(); } diff --git a/crates/tui/src/view/context.rs b/crates/tui/src/view/context.rs index 3762a6bf..0759cff7 100644 --- a/crates/tui/src/view/context.rs +++ b/crates/tui/src/view/context.rs @@ -119,7 +119,7 @@ impl ViewContext { /// Queue a view event to be handled by the component tree pub fn push_event(event: Event) { - Self::with_mut(|context| context.event_queue.push(event)) + Self::with_mut(|context| context.event_queue.push(event)); } /// Pop an event off the event queue diff --git a/crates/tui/src/view/util.rs b/crates/tui/src/view/util.rs index 130c13b0..b2c91f7a 100644 --- a/crates/tui/src/view/util.rs +++ b/crates/tui/src/view/util.rs @@ -3,7 +3,11 @@ pub mod highlight; pub mod persistence; -use crate::{message::Message, util::temp_file, view::ViewContext}; +use crate::{ + message::Message, + util::{spawn_local, temp_file}, + view::ViewContext, +}; use anyhow::Context; use itertools::Itertools; use mime::Mime; @@ -16,7 +20,7 @@ use slumber_core::{ util::ResultTraced, }; use std::{io::Write, path::Path, time::Duration}; -use tokio::{select, sync::broadcast, task, time}; +use tokio::{task::AbortHandle, time}; /// A data structure for representation a yes/no confirmation. This is similar /// to [Prompt], but it only asks a yes/no question. @@ -47,47 +51,40 @@ impl Prompter for PreviewPrompter { #[derive(Debug)] pub struct Debounce { duration: Duration, - /// Broadcast channel to send on when previous tasks should be cancelled - cancel_send: broadcast::Sender<()>, + abort_handle: Option, } impl Debounce { pub fn new(duration: Duration) -> Self { - let (cancel_send, _) = broadcast::channel(1); Self { duration, - cancel_send, + abort_handle: None, } } /// Trigger a debounced callback. The given callback will be invoked after /// the debounce period _if_ this method is not called again during the /// debounce period. - pub fn start(&self, on_complete: impl 'static + Fn()) { - // Cancel existing tasks, _then_ start a new listener, so we don't - // cancel ourselves + pub fn start(&mut self, on_complete: impl 'static + Fn()) { + // Cancel the existing debounce, if any self.cancel(); - let mut cancel_recv = self.cancel_send.subscribe(); // Run debounce in a local task so component behavior can access the // view context, e.g. to push events let duration = self.duration; - task::spawn_local(async move { - // Start a timer. If it expires before cancellation, then submit - select! { - _ = time::sleep(duration) => { - on_complete() - }, - _ = cancel_recv.recv() => {} - }; + let handle = spawn_local(async move { + time::sleep(duration).await; + on_complete(); }); + self.abort_handle = Some(handle.abort_handle()); } /// Cancel the current pending callback (if any) without registering a new /// one - pub fn cancel(&self) { - // An error on the send just means there are no listeners; we can ignore - let _ = self.cancel_send.send(()); + pub fn cancel(&mut self) { + if let Some(abort_handle) = self.abort_handle.take() { + abort_handle.abort(); + } } }