diff --git a/rslib/src/collection/transact.rs b/rslib/src/collection/transact.rs index 5c196286f..e40892616 100644 --- a/rslib/src/collection/transact.rs +++ b/rslib/src/collection/transact.rs @@ -9,6 +9,7 @@ impl Collection { F: FnOnce(&mut Collection) -> Result, { let have_op = op.is_some(); + let skip_undo_queue = op == Some(Op::SkipUndo); self.storage.begin_rust_trx()?; self.begin_undoable_operation(op); @@ -36,7 +37,7 @@ impl Collection { changes: StateChanges::default(), } }; - self.end_undoable_operation(); + self.end_undoable_operation(skip_undo_queue); Ok(OpOutput { output, changes }) }) // roll back on error diff --git a/rslib/src/config/bool.rs b/rslib/src/config/bool.rs index d6bbd25fc..848918f37 100644 --- a/rslib/src/config/bool.rs +++ b/rslib/src/config/bool.rs @@ -76,11 +76,13 @@ impl Collection { value: bool, undoable: bool, ) -> Result> { - self.transact(Op::UpdateConfig, |col| { + let op = if undoable { + Op::UpdateConfig + } else { + Op::SkipUndo + }; + self.transact(op, |col| { col.set_config(key, &value)?; - if !undoable { - col.clear_current_undo_step_changes()?; - } Ok(()) }) } diff --git a/rslib/src/config/mod.rs b/rslib/src/config/mod.rs index 6cc6bcf5f..72490e1ac 100644 --- a/rslib/src/config/mod.rs +++ b/rslib/src/config/mod.rs @@ -76,11 +76,13 @@ impl Collection { val: &T, undoable: bool, ) -> Result> { - self.transact(Op::UpdateConfig, |col| { + let op = if undoable { + Op::UpdateConfig + } else { + Op::SkipUndo + }; + self.transact(op, |col| { col.set_config(key, val)?; - if !undoable { - col.clear_current_undo_step_changes()?; - } Ok(()) }) } diff --git a/rslib/src/config/string.rs b/rslib/src/config/string.rs index edf42c868..ea7d5c739 100644 --- a/rslib/src/config/string.rs +++ b/rslib/src/config/string.rs @@ -31,11 +31,13 @@ impl Collection { val: &str, undoable: bool, ) -> Result> { - self.transact(Op::UpdateConfig, |col| { + let op = if undoable { + Op::UpdateConfig + } else { + Op::SkipUndo + }; + self.transact(op, |col| { col.set_config_string_inner(key, val)?; - if !undoable { - col.clear_current_undo_step_changes()?; - } Ok(()) }) } diff --git a/rslib/src/decks/tree.rs b/rslib/src/decks/tree.rs index 8e1922cf6..c40b065d9 100644 --- a/rslib/src/decks/tree.rs +++ b/rslib/src/decks/tree.rs @@ -307,7 +307,7 @@ impl Collection { collapsed: bool, scope: DeckCollapseScope, ) -> Result> { - self.transact(Op::ExpandCollapse, |col| { + self.transact(Op::SkipUndo, |col| { if let Some(mut deck) = col.storage.get_deck(did)? { let original = deck.clone(); let c = &mut deck.common; diff --git a/rslib/src/ops.rs b/rslib/src/ops.rs index ddf94de33..394f2d310 100644 --- a/rslib/src/ops.rs +++ b/rslib/src/ops.rs @@ -15,7 +15,6 @@ pub enum Op { ChangeNotetype, ClearUnusedTags, EmptyFilteredDeck, - ExpandCollapse, FindAndReplace, RebuildFilteredDeck, RemoveDeck, @@ -42,6 +41,9 @@ pub enum Op { UpdateTag, UpdateNotetype, SetCurrentDeck, + /// Does not register changes in undo queue, but does not clear the current + /// queue either. + SkipUndo, } impl Op { @@ -75,7 +77,6 @@ impl Op { Op::BuildFilteredDeck => tr.actions_build_filtered_deck(), Op::RebuildFilteredDeck => tr.actions_build_filtered_deck(), Op::EmptyFilteredDeck => tr.studying_empty(), - Op::ExpandCollapse => tr.actions_expand_collapse(), Op::SetCurrentDeck => tr.browsing_change_deck(), Op::UpdateDeckConfig => tr.deck_config_title(), Op::AddNotetype => tr.actions_add_notetype(), @@ -84,6 +85,7 @@ impl Op { Op::UpdateConfig => tr.actions_update_config(), Op::Custom(name) => name.into(), Op::ChangeNotetype => tr.browsing_change_notetype(), + Op::SkipUndo => return "".to_string(), } .into() } @@ -133,17 +135,11 @@ impl OpChanges { // These routines should return true even if the GUI may have // special handling for an action, since we need to do the right // thing when undoing, and if multiple windows of the same type are - // open. For example, while toggling the expand/collapse state - // in the sidebar will not normally trigger a full sidebar refresh, - // requires_browser_sidebar_redraw() should still return true. + // open. pub fn requires_browser_table_redraw(&self) -> bool { let c = &self.changes; - c.card - || c.notetype - || c.config - || (c.note && self.op != Op::AddNote) - || (c.deck && self.op != Op::ExpandCollapse) + c.card || c.notetype || c.config || (c.note && self.op != Op::AddNote) || c.deck } pub fn requires_browser_sidebar_redraw(&self) -> bool { @@ -159,7 +155,7 @@ impl OpChanges { pub fn requires_study_queue_rebuild(&self) -> bool { let c = &self.changes; c.card - || (c.deck && self.op != Op::ExpandCollapse) + || c.deck || (c.config && matches!(self.op, Op::SetCurrentDeck | Op::UpdatePreferences)) || c.deck_config } diff --git a/rslib/src/tags/tree.rs b/rslib/src/tags/tree.rs index 20ed15524..a4e4ff9fa 100644 --- a/rslib/src/tags/tree.rs +++ b/rslib/src/tags/tree.rs @@ -17,7 +17,7 @@ impl Collection { } pub fn set_tag_collapsed(&mut self, tag: &str, collapsed: bool) -> Result> { - self.transact(Op::ExpandCollapse, |col| { + self.transact(Op::SkipUndo, |col| { col.set_tag_collapsed_inner(tag, collapsed, col.usn()?) }) } diff --git a/rslib/src/undo/mod.rs b/rslib/src/undo/mod.rs index ed16cd5cf..42961734d 100644 --- a/rslib/src/undo/mod.rs +++ b/rslib/src/undo/mod.rs @@ -94,17 +94,15 @@ impl UndoManager { }); } - fn end_step(&mut self) { + fn end_step(&mut self, skip_undo: bool) { if let Some(step) = self.current_step.take() { - if step.has_changes() { + if step.has_changes() && !skip_undo { if self.mode == UndoMode::Undoing { self.redo_steps.push(step); } else { self.undo_steps.truncate(UNDO_LIMIT - 1); self.undo_steps.push_front(step); } - } else { - println!("no undo changes, discarding step"); } } } @@ -171,15 +169,9 @@ impl UndoManager { /// counter value, which can be used with `merge_undoable_ops`. fn add_custom_step(&mut self, name: String) -> usize { self.begin_step(Some(Op::Custom(name))); - self.end_step(); + self.end_step(false); self.counter } - - fn clear_current_changes(&mut self) { - if let Some(op) = &mut self.current_step { - op.changes.clear(); - } - } } impl Collection { @@ -234,8 +226,8 @@ impl Collection { /// Called at the end of a successful transaction. /// In most instances, this will also clear the study queues. - pub(crate) fn end_undoable_operation(&mut self) { - self.state.undo.end_step(); + pub(crate) fn end_undoable_operation(&mut self, skip_undo: bool) { + self.state.undo.end_step(skip_undo); } pub(crate) fn discard_undo_and_study_queues(&mut self) { @@ -253,17 +245,6 @@ impl Collection { self.state.undo.save(item.into()); } - /// Forget any recorded changes in the current transaction, allowing - /// a minor change like a config update to bypass undo. Bumps mtime if - /// there were pending changes. - pub(crate) fn clear_current_undo_step_changes(&mut self) -> Result<()> { - if self.current_undo_step_has_changes() { - self.set_modified()?; - } - self.state.undo.clear_current_changes(); - Ok(()) - } - pub(crate) fn current_undo_op(&self) -> Option<&UndoableOp> { self.state.undo.current_op() } @@ -554,13 +535,13 @@ mod test { ); assert!(!out.changes.had_change()); - // op output won't reflect changes were made + // op output will reflect changes were made let out = col.set_config_bool(BoolKey::AddingDefaultsToCurrentDeck, true, false)?; assert_ne!( col.storage.get_collection_timestamps()?.collection_change.0, 0 ); - assert!(!out.changes.had_change()); + assert!(out.changes.had_change()); Ok(()) }