Trains should be left associative and hooks should be right associative

2 files changed, 117 insertions, 105 deletions

diff --git a/rprt-engine/src/expression.rs b/rprt-engine/src/expression.rs
index fcb1ff1..3bad172 100644
--- a/rprt-engine/src/expression.rs
+++ b/rprt-engine/src/expression.rs
@@ -59,14 +59,14 @@ impl Display for Composite {
             Train2 { f, g } => write!(fmtr, "({f} {g})"),
             Train3 { f, g, h } => write!(fmtr, "({f} {g} {h})"),
             Group { operations } => {
-                write!(fmtr, "{{")?;
+                write!(fmtr, "{{ ")?;
                 for (i, op) in operations.iter().enumerate() {
                     if i > 0 {
                         write!(fmtr, " , ")?;
                     }
                     write!(fmtr, "{}", op)?;
                 }
-                write!(fmtr, "}}")
+                write!(fmtr, " }}")
             }
         }
     }
diff --git a/rprt-engine/src/parser.rs b/rprt-engine/src/parser.rs
index 9536046..3353dff 100644
--- a/rprt-engine/src/parser.rs
+++ b/rprt-engine/src/parser.rs
@@ -64,8 +64,9 @@ fn parse_composition(context: &mut ParseContext) -> Result<Composite, ParseError
 }
 
 fn build_train(mut functions: Vec<Composite>) -> Composite {
-    // Rust is insane and has no easy way to destructure a vector and own the elements
-    // so this function is littered with unwraps for no reason, we already own the vector!
+    // It would appear that there is no easy way to destructure a vector and own
+    // the elements so this function is littered with unwraps for no reason, we
+    // already own the vector!
     match functions.len() {
         1 => functions.pop().unwrap(),
         2 => {
@@ -84,20 +85,19 @@ fn build_train(mut functions: Vec<Composite>) -> Composite {
             }
         }
         n if n % 2 == 0 => {
-            let rest = functions.split_off(1);
-            let [f] = <[_; 1]>::try_from(functions).unwrap();
+            let g = Box::new(functions.pop().unwrap());
             Composite::Train2 {
-                f: Box::new(f),
-                g: Box::new(build_train(rest)),
+                f: Box::new(build_train(functions)),
+                g,
             }
         }
         _ => {
-            let rest = functions.split_off(2);
-            let [f, g] = <[_; 2]>::try_from(functions).unwrap();
+            let h = Box::new(functions.pop().unwrap());
+            let g = Box::new(functions.pop().unwrap());
             Composite::Train3 {
-                f: Box::new(f),
-                g: Box::new(g),
-                h: Box::new(build_train(rest)),
+                f: Box::new(build_train(functions)),
+                g,
+                h,
             }
         }
     }
@@ -173,30 +173,27 @@ fn parse_base_function(context: &mut ParseContext) -> Result<Composite, ParseErr
 }
 
 fn parse_function_atom(context: &mut ParseContext) -> Result<Composite, ParseError> {
-    let mut result = parse_base_function(context)?;
-
-    loop {
-        match context.current() {
-            Ok(Token::Before) => {
-                context.advance(1, true)?;
-                let right = parse_base_function(context)?;
-                result = Composite::Hook {
-                    kind: HookKind::Before,
-                    left: Box::new(result),
-                    right: Box::new(right),
-                };
-            }
-            Ok(Token::After) => {
-                context.advance(1, true)?;
-                let right = parse_base_function(context)?;
-                result = Composite::Hook {
-                    kind: HookKind::After,
-                    left: Box::new(result),
-                    right: Box::new(right),
-                };
-            }
-            _ => return Ok(result),
+    let result = parse_base_function(context)?;
+    match context.current() {
+        Ok(Token::Before) => {
+            context.advance(1, true)?;
+            let rest = parse_composition(context)?;
+            return Ok(Composite::Hook {
+                kind: HookKind::Before,
+                left: Box::new(result),
+                right: Box::new(rest),
+            });
+        }
+        Ok(Token::After) => {
+            context.advance(1, true)?;
+            let rest = parse_composition(context)?;
+            return Ok(Composite::Hook {
+                kind: HookKind::After,
+                left: Box::new(result),
+                right: Box::new(rest),
+            });
         }
+        _ => return Ok(result),
     }
 }
 
@@ -554,22 +551,6 @@ mod tests {
         match result {
             Composite::Train3 { f, g, h } => {
                 match *f {
-                    Composite::SelectionFunction {
-                        func: BuiltinSelectionFn::CharOffset(n),
-                        search_mod: None,
-                        result_transform: None,
-                    } => assert_eq!(n, 0),
-                    _ => panic!(),
-                }
-                assert!(matches!(
-                    *g,
-                    Composite::SelectionFunction {
-                        func: BuiltinSelectionFn::Span,
-                        search_mod: None,
-                        result_transform: None,
-                    }
-                ));
-                match *h {
                     Composite::Train3 {
                         ref f,
                         ref g,
@@ -580,7 +561,7 @@ mod tests {
                                 func: BuiltinSelectionFn::CharOffset(n),
                                 search_mod: None,
                                 result_transform: None,
-                            } => assert_eq!(n, 10),
+                            } => assert_eq!(n, 0),
                             _ => panic!(),
                         }
                         assert!(matches!(
@@ -596,12 +577,28 @@ mod tests {
                                 func: BuiltinSelectionFn::CharOffset(n),
                                 search_mod: None,
                                 result_transform: None,
-                            } => assert_eq!(n, 20),
+                            } => assert_eq!(n, 10),
                             _ => panic!(),
                         }
                     }
                     _ => panic!(),
                 }
+                assert!(matches!(
+                    *g,
+                    Composite::SelectionFunction {
+                        func: BuiltinSelectionFn::Span,
+                        search_mod: None,
+                        result_transform: None,
+                    }
+                ));
+                match *h {
+                    Composite::SelectionFunction {
+                        func: BuiltinSelectionFn::CharOffset(n),
+                        search_mod: None,
+                        result_transform: None,
+                    } => assert_eq!(n, 20),
+                    _ => panic!(),
+                }
             }
             _ => panic!(),
         }
@@ -701,7 +698,7 @@ mod tests {
 
     #[test]
     fn test_parse_nested_hooks_left_associative() {
-        let input = r#"c<(|<"sort")"#;
+        let input = r#"(c<|)<"sort""#;
         let tokens = tokenise(input).unwrap();
         let result = parse(tokens).unwrap();
         println!("{:?}", result);
@@ -712,14 +709,7 @@ mod tests {
                 right: outer_right,
             } => {
                 assert_eq!(outer_kind, HookKind::After);
-                assert!(matches!(
-                    *outer_left,
-                    Composite::TextFunction {
-                        func: BuiltinTextFn::Change,
-                        swapped: false,
-                    }
-                ));
-                match *outer_right {
+                match *outer_left {
                     Composite::Hook {
                         kind: inner_kind,
                         left: inner_left,
@@ -729,20 +719,27 @@ mod tests {
                         assert!(matches!(
                             *inner_left,
                             Composite::TextFunction {
-                                func: BuiltinTextFn::Pipe,
+                                func: BuiltinTextFn::Change,
                                 swapped: false,
                             }
                         ));
-                        match *inner_right {
+                        assert!(matches!(
+                            *inner_right,
                             Composite::TextFunction {
-                                func: BuiltinTextFn::Literal(ref s),
+                                func: BuiltinTextFn::Pipe,
                                 swapped: false,
-                            } => assert_eq!(s, "sort"),
-                            _ => panic!(),
-                        }
+                            }
+                        ));
                     }
                     _ => panic!(),
                 }
+                match *outer_right {
+                    Composite::TextFunction {
+                        func: BuiltinTextFn::Literal(ref s),
+                        swapped: false,
+                    } => assert_eq!(s, "sort"),
+                    _ => panic!(),
+                }
             }
             _ => panic!(),
         }
@@ -761,7 +758,14 @@ mod tests {
                 right: outer_right,
             } => {
                 assert_eq!(outer_kind, HookKind::After);
-                match *outer_left {
+                assert!(matches!(
+                    *outer_left,
+                    Composite::TextFunction {
+                        func: BuiltinTextFn::Change,
+                        swapped: false,
+                    }
+                ));
+                match *outer_right {
                     Composite::Hook {
                         kind: inner_kind,
                         left: inner_left,
@@ -771,27 +775,20 @@ mod tests {
                         assert!(matches!(
                             *inner_left,
                             Composite::TextFunction {
-                                func: BuiltinTextFn::Change,
+                                func: BuiltinTextFn::Pipe,
                                 swapped: false,
                             }
                         ));
-                        assert!(matches!(
-                            *inner_right,
+                        match *inner_right {
                             Composite::TextFunction {
-                                func: BuiltinTextFn::Pipe,
+                                func: BuiltinTextFn::Literal(ref s),
                                 swapped: false,
-                            }
-                        ));
+                            } => assert_eq!(s, "sort"),
+                            _ => panic!(),
+                        }
                     }
                     _ => panic!(),
                 }
-                match *outer_right {
-                    Composite::TextFunction {
-                        func: BuiltinTextFn::Literal(ref s),
-                        swapped: false,
-                    } => assert_eq!(s, "sort"),
-                    _ => panic!(),
-                }
             }
             _ => panic!(),
         }
@@ -811,13 +808,14 @@ mod tests {
             } => {
                 assert_eq!(k1, HookKind::Before);
                 assert!(matches!(
-                    *r1,
-                    Composite::TextFunction {
-                        func: BuiltinTextFn::Insert,
-                        swapped: false,
+                    *l1,
+                    Composite::SelectionFunction {
+                        func: BuiltinSelectionFn::Empty,
+                        search_mod: None,
+                        result_transform: None,
                     }
                 ));
-                match *l1 {
+                match *r1 {
                     Composite::Hook {
                         kind: k2,
                         left: l2,
@@ -825,13 +823,13 @@ mod tests {
                     } => {
                         assert_eq!(k2, HookKind::After);
                         assert!(matches!(
-                            *r2,
+                            *l2,
                             Composite::TextFunction {
-                                func: BuiltinTextFn::Change,
+                                func: BuiltinTextFn::Delete,
                                 swapped: false,
                             }
                         ));
-                        match *l2 {
+                        match *r2 {
                             Composite::Hook {
                                 kind: k3,
                                 left: l3,
@@ -840,16 +838,15 @@ mod tests {
                                 assert_eq!(k3, HookKind::Before);
                                 assert!(matches!(
                                     *l3,
-                                    Composite::SelectionFunction {
-                                        func: BuiltinSelectionFn::Empty,
-                                        search_mod: None,
-                                        result_transform: None,
+                                    Composite::TextFunction {
+                                        func: BuiltinTextFn::Change,
+                                        swapped: false,
                                     }
                                 ));
                                 assert!(matches!(
                                     *r3,
                                     Composite::TextFunction {
-                                        func: BuiltinTextFn::Delete,
+                                        func: BuiltinTextFn::Insert,
                                         swapped: false,
                                     }
                                 ));
@@ -871,21 +868,36 @@ mod tests {
         let result = parse(tokens).unwrap();
         println!("{:?}", result);
         match result {
-            Composite::Train2 { f, g } => {
-                match *f {
-                    Composite::Hook { kind, .. } => {
-                        assert_eq!(kind, HookKind::Before);
-                    }
-                    _ => panic!(),
-                }
+            Composite::Hook { kind, left, right } => {
+                assert_eq!(kind, HookKind::Before);
                 assert!(matches!(
-                    *g,
+                    *left,
                     Composite::SelectionFunction {
-                        func: BuiltinSelectionFn::EndOfBuffer,
+                        func: BuiltinSelectionFn::Empty,
                         search_mod: None,
                         result_transform: None,
                     }
                 ));
+                match *right {
+                    Composite::Train2 { f, g } => {
+                        assert!(matches!(
+                            *f,
+                            Composite::TextFunction {
+                                func: BuiltinTextFn::Delete,
+                                swapped: false,
+                            }
+                        ));
+                        assert!(matches!(
+                            *g,
+                            Composite::SelectionFunction {
+                                func: BuiltinSelectionFn::EndOfBuffer,
+                                search_mod: None,
+                                result_transform: None,
+                            }
+                        ));
+                    }
+                    _ => panic!(),
+                }
             }
             _ => panic!(),
         }