fix 9766 (#10055)

Conrad Irwin created

Release Notes:

- Fixed a panic in editor::SelectPrevious (`gN` in vim)
([#9766](https://github.com/zed-industries/zed/issues/9766)).

Change summary

crates/editor/src/editor_tests.rs             | 41 ++++++++++++++++++++
crates/editor/src/test/editor_test_context.rs | 42 ++++++++++++++++++++
crates/multi_buffer/src/multi_buffer.rs       | 31 ++++++++++-----
3 files changed, 103 insertions(+), 11 deletions(-)

Detailed changes

crates/editor/src/editor_tests.rs 🔗

@@ -4087,6 +4087,47 @@ let foo = «2ˇ»;"#,
     );
 }
 
+#[gpui::test]
+async fn test_select_previous_multibuffer(cx: &mut gpui::TestAppContext) {
+    init_test(cx, |_| {});
+
+    let mut cx = EditorTestContext::new_multibuffer(
+        cx,
+        [
+            indoc! {
+                "aaa\n«bbb\nccc\n»ddd"
+            },
+            indoc! {
+                "aaa\n«bbb\nccc\n»ddd"
+            },
+        ],
+    );
+
+    cx.assert_editor_state(indoc! {"
+        ˇbbb
+        ccc
+
+        bbb
+        ccc
+        "});
+    cx.dispatch_action(SelectPrevious::default());
+    cx.assert_editor_state(indoc! {"
+                «bbbˇ»
+                ccc
+
+                bbb
+                ccc
+                "});
+    cx.dispatch_action(SelectPrevious::default());
+    cx.assert_editor_state(indoc! {"
+                «bbbˇ»
+                ccc
+
+                «bbbˇ»
+                ccc
+                "});
+}
+
 #[gpui::test]
 async fn test_select_previous_with_single_caret(cx: &mut gpui::TestAppContext) {
     init_test(cx, |_| {});

crates/editor/src/test/editor_test_context.rs 🔗

@@ -10,6 +10,7 @@ use gpui::{
 use indoc::indoc;
 use itertools::Itertools;
 use language::{Buffer, BufferSnapshot, LanguageRegistry};
+use multi_buffer::ExcerptRange;
 use parking_lot::RwLock;
 use project::{FakeFs, Project};
 use std::{
@@ -20,12 +21,14 @@ use std::{
         Arc,
     },
 };
+use text::BufferId;
+use ui::Context;
 use util::{
     assert_set_eq,
     test::{generate_marked_text, marked_text_ranges},
 };
 
-use super::build_editor_with_project;
+use super::{build_editor, build_editor_with_project};
 
 pub struct EditorTestContext {
     pub cx: gpui::VisualTestContext,
@@ -67,6 +70,43 @@ impl EditorTestContext {
         }
     }
 
+    pub fn new_multibuffer<const COUNT: usize>(
+        cx: &mut gpui::TestAppContext,
+        excerpts: [&str; COUNT],
+    ) -> EditorTestContext {
+        let mut multibuffer = MultiBuffer::new(0, language::Capability::ReadWrite);
+        let buffer = cx.new_model(|cx| {
+            for (i, excerpt) in excerpts.into_iter().enumerate() {
+                let (text, ranges) = marked_text_ranges(excerpt, false);
+                let buffer =
+                    cx.new_model(|_| Buffer::new(0, BufferId::new(i as u64 + 1).unwrap(), text));
+                multibuffer.push_excerpts(
+                    buffer,
+                    ranges.into_iter().map(|range| ExcerptRange {
+                        context: range,
+                        primary: None,
+                    }),
+                    cx,
+                );
+            }
+            multibuffer
+        });
+
+        let editor = cx.add_window(|cx| {
+            let editor = build_editor(buffer, cx);
+            editor.focus(cx);
+            editor
+        });
+
+        let editor_view = editor.root_view(cx).unwrap();
+        Self {
+            cx: VisualTestContext::from_window(*editor.deref(), cx),
+            window: editor.into(),
+            editor: editor_view,
+            assertion_cx: AssertionContextManager::new(),
+        }
+    }
+
     pub fn condition(
         &self,
         predicate: impl FnMut(&Editor, &AppContext) -> bool,

crates/multi_buffer/src/multi_buffer.rs 🔗

@@ -262,7 +262,8 @@ struct ExcerptChunks<'a> {
 
 struct ExcerptBytes<'a> {
     content_bytes: text::Bytes<'a>,
-    footer_height: usize,
+    padding_height: usize,
+    reversed: bool,
 }
 
 impl MultiBuffer {
@@ -2165,7 +2166,7 @@ impl MultiBufferSnapshot {
         let mut chunk = &[][..];
         let excerpt_bytes = if let Some(excerpt) = excerpts.item() {
             let mut excerpt_bytes = excerpt.reversed_bytes_in_range(
-                range.start - excerpts.start()..range.end - excerpts.start(),
+                range.start.saturating_sub(*excerpts.start())..range.end - *excerpts.start(),
             );
             chunk = excerpt_bytes.next().unwrap_or(&[][..]);
             Some(excerpt_bytes)
@@ -3724,7 +3725,8 @@ impl Excerpt {
 
         ExcerptBytes {
             content_bytes,
-            footer_height,
+            padding_height: footer_height,
+            reversed: false,
         }
     }
 
@@ -3744,7 +3746,8 @@ impl Excerpt {
 
         ExcerptBytes {
             content_bytes,
-            footer_height,
+            padding_height: footer_height,
+            reversed: true,
         }
     }
 
@@ -4130,14 +4133,16 @@ impl<'a> ReversedMultiBufferBytes<'a> {
             if let Some(chunk) = self.excerpt_bytes.as_mut().and_then(|bytes| bytes.next()) {
                 self.chunk = chunk;
             } else {
-                self.excerpts.next(&());
+                self.excerpts.prev(&());
                 if let Some(excerpt) = self.excerpts.item() {
-                    let mut excerpt_bytes =
-                        excerpt.bytes_in_range(0..self.range.end - self.excerpts.start());
+                    let mut excerpt_bytes = excerpt.reversed_bytes_in_range(
+                        self.range.start.saturating_sub(*self.excerpts.start())..usize::MAX,
+                    );
                     self.chunk = excerpt_bytes.next().unwrap();
                     self.excerpt_bytes = Some(excerpt_bytes);
                 }
             }
+        } else {
         }
     }
 }
@@ -4157,15 +4162,21 @@ impl<'a> Iterator for ExcerptBytes<'a> {
     type Item = &'a [u8];
 
     fn next(&mut self) -> Option<Self::Item> {
+        if self.reversed && self.padding_height > 0 {
+            let result = &NEWLINES[..self.padding_height];
+            self.padding_height = 0;
+            return Some(result);
+        }
+
         if let Some(chunk) = self.content_bytes.next() {
             if !chunk.is_empty() {
                 return Some(chunk);
             }
         }
 
-        if self.footer_height > 0 {
-            let result = &NEWLINES[..self.footer_height];
-            self.footer_height = 0;
+        if self.padding_height > 0 {
+            let result = &NEWLINES[..self.padding_height];
+            self.padding_height = 0;
             return Some(result);
         }