From 98c96ba1cb8a46e327de313118e4ce1a84795ae9 Mon Sep 17 00:00:00 2001
From: Martin Haug <mhaug@live.de>
Date: Sun, 2 Jan 2022 14:46:08 +0100
Subject: [PATCH] Fix parser / space / error bug

---
 src/parse/incremental.rs |  1 +
 src/parse/parser.rs      | 16 ++--------------
 src/parse/tokens.rs      |  1 +
 tests/typ/code/let.typ   |  1 +
 tests/typeset.rs         | 12 ++++++++++--
 5 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/src/parse/incremental.rs b/src/parse/incremental.rs
index 1ee37a511..5cb016d2c 100644
--- a/src/parse/incremental.rs
+++ b/src/parse/incremental.rs
@@ -623,6 +623,7 @@ mod tests {
         test("x = y", 1 .. 1, " + y\n", 0 .. 10);
         test("abc\n= a heading\njoke", 3 .. 4, "\nmore\n\n", 0 .. 21);
         test("abc\n= a heading\njoke", 3 .. 4, "\nnot ", 0 .. 19);
+        test("#let x = (1, 2 + ; Five\r\n\r", 19..22, "2.", 18..22);
         test("hey #myfriend", 4 .. 4, "\\", 0 .. 14);
         test("hey  #myfriend", 4 .. 4, "\\", 3 .. 6);
 
diff --git a/src/parse/parser.rs b/src/parse/parser.rs
index b31f69d3b..f36155d5d 100644
--- a/src/parse/parser.rs
+++ b/src/parse/parser.rs
@@ -282,12 +282,6 @@ impl<'s> Parser<'s> {
                 self.eat();
                 rescan = false;
             } else if required {
-                // FIXME The error has to be inserted before any space rolls
-                // around because the rescan will set the cursor back in front
-                // of the space and reconsume it. Supressing the rescan is not
-                // an option since additional rescans (e.g. for statements) can
-                // be triggered directly afterwards, without processing any
-                // other token.
                 self.push_error(format_eco!("expected {}", end));
                 self.last_unterminated = Some(self.prev_end());
             }
@@ -380,14 +374,8 @@ impl Parser<'_> {
     /// Push an error into the children list.
     pub fn push_error(&mut self, msg: impl Into<EcoString>) {
         let error = NodeKind::Error(ErrorPos::Full, msg.into());
-        for i in (0 .. self.children.len()).rev() {
-            if Self::is_trivia_ext(self.children[i].kind(), false) {
-                self.children.remove(i);
-            } else {
-                break;
-            }
-        }
-        self.children.push(GreenData::new(error, 0).into());
+        let idx = self.trivia_start();
+        self.children.insert(idx.0, GreenData::new(error, 0).into());
     }
 
     /// Eat the current token and add an error that it is unexpected.
diff --git a/src/parse/tokens.rs b/src/parse/tokens.rs
index 3a0ad1ade..7dfca2bf4 100644
--- a/src/parse/tokens.rs
+++ b/src/parse/tokens.rs
@@ -727,6 +727,7 @@ mod tests {
         t!(Both["a1/"]: "  \n"         => Space(1));
         t!(Both["a1/"]: "  \n   "      => Space(1));
         t!(Both["a1/"]: "\r\n"         => Space(1));
+        t!(Both["a1/"]: "\r\n\r"       => Space(2));
         t!(Both["a1/"]: "  \n\t \n  "  => Space(2));
         t!(Both["a1/"]: "\n\r"         => Space(2));
         t!(Both["a1/"]: " \r\r\n \x0D" => Space(3));
diff --git a/tests/typ/code/let.typ b/tests/typ/code/let.typ
index a95d651aa..d4765ea5d 100644
--- a/tests/typ/code/let.typ
+++ b/tests/typ/code/let.typ
@@ -59,6 +59,7 @@ Three
 // Error: 18 expected expression
 // Error: 18 expected closing paren
 #let v5 = (1, 2 + ; Five
+                   ^^^^^ + \r\n
 
 ---
 // Error: 13 expected body
diff --git a/tests/typeset.rs b/tests/typeset.rs
index f23de5cd9..aa3bcf9d5 100644
--- a/tests/typeset.rs
+++ b/tests/typeset.rs
@@ -412,6 +412,15 @@ fn test_reparse(src: &str, i: usize, rng: &mut LinearShift) -> bool {
 
     let apply = |replace: std::ops::Range<usize>, with| {
         let mut incr_source = SourceFile::detached(src);
+        if incr_source.root().len() != src.len() {
+            println!(
+                "    Subtest {} tree length {} does not match string length {} ❌",
+                i,
+                incr_source.root().len(),
+                src.len(),
+            );
+            return false;
+        }
 
         incr_source.edit(replace.clone(), with);
         let edited_src = incr_source.src();
@@ -428,7 +437,7 @@ fn test_reparse(src: &str, i: usize, rng: &mut LinearShift) -> bool {
                 "\n    Expected reference tree:\n{:#?}\n\n    Found incremental tree:\n{:#?}",
                 ref_root, incr_root
             );
-            println!("Full source ({}):\n\"{}\"", edited_src.len(), edited_src);
+            println!("Full source ({}):\n\"{:?}\"", edited_src.len(), edited_src);
             false
         } else {
             true
@@ -454,7 +463,6 @@ fn test_reparse(src: &str, i: usize, rng: &mut LinearShift) -> bool {
 
         if !apply(start .. end, supplement) {
             println!("original tree: {:#?}", SourceFile::detached(src).root());
-
             ok = false;
         }
     }