Removed DisplayRow abstraction

Mikayla Maki created

Change summary

crates/editor/src/display_map.rs | 125 +++++++++------------------------
crates/editor/src/editor.rs      |  45 +++++++-----
crates/editor/src/element.rs     |   4 
crates/vim/src/normal.rs         |   6 
4 files changed, 66 insertions(+), 114 deletions(-)

Detailed changes

crates/editor/src/display_map.rs 🔗

@@ -12,7 +12,6 @@ use gpui::{
     Entity, ModelContext, ModelHandle,
 };
 use language::{OffsetUtf16, Point, Subscription as BufferSubscription};
-use serde::{Deserialize, Serialize};
 use settings::Settings;
 use std::{any::TypeId, fmt::Debug, num::NonZeroU32, ops::Range, sync::Arc};
 use sum_tree::{Bias, TreeMap};
@@ -576,10 +575,10 @@ impl DisplaySnapshot {
         result
     }
 
-    pub fn line_indent(&self, display_row: DisplayRow) -> (u32, bool) {
+    pub fn line_indent(&self, display_row: u32) -> (u32, bool) {
         let mut indent = 0;
         let mut is_blank = true;
-        for (c, _) in self.chars_at(display_row.start()) {
+        for (c, _) in self.chars_at(DisplayPoint::new(display_row, 0)) {
             if c == ' ' {
                 indent += 1;
             } else {
@@ -599,8 +598,7 @@ impl DisplaySnapshot {
     }
 
     pub fn fold_for_line(self: &Self, display_row: u32) -> Option<FoldStatus> {
-        let display_row_typed = DisplayRow::new(display_row);
-        if self.is_foldable(display_row_typed) {
+        if self.is_foldable(display_row) {
             Some(FoldStatus::Foldable)
         } else if self.is_line_folded(display_row) {
             Some(FoldStatus::Folded)
@@ -609,40 +607,44 @@ impl DisplaySnapshot {
         }
     }
 
-    pub fn is_foldable(self: &Self, row: DisplayRow) -> bool {
-        if let Some(next_lines) = row.next_rows(self) {
-            let (start_indent, is_blank) = self.line_indent(row);
-            if is_blank {
-                return false;
-            }
+    pub fn is_foldable(self: &Self, row: u32) -> bool {
+        let max_point = self.max_point();
+        if row >= max_point.row() {
+            return false;
+        }
 
-            for display_row in next_lines {
-                let (indent, is_blank) = self.line_indent(display_row);
-                if !is_blank {
-                    return indent > start_indent;
-                }
+        let (start_indent, is_blank) = self.line_indent(row);
+        if is_blank {
+            return false;
+        }
+
+        for display_row in next_rows(row, self) {
+            let (indent, is_blank) = self.line_indent(display_row);
+            if !is_blank {
+                return indent > start_indent;
             }
         }
+
         return false;
     }
 
-    pub fn foldable_range(self: &Self, row: DisplayRow) -> Option<Range<DisplayPoint>> {
-        let start = row.end(&self);
+    pub fn foldable_range(self: &Self, row: u32) -> Option<Range<DisplayPoint>> {
+        let start = DisplayPoint::new(row, self.line_len(row));
 
         if self.is_foldable(row) && !self.is_line_folded(start.row()) {
             let (start_indent, _) = self.line_indent(row);
             let max_point = self.max_point();
             let mut end = None;
 
-            for row in row.next_rows(self).unwrap() {
+            for row in next_rows(row, self) {
                 let (indent, is_blank) = self.line_indent(row);
                 if !is_blank && indent <= start_indent {
-                    end = row.previous_row();
+                    end = Some(DisplayPoint::new(row - 1, self.line_len(row - 1)));
                     break;
                 }
             }
 
-            let end = end.map(|end_row| end_row.end(self)).unwrap_or(max_point);
+            let end = end.unwrap_or(max_point);
             Some(start..end)
         } else {
             None
@@ -736,79 +738,22 @@ impl ToDisplayPoint for Anchor {
     }
 }
 
-#[derive(Copy, Clone, Default, Eq, Ord, PartialOrd, PartialEq, Deserialize, Serialize)]
-#[repr(transparent)]
-pub struct DisplayRow(pub u32);
-
-// TODO: Move display_map check into new, and then remove it from everywhere else
-impl DisplayRow {
-    pub fn new(display_row: u32) -> Self {
-        DisplayRow(display_row)
-    }
-
-    pub fn to_points(&self, display_map: &DisplaySnapshot) -> Range<DisplayPoint> {
-        self.start()..self.end(&display_map)
-    }
-
-    pub fn previous_row(&self) -> Option<DisplayRow> {
-        self.0.checked_sub(1).map(|prev_row| DisplayRow(prev_row))
-    }
-
-    pub fn next_row(&self, display_map: &DisplaySnapshot) -> Option<DisplayRow> {
-        if self.0 + 1 > display_map.max_point().row() {
-            None
+pub fn next_rows(display_row: u32, display_map: &DisplaySnapshot) -> impl Iterator<Item = u32> {
+    let max_row = display_map.max_point().row();
+    let start_row = display_row + 1;
+    let mut current = None;
+    std::iter::from_fn(move || {
+        if current == None {
+            current = Some(start_row);
         } else {
-            Some(DisplayRow(self.0 + 1))
+            current = Some(current.unwrap() + 1)
         }
-    }
-
-    pub fn next_rows(
-        &self,
-        display_map: &DisplaySnapshot,
-    ) -> Option<impl Iterator<Item = DisplayRow>> {
-        self.next_row(display_map)
-            .and_then(|next_row| next_row.span_to(display_map.max_point()))
-    }
-
-    pub fn span_to<I: Into<DisplayRow>>(
-        &self,
-        end_row: I,
-    ) -> Option<impl Iterator<Item = DisplayRow>> {
-        let end_row = end_row.into();
-        if self.0 <= end_row.0 {
-            let start = *self;
-            let mut current = None;
-
-            Some(std::iter::from_fn(move || {
-                if current == None {
-                    current = Some(start);
-                } else {
-                    current = Some(DisplayRow(current.unwrap().0 + 1))
-                }
-                if current.unwrap().0 > end_row.0 {
-                    None
-                } else {
-                    current
-                }
-            }))
-        } else {
+        if current.unwrap() > max_row {
             None
+        } else {
+            current
         }
-    }
-
-    pub fn start(&self) -> DisplayPoint {
-        DisplayPoint::new(self.0, 0)
-    }
-
-    pub fn end(&self, display_map: &DisplaySnapshot) -> DisplayPoint {
-        DisplayPoint::new(self.0, display_map.line_len(self.0))
-    }
-}
-
-impl From<DisplayPoint> for DisplayRow {
-    fn from(value: DisplayPoint) -> Self {
-        DisplayRow(value.row())
-    }
+    })
 }
 
 #[cfg(test)]

crates/editor/src/editor.rs 🔗

@@ -164,12 +164,12 @@ pub struct ToggleComments {
 
 #[derive(Clone, Default, Deserialize, PartialEq)]
 pub struct FoldAt {
-    pub display_row: DisplayRow,
+    pub display_row: u32,
 }
 
 #[derive(Clone, Default, Deserialize, PartialEq)]
 pub struct UnfoldAt {
-    pub display_row: DisplayRow,
+    pub display_row: u32,
 }
 
 #[derive(Clone, Default, Deserialize, PartialEq)]
@@ -2753,10 +2753,10 @@ impl Editor {
                                 move |_, cx| {
                                     cx.dispatch_any_action(match fold_status {
                                         FoldStatus::Folded => Box::new(UnfoldAt {
-                                            display_row: DisplayRow::new(fold_location),
+                                            display_row: fold_location,
                                         }),
                                         FoldStatus::Foldable => Box::new(FoldAt {
-                                            display_row: DisplayRow::new(fold_location),
+                                            display_row: fold_location,
                                         }),
                                     });
                                 }
@@ -5755,11 +5755,9 @@ impl Editor {
             let buffer_start_row = range.start.to_point(&display_map).row;
 
             for row in (0..=range.end.row()).rev() {
-                let fold_range = display_map
-                    .foldable_range(DisplayRow::new(row))
-                    .map(|range| {
-                        range.start.to_point(&display_map)..range.end.to_point(&display_map)
-                    });
+                let fold_range = display_map.foldable_range(row).map(|range| {
+                    range.start.to_point(&display_map)..range.end.to_point(&display_map)
+                });
 
                 if let Some(fold_range) = fold_range {
                     if fold_range.end.row >= buffer_start_row {
@@ -5809,23 +5807,32 @@ impl Editor {
                 start..end
             })
             .collect::<Vec<_>>();
+
         self.unfold_ranges(ranges, true, true, cx);
     }
 
-    pub fn unfold_at(&mut self, fold_at: &UnfoldAt, cx: &mut ViewContext<Self>) {
+    pub fn unfold_at(&mut self, unfold_at: &UnfoldAt, cx: &mut ViewContext<Self>) {
         let display_map = self.display_map.update(cx, |map, cx| map.snapshot(cx));
 
-        let unfold_range = fold_at.display_row.to_points(&display_map);
+        let intersection_range = DisplayPoint::new(unfold_at.display_row, 0)
+            ..DisplayPoint::new(
+                unfold_at.display_row,
+                display_map.line_len(unfold_at.display_row),
+            );
 
-        let autoscroll = self
-            .selections
-            .all::<Point>(cx)
-            .iter()
-            .any(|selection| unfold_range.overlaps(&selection.display_range(&display_map)));
+        let autoscroll =
+            self.selections.all::<Point>(cx).iter().any(|selection| {
+                intersection_range.overlaps(&selection.display_range(&display_map))
+            });
+
+        let display_point = DisplayPoint::new(unfold_at.display_row, 0).to_point(&display_map);
+
+        let mut point_range = display_point..display_point;
+
+        point_range.start.column = 0;
+        point_range.end.column = display_map.buffer_snapshot.line_len(point_range.end.row);
 
-        let unfold_range =
-            unfold_range.start.to_point(&display_map)..unfold_range.end.to_point(&display_map);
-        self.unfold_ranges(std::iter::once(unfold_range), true, autoscroll, cx)
+        self.unfold_ranges(std::iter::once(point_range), true, autoscroll, cx)
     }
 
     pub fn fold_selected_ranges(&mut self, _: &FoldSelectedRanges, cx: &mut ViewContext<Self>) {

crates/editor/src/element.rs 🔗

@@ -4,7 +4,7 @@ use super::{
     ToPoint, MAX_LINE_LEN,
 };
 use crate::{
-    display_map::{BlockStyle, DisplayRow, DisplaySnapshot, FoldStatus, TransformBlock},
+    display_map::{BlockStyle, DisplaySnapshot, FoldStatus, TransformBlock},
     git::{diff_hunk_to_display, DisplayDiffHunk},
     hover_popover::{
         HideHover, HoverAt, HOVER_POPOVER_GAP, MIN_POPOVER_CHARACTER_WIDTH, MIN_POPOVER_LINE_HEIGHT,
@@ -2399,7 +2399,7 @@ impl ClickRange for FoldMarker {
         cx: &mut EventContext,
     ) {
         cx.dispatch_action(UnfoldAt {
-            display_row: DisplayRow(range.start.row()),
+            display_row: range.start.row(),
         })
     }
 }

crates/vim/src/normal.rs 🔗

@@ -12,7 +12,7 @@ use crate::{
 };
 use collections::{HashMap, HashSet};
 use editor::{
-    display_map::{DisplayRow, ToDisplayPoint},
+    display_map::ToDisplayPoint,
     scroll::{autoscroll::Autoscroll, scroll_amount::ScrollAmount},
     Anchor, Bias, ClipboardSelection, DisplayPoint, Editor,
 };
@@ -195,7 +195,7 @@ fn insert_line_above(_: &mut Workspace, _: &InsertLineAbove, cx: &mut ViewContex
                     .map(|selection| selection.start.row())
                     .collect();
                 let edits = selection_start_rows.into_iter().map(|row| {
-                    let (indent, _) = map.line_indent(DisplayRow::new(row));
+                    let (indent, _) = map.line_indent(row);
                     let start_of_line = map
                         .clip_point(DisplayPoint::new(row, 0), Bias::Left)
                         .to_point(&map);
@@ -229,7 +229,7 @@ fn insert_line_below(_: &mut Workspace, _: &InsertLineBelow, cx: &mut ViewContex
                     .map(|selection| selection.end.row())
                     .collect();
                 let edits = selection_end_rows.into_iter().map(|row| {
-                    let (indent, _) = map.line_indent(DisplayRow::new(row));
+                    let (indent, _) = map.line_indent(row);
                     let end_of_line = map
                         .clip_point(DisplayPoint::new(row, map.line_len(row)), Bias::Left)
                         .to_point(&map);