coalesce note updates; avoid unnecessary saving due to mtime changes

qt/aqt/editor.py

@@ -449,7 +449,7 @@ class Editor:
             self.note.fields[ord] = self.mungeHTML(txt)
 
             if not self.addMode:
-                self.note.flush()
+                self._save_current_note()
                 self.mw.requireReset(reason=ResetReason.EditorBridgeCmd, context=self)
             if type == "blur":
                 self.currentField = None
@@ -542,6 +542,10 @@ class Editor:
         js = gui_hooks.editor_will_load_note(js, self.note, self)
         self.web.evalWithCallback(js, oncallback)
 
+    def _save_current_note(self) -> None:
+        "Call after note is updated with data from webview."
+        self.mw.col.update_note(self.note)
+
     def fonts(self) -> List[Tuple[str, int, bool]]:
         return [
             (gui_hooks.editor_will_use_font_for_field(f["font"]), f["size"], f["rtl"])
@@ -633,7 +637,7 @@ class Editor:
                 )
         self.note.fields[field] = html
         if not self.addMode:
-            self.note.flush()
+            self._save_current_note()
         self.loadNote(focusTo=field)
         saveGeom(d, "htmlEditor")
 
@@ -673,7 +677,7 @@ class Editor:
             return
         self.note.tags = self.mw.col.tags.split(self.tags.text())
         if not self.addMode:
-            self.note.flush()
+            self._save_current_note()
         gui_hooks.editor_did_update_tags(self.note)
 
     def saveAddModeVars(self) -> None:

rslib/src/notes/mod.rs

@@ -334,7 +334,7 @@ impl Collection {
         op: Option<UndoableOpKind>,
     ) -> Result<()> {
         let mut existing_note = self.storage.get_note(note.id)?.ok_or(AnkiError::NotFound)?;
-        if !note_modified(&mut existing_note, note) {
+        if !note_differs_from_db(&mut existing_note, note) {
             // nothing to do
             return Ok(());
         }
@@ -382,7 +382,7 @@ impl Collection {
         if mark_note_modified {
             note.set_modified(usn);
         }
-        self.update_note_undoable(note, original)
+        self.update_note_undoable(note, original, true)
     }
 
     /// Remove provided notes, and any cards that use them.
@@ -537,9 +537,12 @@ impl Collection {
 /// The existing note pulled from the DB will have sfld and csum set, but the
 /// note we receive from the frontend won't. Temporarily zero them out and
 /// compare, then restore them again.
-fn note_modified(existing_note: &mut Note, note: &Note) -> bool {
+/// Also set mtime to existing, since the frontend may have a stale mtime, and
+/// we'll bump it as we save in any case.
+fn note_differs_from_db(existing_note: &mut Note, note: &mut Note) -> bool {
     let sort_field = existing_note.sort_field.take();
     let checksum = existing_note.checksum.take();
+    note.mtime = existing_note.mtime;
     let notes_differ = existing_note != note;
     existing_note.sort_field = sort_field;
     existing_note.checksum = checksum;

rslib/src/notes/undo.rs

@@ -1,7 +1,7 @@
 // Copyright: Ankitects Pty Ltd and contributors
 // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 
-use crate::prelude::*;
+use crate::{prelude::*, undo::UndoableChange};
 
 #[derive(Debug)]
 pub(crate) enum UndoableNoteChange {
@@ -16,12 +16,12 @@ impl Collection {
     pub(crate) fn undo_note_change(&mut self, change: UndoableNoteChange) -> Result<()> {
         match change {
             UndoableNoteChange::Added(note) => self.remove_note_without_grave(*note),
-            UndoableNoteChange::Updated(mut note) => {
+            UndoableNoteChange::Updated(note) => {
                 let current = self
                     .storage
                     .get_note(note.id)?
                     .ok_or_else(|| AnkiError::invalid_input("note disappeared"))?;
-                self.update_note_undoable(&mut *note, &current)
+                self.update_note_undoable(&note, &current, false)
             }
             UndoableNoteChange::Removed(note) => self.restore_deleted_note(*note),
             UndoableNoteChange::GraveAdded(e) => self.remove_note_grave(e.0, e.1),
@@ -31,8 +31,17 @@ impl Collection {
 
     /// Saves in the undo queue, and commits to DB.
     /// No validation, card generation or normalization is done.
-    pub(super) fn update_note_undoable(&mut self, note: &mut Note, original: &Note) -> Result<()> {
+    /// If `coalesce_updates` is true, successive updates within a 1 minute
+    /// period will not result in further undo entries.
+    pub(super) fn update_note_undoable(
+        &mut self,
+        note: &Note,
+        original: &Note,
+        coalesce_updates: bool,
+    ) -> Result<()> {
+        if !coalesce_updates || !self.note_was_just_updated(note) {
             self.save_undo(UndoableNoteChange::Updated(Box::new(original.clone())));
+        }
         self.storage.update_note(note)?;
 
         Ok(())
@@ -77,4 +86,22 @@ impl Collection {
         self.save_undo(UndoableNoteChange::GraveRemoved(Box::new((nid, usn))));
         self.storage.remove_note_grave(nid)
     }
+
+    /// True only if the last operation was UpdateNote, and the same note was just updated less than
+    /// a minute ago.
+    fn note_was_just_updated(&self, before_change: &Note) -> bool {
+        self.previous_undo_op()
+            .map(|op| {
+                if let Some(UndoableChange::Note(UndoableNoteChange::Updated(note))) =
+                    op.changes.last()
+                {
+                    note.id == before_change.id
+                        && op.kind == UndoableOpKind::UpdateNote
+                        && op.timestamp.elapsed_secs() < 60
+                } else {
+                    false
+                }
+            })
+            .unwrap_or(false)
+    }
 }

rslib/src/undo/mod.rs

@@ -14,9 +14,10 @@ use std::collections::VecDeque;
 const UNDO_LIMIT: usize = 30;
 
 #[derive(Debug)]
-struct UndoableOp {
+pub(crate) struct UndoableOp {
-    kind: UndoableOpKind,
+    pub kind: UndoableOpKind,
-    changes: Vec<UndoableChange>,
+    pub timestamp: TimestampSecs,
+    pub changes: Vec<UndoableChange>,
 }
 
 #[derive(Debug, PartialEq)]
@@ -61,12 +62,14 @@ impl UndoManager {
         }
         self.current_step = op.map(|op| UndoableOp {
             kind: op,
+            timestamp: TimestampSecs::now(),
             changes: vec![],
         });
     }
 
     fn end_step(&mut self) {
         if let Some(step) = self.current_step.take() {
+            if !step.changes.is_empty() {
                 if self.mode == UndoMode::Undoing {
                     self.redo_steps.push(step);
                 } else {
@@ -74,6 +77,7 @@ impl UndoManager {
                     self.undo_steps.push_front(step);
                 }
             }
+        }
         println!("ended, undo steps count now {}", self.undo_steps.len());
     }
 
@@ -91,6 +95,10 @@ impl UndoManager {
     fn can_redo(&self) -> Option<UndoableOpKind> {
         self.redo_steps.last().map(|s| s.kind)
     }
+
+    pub(crate) fn previous_op(&self) -> Option<&UndoableOp> {
+        self.undo_steps.front()
+    }
 }
 
 impl Collection {
@@ -170,6 +178,10 @@ impl Collection {
     pub(crate) fn save_undo(&mut self, item: impl Into<UndoableChange>) {
         self.state.undo.save(item.into());
     }
+
+    pub(crate) fn previous_undo_op(&self) -> Option<&UndoableOp> {
+        self.state.undo.previous_op()
+    }
 }
 
 #[cfg(test)]