think the commit logic is worked out now for eager commits

4 files changed, 73 insertions, 107 deletions

diff --git a/rprt-engine/src/evaluate.rs b/rprt-engine/src/evaluate.rs
index b23c1be..a139aa3 100644
--- a/rprt-engine/src/evaluate.rs
+++ b/rprt-engine/src/evaluate.rs
@@ -1,8 +1,8 @@
 use crate::{
     expression::Composite,
-    selection::{Selection, VectoriseError},
+    selection::{Selection, SelectionError, VectoriseError},
     selection_functions::{evaluate_selection_function, SFError},
-    state::{EditorState, StateResult},
+    state::{EditorState, GroupedChangeError, StateChange, StateResult},
 };
 
 use thiserror::Error;
@@ -11,17 +11,35 @@ use thiserror::Error;
 pub enum EvaluationError {
     #[error("{0}")]
     SelectionFunctionError(VectoriseError<SFError>),
+    #[error("{0}")]
+    SelectionUnionError(SelectionError),
+    #[error("Error in processing group {0}")]
+    GroupError(GroupedChangeError),
     #[error("Invalid right-only application")]
     IROApplication,
     #[error("Not yet implemented {0}")]
     UnimplementedError(&'static str),
 }
 
+fn commit_if_needed(
+    es: &mut EditorState,
+    changes: Vec<StateChange>,
+    commit: bool,
+) -> Vec<StateChange> {
+    if commit {
+        es.commit_changes(changes);
+        Vec::new()
+    } else {
+        changes
+    }
+}
+
 pub fn evaluate(
-    es: &EditorState,
+    es: &mut EditorState,
     comp: Composite,
     left: Option<Selection>,
     right: Option<Selection>,
+    commit: bool,
 ) -> Result<StateResult, EvaluationError> {
     if left.is_none() && right.is_some() {
         return Err(EvaluationError::IROApplication);
@@ -43,28 +61,42 @@ pub fn evaluate(
         }
         Composite::Hook { kind, left, right } => Err(EvaluationError::UnimplementedError("hook")),
         Composite::Train2 { f, g } => {
-            let (left, mut state) = evaluate(es, *f, left, right)?;
-            let (sel, more_state) = evaluate(es, *g, Some(left), None)?;
-            state.extend(more_state);
-            Ok((sel, state))
+            let (left, mut f_changes) = evaluate(es, *f, left, right, commit)?;
+            f_changes = commit_if_needed(es, f_changes, commit);
+            let (sel, g_changes) = evaluate(es, *g, Some(left), None, commit)?;
+            let more_changes = commit_if_needed(es, g_changes, commit);
+            f_changes.extend(more_changes);
+            Ok((sel, f_changes))
         }
         Composite::Train3 { f, g, h } => {
-            let (f_left, mut f_state) = evaluate(es, *f, left.clone(), right.clone())?;
-            let (g_right, h_state) = evaluate(es, *h, left, right)?;
-            let (sel, g_state) = evaluate(es, *g, Some(f_left), Some(g_right))?;
-            f_state.extend(h_state);
-            f_state.extend(g_state);
-            Ok((sel, f_state))
+            let (f_left, mut f_changes) = evaluate(es, *f, left.clone(), right.clone(), commit)?;
+            f_changes = commit_if_needed(es, f_changes, commit);
+            let (g_right, h_changes) = evaluate(es, *h, left, right, commit)?;
+            let h_changes = commit_if_needed(es, h_changes, commit);
+            let (sel, g_changes) = evaluate(es, *g, Some(f_left), Some(g_right), commit)?;
+            let g_changes = commit_if_needed(es, g_changes, commit);
+            f_changes.extend(h_changes);
+            f_changes.extend(g_changes);
+            Ok((sel, f_changes))
         }
         Composite::Group { operations } => {
-            // TODO: does rust have some monadic failure map thing on first failure?
-            let mut selections = Vec::new();
-            let mut states = Vec::new();
-            for f in operations {
-                let (sel, state) = evaluate(es, f, left.clone(), right.clone())?;
-                selections.extend(sel);
-                states.extend(state);
+            let results: Vec<(Selection, Vec<StateChange>)> = operations
+                .into_iter()
+                .map(|op| evaluate(es, op, left.clone(), right.clone(), commit))
+                .collect::<Result<_, _>>()?;
+
+            let (selections, arms) = results.into_iter().unzip();
+
+            let states = if commit {
+                es.commit_changes_grouped(arms).map(|_| Vec::new())
+            } else {
+                es.validate_grouped_changes(&arms)
+                    .map(|_| arms.into_iter().flatten().collect())
             }
+            .map_err(EvaluationError::GroupError)?;
+
+            let sel = Selection::union(selections).map_err(EvaluationError::SelectionUnionError)?;
+            Ok((sel, states))
         }
     }
 }
diff --git a/rprt-engine/src/lib.rs b/rprt-engine/src/lib.rs
index 6f552bd..e002005 100644
--- a/rprt-engine/src/lib.rs
+++ b/rprt-engine/src/lib.rs
@@ -1,7 +1,6 @@
 pub mod buffer;
 pub mod evaluate;
 pub mod expression;
-pub mod monad;
 pub mod parser;
 pub mod selection;
 pub mod selection_functions;
diff --git a/rprt-engine/src/monad.rs b/rprt-engine/src/monad.rs
deleted file mode 100644
index ac35722..0000000
--- a/rprt-engine/src/monad.rs
+++ /dev/null
@@ -1,75 +0,0 @@
-use crate::{
-    selection::{Selection, SelectionError},
-    state::{EditorState, GroupedChangeError, StateResult},
-};
-
-pub enum EvaluationStrategy {
-    Sequential,
-    Grouped,
-}
-
-#[derive(Debug)]
-pub enum MonadError {
-    Selection(SelectionError),
-    GroupedChange(GroupedChangeError),
-}
-
-impl From<SelectionError> for MonadError {
-    fn from(err: SelectionError) -> Self {
-        MonadError::Selection(err)
-    }
-}
-
-impl From<GroupedChangeError> for MonadError {
-    fn from(err: GroupedChangeError) -> Self {
-        MonadError::GroupedChange(err)
-    }
-}
-
-pub struct EditorStateMonad {
-    func: Box<dyn Fn(&EditorState) -> StateResult>,
-}
-
-impl EditorStateMonad {
-    pub fn new(f: impl Fn(&EditorState) -> StateResult + 'static) -> Self {
-        Self { func: Box::new(f) }
-    }
-
-    pub fn run(&self, state: &EditorState) -> StateResult {
-        (self.func)(state)
-    }
-
-    pub fn run_with_strategy(
-        monads: Vec<EditorStateMonad>,
-        strategy: EvaluationStrategy,
-        state: &mut EditorState,
-    ) -> Result<Selection, MonadError> {
-        match strategy {
-            EvaluationStrategy::Sequential => {
-                let mut last_selection = Selection::empty();
-
-                for monad in monads {
-                    let (sel, state_changes) = monad.run(state);
-                    state.commit_changes(state_changes);
-                    last_selection = sel;
-                }
-
-                Ok(last_selection)
-            }
-            EvaluationStrategy::Grouped => {
-                let mut arms = Vec::new();
-                let mut all_selections = Vec::new();
-
-                for monad in monads {
-                    let (sel, state_changes) = monad.run(state);
-                    all_selections.push(sel);
-                    arms.push(state_changes);
-                }
-
-                state.commit_changes_grouped(arms)?;
-
-                Ok(Selection::union(all_selections)?)
-            }
-        }
-    }
-}
diff --git a/rprt-engine/src/state.rs b/rprt-engine/src/state.rs
index 80b5e86..7ef5a19 100644
--- a/rprt-engine/src/state.rs
+++ b/rprt-engine/src/state.rs
@@ -4,6 +4,7 @@ use crate::{
 };
 use std::collections::{HashMap, HashSet};
 use std::path::PathBuf;
+use thiserror::Error;
 
 #[derive(Debug, Clone)]
 pub enum StateChange {
@@ -23,19 +24,19 @@ pub enum StateChange {
     },
 }
 
-#[derive(Debug, Clone)]
+#[derive(Error, Debug, Clone)]
 pub enum GroupedChangeError {
+    #[error("Changes for buffer ID {buffer} overlap in regions {first:#?} and {second:#?}")]
     OverlappingChanges {
         buffer: BufferID,
         first: (usize, usize),
         second: (usize, usize),
     },
-    BufferNotLive {
-        buffer: BufferID,
-    },
-    BufferAlreadyExists {
-        buffer: BufferID,
-    },
+    #[error("Buffer ID {buffer} does not exist at time of processing")]
+    BufferNotLive { buffer: BufferID },
+    #[error("Buffer ID {buffer} already exists at time of processing")]
+    BufferAlreadyExists { buffer: BufferID },
+    #[error("Inconsistent final buffer set")]
     InconsistentFinalBufferSet,
 }
 
@@ -154,14 +155,14 @@ impl EditorState {
         }
     }
 
-    pub fn commit_changes_grouped(
-        &mut self,
-        arms: Vec<Vec<StateChange>>,
+    pub fn validate_grouped_changes(
+        &self,
+        arms: &Vec<Vec<StateChange>>,
     ) -> Result<(), GroupedChangeError> {
         let initial_buffers: std::collections::HashSet<BufferID> =
             self.buffers.keys().copied().collect();
 
-        self.validate_arms_buffer_liveness(&arms, &initial_buffers)?;
+        self.validate_arms_buffer_liveness(arms, &initial_buffers)?;
 
         let all_modifications: Vec<&StateChange> = arms
             .iter()
@@ -171,6 +172,15 @@ impl EditorState {
 
         Self::validate_modifications(&all_modifications)?;
 
+        Ok(())
+    }
+
+    pub fn commit_changes_grouped(
+        &mut self,
+        arms: Vec<Vec<StateChange>>,
+    ) -> Result<(), GroupedChangeError> {
+        self.validate_grouped_changes(&arms)?;
+
         let all_changes: Vec<StateChange> = arms.into_iter().flatten().collect();
 
         let mut creates = Vec::new();