Move unwrap_math() into tokens.next()

crates/math-core/src/parser.rs

@@ -233,11 +233,6 @@ where
             .tokens
             .peek_class_token()?
             .class(parse_as.in_sequence(), self.state.right_boundary_hack);
-        let cur_token = if let Token::MathOrTextMode(tok, _) = cur_token {
-            *tok
-        } else {
-            cur_token
-        };
         let node: Result<Node, LatexError> = match cur_token {
             Token::Digit(number) => 'digit: {
                 if let Some(MathVariant::Transform(tf)) = self.state.transform {
@@ -420,7 +415,8 @@ where
                         ));
                     }
                 };
-                let op = match tokspan.token().unwrap_math() {
+                let (tok, span) = tokspan.into_parts();
+                let op = match tok {
                     Token::Ord(op) | Token::Open(op) | Token::Close(op) => op.as_op(),
                     Token::Op(op) | Token::Inner(op) => op.as_op(),
                     Token::BinaryOp(op) => op.as_op(),
@@ -433,7 +429,7 @@ where
                     Token::SquareBracketClose => symbol::RIGHT_SQUARE_BRACKET.as_op(),
                     _ => {
                         break 'mathbin Err(LatexError(
-                            tokspan.span().into(),
+                            span.into(),
                             LatexErrKind::ExpectedRelation,
                         ));
                     }
@@ -816,7 +812,7 @@ where
                     .tokens
                     .peek_class_token()?
                     .class(parse_as.in_sequence(), self.state.right_boundary_hack);
-                match tok.unwrap_math() {
+                match tok {
                     Token::Relation(op) => {
                         let (left, right) = self.state.relation_spacing(prev_class, next_class);
                         if let Some(negated) = get_negated_op(op) {
@@ -1549,7 +1545,7 @@ where
             symbol::LEFT_SQUARE_BRACKET.as_stretchable_op().unwrap();
         const SQ_R_BRACKET: StretchableOp =
             symbol::RIGHT_SQUARE_BRACKET.as_stretchable_op().unwrap();
-        let delim = match tok.unwrap_math() {
+        let delim = match tok {
             Token::Open(paren) => paren.as_stretchable_op(),
             Token::Close(paren) => paren.as_stretchable_op(),
             Token::Ord(ord) => ord.as_stretchable_op(),

crates/math-core/src/snapshots/math_core__token_queue__tests__consecutive_whitespace.snap

@@ -0,0 +1,7 @@
+---
+source: crates/math-core/src/token_queue.rs
+expression: "{x   y} z"
+---
+1..2: Letter('x', MathOrText)
+2..3: Whitespace
+5..6: Letter('y', MathOrText)

crates/math-core/src/snapshots/math_core__token_queue__tests__consecutive_whitespace_skip.snap

@@ -0,0 +1,6 @@
+---
+source: crates/math-core/src/token_queue.rs
+expression: "{x   y} z"
+---
+1..2: Letter('x', MathOrText)
+5..6: Letter('y', MathOrText)

crates/math-core/src/snapshots/math_core__token_queue__tests__early_error.snap

@@ -2,4 +2,10 @@
 source: crates/math-core/src/token_queue.rs
 expression: "{x + \\unknowncmd + y}"
 ---
-Error at 5..16: UnknownCommand("unknowncmd")
+Error: Unknown command "\unknowncmd".
+   ╭─[ <input>:1:6 ]
+   │
+ 1 │ {x + \unknowncmd + y}
+   │      ─────┬─────  
+   │           ╰─────── unknown command
+───╯

crates/math-core/src/snapshots/math_core__token_queue__tests__hyphen.snap

@@ -0,0 +1,5 @@
+---
+source: crates/math-core/src/token_queue.rs
+expression: "-xy"
+---
+0..1: MathOrTextMode(BinaryOp(Bin { char: '−', cat: BD }), '-')

crates/math-core/src/snapshots/math_core__token_queue__tests__hyphen_math_mode.snap

@@ -0,0 +1,5 @@
+---
+source: crates/math-core/src/token_queue.rs
+expression: "-xy"
+---
+0..1: BinaryOp(Bin { char: '−', cat: BD })

crates/math-core/src/snapshots/math_core__token_queue__tests__unclosed_group.snap

@@ -2,4 +2,10 @@
 source: crates/math-core/src/token_queue.rs
 expression: "{x + y"
 ---
-Error at 6..6: UnclosedGroup(GroupClose)
+Error: Expected token "}", but not found.
+   ╭─[ <input>:1:7 ]
+   │
+ 1 │ {x + y
+   │       │ 
+   │       ╰─ expected "}" to close this group
+───╯

crates/math-core/src/snapshots/math_core__token_queue__tests__unclosed_nested_group.snap

@@ -2,4 +2,10 @@
 source: crates/math-core/src/token_queue.rs
 expression: "{x + {y + z}"
 ---
-Error at 12..12: UnclosedGroup(GroupClose)
+Error: Expected token "}", but not found.
+   ╭─[ <input>:1:13 ]
+   │
+ 1 │ {x + {y + z}
+   │             │ 
+   │             ╰─ expected "}" to close this group
+───╯

crates/math-core/src/text_parser.rs

@@ -26,7 +26,7 @@ impl<'cell, 'arena, 'source, 'config> Parser<'cell, 'arena, 'source, 'config> {
 
         while let Some((previous_nesting, current_style)) = style_stack.last().copied() {
             let tokloc = if text_mode {
-                self.tokens.next_with_whitespace()
+                self.tokens.next_any_token()
             } else {
                 self.tokens.next()
             };

crates/math-core/src/token_queue.rs

@@ -169,7 +169,9 @@ impl<'source, 'config> TokenQueue<'source, 'config> {
         self.find_or_load_after_next(SkipMode::NoClass)
     }
 
-    /// Get the next non-whitespace token.
+    /// Get the next math-mode token.
+    ///
+    /// This method skips any whitespace tokens and unwraps [`Token::MathOrTextMode`].
     ///
     /// This method also ensures that there is always a peekable token after this one.
     pub(super) fn next(&mut self) -> Result<TokSpan<'source>, Box<LatexError>> {
@@ -181,6 +183,12 @@ impl<'source, 'config> TokenQueue<'source, 'config> {
         // Now pop the next token.
         if let Some(ret) = self.queue.pop_front() {
             self.ensure_next_non_whitespace()?;
+            let (tok, span) = ret.into_parts();
+            let ret = TokSpan::new(tok.unwrap_math(), span);
+            debug_assert!(!matches!(
+                ret.token(),
+                Token::Whitespace | Token::MathOrTextMode(_, _)
+            ));
             Ok(ret)
         } else {
             // We must have reached EOI previously.
@@ -189,8 +197,10 @@ impl<'source, 'config> TokenQueue<'source, 'config> {
         }
     }
 
-    /// Get the next token which may be whitespace.
-    pub(super) fn next_with_whitespace(&mut self) -> Result<TokSpan<'source>, Box<LatexError>> {
+    /// Get the next token without skipping or unwrapping anything.
+    ///
+    /// This method may return whitespace tokens and [`Token::MathOrTextMode`].
+    pub(super) fn next_any_token(&mut self) -> Result<TokSpan<'source>, Box<LatexError>> {
         if let Some(ret) = self.queue.pop_front() {
             // `next_non_whitespace` may need to be updated.
             if let Some(new_pos) = self.next_non_whitespace.checked_sub(1) {
@@ -235,12 +245,12 @@ impl<'source, 'config> TokenQueue<'source, 'config> {
     pub(super) fn record_group(
         &mut self,
         tokens: &mut Vec<TokSpan<'source>>,
-        with_whitespace: bool,
+        preserve_all: bool,
     ) -> Result<usize, Box<LatexError>> {
         let mut nesting_level = 0usize;
         let end = loop {
-            let tokloc = if with_whitespace {
-                self.next_with_whitespace()
+            let tokloc = if preserve_all {
+                self.next_any_token()
             } else {
                 self.next()
             };
@@ -274,17 +284,26 @@ impl<'source, 'config> TokenQueue<'source, 'config> {
     /// Read one macro argument, which is either a single token or a group of tokens.
     ///
     /// Any immediately following whitespace is always skipped. If the argument is a group, then
-    /// the parameter `with_whitespace` determines whether the whitespace tokens within the group
+    /// the parameter `preserve_all` determines whether the whitespace tokens within the group
     /// are included in the output vector or not.
     pub fn read_argument(
         &mut self,
-        with_whitespace: bool,
+        preserve_all: bool,
     ) -> Result<MacroArgument<'source>, Box<LatexError>> {
-        let first = self.next()?;
+        let first = if preserve_all {
+            // For `preserve_all`, we still want to skip leading whitespace, but we don't want to
+            // perform the unwrapping that `next()` does. So we use this hack here of copying the
+            // peek token and then discarding it with `next()`.
+            let tok = *self.peek();
+            self.next()?;
+            tok
+        } else {
+            self.next()?
+        };
         if matches!(first.token(), Token::GroupBegin) {
             let mut tokens = Vec::new();
             // Read until the matching `}`.
-            let end_loc = self.record_group(&mut tokens, with_whitespace)?;
+            let end_loc = self.record_group(&mut tokens, preserve_all)?;
             Ok(MacroArgument::Group(tokens, first.span().start()..end_loc))
         } else {
             Ok(MacroArgument::Token(first))
@@ -359,7 +378,7 @@ mod tests {
     use super::*;
 
     #[test]
-    fn test_read_group() {
+    fn test_record_group() {
         let problems = [
             ("simple_group", r"{x+y}"),
             ("group_followed", r"{x+y} b"),
@@ -390,7 +409,14 @@ mod tests {
                     }
                     token_str
                 }
-                Err(err) => format!("Error at {}..{}: {:?}", err.0.start, err.0.end, err.1),
+                Err(error) => {
+                    let report = error.to_report("<input>", false);
+                    let mut buf = Vec::new();
+                    report
+                        .write(("<input>", ariadne::Source::from(problem)), &mut buf)
+                        .expect("failed to write report");
+                    String::from_utf8(buf).expect("report should be valid UTF-8")
+                }
             };
             assert_snapshot!(name, &tokens, problem);
         }
@@ -406,7 +432,7 @@ mod tests {
         let mut token_str = String::new();
 
         loop {
-            let (tok, span) = manager.next_with_whitespace().unwrap().into_parts();
+            let (tok, span) = manager.next_any_token().unwrap().into_parts();
             if matches!(tok, Token::Eoi) {
                 break;
             }
@@ -442,4 +468,42 @@ mod tests {
         assert!(matches!(queue.queue[0].token(), Token::Whitespace));
         assert!(matches!(queue.queue[2].token(), Token::Whitespace));
     }
+
+    #[test]
+    fn text_read_argument() {
+        let problems = [
+            ("hyphen", r"-xy", true),
+            ("hyphen_math_mode", r"-xy", false),
+            ("consecutive_whitespace", r"{x   y} z", true),
+            ("consecutive_whitespace_skip", r"{x   y} z", false),
+        ];
+
+        for (name, problem, preserve_all) in problems.into_iter() {
+            let lexer = Lexer::new(problem, false, None);
+            let mut manager = TokenQueue::new(lexer).expect("Failed to create TokenManager");
+            let tokens = match manager.read_argument(preserve_all) {
+                Ok(MacroArgument::Group(tokens, _)) => {
+                    let mut token_str = String::new();
+                    for tokloc in tokens {
+                        let (tok, span) = tokloc.into_parts();
+                        write!(token_str, "{}..{}: {:?}\n", span.start(), span.end(), tok).unwrap();
+                    }
+                    token_str
+                }
+                Ok(MacroArgument::Token(tok)) => {
+                    let (tok, span) = tok.into_parts();
+                    format!("{}..{}: {:?}\n", span.start(), span.end(), tok)
+                }
+                Err(error) => {
+                    let report = error.to_report("<input>", false);
+                    let mut buf = Vec::new();
+                    report
+                        .write(("<input>", ariadne::Source::from(problem)), &mut buf)
+                        .expect("failed to write report");
+                    String::from_utf8(buf).expect("report should be valid UTF-8")
+                }
+            };
+            assert_snapshot!(name, &tokens, problem);
+        }
+    }
 }

crates/math-core/tests/conversion_test.rs

@@ -200,7 +200,7 @@ fn main() {
         ),
         ("genfrac_0.4pt", r"\genfrac(]{0.4pt}{2}{a+b}{c+d}"),
         ("genfrac_0.4ex", r"\genfrac(]{0.4ex}{2}{a+b}{c+d}"),
-        ("genfrac_4em", r"\genfrac(]{4em}{2}{a+b}{c+d}"),
+        ("genfrac_4em", r"\genfrac(]{4em}2{a+b}{c+d}"),
         ("not_subset", r"\not\subset"),
         ("not_less_than", r"\not\lt"),
         ("not_less_than_symbol", r"\not< x"),

crates/math-core/tests/error_test.rs

@@ -89,6 +89,8 @@ fn main() {
         ("mathbin_no_arg", r"x \mathbin{} y"),
         ("mathbin_letter", r"x \mathbin a y"),
         ("genfrac_too_many_numbers", r"\genfrac[]{0pt}{00}{a}{b}"),
+        ("unknown_color", r"{\color{foobar} x}"),
+        ("unknown_color_no_braces", r"{\color - x}"),
     ];
 
     let config = MathCoreConfig {

crates/math-core/tests/snapshots/conversion_test__genfrac_4em.snap

@@ -1,6 +1,6 @@
 ---
 source: crates/math-core/tests/conversion_test.rs
-expression: "\\genfrac(]{4em}{2}{a+b}{c+d}"
+expression: "\\genfrac(]{4em}2{a+b}{c+d}"
 ---
 <math>
     <mrow displaystyle="false" scriptlevel="1">

crates/math-core/tests/snapshots/error_test__unknown_color.snap

@@ -0,0 +1,11 @@
+---
+source: crates/math-core/tests/error_test.rs
+expression: "{\\color{foobar} x}"
+---
+Error: Unknown color "foobar".
+   ╭─[ <input>:1:8 ]
+   │
+ 1 │ {\color{foobar} x}
+   │        ────┬───  
+   │            ╰───── unknown color
+───╯

crates/math-core/tests/snapshots/error_test__unknown_color_no_braces.snap

@@ -0,0 +1,11 @@
+---
+source: crates/math-core/tests/error_test.rs
+expression: "{\\color - x}"
+---
+Error: Unknown color "-".
+   ╭─[ <input>:1:9 ]
+   │
+ 1 │ {\color - x}
+   │         ┬  
+   │         ╰── unknown color
+───╯