Refactor to pass a TextLayoutDetails around

Conrad Irwin created

Change summary

crates/editor/src/display_map.rs | 44 +++++++++++---------------------
crates/editor/src/editor.rs      | 31 +++++-----------------
crates/editor/src/movement.rs    | 46 +++++++++++++++++++--------------
3 files changed, 48 insertions(+), 73 deletions(-)

Detailed changes

crates/editor/src/display_map.rs 🔗

@@ -5,8 +5,8 @@ mod tab_map;
 mod wrap_map;
 
 use crate::{
-    link_go_to_definition::InlayHighlight, Anchor, AnchorRangeExt, EditorStyle, InlayId,
-    MultiBuffer, MultiBufferSnapshot, ToOffset, ToPoint,
+    link_go_to_definition::InlayHighlight, movement::TextLayoutDetails, Anchor, AnchorRangeExt,
+    EditorStyle, InlayId, MultiBuffer, MultiBufferSnapshot, ToOffset, ToPoint,
 };
 pub use block_map::{BlockMap, BlockPoint};
 use collections::{BTreeMap, HashMap, HashSet};
@@ -565,9 +565,11 @@ impl DisplaySnapshot {
     fn layout_line_for_row(
         &self,
         display_row: u32,
-        font_cache: &FontCache,
-        text_layout_cache: &TextLayoutCache,
-        editor_style: &EditorStyle,
+        TextLayoutDetails {
+            font_cache,
+            text_layout_cache,
+            editor_style,
+        }: &TextLayoutDetails,
     ) -> Line {
         let mut styles = Vec::new();
         let mut line = String::new();
@@ -605,16 +607,9 @@ impl DisplaySnapshot {
     pub fn x_for_point(
         &self,
         display_point: DisplayPoint,
-        font_cache: &FontCache,
-        text_layout_cache: &TextLayoutCache,
-        editor_style: &EditorStyle,
+        text_layout_details: &TextLayoutDetails,
     ) -> f32 {
-        let layout_line = self.layout_line_for_row(
-            display_point.row(),
-            font_cache,
-            text_layout_cache,
-            editor_style,
-        );
+        let layout_line = self.layout_line_for_row(display_point.row(), text_layout_details);
         layout_line.x_for_index(display_point.column() as usize)
     }
 
@@ -622,12 +617,9 @@ impl DisplaySnapshot {
         &self,
         display_row: u32,
         x_coordinate: f32,
-        font_cache: &FontCache,
-        text_layout_cache: &TextLayoutCache,
-        editor_style: &EditorStyle,
+        text_layout_details: &TextLayoutDetails,
     ) -> Option<u32> {
-        let layout_line =
-            self.layout_line_for_row(display_row, font_cache, text_layout_cache, editor_style);
+        let layout_line = self.layout_line_for_row(display_row, text_layout_details);
         layout_line.index_for_x(x_coordinate).map(|c| c as u32)
     }
 
@@ -1339,7 +1331,8 @@ pub mod tests {
         let window = cx.window.clone();
 
         cx.update_window(window, |cx| {
-            let editor_style = editor.read(&cx).style(cx);
+            let text_layout_details =
+                editor.read_with(cx, |editor, cx| TextLayoutDetails::new(editor, cx));
 
             let font_cache = cx.font_cache().clone();
 
@@ -1380,12 +1373,7 @@ pub mod tests {
                 DisplayPoint::new(0, 7)
             );
 
-            let x = snapshot.x_for_point(
-                DisplayPoint::new(1, 10),
-                cx.font_cache(),
-                cx.text_layout_cache(),
-                &editor_style,
-            );
+            let x = snapshot.x_for_point(DisplayPoint::new(1, 10), &text_layout_details);
             dbg!(x);
             assert_eq!(
                 movement::up(
@@ -1393,9 +1381,7 @@ pub mod tests {
                     DisplayPoint::new(1, 10),
                     SelectionGoal::None,
                     false,
-                    cx.font_cache(),
-                    cx.text_layout_cache(),
-                    &editor_style,
+                    &text_layout_details,
                 ),
                 (
                     DisplayPoint::new(0, 7),

crates/editor/src/editor.rs 🔗

@@ -71,6 +71,7 @@ use link_go_to_definition::{
 };
 use log::error;
 use lsp::LanguageServerId;
+use movement::TextLayoutDetails;
 use multi_buffer::ToOffsetUtf16;
 pub use multi_buffer::{
     Anchor, AnchorRangeExt, ExcerptId, ExcerptRange, MultiBuffer, MultiBufferSnapshot, ToOffset,
@@ -5274,9 +5275,7 @@ impl Editor {
             return;
         }
 
-        let font_cache = cx.font_cache().clone();
-        let text_layout_cache = cx.text_layout_cache().clone();
-        let editor_style = self.style(cx);
+        let text_layout_details = TextLayoutDetails::new(&self, cx);
 
         self.change_selections(Some(Autoscroll::fit()), cx, |s| {
             let line_mode = s.line_mode;
@@ -5289,9 +5288,7 @@ impl Editor {
                     selection.start,
                     selection.goal,
                     false,
-                    &font_cache,
-                    &text_layout_cache,
-                    &editor_style,
+                    &text_layout_details,
                 );
                 selection.collapse_to(cursor, goal);
             });
@@ -5320,9 +5317,7 @@ impl Editor {
             Autoscroll::fit()
         };
 
-        let font_cache = cx.font_cache().clone();
-        let text_layout = cx.text_layout_cache().clone();
-        let editor_style = self.style(cx);
+        let text_layout_details = TextLayoutDetails::new(&self, cx);
 
         self.change_selections(Some(autoscroll), cx, |s| {
             let line_mode = s.line_mode;
@@ -5336,9 +5331,7 @@ impl Editor {
                     row_count,
                     selection.goal,
                     false,
-                    &font_cache,
-                    &text_layout,
-                    &editor_style,
+                    &text_layout_details,
                 );
                 selection.collapse_to(cursor, goal);
             });
@@ -5346,20 +5339,10 @@ impl Editor {
     }
 
     pub fn select_up(&mut self, _: &SelectUp, cx: &mut ViewContext<Self>) {
-        let font_cache = cx.font_cache().clone();
-        let text_layout = cx.text_layout_cache().clone();
-        let editor_style = self.style(cx);
+        let text_layout_details = TextLayoutDetails::new(&self, cx);
         self.change_selections(Some(Autoscroll::fit()), cx, |s| {
             s.move_heads_with(|map, head, goal| {
-                movement::up(
-                    map,
-                    head,
-                    goal,
-                    false,
-                    &font_cache,
-                    &text_layout,
-                    &editor_style,
-                )
+                movement::up(map, head, goal, false, &text_layout_details)
             })
         })
     }

crates/editor/src/movement.rs 🔗

@@ -1,8 +1,8 @@
 use super::{Bias, DisplayPoint, DisplaySnapshot, SelectionGoal, ToDisplayPoint};
-use crate::{char_kind, CharKind, EditorStyle, ToOffset, ToPoint};
-use gpui::{FontCache, TextLayoutCache, WindowContext};
+use crate::{char_kind, CharKind, Editor, EditorStyle, ToOffset, ToPoint};
+use gpui::{text_layout, FontCache, TextLayoutCache, WindowContext};
 use language::Point;
-use std::ops::Range;
+use std::{ops::Range, sync::Arc};
 
 #[derive(Debug, PartialEq)]
 pub enum FindRange {
@@ -10,6 +10,24 @@ pub enum FindRange {
     MultiLine,
 }
 
+/// TextLayoutDetails encompasses everything we need to move vertically
+/// taking into account variable width characters.
+pub struct TextLayoutDetails {
+    pub font_cache: Arc<FontCache>,
+    pub text_layout_cache: Arc<TextLayoutCache>,
+    pub editor_style: EditorStyle,
+}
+
+impl TextLayoutDetails {
+    pub fn new(editor: &Editor, cx: &WindowContext) -> TextLayoutDetails {
+        TextLayoutDetails {
+            font_cache: cx.font_cache().clone(),
+            text_layout_cache: cx.text_layout_cache().clone(),
+            editor_style: editor.style(cx),
+        }
+    }
+}
+
 pub fn left(map: &DisplaySnapshot, mut point: DisplayPoint) -> DisplayPoint {
     if point.column() > 0 {
         *point.column_mut() -= 1;
@@ -48,9 +66,7 @@ pub fn up(
     start: DisplayPoint,
     goal: SelectionGoal,
     preserve_column_at_start: bool,
-    font_cache: &FontCache,
-    text_layout_cache: &TextLayoutCache,
-    editor_style: &EditorStyle,
+    text_layout_details: &TextLayoutDetails,
 ) -> (DisplayPoint, SelectionGoal) {
     up_by_rows(
         map,
@@ -58,9 +74,7 @@ pub fn up(
         1,
         goal,
         preserve_column_at_start,
-        font_cache,
-        text_layout_cache,
-        editor_style,
+        text_layout_details,
     )
 }
 
@@ -79,14 +93,12 @@ pub fn up_by_rows(
     row_count: u32,
     goal: SelectionGoal,
     preserve_column_at_start: bool,
-    font_cache: &FontCache,
-    text_layout_cache: &TextLayoutCache,
-    editor_style: &EditorStyle,
+    text_layout_details: &TextLayoutDetails,
 ) -> (DisplayPoint, SelectionGoal) {
     let mut goal_x = match goal {
         SelectionGoal::HorizontalPosition(x) => x,
         SelectionGoal::HorizontalRange { end, .. } => end,
-        _ => map.x_for_point(start, font_cache, text_layout_cache, editor_style),
+        _ => map.x_for_point(start, text_layout_details),
     };
 
     let prev_row = start.row().saturating_sub(row_count);
@@ -96,13 +108,7 @@ pub fn up_by_rows(
     );
     if point.row() < start.row() {
         *point.column_mut() = map
-            .column_for_x(
-                point.row(),
-                goal_x,
-                font_cache,
-                text_layout_cache,
-                editor_style,
-            )
+            .column_for_x(point.row(), goal_x, text_layout_details)
             .unwrap_or(point.column());
     } else if preserve_column_at_start {
         return (start, goal);