Implement full macro undo

src/calc.rs

@@ -8,20 +8,27 @@ use constants::{
     CalculatorState, RegisterState,
 };
 use errors::{CalculatorError, CalculatorResult};
-use operations::CalculatorOperation;
+use operations::{CalculatorOperation, MacroState};
 use std::collections::{HashSet, VecDeque};
 
-#[derive(PartialEq)]
+#[derive(PartialEq, Debug)]
+enum HistoryMode {
+    One,
+    Macro,
+}
+
+#[derive(PartialEq, Debug)]
 enum OpArgs {
+    Macro(MacroState),
     Unary(f64),
     Binary([f64; 2]),
     None,
 }
 
+#[derive(PartialEq, Debug)]
 struct CalculatorStateChange {
     pop: OpArgs,
     push: OpArgs,
-    within_macro: bool,
 }
 
 impl CalculatorStateChange {
@@ -201,18 +208,15 @@ impl<'a> Calculator<'a> {
                     return Err(CalculatorError::RecursiveMacro(c));
                 }
 
+                // Record the macro started, if this is the outer macro
+                self.op(CalculatorOperation::Macro(MacroState::Start))?;
+
+                // Record that we are running macro c
+                self.active_macros.insert(c);
+
                 // The macro needs to run in normal mode
                 self.state = CalculatorState::Normal;
 
-                // Add a no-op undo event as a stopgap for undo/redo operations (fixes undo after multiple macros running)
-                // self.direct_state_change(CalculatorStateChange {
-                //     pop: OpArgs::None,
-                //     push: OpArgs::None,
-                //     within_macro: self.within_macro(),
-                // })?;
-                self.op(CalculatorOperation::Macro(mac))?;
-                // Record that we are running macro c
-                self.active_macros.insert(c);
                 for c in value.chars() {
                     self.take_input(c).map_err(|e| {
                         self.cancel();
@@ -221,9 +225,11 @@ impl<'a> Calculator<'a> {
                 }
                 // Macro c should be over now
                 self.active_macros.remove(&c);
-                Ok(())
 
-                // self.push(value)
+                // Record the macro is over, if this is the outer macro
+                self.op(CalculatorOperation::Macro(MacroState::End))?;
+
+                Ok(())
             }
             CalculatorState::WaitingForRegister(register_state) => {
                 match register_state {
@@ -328,7 +334,6 @@ impl<'a> Calculator<'a> {
         self.direct_state_change(CalculatorStateChange {
             pop: OpArgs::None,
             push: OpArgs::Unary(f),
-            within_macro: self.within_macro(),
         })
     }
     fn pop(&mut self) -> CalculatorResult<f64> {
@@ -336,7 +341,6 @@ impl<'a> Calculator<'a> {
         self.direct_state_change(CalculatorStateChange {
             pop: OpArgs::Unary(f),
             push: OpArgs::None,
-            within_macro: self.within_macro(),
         })?;
         Ok(f)
     }
@@ -362,9 +366,7 @@ impl<'a> Calculator<'a> {
             CalculatorOperation::AbsoluteValue => self.unary_op(|a| OpArgs::Unary(a.abs())),
             CalculatorOperation::Inverse => self.unary_op(|a| OpArgs::Unary(1.0 / a)),
             CalculatorOperation::Modulo => self.binary_op(|[a, b]| OpArgs::Unary(b % a)),
-            //CalculatorOperation::Remainder => {
+            //CalculatorOperation::Remainder => self.binary_op(|[a, b]| OpArgs::Unary(b.rem_euclid(a))),
-            //    self.binary_op(|[a, b]| OpArgs::Unary(b.rem_euclid(a)))
-            //}
             CalculatorOperation::Dup => self.unary_op(|a| OpArgs::Binary([a, a])),
             CalculatorOperation::Drop => self.unary_op(|_| OpArgs::None),
             CalculatorOperation::Swap => self.binary_op(|[a, b]| OpArgs::Binary([b, a])),
@@ -375,95 +377,83 @@ impl<'a> Calculator<'a> {
             CalculatorOperation::ACos => self.unary_op(|a| OpArgs::Unary(a.acos())),
             CalculatorOperation::ATan => self.unary_op(|a| OpArgs::Unary(a.atan())),
             CalculatorOperation::Sqrt => self.unary_op(|a| OpArgs::Unary(a.sqrt())),
-            // CalculatorOperation::Factorial => vec![args[0].()],
+            // CalculatorOperation::Factorial => self.unary_op(|a| OpArgs::Unary(a.())),
             CalculatorOperation::Log => self.unary_op(|a| OpArgs::Unary(a.log10())),
             CalculatorOperation::Ln => self.unary_op(|a| OpArgs::Unary(a.ln())),
             CalculatorOperation::Pow => self.binary_op(|[a, b]| OpArgs::Unary(b.powf(a))),
             CalculatorOperation::E => self.binary_op(|[a, b]| OpArgs::Unary(b * 10.0f64.powf(a))),
-            CalculatorOperation::Undo => {
+            CalculatorOperation::Undo => return self.history_op(false),
-                let mut has_run = false;
+            CalculatorOperation::Redo => return self.history_op(true),
-                loop {
-                    let s = self
-                        .undo_buf
-                        .pop()
-                        .ok_or_else(|| CalculatorError::EmptyHistory(String::from("undo")))?;
-                    let quit = !s.within_macro;
-                    let is_noop = s.is_noop();
-                    self.apply_state_change(s, false)?;
-                    if is_noop && !has_run {
-                        continue;
-                    }
-                    if quit {
-                        return Ok(());
-                    }
-                    has_run = true;
-                }
-            }
-            CalculatorOperation::Redo => {
-                let mut has_run = false;
-                loop {
-                    // If we have already redone, but the buffer is empty, last cmd was a macro; don't error
-                    if has_run && self.redo_buf.get(0).is_none() {
-                        return Ok(());
-                    }
-                    let s = self
-                        .redo_buf
-                        .pop()
-                        .ok_or_else(|| CalculatorError::EmptyHistory(String::from("redo")))?;
-
-                    // We should quit if we will redo something out of a macro
-                    let quit = !s.within_macro;
-                    let is_noop = s.is_noop();
-                    self.apply_state_change(s, true)?;
-                    // If this is a noop and it is the first we have encountered, try again. We have hit the beginning of a macro and need to jump straight to the macro contents
-                    // We don't want to mark as has_run
-                    if is_noop && !has_run {
-                        continue;
-                    }
-                    if quit {
-                        return Ok(());
-                    }
-                    has_run = true;
-                }
-            }
-            // CalculatorOperation::Redo => {
-            //     let mut has_run = false;
-            //     loop {
-            //         // If we have already redone, but the buffer is empty, last cmd was a macro; don't error
-            //         if has_run && self.redo_buf.get(0).is_none() {
-            //             return Ok(());
-            //         }
-            //         let s = self
-            //             .redo_buf
-            //             .pop()
-            //             .ok_or_else(|| CalculatorError::EmptyHistory(String::from("redo")))?;
-
-            //         // We should quit if we will redo something out of a macro
-            //         let quit = !s.within_macro;
-            //         let is_noop = s.is_noop();
-            //         self.apply_state_change(s, true)?;
-            //         // If this is a noop and it is the first we have encountered, try again. We have hit the beginning of a macro and need to jump straight to the macro contents
-            //         // We don't want to mark as has_run
-            //         if is_noop && !has_run {
-            //             continue;
-            //         }
-            //         if quit {
-            //             return Ok(());
-            //         }
-            //         has_run = true;
-            //     }
-            // }
             // Macros are a no-op operator; need to insert for undo/redo
-            CalculatorOperation::Macro(_) => Ok(CalculatorStateChange {
+            CalculatorOperation::Macro(state) => {
-                pop: OpArgs::None,
+                if self.within_macro() {
-                push: OpArgs::None,
+                    // Do not push any states if we are already in a macro
-                within_macro: self.within_macro(),
+                    return Ok(());
-            }),
+                }
+
+                Ok(CalculatorStateChange {
+                    pop: OpArgs::None,
+                    push: OpArgs::Macro(state),
+                })
+            }
         };
 
         self.direct_state_change(state_change?)
     }
 
+    fn history_op(&mut self, forward: bool) -> CalculatorResult<()> {
+        let s = if forward {
+            &self.redo_buf
+        } else {
+            &self.undo_buf
+        }
+        .last()
+        .ok_or_else(|| {
+            CalculatorError::EmptyHistory(String::from(if forward { "redo" } else { "undo" }))
+        })?;
+
+        let target_history_mode = if forward {
+            MacroState::Start
+        } else {
+            MacroState::End
+        };
+
+        let history_mode = match s {
+            CalculatorStateChange {
+                push: OpArgs::Macro(m),
+                ..
+            } if *m == target_history_mode => Ok(HistoryMode::Macro),
+            CalculatorStateChange {
+                push: OpArgs::Macro(_),
+                ..
+            } => Err(CalculatorError::CorruptStateChange(String::from(
+                "macro start is out of bounds",
+            ))),
+            _ => Ok(HistoryMode::One),
+        }?;
+
+        loop {
+            let s = if forward {
+                self.redo_buf.pop()
+            } else {
+                self.undo_buf.pop()
+            }
+            .ok_or_else(|| CalculatorError::EmptyHistory(String::from("redo")))?;
+
+            let macro_end = s.push
+                == OpArgs::Macro(if forward {
+                    MacroState::End
+                } else {
+                    MacroState::Start
+                });
+            // println!("{:?} {:?}", s, history_mode);
+            self.apply_state_change(s, forward)?;
+
+            if history_mode == HistoryMode::One || macro_end {
+                return Ok(());
+            }
+        }
+    }
     fn unary_op(
         &mut self,
         op: impl FnOnce(f64) -> OpArgs,
@@ -475,7 +465,6 @@ impl<'a> Calculator<'a> {
         Ok(CalculatorStateChange {
             pop: OpArgs::Unary(*arg),
             push: op(*arg),
-            within_macro: self.within_macro(),
         })
     }
     fn binary_op(
@@ -495,7 +484,6 @@ impl<'a> Calculator<'a> {
         Ok(CalculatorStateChange {
             pop: OpArgs::Binary(args),
             push: op(args),
-            within_macro: self.within_macro(),
         })
     }
 
@@ -530,7 +518,7 @@ impl<'a> Calculator<'a> {
                     return Err(CalculatorError::ArithmeticError);
                 }
             }
-            OpArgs::None => {}
+            OpArgs::Macro(_) | OpArgs::None => {}
         };
 
         match to_pop {
@@ -551,7 +539,7 @@ impl<'a> Calculator<'a> {
                 self.stack.pop_front();
                 self.stack.pop_front();
             }
-            OpArgs::None => {}
+            OpArgs::Macro(_) | OpArgs::None => {}
         };
 
         match to_push {
@@ -562,7 +550,7 @@ impl<'a> Calculator<'a> {
                 self.stack.push_front(*b);
                 self.stack.push_front(*a);
             }
-            OpArgs::None => {}
+            OpArgs::Macro(_) | OpArgs::None => {}
         };
 
         if forward {

src/calc/operations.rs

@@ -1,7 +1,11 @@
-use super::constants::CalculatorMacro;
+#[derive(PartialEq, Debug)]
-// use super::errors::{CalculatorError, CalculatorResult};
+pub enum MacroState {
+    Start,
+    End,
+}
 
-pub enum CalculatorOperation<'a> {
+#[derive(PartialEq, Debug)]
+pub enum CalculatorOperation {
     Add,
     Subtract,
     Multiply,
@@ -29,51 +33,5 @@ pub enum CalculatorOperation<'a> {
     Log,
     Ln,
     E,
-    Macro(CalculatorMacro<'a>),
+    Macro(MacroState),
 }
-
-// impl CalculatorOperation<'_> {
-//     pub fn from_char<'a>(key: char) -> CalculatorResult<CalculatorOperation<'a>> {
-//         match key {
-//             _ => Err(CalculatorError::NoSuchOperator),
-//         }
-//     }
-
-//     pub fn num_stack(&self) -> usize {
-//         match self {
-//             CalculatorOperation::Add
-//             | CalculatorOperation::Subtract
-//             | CalculatorOperation::Multiply
-//             | CalculatorOperation::Divide
-//             | CalculatorOperation::Modulo
-//             | CalculatorOperation::IntegerDivide
-//             | CalculatorOperation::Swap
-//             | CalculatorOperation::Pow
-//             | CalculatorOperation::E
-//             //| CalculatorOperation::Remainder
-//             => 2,
-//             CalculatorOperation::Negate
-//             | CalculatorOperation::AbsoluteValue
-//             | CalculatorOperation::Inverse
-//             | CalculatorOperation::Drop
-
-//             | CalculatorOperation::Sin
-//             | CalculatorOperation::Cos
-//             | CalculatorOperation::Tan
-//             | CalculatorOperation::ASin
-//             | CalculatorOperation::ACos
-//             | CalculatorOperation::ATan
-//             | CalculatorOperation::Sqrt
-//             | CalculatorOperation::Dup
-
-//             //|CalculatorOperation::Factorial
-//             | CalculatorOperation::Log
-//             | CalculatorOperation::Ln
-//             => 1,
-//             CalculatorOperation::Macro(_)
-//              |CalculatorOperation::Undo
-//             | CalculatorOperation::Redo
-//              => 0,
-//         }
-//     }
-// }