From 7ffe7d69c51294b899859a3c8ce33225f359bbad Mon Sep 17 00:00:00 2001 From: Matt Armstrong Date: Fri, 21 Jun 2024 09:19:38 -0700 Subject: [PATCH] Iterate on transactions --- src/lib.rs | 2 +- src/persist/mod.rs | 3 +- src/persist/store.rs | 355 +++++++++++++++++++++++++------------------ src/ui_state.rs | 13 +- 4 files changed, 216 insertions(+), 157 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index bb5f685..2afbb5c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -29,7 +29,7 @@ pub fn save_name() -> PathBuf { } fn handle_key_event(state: &mut ui_state::State, key_event: crossterm::event::KeyEvent) { - // TODO: do this combinding earlier, properly. + // TODO: do this combining earlier, properly. let key_combination: crokey::KeyCombination = key_event.into(); if let Some(screen) = state.current_screen.take() { diff --git a/src/persist/mod.rs b/src/persist/mod.rs index e8ee404..b2e9444 100644 --- a/src/persist/mod.rs +++ b/src/persist/mod.rs @@ -3,7 +3,8 @@ pub(crate) use document::{load_tasks, save_tasks, TaskList}; pub(crate) use task::{Task, TaskId}; -pub(crate) use self::store::{MemoryStore, Store}; +pub(crate) use self::store::memory::MemoryStore; +pub(crate) use self::store::Store; mod container; mod crc; diff --git a/src/persist/store.rs b/src/persist/store.rs index fc822f7..5cf70f8 100644 --- a/src/persist/store.rs +++ b/src/persist/store.rs @@ -1,209 +1,264 @@ -use std::path::Path; +use super::{Task, TaskId}; -use anyhow::bail; +pub(crate) trait Transaction { + fn delete_task(&mut self, id: &TaskId) -> anyhow::Result<()>; -use super::{load_tasks, save_tasks, Task, TaskId, TaskList}; + // Commit and consume the transaction. + // + // See https://stackoverflow.com/q/46620790 for why this argument + // is boxed. + fn commit(self: Box) -> anyhow::Result<()>; +} pub(crate) trait Store { + // TODO: copy to Transaction, above. fn get_task(&mut self, id: &TaskId) -> anyhow::Result; + // TODO: move to Transaction, above. fn put_task(&mut self, task: &Task) -> anyhow::Result<()>; + // TODO: move to Transaction, above. fn insert_task(&mut self, previous: Option<&TaskId>, task: &Task) -> anyhow::Result<()>; - fn delete_task(&mut self, id: &TaskId) -> anyhow::Result<()>; - + // TODO: move to Transaction, above. // If prevous is None moves to the front, otherwise moves after previous. fn move_task(&mut self, previous: Option<&TaskId>, task: &TaskId) -> anyhow::Result<()>; + // TODO: move to Transaction, above. fn list_tasks(&mut self) -> anyhow::Result>; fn undo(&mut self) -> anyhow::Result<()>; fn redo(&mut self) -> anyhow::Result<()>; -} -#[derive(Default, Clone)] -struct Record { - tasks: im::HashMap, - order: im::Vector, -} + fn transaction<'a>(&'a mut self) -> Box; -impl Record { - fn get_task(&mut self, id: &TaskId) -> Result { - self.tasks - .get(id) - .map_or_else(|| bail!("task not found"), |task| Ok(task.clone())) + fn with_transaction(&mut self, callback: F) -> anyhow::Result<()> + where + F: FnOnce(&mut dyn Transaction) -> anyhow::Result<()>, + { + let mut txn = self.transaction(); + callback(txn.as_mut())?; + txn.commit()?; + Ok(()) } +} - fn put_task(&mut self, task: &Task) -> anyhow::Result<()> { - use im::hashmap::Entry::{Occupied, Vacant}; - debug_assert!( - self.order.contains(&task.id()), - "MemoryStore::put called with task not in the order list" - ); - match self.tasks.entry(task.id()) { - Occupied(mut entry) => *entry.get_mut() = task.clone(), - Vacant(_) => { - panic!("MemoryStore::put called with task not in the tasks map") - } - } +pub(crate) mod memory { + use std::path::Path; - Ok(()) - } + use anyhow::bail; - fn insert_task(&mut self, previous: Option<&TaskId>, task: &Task) -> anyhow::Result<()> { - let index = if let Some(previous) = previous { - self.order - .index_of(previous) - .map_or(0, |index| index.saturating_add(1)) - } else { - 0 - }; - - debug_assert!( - !self.order.contains(&task.id()), - "MemoryStore::insert called with task.id already in the order list" - ); - self.order.insert(index, task.id()); - self.tasks.entry(task.id()).or_insert(task.clone()); + use super::{Store, Transaction}; + use crate::persist::{load_tasks, save_tasks, Task, TaskId, TaskList}; - Ok(()) + #[derive(Default, Clone)] + struct Record { + tasks: im::HashMap, + order: im::Vector, } - fn delete_task(&mut self, id: &TaskId) -> anyhow::Result<()> { - self.order.retain(|entry| entry != id); - self.tasks.retain(|key, _| key != id); - Ok(()) - } + impl Record { + fn get_task(&mut self, id: &TaskId) -> Result { + self.tasks + .get(id) + .map_or_else(|| bail!("task not found"), |task| Ok(task.clone())) + } - fn move_task(&mut self, previous: Option<&TaskId>, id: &TaskId) -> anyhow::Result<()> { - self.order.retain(|other| other != id); + fn put_task(&mut self, task: &Task) -> anyhow::Result<()> { + use im::hashmap::Entry::{Occupied, Vacant}; + debug_assert!( + self.order.contains(&task.id()), + "MemoryStore::put called with task not in the order list" + ); + match self.tasks.entry(task.id()) { + Occupied(mut entry) => *entry.get_mut() = task.clone(), + Vacant(_) => { + panic!("MemoryStore::put called with task not in the tasks map") + } + } - let index = if let Some(previous_id) = previous { - self.order.index_of(previous_id).map(|index| index + 1) - } else { - None + Ok(()) } - .unwrap_or(0); - self.order.insert(index, *id); + fn insert_task(&mut self, previous: Option<&TaskId>, task: &Task) -> anyhow::Result<()> { + let index = if let Some(previous) = previous { + self.order + .index_of(previous) + .map_or(0, |index| index.saturating_add(1)) + } else { + 0 + }; + + debug_assert!( + !self.order.contains(&task.id()), + "MemoryStore::insert called with task.id already in the order list" + ); + self.order.insert(index, task.id()); + self.tasks.entry(task.id()).or_insert(task.clone()); - Ok(()) - } + Ok(()) + } - fn list_tasks(&mut self) -> anyhow::Result> { - Ok(self - .order - .iter() - .map(|id| { - self.tasks - .get(id) - .expect("all items in MemoryStore::order must be in MemoryStore::tasks") - .clone() - }) - .collect()) - } -} + fn delete_task(&mut self, id: &TaskId) -> anyhow::Result<()> { + self.order.retain(|entry| entry != id); + self.tasks.retain(|key, _| key != id); + Ok(()) + } -#[derive(Default)] -pub(crate) struct MemoryStore { - current: Record, - undo_stack: Vec, - redo_stack: Vec, -} + fn move_task(&mut self, previous: Option<&TaskId>, id: &TaskId) -> anyhow::Result<()> { + self.order.retain(|other| other != id); -impl MemoryStore { - #[allow(dead_code)] - pub(crate) fn new() -> Self { - Self::default() - } + let index = if let Some(previous_id) = previous { + self.order.index_of(previous_id).map(|index| index + 1) + } else { + None + } + .unwrap_or(0); - pub(crate) fn load(path: &Path) -> Result { - let tasks = load_tasks(path)?; - - let order: im::Vector = tasks.tasks.iter().map(Task::id).collect(); - let tasks: im::HashMap = tasks - .tasks - .into_iter() - .map(|task| (task.id(), task)) - .collect(); - - Ok(MemoryStore { - current: Record { tasks, order }, - undo_stack: Vec::new(), - redo_stack: Vec::new(), - }) - } + self.order.insert(index, *id); - pub(crate) fn save(&self, path: &Path) -> Result<(), anyhow::Error> { - let tasks = TaskList { - tasks: self - .current + Ok(()) + } + + fn list_tasks(&mut self) -> anyhow::Result> { + Ok(self .order .iter() - .map(|id| self.current.tasks.get(id).unwrap()) - .cloned() - .collect(), - }; - save_tasks(path, &tasks)?; - Ok(()) + .map(|id| { + self.tasks + .get(id) + .expect("all items in MemoryStore::order must be in MemoryStore::tasks") + .clone() + }) + .collect()) + } } -} -impl Store for MemoryStore { - fn get_task(&mut self, id: &TaskId) -> anyhow::Result { - self.current.get_task(id) + #[derive(Default)] + pub(crate) struct MemoryStore { + current: Record, + undo_stack: Vec, + redo_stack: Vec, } - fn put_task(&mut self, task: &Task) -> anyhow::Result<()> { - let saved = self.current.clone(); - self.current.put_task(task)?; - self.undo_stack.push(saved); - Ok(()) + struct MemoryTransaction<'a> { + store: &'a mut MemoryStore, + start: Record, } - fn insert_task(&mut self, previous: Option<&TaskId>, task: &Task) -> anyhow::Result<()> { - let saved = self.current.clone(); - self.current.insert_task(previous, task)?; - self.undo_stack.push(saved); - Ok(()) + impl<'a> MemoryTransaction<'a> { + fn new(store: &'a mut MemoryStore) -> Self { + let start = store.current.clone(); + Self { start, store } + } } - fn delete_task(&mut self, id: &TaskId) -> anyhow::Result<()> { - let saved = self.current.clone(); - self.current.delete_task(id)?; - self.undo_stack.push(saved); - Ok(()) - } + impl Transaction for MemoryTransaction<'_> { + fn delete_task(&mut self, id: &TaskId) -> anyhow::Result<()> { + self.store.delete_task(id) + } - fn move_task(&mut self, previous: Option<&TaskId>, task: &TaskId) -> anyhow::Result<()> { - let saved = self.current.clone(); - self.current.move_task(previous, task)?; - self.undo_stack.push(saved); - Ok(()) + fn commit(self: Box) -> anyhow::Result<()> { + self.store.undo_stack.push(self.start); + Ok(()) + } } - fn list_tasks(&mut self) -> anyhow::Result> { - self.current.list_tasks() - } + impl MemoryStore { + #[allow(dead_code)] + pub(crate) fn new() -> Self { + Self::default() + } + + pub(crate) fn load(path: &Path) -> Result { + let tasks = load_tasks(path)?; - fn undo(&mut self) -> anyhow::Result<()> { - if let Some(record) = self.undo_stack.pop() { - self.redo_stack.push(record.clone()); - self.current = record; + let order: im::Vector = tasks.tasks.iter().map(Task::id).collect(); + let tasks: im::HashMap = tasks + .tasks + .into_iter() + .map(|task| (task.id(), task)) + .collect(); + + Ok(MemoryStore { + current: Record { tasks, order }, + undo_stack: Vec::new(), + redo_stack: Vec::new(), + }) + } + + pub(crate) fn save(&self, path: &Path) -> Result<(), anyhow::Error> { + let tasks = TaskList { + tasks: self + .current + .order + .iter() + .map(|id| self.current.tasks.get(id).unwrap()) + .cloned() + .collect(), + }; + save_tasks(path, &tasks)?; Ok(()) - } else { - bail!("undo is not available") + } + + fn delete_task(&mut self, id: &TaskId) -> Result<(), anyhow::Error> { + self.current.delete_task(id) } } - fn redo(&mut self) -> anyhow::Result<()> { - if let Some(record) = self.redo_stack.pop() { - self.current = record; + impl Store for MemoryStore { + fn get_task(&mut self, id: &TaskId) -> anyhow::Result { + self.current.get_task(id) + } + + fn put_task(&mut self, task: &Task) -> anyhow::Result<()> { + let saved = self.current.clone(); + self.current.put_task(task)?; + self.undo_stack.push(saved); + Ok(()) + } + + fn insert_task(&mut self, previous: Option<&TaskId>, task: &Task) -> anyhow::Result<()> { + let saved = self.current.clone(); + self.current.insert_task(previous, task)?; + self.undo_stack.push(saved); Ok(()) - } else { - bail!("redo is not available") + } + + fn move_task(&mut self, previous: Option<&TaskId>, task: &TaskId) -> anyhow::Result<()> { + let saved = self.current.clone(); + self.current.move_task(previous, task)?; + self.undo_stack.push(saved); + Ok(()) + } + + fn list_tasks(&mut self) -> anyhow::Result> { + self.current.list_tasks() + } + + fn undo(&mut self) -> anyhow::Result<()> { + if let Some(record) = self.undo_stack.pop() { + self.redo_stack.push(record.clone()); + self.current = record; + Ok(()) + } else { + bail!("undo is not available") + } + } + + fn redo(&mut self) -> anyhow::Result<()> { + if let Some(record) = self.redo_stack.pop() { + self.current = record; + Ok(()) + } else { + bail!("redo is not available") + } + } + + fn transaction(&mut self) -> Box { + let transaction = MemoryTransaction::new(self); + Box::new(transaction) } } } diff --git a/src/ui_state.rs b/src/ui_state.rs index 2a2ffb8..e5b8dab 100644 --- a/src/ui_state.rs +++ b/src/ui_state.rs @@ -216,11 +216,14 @@ impl CommonState { } self.selected = new_selected; - for id in &deletions { - self.store - .delete_task(id) - .expect("FIXME: handle error here"); - } + self.store + .with_transaction(|txn| { + for id in &deletions { + txn.delete_task(id).expect("FIXME: handle error here"); + } + Ok(()) + }) + .expect("TODO: handle errors here"); } pub(crate) fn undo(&mut self) {