initial work on undo support

pylib/tests/test_schedv1.py

@@ -681,7 +681,7 @@ def test_cram():
     c.col = None
     c2 = copy.deepcopy(c)
     c2.col = c.col = d
-    c2.id = 123
+    c2.id = 0
     c2.ord = 1
     c2.due = 325
     c2.col = c.col

rslib/src/backend/mod.rs

@@ -635,7 +635,15 @@ impl Backend {
 
     fn update_card(&self, pbcard: pb::Card) -> Result<()> {
         let mut card = pbcard_to_native(pbcard)?;
-        self.with_col(|col| col.transact(None, |ctx| ctx.update_card(&mut card)))
+        self.with_col(|col| {
+            col.transact(None, |ctx| {
+                let orig = ctx
+                    .storage
+                    .get_card(card.id)?
+                    .ok_or_else(|| AnkiError::invalid_input("missing card"))?;
+                ctx.update_card(&mut card, &orig)
+            })
+        })
     }
 
     fn add_card(&self, pbcard: pb::Card) -> Result<i64> {

rslib/src/card.rs

@@ -5,7 +5,7 @@ use crate::decks::DeckID;
 use crate::define_newtype;
 use crate::err::{AnkiError, Result};
 use crate::notes::NoteID;
-use crate::{collection::Collection, timestamp::TimestampSecs, types::Usn};
+use crate::{collection::Collection, timestamp::TimestampSecs, types::Usn, undo::Undoable};
 use num_enum::TryFromPrimitive;
 use serde_repr::{Deserialize_repr, Serialize_repr};
 
@@ -86,11 +86,43 @@ impl Default for Card {
     }
 }
 
+#[derive(Debug)]
+pub(crate) struct UpdateCardUndo(Card);
+
+impl Undoable for UpdateCardUndo {
+    fn apply(&self, col: &mut crate::collection::Collection) -> Result<()> {
+        let current = col
+            .storage
+            .get_card(self.0.id)?
+            .ok_or_else(|| AnkiError::invalid_input("card disappeared"))?;
+        // when  called here, update_card should be placing the original content into the redo queue
+        col.update_card(&mut self.0.clone(), &current)
+    }
+}
+
 impl Collection {
-    pub(crate) fn update_card(&mut self, card: &mut Card) -> Result<()> {
+    #[cfg(test)]
+    pub(crate) fn get_and_update_card<F, T>(&mut self, cid: CardID, func: F) -> Result<Card>
+    where
+        F: FnOnce(&mut Card) -> Result<T>,
+    {
+        let orig = self
+            .storage
+            .get_card(cid)?
+            .ok_or_else(|| AnkiError::invalid_input("no such card"))?;
+        let mut card = orig.clone();
+        func(&mut card)?;
+        self.update_card(&mut card, &orig)?;
+        Ok(card)
+    }
+
+    pub(crate) fn update_card(&mut self, card: &mut Card, original: &Card) -> Result<()> {
         if card.id.0 == 0 {
             return Err(AnkiError::invalid_input("card id not set"));
         }
+        self.state
+            .undo
+            .save_undoable(Box::new(UpdateCardUndo(original.clone())));
         card.mtime = TimestampSecs::now();
         card.usn = self.usn()?;
         self.storage.update_card(card)
@@ -106,3 +138,99 @@ impl Collection {
         self.storage.add_card(card)
     }
 }
+
+#[cfg(test)]
+mod test {
+    use super::Card;
+    use crate::collection::{open_test_collection, CollectionOp};
+
+    #[test]
+    fn undo() {
+        let mut col = open_test_collection();
+
+        let mut card = Card::default();
+        card.ivl = 1;
+        col.add_card(&mut card).unwrap();
+        let cid = card.id;
+
+        assert_eq!(col.can_undo(), None);
+        assert_eq!(col.can_redo(), None);
+
+        // outside of a transaction, no undo info recorded
+        let card = col
+            .get_and_update_card(cid, |card| {
+                card.ivl = 2;
+                Ok(())
+            })
+            .unwrap();
+        assert_eq!(card.ivl, 2);
+        assert_eq!(col.can_undo(), None);
+        assert_eq!(col.can_redo(), None);
+
+        // record a few undo steps
+        for i in 3..=4 {
+            col.transact(Some(CollectionOp::UpdateCard), |col| {
+                col.get_and_update_card(cid, |card| {
+                    card.ivl = i;
+                    Ok(())
+                })
+                .unwrap();
+                Ok(())
+            })
+            .unwrap();
+        }
+
+        assert_eq!(col.storage.get_card(cid).unwrap().unwrap().ivl, 4);
+        assert_eq!(col.can_undo(), Some(CollectionOp::UpdateCard));
+        assert_eq!(col.can_redo(), None);
+
+        // undo a step
+        col.undo().unwrap();
+        assert_eq!(col.storage.get_card(cid).unwrap().unwrap().ivl, 3);
+        assert_eq!(col.can_undo(), Some(CollectionOp::UpdateCard));
+        assert_eq!(col.can_redo(), Some(CollectionOp::UpdateCard));
+
+        // and again
+        col.undo().unwrap();
+        assert_eq!(col.storage.get_card(cid).unwrap().unwrap().ivl, 2);
+        assert_eq!(col.can_undo(), None);
+        assert_eq!(col.can_redo(), Some(CollectionOp::UpdateCard));
+
+        // redo a step
+        col.redo().unwrap();
+        assert_eq!(col.storage.get_card(cid).unwrap().unwrap().ivl, 3);
+        assert_eq!(col.can_undo(), Some(CollectionOp::UpdateCard));
+        assert_eq!(col.can_redo(), Some(CollectionOp::UpdateCard));
+
+        // and another
+        col.redo().unwrap();
+        assert_eq!(col.storage.get_card(cid).unwrap().unwrap().ivl, 4);
+        assert_eq!(col.can_undo(), Some(CollectionOp::UpdateCard));
+        assert_eq!(col.can_redo(), None);
+
+        // and undo the redo
+        col.undo().unwrap();
+        assert_eq!(col.storage.get_card(cid).unwrap().unwrap().ivl, 3);
+        assert_eq!(col.can_undo(), Some(CollectionOp::UpdateCard));
+        assert_eq!(col.can_redo(), Some(CollectionOp::UpdateCard));
+
+        // if any action is performed, it should clear the redo queue
+        col.transact(Some(CollectionOp::UpdateCard), |col| {
+            col.get_and_update_card(cid, |card| {
+                card.ivl = 5;
+                Ok(())
+            })
+            .unwrap();
+            Ok(())
+        })
+        .unwrap();
+        assert_eq!(col.storage.get_card(cid).unwrap().unwrap().ivl, 5);
+        assert_eq!(col.can_undo(), Some(CollectionOp::UpdateCard));
+        assert_eq!(col.can_redo(), None);
+
+        // and any action that doesn't support undoing will clear both queues
+        col.transact(None, |_col| Ok(())).unwrap();
+        assert_eq!(col.can_undo(), None);
+        assert_eq!(col.can_redo(), None);
+    }
+}

rslib/src/collection.rs

@@ -6,7 +6,7 @@ use crate::i18n::I18n;
 use crate::log::Logger;
 use crate::timestamp::TimestampSecs;
 use crate::types::Usn;
-use crate::{sched::cutoff::SchedTimingToday, storage::SqliteStorage};
+use crate::{sched::cutoff::SchedTimingToday, storage::SqliteStorage, undo::UndoManager};
 use std::path::PathBuf;
 
 pub fn open_collection<P: Into<PathBuf>>(
@@ -34,9 +34,17 @@ pub fn open_collection<P: Into<PathBuf>>(
     Ok(col)
 }
 
+#[cfg(test)]
+pub fn open_test_collection() -> Collection {
+    use crate::log;
+    let i18n = I18n::new(&[""], "", log::terminal());
+    open_collection(":memory:", "", "", false, i18n, log::terminal()).unwrap()
+}
+
 #[derive(Debug, Default)]
 pub struct CollectionState {
     task_state: CollectionTaskState,
+    pub(crate) undo: UndoManager,
     timing_today: Option<SchedTimingToday>,
 }
 
@@ -62,10 +70,13 @@ pub struct Collection {
     pub(crate) i18n: I18n,
     pub(crate) log: Logger,
     pub(crate) server: bool,
-    state: CollectionState,
+    pub(crate) state: CollectionState,
 }
 
-pub(crate) enum CollectionOp {}
+#[derive(Debug, Clone, PartialEq)]
+pub enum CollectionOp {
+    UpdateCard,
+}
 
 impl Collection {
     /// Execute the provided closure in a transaction, rolling back if
@@ -75,19 +86,23 @@ impl Collection {
         F: FnOnce(&mut Collection) -> Result<R>,
     {
         self.storage.begin_rust_trx()?;
+        self.state.undo.begin_step(op);
 
         let mut res = func(self);
 
         if res.is_ok() {
             if let Err(e) = self.storage.mark_modified() {
                 res = Err(e);
-            } else if let Err(e) = self.storage.commit_rust_op(op) {
+            } else if let Err(e) = self.storage.commit_rust_trx() {
                 res = Err(e);
             }
         }
 
         if res.is_err() {
+            self.state.undo.discard_step();
             self.storage.rollback_rust_trx()?;
+        } else {
+            self.state.undo.end_step();
         }
 
         res

rslib/src/lib.rs

@@ -30,3 +30,4 @@ pub mod template_filters;
 pub mod text;
 pub mod timestamp;
 pub mod types;
+pub mod undo;

rslib/src/storage/sqlite.rs

@@ -1,7 +1,6 @@
 // Copyright: Ankitects Pty Ltd and contributors
 // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 
-use crate::collection::CollectionOp;
 use crate::config::Config;
 use crate::decks::DeckID;
 use crate::err::Result;
@@ -232,10 +231,6 @@ impl SqliteStorage {
         Ok(())
     }
 
-    pub(crate) fn commit_rust_op(&self, _op: Option<CollectionOp>) -> Result<()> {
-        self.commit_rust_trx()
-    }
-
     pub(crate) fn rollback_rust_trx(&self) -> Result<()> {
         self.db
             .prepare_cached("rollback to rust")?

rslib/src/undo.rs

@@ -0,0 +1,126 @@
+// Copyright: Ankitects Pty Ltd and contributors
+// License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
+
+use crate::{
+    collection::{Collection, CollectionOp},
+    err::Result,
+};
+use std::fmt;
+
+pub(crate) trait Undoable: fmt::Debug + Send {
+    fn apply(&self, ctx: &mut Collection) -> Result<()>;
+}
+
+#[derive(Debug)]
+struct UndoStep {
+    kind: CollectionOp,
+    changes: Vec<Box<dyn Undoable>>,
+}
+
+#[derive(Debug, PartialEq)]
+enum UndoMode {
+    NormalOp,
+    Undoing,
+    Redoing,
+}
+
+impl Default for UndoMode {
+    fn default() -> Self {
+        Self::NormalOp
+    }
+}
+
+#[derive(Debug, Default)]
+pub(crate) struct UndoManager {
+    undo_steps: Vec<UndoStep>,
+    redo_steps: Vec<UndoStep>,
+    mode: UndoMode,
+    current_step: Option<UndoStep>,
+}
+
+impl UndoManager {
+    pub(crate) fn save_undoable(&mut self, item: Box<dyn Undoable>) {
+        if let Some(step) = self.current_step.as_mut() {
+            step.changes.push(item)
+        }
+    }
+
+    pub(crate) fn begin_step(&mut self, op: Option<CollectionOp>) {
+        if op.is_none() {
+            // action doesn't support undoing; clear the queue
+            self.undo_steps.clear();
+            self.redo_steps.clear();
+        } else if self.mode == UndoMode::NormalOp {
+            // a normal op clears the redo queue
+            self.redo_steps.clear();
+        }
+        self.current_step = op.map(|op| UndoStep {
+            kind: op,
+            changes: vec![],
+        });
+    }
+
+    pub(crate) fn end_step(&mut self) {
+        if let Some(step) = self.current_step.take() {
+            if self.mode == UndoMode::Undoing {
+                self.redo_steps.push(step);
+            } else {
+                self.undo_steps.push(step);
+            }
+        }
+    }
+
+    pub(crate) fn discard_step(&mut self) {
+        self.begin_step(None)
+    }
+
+    fn can_undo(&self) -> Option<CollectionOp> {
+        self.undo_steps.last().map(|s| s.kind.clone())
+    }
+
+    fn can_redo(&self) -> Option<CollectionOp> {
+        self.redo_steps.last().map(|s| s.kind.clone())
+    }
+}
+
+impl Collection {
+    pub fn can_undo(&self) -> Option<CollectionOp> {
+        self.state.undo.can_undo()
+    }
+
+    pub fn can_redo(&self) -> Option<CollectionOp> {
+        self.state.undo.can_redo()
+    }
+
+    pub fn undo(&mut self) -> Result<()> {
+        if let Some(step) = self.state.undo.undo_steps.pop() {
+            let changes = step.changes;
+            self.state.undo.mode = UndoMode::Undoing;
+            let res = self.transact(Some(step.kind), |col| {
+                for change in changes.iter().rev() {
+                    change.apply(col)?;
+                }
+                Ok(())
+            });
+            self.state.undo.mode = UndoMode::NormalOp;
+            res?;
+        }
+        Ok(())
+    }
+
+    pub fn redo(&mut self) -> Result<()> {
+        if let Some(step) = self.state.undo.redo_steps.pop() {
+            let changes = step.changes;
+            self.state.undo.mode = UndoMode::Redoing;
+            let res = self.transact(Some(step.kind), |col| {
+                for change in changes.iter().rev() {
+                    change.apply(col)?;
+                }
+                Ok(())
+            });
+            self.state.undo.mode = UndoMode::NormalOp;
+            res?;
+        }
+        Ok(())
+    }
+}