refactor DocumentDidChange events

in the past DocumentDidChange and SelectionDidChange events were implemented in
a simplistic manner to get a simple prototype out. However, if you want to use
these events in more complex scenarios with interdependencies between the two
handlers the system fell short.

The `SelectionDidChange` event was dispatched before the DocumentDidChange (and
not at all if the selection wasn't manually set) so any handlers that wants to
track selection was not able to map their ranges yet.

The reason for this was actually the way that apply_impl was structured. The
function was slightly refactored to address these problems and enable moving
other range mappings to event handlers.

helix-term/src/handlers/signature_help.rs

@@ -341,7 +341,7 @@ pub(super) fn register_hooks(handlers: &Handlers) {
 
     let tx = handlers.signature_hints.clone();
     register_hook!(move |event: &mut DocumentDidChange<'_>| {
-        if event.doc.config.load().lsp.auto_signature_help {
+        if event.doc.config.load().lsp.auto_signature_help && !event.ghost_transaction {
             send_blocking(&tx, SignatureHelpEvent::ReTrigger);
         }
         Ok(())

helix-view/src/document.rs

@@ -1189,28 +1189,12 @@ impl Document {
         use helix_core::Assoc;
 
         let old_doc = self.text().clone();
-
+        let changes = transaction.changes();
-        let success = transaction.changes().apply(&mut self.text);
+        if !changes.apply(&mut self.text) {
-
+            return false;
-        if success {
-            if emit_lsp_notification {
-                helix_event::dispatch(DocumentDidChange {
-                    doc: self,
-                    view: view_id,
-                    old_text: &old_doc,
-                });
-            }
-
-            for selection in self.selections.values_mut() {
-                *selection = selection
-                    .clone()
-                    // Map through changes
-                    .map(transaction.changes())
-                    // Ensure all selections across all views still adhere to invariants.
-                    .ensure_invariants(self.text.slice(..));
         }
 
-            // if specified, the current selection should instead be replaced by transaction.selection
+        if changes.is_empty() {
             if let Some(selection) = transaction.selection() {
                 self.selections.insert(
                     view_id,
@@ -1221,15 +1205,18 @@ impl Document {
                     view: view_id,
                 });
             }
-
+            return true;
-            self.modified_since_accessed = true;
         }
 
-        if !transaction.changes().is_empty() {
+        self.modified_since_accessed = true;
         self.version += 1;
-            // start computing the diff in parallel
+        for selection in self.selections.values_mut() {
-            if let Some(diff_handle) = &self.diff_handle {
+            *selection = selection
-                diff_handle.update_document(self.text.clone(), false);
+                .clone()
+                // Map through changes
+                .map(transaction.changes())
+                // Ensure all selections across all views still adhere to invariants.
+                .ensure_invariants(self.text.slice(..));
         }
 
         // generate revert to savepoint
@@ -1261,7 +1248,11 @@ impl Document {
             }
         }
 
-            let changes = transaction.changes();
+        // TODO: all of that should likely just be hooks
+        // start computing the diff in parallel
+        if let Some(diff_handle) = &self.diff_handle {
+            diff_handle.update_document(self.text.clone(), false);
+        }
 
         // map diagnostics over changes too
         changes.update_positions(self.diagnostics.iter_mut().map(|diagnostic| {
@@ -1326,6 +1317,14 @@ impl Document {
             apply_inlay_hint_changes(padding_after_inlay_hints);
         }
 
+        helix_event::dispatch(DocumentDidChange {
+            doc: self,
+            view: view_id,
+            old_text: &old_doc,
+            changes,
+            ghost_transaction: !emit_lsp_notification,
+        });
+
         if emit_lsp_notification {
             // TODO: move to hook
             // emit lsp notification
@@ -1342,8 +1341,19 @@ impl Document {
                 }
             }
         }
+
+        if let Some(selection) = transaction.selection() {
+            self.selections.insert(
+                view_id,
+                selection.clone().ensure_invariants(self.text.slice(..)),
+            );
+            helix_event::dispatch(SelectionDidChange {
+                doc: self,
+                view: view_id,
+            });
         }
-        success
+
+        true
     }
 
     fn apply_inner(

helix-view/src/events.rs

@@ -1,9 +1,15 @@
-use helix_core::Rope;
+use helix_core::{ChangeSet, Rope};
 use helix_event::events;
 
 use crate::{Document, ViewId};
 
 events! {
-    DocumentDidChange<'a> { doc: &'a mut Document, view: ViewId, old_text: &'a Rope  }
+    DocumentDidChange<'a> {
+        doc: &'a mut Document,
+        view: ViewId,
+        old_text: &'a Rope,
+        changes: &'a ChangeSet,
+        ghost_transaction: bool
+    }
     SelectionDidChange<'a> { doc: &'a mut Document, view: ViewId }
 }