From e239aa82295762622069ca300b38560da47b8a3b Mon Sep 17 00:00:00 2001
From: Max Brunsfeld <maxbrunsfeld@gmail.com>
Date: Wed, 20 Feb 2019 16:45:51 -0800
Subject: [PATCH] highlight: don't include scope in ScopeEnd events

When there are embedded documents, multiple scopes can start or
end at the same position. Previously, there was no guarantee that
the ScopeEnd events would always occur in the reverse order of the
ScopeStart events. The easiest way to avoid exposing inconsistency
is to not surface the scopes being ended.
---
 cli/src/highlight.rs            |  2 +-
 cli/src/tests/highlight_test.rs |  3 +--
 highlight/src/lib.rs            | 24 +++++++++++++++---------
 3 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/cli/src/highlight.rs b/cli/src/highlight.rs
index 6cd19392..55ef4bc2 100644
--- a/cli/src/highlight.rs
+++ b/cli/src/highlight.rs
@@ -209,7 +209,7 @@ pub fn ansi(
             HighlightEvent::ScopeStart(s) => {
                 scope_stack.push(s);
             }
-            HighlightEvent::ScopeEnd(_) => {
+            HighlightEvent::ScopeEnd => {
                 scope_stack.pop();
             }
         }
diff --git a/cli/src/tests/highlight_test.rs b/cli/src/tests/highlight_test.rs
index 6e07ab4a..57f61e16 100644
--- a/cli/src/tests/highlight_test.rs
+++ b/cli/src/tests/highlight_test.rs
@@ -162,8 +162,7 @@ fn to_token_vector<'a>(
     )? {
         match event {
             HighlightEvent::ScopeStart(s) => scopes.push(s),
-            HighlightEvent::ScopeEnd(s) => {
-                assert_eq!(*scopes.last().unwrap(), s);
+            HighlightEvent::ScopeEnd => {
                 scopes.pop();
             }
             HighlightEvent::Source(s) => {
diff --git a/highlight/src/lib.rs b/highlight/src/lib.rs
index bbe0b424..7ec186d8 100644
--- a/highlight/src/lib.rs
+++ b/highlight/src/lib.rs
@@ -96,7 +96,7 @@ where
 pub enum HighlightEvent<'a> {
     Source(&'a str),
     ScopeStart(Scope),
-    ScopeEnd(Scope),
+    ScopeEnd,
 }
 
 #[derive(Debug, Deserialize)]
@@ -565,10 +565,7 @@ where
                 .parse(self.source, None)
                 .expect("Failed to parse");
             let layer = Layer::new(self.source, tree, property_sheet, ranges);
-            match self
-                .layers
-                .binary_search_by_key(&(layer.offset(), 1), |l| (l.offset(), 0))
-            {
+            match self.layers.binary_search_by(|l| l.cmp(&layer)) {
                 Ok(i) | Err(i) => self.layers.insert(i, layer),
             };
         }
@@ -625,7 +622,7 @@ impl<'a, T: Fn(&str) -> Option<(Language, &'a PropertySheet<Properties>)>> Itera
                 }
 
                 scope_event = if self.layers[0].at_node_end {
-                    Some(HighlightEvent::ScopeEnd(scope))
+                    Some(HighlightEvent::ScopeEnd)
                 } else {
                     Some(HighlightEvent::ScopeStart(scope))
                 };
@@ -638,7 +635,7 @@ impl<'a, T: Fn(&str) -> Option<(Language, &'a PropertySheet<Properties>)>> Itera
             // to re-sort the layers. If the cursor is already at the end of its syntax tree,
             // remove it.
             if self.layers[0].advance() {
-                self.layers.sort_unstable_by_key(|layer| layer.offset());
+                self.layers.sort_unstable_by(|a, b| a.cmp(&b));
             } else {
                 self.layers.remove(0);
             }
@@ -676,6 +673,15 @@ impl<'a> Layer<'a> {
         }
     }
 
+    fn cmp(&self, other: &Layer) -> cmp::Ordering {
+        // Events are ordered primarily by their position in the document. But if
+        // one scope starts at a given position and another scope ends at that
+        // same position, return the scope end event before the scope start event.
+        self.offset()
+            .cmp(&other.offset())
+            .then_with(|| other.at_node_end.cmp(&self.at_node_end))
+    }
+
     fn offset(&self) -> usize {
         if self.at_node_end {
             self.cursor.node().end_byte()
@@ -770,8 +776,8 @@ where
                 scopes.push(s);
                 renderer.start_scope(s);
             }
-            HighlightEvent::ScopeEnd(s) => {
-                assert_eq!(scopes.pop(), Some(s));
+            HighlightEvent::ScopeEnd => {
+                scopes.pop();
                 renderer.end_scope();
             }
             HighlightEvent::Source(src) => {