Finish implementing ProjectDiagnostics::open_excerpts

Max Brunsfeld and Nathan Sobo created

* Build workspace item views with a reference to the workspace
* Add randomized test for MultiBuffer::excerpted_buffers and fix a small bug

Co-Authored-By: Nathan Sobo <nathan@zed.dev>

Change summary

crates/diagnostics/src/diagnostics.rs | 64 +++++++++++++++++++---------
crates/editor/src/items.rs            |  9 +++
crates/editor/src/multi_buffer.rs     | 46 +++++++++++++++-----
crates/workspace/src/pane.rs          |  5 +
crates/workspace/src/workspace.rs     | 26 ++++++++---
5 files changed, 106 insertions(+), 44 deletions(-)

Detailed changes

crates/diagnostics/src/diagnostics.rs 🔗

@@ -5,16 +5,17 @@ use collections::{HashMap, HashSet};
 use editor::{
     context_header_renderer, diagnostic_block_renderer, diagnostic_header_renderer,
     display_map::{BlockDisposition, BlockId, BlockProperties},
-    BuildSettings, Editor, ExcerptId, ExcerptProperties, MultiBuffer,
+    items::BufferItemHandle,
+    Autoscroll, BuildSettings, Editor, ExcerptId, ExcerptProperties, MultiBuffer,
 };
 use gpui::{
     action, elements::*, keymap::Binding, AppContext, Entity, ModelHandle, MutableAppContext,
-    RenderContext, Task, View, ViewContext, ViewHandle,
+    RenderContext, Task, View, ViewContext, ViewHandle, WeakViewHandle,
 };
 use language::{Bias, Buffer, Diagnostic, DiagnosticEntry, Point, Selection, SelectionGoal};
 use postage::watch;
 use project::{Project, ProjectPath, WorktreeId};
-use std::{cmp::Ordering, ops::Range, path::Path, sync::Arc};
+use std::{cmp::Ordering, mem, ops::Range, path::Path, sync::Arc};
 use util::TryFutureExt;
 use workspace::Workspace;
 
@@ -44,6 +45,7 @@ struct ProjectDiagnostics {
 
 struct ProjectDiagnosticsEditor {
     model: ModelHandle<ProjectDiagnostics>,
+    workspace: WeakViewHandle<Workspace>,
     editor: ViewHandle<Editor>,
     excerpts: ModelHandle<MultiBuffer>,
     path_states: Vec<(Arc<Path>, Vec<DiagnosticGroupState>)>,
@@ -110,6 +112,7 @@ impl View for ProjectDiagnosticsEditor {
 impl ProjectDiagnosticsEditor {
     fn new(
         model: ModelHandle<ProjectDiagnostics>,
+        workspace: WeakViewHandle<Workspace>,
         settings: watch::Receiver<workspace::Settings>,
         cx: &mut ViewContext<Self>,
     ) -> Self {
@@ -144,6 +147,7 @@ impl ProjectDiagnosticsEditor {
             .collect();
         let this = Self {
             model,
+            workspace,
             excerpts,
             editor,
             build_settings,
@@ -166,20 +170,37 @@ impl ProjectDiagnosticsEditor {
     }
 
     fn open_excerpts(&mut self, _: &OpenExcerpts, cx: &mut ViewContext<Self>) {
-        let editor = self.editor.read(cx);
-        let excerpts = self.excerpts.read(cx);
-        let mut new_selections_by_buffer = HashMap::default();
-        for selection in editor.local_selections::<usize>(cx) {
-            for (buffer, range) in excerpts.excerpted_buffers(selection.start..selection.end, cx) {
-                new_selections_by_buffer
-                    .entry(buffer)
-                    .or_insert(Vec::new())
-                    .push((range.start, range.end, selection.reversed))
+        if let Some(workspace) = self.workspace.upgrade(cx) {
+            let editor = self.editor.read(cx);
+            let excerpts = self.excerpts.read(cx);
+            let mut new_selections_by_buffer = HashMap::default();
+
+            for selection in editor.local_selections::<usize>(cx) {
+                for (buffer, mut range) in
+                    excerpts.excerpted_buffers(selection.start..selection.end, cx)
+                {
+                    if selection.reversed {
+                        mem::swap(&mut range.start, &mut range.end);
+                    }
+                    new_selections_by_buffer
+                        .entry(buffer)
+                        .or_insert(Vec::new())
+                        .push(range)
+                }
             }
-        }
 
-        for (buffer, selections) in new_selections_by_buffer {
-            // buffer.read(cx).
+            workspace.update(cx, |workspace, cx| {
+                for (buffer, ranges) in new_selections_by_buffer {
+                    let editor = workspace
+                        .open_item(BufferItemHandle(buffer), cx)
+                        .to_any()
+                        .downcast::<Editor>()
+                        .unwrap();
+                    editor.update(cx, |editor, cx| {
+                        editor.select_ranges(ranges, Some(Autoscroll::Center), cx)
+                    });
+                }
+            });
         }
     }
 
@@ -459,10 +480,10 @@ impl workspace::Item for ProjectDiagnostics {
 
     fn build_view(
         handle: ModelHandle<Self>,
-        settings: watch::Receiver<workspace::Settings>,
+        workspace: &Workspace,
         cx: &mut ViewContext<Self::View>,
     ) -> Self::View {
-        ProjectDiagnosticsEditor::new(handle, settings, cx)
+        ProjectDiagnosticsEditor::new(handle, workspace.weak_handle(), workspace.settings(), cx)
     }
 
     fn project_path(&self) -> Option<project::ProjectPath> {
@@ -554,7 +575,8 @@ mod tests {
 
     #[gpui::test]
     async fn test_diagnostics(mut cx: TestAppContext) {
-        let settings = cx.update(WorkspaceParams::test).settings;
+        let workspace_params = cx.update(WorkspaceParams::test);
+        let settings = workspace_params.settings.clone();
         let http_client = FakeHttpClient::new(|_| async move { Ok(ServerResponse::new(404)) });
         let client = Client::new(http_client.clone());
         let user_store = cx.add_model(|cx| UserStore::new(client.clone(), http_client, cx));
@@ -684,8 +706,10 @@ mod tests {
         });
 
         let model = cx.add_model(|_| ProjectDiagnostics::new(project.clone()));
-        let view = cx.add_view(Default::default(), |cx| {
-            ProjectDiagnosticsEditor::new(model, settings, cx)
+        let workspace = cx.add_view(0, |cx| Workspace::new(&workspace_params, cx));
+
+        let view = cx.add_view(0, |cx| {
+            ProjectDiagnosticsEditor::new(model, workspace.downgrade(), settings, cx)
         });
 
         view.condition(&mut cx, |view, cx| view.text(cx).contains("fn main()"))

crates/editor/src/items.rs 🔗

@@ -13,6 +13,7 @@ use std::path::Path;
 use text::{Point, Selection};
 use workspace::{
     ItemHandle, ItemView, ItemViewHandle, PathOpener, Settings, StatusItemView, WeakItemHandle,
+    Workspace,
 };
 
 pub struct BufferOpener;
@@ -43,13 +44,17 @@ impl ItemHandle for BufferItemHandle {
     fn add_view(
         &self,
         window_id: usize,
-        settings: watch::Receiver<Settings>,
+        workspace: &Workspace,
         cx: &mut MutableAppContext,
     ) -> Box<dyn ItemViewHandle> {
         let buffer = cx.add_model(|cx| MultiBuffer::singleton(self.0.clone(), cx));
         let weak_buffer = buffer.downgrade();
         Box::new(cx.add_view(window_id, |cx| {
-            Editor::for_buffer(buffer, crate::settings_builder(weak_buffer, settings), cx)
+            Editor::for_buffer(
+                buffer,
+                crate::settings_builder(weak_buffer, workspace.settings()),
+                cx,
+            )
         }))
     }
 

crates/editor/src/multi_buffer.rs 🔗

@@ -706,11 +706,16 @@ impl MultiBuffer {
                 break;
             }
 
+            let mut end_before_newline = cursor.end(&());
+            if excerpt.has_trailing_newline {
+                end_before_newline -= 1;
+            }
             let excerpt_start = excerpt.range.start.to_offset(&excerpt.buffer);
             let start = excerpt_start + (cmp::max(start, *cursor.start()) - *cursor.start());
-            let end = excerpt_start + (cmp::min(end, cursor.end(&())) - *cursor.start());
+            let end = excerpt_start + (cmp::min(end, end_before_newline) - *cursor.start());
             let buffer = self.buffers.borrow()[&excerpt.buffer_id].buffer.clone();
             result.push((buffer, start..end));
+            cursor.next(&());
         }
 
         result
@@ -2582,7 +2587,7 @@ mod tests {
             .unwrap_or(10);
 
         let mut buffers: Vec<ModelHandle<Buffer>> = Vec::new();
-        let list = cx.add_model(|_| MultiBuffer::new(0));
+        let multibuffer = cx.add_model(|_| MultiBuffer::new(0));
         let mut excerpt_ids = Vec::new();
         let mut expected_excerpts = Vec::<(ModelHandle<Buffer>, Range<text::Anchor>)>::new();
         let mut old_versions = Vec::new();
@@ -2613,7 +2618,9 @@ mod tests {
                         );
                     }
                     ids_to_remove.sort_unstable();
-                    list.update(cx, |list, cx| list.remove_excerpts(&ids_to_remove, cx));
+                    multibuffer.update(cx, |multibuffer, cx| {
+                        multibuffer.remove_excerpts(&ids_to_remove, cx)
+                    });
                 }
                 _ => {
                     let buffer_handle = if buffers.is_empty() || rng.gen_bool(0.4) {
@@ -2645,8 +2652,8 @@ mod tests {
                         &buffer.text()[start_ix..end_ix]
                     );
 
-                    let excerpt_id = list.update(cx, |list, cx| {
-                        list.insert_excerpt_after(
+                    let excerpt_id = multibuffer.update(cx, |multibuffer, cx| {
+                        multibuffer.insert_excerpt_after(
                             &prev_excerpt_id,
                             ExcerptProperties {
                                 buffer: &buffer_handle,
@@ -2662,12 +2669,12 @@ mod tests {
             }
 
             if rng.gen_bool(0.3) {
-                list.update(cx, |list, cx| {
-                    old_versions.push((list.snapshot(cx), list.subscribe()));
+                multibuffer.update(cx, |multibuffer, cx| {
+                    old_versions.push((multibuffer.snapshot(cx), multibuffer.subscribe()));
                 })
             }
 
-            let snapshot = list.read(cx).snapshot(cx);
+            let snapshot = multibuffer.read(cx).snapshot(cx);
 
             let mut excerpt_starts = Vec::new();
             let mut expected_text = String::new();
@@ -2862,15 +2869,30 @@ mod tests {
                 let end_ix = text_rope.clip_offset(rng.gen_range(0..=text_rope.len()), Bias::Right);
                 let start_ix = text_rope.clip_offset(rng.gen_range(0..=end_ix), Bias::Left);
 
+                let text_for_range = snapshot
+                    .text_for_range(start_ix..end_ix)
+                    .collect::<String>();
                 assert_eq!(
-                    snapshot
-                        .text_for_range(start_ix..end_ix)
-                        .collect::<String>(),
+                    text_for_range,
                     &expected_text[start_ix..end_ix],
                     "incorrect text for range {:?}",
                     start_ix..end_ix
                 );
 
+                let excerpted_buffer_ranges =
+                    multibuffer.read(cx).excerpted_buffers(start_ix..end_ix, cx);
+                let excerpted_buffers_text = excerpted_buffer_ranges
+                    .into_iter()
+                    .map(|(buffer, buffer_range)| {
+                        buffer
+                            .read(cx)
+                            .text_for_range(buffer_range)
+                            .collect::<String>()
+                    })
+                    .collect::<Vec<_>>()
+                    .join("\n");
+                assert_eq!(excerpted_buffers_text, text_for_range);
+
                 let expected_summary = TextSummary::from(&expected_text[start_ix..end_ix]);
                 assert_eq!(
                     snapshot.text_summary_for_range::<TextSummary, _>(start_ix..end_ix),
@@ -2904,7 +2926,7 @@ mod tests {
             }
         }
 
-        let snapshot = list.read(cx).snapshot(cx);
+        let snapshot = multibuffer.read(cx).snapshot(cx);
         for (old_snapshot, subscription) in old_versions {
             let edits = subscription.consume().into_inner();
 

crates/workspace/src/pane.rs 🔗

@@ -1,5 +1,5 @@
 use super::{ItemViewHandle, SplitDirection};
-use crate::{ItemHandle, Settings};
+use crate::{ItemHandle, Settings, Workspace};
 use gpui::{
     action,
     elements::*,
@@ -90,6 +90,7 @@ impl Pane {
     pub fn open_item<T>(
         &mut self,
         item_handle: T,
+        workspace: &Workspace,
         cx: &mut ViewContext<Self>,
     ) -> Box<dyn ItemViewHandle>
     where
@@ -103,7 +104,7 @@ impl Pane {
             }
         }
 
-        let item_view = item_handle.add_view(cx.window_id(), self.settings.clone(), cx);
+        let item_view = item_handle.add_view(cx.window_id(), workspace, cx);
         self.add_item_view(item_view.boxed_clone(), cx);
         item_view
     }

crates/workspace/src/workspace.rs 🔗

@@ -18,7 +18,7 @@ use gpui::{
     platform::{CursorStyle, WindowOptions},
     AnyViewHandle, AppContext, ClipboardItem, Entity, ModelContext, ModelHandle, MutableAppContext,
     PathPromptOptions, PromptLevel, RenderContext, Task, View, ViewContext, ViewHandle,
-    WeakModelHandle,
+    WeakModelHandle, WeakViewHandle,
 };
 use language::LanguageRegistry;
 use log::error;
@@ -131,7 +131,7 @@ pub trait Item: Entity + Sized {
 
     fn build_view(
         handle: ModelHandle<Self>,
-        settings: watch::Receiver<Settings>,
+        workspace: &Workspace,
         cx: &mut ViewContext<Self::View>,
     ) -> Self::View;
 
@@ -181,7 +181,7 @@ pub trait ItemHandle: Send + Sync {
     fn add_view(
         &self,
         window_id: usize,
-        settings: watch::Receiver<Settings>,
+        workspace: &Workspace,
         cx: &mut MutableAppContext,
     ) -> Box<dyn ItemViewHandle>;
     fn boxed_clone(&self) -> Box<dyn ItemHandle>;
@@ -224,10 +224,10 @@ impl<T: Item> ItemHandle for ModelHandle<T> {
     fn add_view(
         &self,
         window_id: usize,
-        settings: watch::Receiver<Settings>,
+        workspace: &Workspace,
         cx: &mut MutableAppContext,
     ) -> Box<dyn ItemViewHandle> {
-        Box::new(cx.add_view(window_id, |cx| T::build_view(self.clone(), settings, cx)))
+        Box::new(cx.add_view(window_id, |cx| T::build_view(self.clone(), workspace, cx)))
     }
 
     fn boxed_clone(&self) -> Box<dyn ItemHandle> {
@@ -251,10 +251,10 @@ impl ItemHandle for Box<dyn ItemHandle> {
     fn add_view(
         &self,
         window_id: usize,
-        settings: watch::Receiver<Settings>,
+        workspace: &Workspace,
         cx: &mut MutableAppContext,
     ) -> Box<dyn ItemViewHandle> {
-        ItemHandle::add_view(self.as_ref(), window_id, settings, cx)
+        ItemHandle::add_view(self.as_ref(), window_id, workspace, cx)
     }
 
     fn boxed_clone(&self) -> Box<dyn ItemHandle> {
@@ -455,6 +455,7 @@ impl WorkspaceParams {
 
 pub struct Workspace {
     pub settings: watch::Receiver<Settings>,
+    weak_self: WeakViewHandle<Self>,
     client: Arc<Client>,
     user_store: ModelHandle<client::UserStore>,
     fs: Arc<dyn Fs>,
@@ -509,6 +510,7 @@ impl Workspace {
 
         Workspace {
             modal: None,
+            weak_self: cx.weak_handle(),
             center: PaneGroup::new(pane.id()),
             panes: vec![pane.clone()],
             active_pane: pane.clone(),
@@ -526,6 +528,14 @@ impl Workspace {
         }
     }
 
+    pub fn weak_handle(&self) -> WeakViewHandle<Self> {
+        self.weak_self.clone()
+    }
+
+    pub fn settings(&self) -> watch::Receiver<Settings> {
+        self.settings.clone()
+    }
+
     pub fn left_sidebar_mut(&mut self) -> &mut Sidebar {
         &mut self.left_sidebar
     }
@@ -914,7 +924,7 @@ impl Workspace {
         T: 'static + ItemHandle,
     {
         self.items.insert(item_handle.downgrade());
-        pane.update(cx, |pane, cx| pane.open_item(item_handle, cx))
+        pane.update(cx, |pane, cx| pane.open_item(item_handle, self, cx))
     }
 
     fn activate_pane(&mut self, pane: ViewHandle<Pane>, cx: &mut ViewContext<Self>) {