diff --git a/ftl/core/undo.ftl b/ftl/core/undo.ftl index e60c9a2a4..e3ae07497 100644 --- a/ftl/core/undo.ftl +++ b/ftl/core/undo.ftl @@ -11,7 +11,9 @@ undo-action-redone = { $action } redone undo-answer-card = Answer Card undo-unbury-unsuspend = Unbury/Unsuspend +undo-add-deck = Add Deck undo-add-note = Add Note undo-update-tag = Update Tag undo-update-note = Update Note undo-update-card = Update Card +undo-update-deck = Update Deck diff --git a/qt/aqt/deckbrowser.py b/qt/aqt/deckbrowser.py index 565079f06..96d839fe3 100644 --- a/qt/aqt/deckbrowser.py +++ b/qt/aqt/deckbrowser.py @@ -258,7 +258,6 @@ class DeckBrowser: self.mw.onExport(did=did) def _rename(self, did: int) -> None: - self.mw.checkpoint(tr(TR.ACTIONS_RENAME_DECK)) deck = self.mw.col.decks.get(did) oldName = deck["name"] newName = getOnlyText(tr(TR.DECKS_NEW_DECK_NAME), default=oldName) @@ -271,6 +270,7 @@ class DeckBrowser: except DeckIsFilteredError as err: showWarning(str(err)) return + self.mw.update_undo_actions() self.show() def _options(self, did: str) -> None: @@ -318,10 +318,10 @@ class DeckBrowser: return self.mw.col.decks.rem(did, True) def on_done(fut: Future) -> None: + self.mw.update_undo_actions() self.show() res = fut.result() # Required to check for errors - self.mw.checkpoint(tr(TR.DECKS_DELETE_DECK)) self.mw.taskman.with_progress(do_delete, on_done) # Top buttons diff --git a/qt/aqt/sidebar.py b/qt/aqt/sidebar.py index 513144057..bb5e242b3 100644 --- a/qt/aqt/sidebar.py +++ b/qt/aqt/sidebar.py @@ -989,7 +989,6 @@ class SidebarTreeView(QTreeView): new_name = new_name.replace('"', "") if not new_name or new_name == old_name: return - self.mw.checkpoint(tr(TR.ACTIONS_RENAME_DECK)) try: self.mw.col.decks.rename(deck, new_name) except DeckIsFilteredError as err: @@ -997,6 +996,7 @@ class SidebarTreeView(QTreeView): return self.refresh() self.mw.deckBrowser.refresh() + self.mw.update_undo_actions() def remove_tag(self, item: SidebarItem) -> None: self.browser.editor.saveNow(lambda: self._remove_tag(item)) @@ -1063,7 +1063,6 @@ class SidebarTreeView(QTreeView): self.refresh() res = fut.result() # Required to check for errors - self.mw.checkpoint(tr(TR.DECKS_DELETE_DECK)) self.browser.model.beginReset() self.mw.taskman.run_in_background(do_delete, on_done) diff --git a/rslib/src/decks/mod.rs b/rslib/src/decks/mod.rs index 8a70f5470..16cd714b5 100644 --- a/rslib/src/decks/mod.rs +++ b/rslib/src/decks/mod.rs @@ -10,7 +10,10 @@ pub use crate::backend_proto::{ deck_kind::Kind as DeckKind, filtered_search_term::FilteredSearchOrder, Deck as DeckProto, DeckCommon, DeckKind as DeckKindProto, FilteredDeck, FilteredSearchTerm, NormalDeck, }; -use crate::{backend_proto as pb, markdown::render_markdown, text::sanitize_html_no_images}; +use crate::{ + backend_proto as pb, markdown::render_markdown, text::sanitize_html_no_images, + undo::UndoableOpKind, +}; use crate::{ collection::Collection, deckconf::DeckConfID, @@ -263,21 +266,38 @@ impl Collection { } /// Add or update an existing deck modified by the user. May add parents, - /// or rename children as required. + /// or rename children as required. Prefer add_deck() or update_deck() to + /// be explicit about your intentions; this function mainly exists so we + /// can integrate with older Python code that behaved this way. pub(crate) fn add_or_update_deck(&mut self, deck: &mut Deck) -> Result<()> { - self.state.deck_cache.clear(); + if deck.id.0 == 0 { + self.add_deck(deck) + } else { + self.update_deck(deck) + } + } - self.transact(None, |col| { + /// Add a new deck. The id must be 0, as it will be automatically assigned. + pub fn add_deck(&mut self, deck: &mut Deck) -> Result<()> { + if deck.id.0 != 0 { + return Err(AnkiError::invalid_input("deck to add must have id 0")); + } + + self.transact(Some(UndoableOpKind::AddDeck), |col| { let usn = col.usn()?; - col.prepare_deck_for_update(deck, usn)?; deck.set_modified(usn); + col.match_or_create_parents(deck, usn)?; + col.add_deck_undoable(deck) + }) + } - if deck.id.0 == 0 { - // TODO: undo support - col.match_or_create_parents(deck, usn)?; - col.storage.add_deck(deck) - } else if let Some(existing_deck) = col.storage.get_deck(deck.id)? { + pub(crate) fn update_deck(&mut self, deck: &mut Deck) -> Result<()> { + self.transact(Some(UndoableOpKind::UpdateDeck), |col| { + let usn = col.usn()?; + col.prepare_deck_for_update(deck, usn)?; + deck.set_modified(usn); + if let Some(existing_deck) = col.storage.get_deck(deck.id)? { let name_changed = existing_deck.name != deck.name; if name_changed { // match closest parent name @@ -301,15 +321,13 @@ impl Collection { /// Add/update a single deck when syncing/importing. Ensures name is unique /// & normalized, but does not check parents/children or update mtime /// (unless the name was changed). Caller must set up transaction. - /// TODO: undo support pub(crate) fn add_or_update_single_deck_with_existing_id( &mut self, deck: &mut Deck, usn: Usn, ) -> Result<()> { - self.state.deck_cache.clear(); self.prepare_deck_for_update(deck, usn)?; - self.storage.add_or_update_deck_with_existing_id(deck) + self.add_or_update_deck_with_existing_id_undoable(deck) } pub(crate) fn ensure_deck_name_unique(&self, deck: &mut Deck, usn: Usn) -> Result<()> { @@ -367,12 +385,11 @@ impl Collection { /// Add a single, normal deck with the provided name for a child deck. /// Caller must have done necessarily validation on name. - fn add_parent_deck(&self, machine_name: &str, usn: Usn) -> Result<()> { + fn add_parent_deck(&mut self, machine_name: &str, usn: Usn) -> Result<()> { let mut deck = Deck::new_normal(); deck.name = machine_name.into(); deck.set_modified(usn); - // fixme: undo - self.storage.add_deck(&mut deck) + self.add_deck_undoable(&mut deck) } /// If parent deck(s) exist, rewrite name to match their case. @@ -404,7 +421,7 @@ impl Collection { } } - fn create_missing_parents(&self, mut machine_name: &str, usn: Usn) -> Result<()> { + fn create_missing_parents(&mut self, mut machine_name: &str, usn: Usn) -> Result<()> { while let Some(parent_name) = immediate_parent_name(machine_name) { if self.storage.get_deck_id(parent_name)?.is_none() { self.add_parent_deck(parent_name, usn)?; @@ -441,12 +458,8 @@ impl Collection { } pub fn remove_deck_and_child_decks(&mut self, did: DeckID) -> Result<()> { - // fixme: vet cache clearing - self.state.deck_cache.clear(); - - self.transact(None, |col| { + self.transact(Some(UndoableOpKind::RemoveDeck), |col| { let usn = col.usn()?; - if let Some(deck) = col.storage.get_deck(did)? { let child_decks = col.storage.child_decks(&deck)?; @@ -463,23 +476,20 @@ impl Collection { } pub(crate) fn remove_single_deck(&mut self, deck: &Deck, usn: Usn) -> Result<()> { - // fixme: undo match deck.kind { DeckKind::Normal(_) => self.delete_all_cards_in_normal_deck(deck.id)?, DeckKind::Filtered(_) => self.return_all_cards_in_filtered_deck(deck.id)?, } self.clear_aux_config_for_deck(deck.id)?; if deck.id.0 == 1 { + // if deleting the default deck, ensure there's a new one, and avoid the grave let mut deck = deck.to_owned(); - // fixme: separate key deck.name = self.i18n.tr(TR::DeckConfigDefaultName).into(); deck.set_modified(usn); - self.add_or_update_single_deck_with_existing_id(&mut deck, usn)?; + self.add_or_update_single_deck_with_existing_id(&mut deck, usn) } else { - self.storage.remove_deck(deck.id)?; - self.storage.add_deck_grave(deck.id, usn)?; + self.remove_deck_and_add_grave_undoable(deck.clone(), usn) } - Ok(()) } fn delete_all_cards_in_normal_deck(&mut self, did: DeckID) -> Result<()> { diff --git a/rslib/src/decks/undo.rs b/rslib/src/decks/undo.rs index 3251a0a91..e4fe37749 100644 --- a/rslib/src/decks/undo.rs +++ b/rslib/src/decks/undo.rs @@ -6,12 +6,17 @@ use crate::prelude::*; #[derive(Debug)] pub(crate) enum UndoableDeckChange { + Added(Box), Updated(Box), + Removed(Box), + GraveAdded(Box<(DeckID, Usn)>), + GraveRemoved(Box<(DeckID, Usn)>), } impl Collection { pub(crate) fn undo_deck_change(&mut self, change: UndoableDeckChange) -> Result<()> { match change { + UndoableDeckChange::Added(deck) => self.remove_deck_undoable(*deck), UndoableDeckChange::Updated(mut deck) => { let current = self .storage @@ -19,9 +24,28 @@ impl Collection { .ok_or_else(|| AnkiError::invalid_input("deck disappeared"))?; self.update_single_deck_undoable(&mut *deck, ¤t) } + UndoableDeckChange::Removed(deck) => self.restore_deleted_deck(*deck), + UndoableDeckChange::GraveAdded(e) => self.remove_deck_grave(e.0, e.1), + UndoableDeckChange::GraveRemoved(e) => self.add_deck_grave_undoable(e.0, e.1), } } + pub(super) fn add_deck_undoable(&mut self, deck: &mut Deck) -> Result<(), AnkiError> { + self.storage.add_deck(deck)?; + self.save_undo(UndoableDeckChange::Added(Box::new(deck.clone()))); + Ok(()) + } + + pub(super) fn add_or_update_deck_with_existing_id_undoable( + &mut self, + deck: &mut Deck, + ) -> Result<(), AnkiError> { + self.state.deck_cache.clear(); + self.storage.add_or_update_deck_with_existing_id(deck)?; + self.save_undo(UndoableDeckChange::Added(Box::new(deck.clone()))); + Ok(()) + } + /// Update an individual, existing deck. Caller is responsible for ensuring deck /// is normalized, matches parents, is not a duplicate name, and bumping mtime. /// Clears deck cache. @@ -34,4 +58,38 @@ impl Collection { self.save_undo(UndoableDeckChange::Updated(Box::new(original.clone()))); self.storage.update_deck(deck) } + + pub(crate) fn remove_deck_and_add_grave_undoable( + &mut self, + deck: Deck, + usn: Usn, + ) -> Result<()> { + self.state.deck_cache.clear(); + self.add_deck_grave_undoable(deck.id, usn)?; + self.storage.remove_deck(deck.id)?; + self.save_undo(UndoableDeckChange::Removed(Box::new(deck))); + Ok(()) + } + + fn restore_deleted_deck(&mut self, deck: Deck) -> Result<()> { + self.storage.add_or_update_deck_with_existing_id(&deck)?; + self.save_undo(UndoableDeckChange::Added(Box::new(deck))); + Ok(()) + } + + fn remove_deck_undoable(&mut self, deck: Deck) -> Result<()> { + self.storage.remove_deck(deck.id)?; + self.save_undo(UndoableDeckChange::Removed(Box::new(deck))); + Ok(()) + } + + fn add_deck_grave_undoable(&mut self, did: DeckID, usn: Usn) -> Result<()> { + self.save_undo(UndoableDeckChange::GraveAdded(Box::new((did, usn)))); + self.storage.add_deck_grave(did, usn) + } + + fn remove_deck_grave(&mut self, did: DeckID, usn: Usn) -> Result<()> { + self.save_undo(UndoableDeckChange::GraveRemoved(Box::new((did, usn)))); + self.storage.remove_deck_grave(did) + } } diff --git a/rslib/src/storage/deck/mod.rs b/rslib/src/storage/deck/mod.rs index 14e50597c..e2f76e88a 100644 --- a/rslib/src/storage/deck/mod.rs +++ b/rslib/src/storage/deck/mod.rs @@ -138,8 +138,8 @@ impl SqliteStorage { } } - /// Used for syncing; will keep existing ID. Shouldn't be used to add new decks locally, - /// since it does not allocate an id. + /// Used for syncing&undo; will keep existing ID. Shouldn't be used to add + /// new decks locally, since it does not allocate an id. pub(crate) fn add_or_update_deck_with_existing_id(&self, deck: &Deck) -> Result<()> { if deck.id.0 == 0 { return Err(AnkiError::invalid_input("deck with id 0")); diff --git a/rslib/src/storage/graves/mod.rs b/rslib/src/storage/graves/mod.rs index e9884126f..b8d9c0021 100644 --- a/rslib/src/storage/graves/mod.rs +++ b/rslib/src/storage/graves/mod.rs @@ -48,6 +48,10 @@ impl SqliteStorage { self.remove_grave(nid.0, GraveKind::Note) } + pub(crate) fn remove_deck_grave(&self, did: DeckID) -> Result<()> { + self.remove_grave(did.0, GraveKind::Deck) + } + pub(crate) fn pending_graves(&self, pending_usn: Usn) -> Result { let mut stmt = self.db.prepare(&format!( "select oid, type from graves where {}", diff --git a/rslib/src/undo/ops.rs b/rslib/src/undo/ops.rs index 028549224..d708ef943 100644 --- a/rslib/src/undo/ops.rs +++ b/rslib/src/undo/ops.rs @@ -5,16 +5,19 @@ use crate::prelude::*; #[derive(Debug, Clone, Copy, PartialEq)] pub enum UndoableOpKind { - UpdateCard, + AddDeck, + AddNote, AnswerCard, Bury, + RemoveDeck, + RemoveNote, Suspend, UnburyUnsuspend, - AddNote, - RemoveNote, - UpdateTag, + UpdateCard, + UpdateDeck, UpdateNote, UpdatePreferences, + UpdateTag, } impl UndoableOpKind { @@ -26,16 +29,19 @@ impl UndoableOpKind { impl Collection { pub fn describe_op_kind(&self, op: UndoableOpKind) -> String { let key = match op { - UndoableOpKind::UpdateCard => TR::UndoUpdateCard, + UndoableOpKind::AddDeck => TR::UndoAddDeck, + UndoableOpKind::AddNote => TR::UndoAddNote, UndoableOpKind::AnswerCard => TR::UndoAnswerCard, UndoableOpKind::Bury => TR::StudyingBury, + UndoableOpKind::RemoveDeck => TR::DecksDeleteDeck, + UndoableOpKind::RemoveNote => TR::StudyingDeleteNote, UndoableOpKind::Suspend => TR::StudyingSuspend, UndoableOpKind::UnburyUnsuspend => TR::UndoUnburyUnsuspend, - UndoableOpKind::AddNote => TR::UndoAddNote, - UndoableOpKind::RemoveNote => TR::StudyingDeleteNote, - UndoableOpKind::UpdateTag => TR::UndoUpdateTag, + UndoableOpKind::UpdateCard => TR::UndoUpdateCard, + UndoableOpKind::UpdateDeck => TR::UndoUpdateDeck, UndoableOpKind::UpdateNote => TR::UndoUpdateNote, UndoableOpKind::UpdatePreferences => TR::PreferencesPreferences, + UndoableOpKind::UpdateTag => TR::UndoUpdateTag, }; self.i18n.tr(key).to_string()