Delay auto-save until exiting insert mode (#11047)

Saving while in insert mode causes issues with the modification
indicator and this is very easy to reproduce with the current state of
the auto-save hook. We can tweak the hook slightly to await the mode
switch out of insert mode to perform the save.

The debounce is preserved: if you save and then immediately exit insert
mode the debounce will be respected. If the debounce lapses while you
are in insert mode, the save occurs as you switch out of insert mode
immediately.

helix-term/src/handlers/auto_save.rs

@@ -1,39 +1,82 @@
-use std::time::Duration;
+use std::{
+    sync::{
+        atomic::{self, AtomicBool},
+        Arc,
+    },
+    time::Duration,
+};
 
 use anyhow::Ok;
 use arc_swap::access::Access;
 
 use helix_event::{register_hook, send_blocking};
-use helix_view::{events::DocumentDidChange, handlers::Handlers, Editor};
+use helix_view::{
+    document::Mode,
+    events::DocumentDidChange,
+    handlers::{AutoSaveEvent, Handlers},
+    Editor,
+};
 use tokio::time::Instant;
 
 use crate::{
     commands, compositor,
+    events::OnModeSwitch,
     job::{self, Jobs},
 };
 
 #[derive(Debug)]
-pub(super) struct AutoSaveHandler;
+pub(super) struct AutoSaveHandler {
+    save_pending: Arc<AtomicBool>,
+}
 
 impl AutoSaveHandler {
     pub fn new() -> AutoSaveHandler {
-        AutoSaveHandler
+        AutoSaveHandler {
+            save_pending: Default::default(),
+        }
     }
 }
 
 impl helix_event::AsyncHook for AutoSaveHandler {
-    type Event = u64;
+    type Event = AutoSaveEvent;
 
     fn handle_event(
         &mut self,
-        timeout: Self::Event,
+        event: Self::Event,
-        _: Option<tokio::time::Instant>,
+        existing_debounce: Option<tokio::time::Instant>,
     ) -> Option<Instant> {
-        Some(Instant::now() + Duration::from_millis(timeout))
+        match event {
+            Self::Event::DocumentChanged { save_after } => {
+                Some(Instant::now() + Duration::from_millis(save_after))
+            }
+            Self::Event::LeftInsertMode => {
+                if existing_debounce.is_some() {
+                    // If the change happened more recently than the debounce, let the
+                    // debounce run down before saving.
+                    existing_debounce
+                } else {
+                    // Otherwise if there is a save pending, save immediately.
+                    if self.save_pending.load(atomic::Ordering::Relaxed) {
+                        self.finish_debounce();
+                    }
+                    None
+                }
+            }
+        }
     }
 
     fn finish_debounce(&mut self) {
-        job::dispatch_blocking(move |editor, _| request_auto_save(editor))
+        let save_pending = self.save_pending.clone();
+        job::dispatch_blocking(move |editor, _| {
+            if editor.mode() == Mode::Insert {
+                // Avoid saving while in insert mode since this mixes up
+                // the modification indicator and prevents future saves.
+                save_pending.store(true, atomic::Ordering::Relaxed);
+            } else {
+                request_auto_save(editor);
+                save_pending.store(false, atomic::Ordering::Relaxed);
+            }
+        })
     }
 }
 
@@ -54,7 +97,20 @@ pub(super) fn register_hooks(handlers: &Handlers) {
     register_hook!(move |event: &mut DocumentDidChange<'_>| {
         let config = event.doc.config.load();
         if config.auto_save.after_delay.enable {
-            send_blocking(&tx, config.auto_save.after_delay.timeout);
+            send_blocking(
+                &tx,
+                AutoSaveEvent::DocumentChanged {
+                    save_after: config.auto_save.after_delay.timeout,
+                },
+            );
+        }
+        Ok(())
+    });
+
+    let tx = handlers.auto_save.clone();
+    register_hook!(move |event: &mut OnModeSwitch<'_, '_>| {
+        if event.old_mode == Mode::Insert {
+            send_blocking(&tx, AutoSaveEvent::LeftInsertMode)
         }
         Ok(())
     });

helix-view/src/handlers.rs

@@ -7,11 +7,17 @@
 pub mod dap;
 pub mod lsp;
 
+#[derive(Debug)]
+pub enum AutoSaveEvent {
+    DocumentChanged { save_after: u64 },
+    LeftInsertMode,
+}
+
 pub struct Handlers {
     // only public because most of the actual implementation is in helix-term right now :/
     pub completions: Sender<lsp::CompletionEvent>,
     pub signature_hints: Sender<lsp::SignatureHelpEvent>,
-    pub auto_save: Sender<u64>,
+    pub auto_save: Sender<AutoSaveEvent>,
 }
 
 impl Handlers {