Clip find preceding boundary (#2689)

Kirill Bulatov created 2 years ago

Fixes inability to do `alt + left arrow` when an inlay with `Bias::Left`
is right to the left of the caret.

Release Notes:

- N/A

Change summary

crates/editor/src/movement.rs | 88 ++++++++++++++++++++++++++++++++++--
1 file changed, 83 insertions(+), 5 deletions(-)

Detailed changes

crates/editor/src/movement.rs

@@ -263,13 +263,13 @@ pub fn find_preceding_boundary(
 
         if let Some((prev_ch, prev_point)) = prev {
             if is_boundary(ch, prev_ch) {
-                return prev_point;
+                return map.clip_point(prev_point, Bias::Left);
             }
         }
 
         prev = Some((ch, point));
     }
-    DisplayPoint::zero()
+    map.clip_point(DisplayPoint::zero(), Bias::Left)
 }
 
 /// Scans for a boundary preceding the given start point `from` until a boundary is found, indicated by the
@@ -292,7 +292,7 @@ pub fn find_preceding_boundary_in_line(
     for (ch, point) in map.reverse_chars_at(from) {
         if let Some((prev_ch, prev_point)) = prev {
             if is_boundary(ch, prev_ch) {
-                return prev_point;
+                return map.clip_point(prev_point, Bias::Left);
             }
         }
 
@@ -303,7 +303,7 @@ pub fn find_preceding_boundary_in_line(
         prev = Some((ch, point));
     }
 
-    prev.map(|(_, point)| point).unwrap_or(from)
+    map.clip_point(prev.map(|(_, point)| point).unwrap_or(from), Bias::Left)
 }
 
 /// Scans for a boundary following the given start point until a boundary is found, indicated by the
@@ -406,8 +406,12 @@ pub fn split_display_range_by_lines(
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::{test::marked_display_snapshot, Buffer, DisplayMap, ExcerptRange, MultiBuffer};
+    use crate::{
+        display_map::Inlay, test::marked_display_snapshot, Buffer, DisplayMap, ExcerptRange,
+        InlayId, MultiBuffer,
+    };
     use settings::SettingsStore;
+    use util::post_inc;
 
     #[gpui::test]
     fn test_previous_word_start(cx: &mut gpui::AppContext) {
@@ -505,6 +509,80 @@ mod tests {
         });
     }
 
+    #[gpui::test]
+    fn test_find_preceding_boundary_with_inlays(cx: &mut gpui::AppContext) {
+        init_test(cx);
+
+        let input_text = "abcdefghijklmnopqrstuvwxys";
+        let family_id = cx
+            .font_cache()
+            .load_family(&["Helvetica"], &Default::default())
+            .unwrap();
+        let font_id = cx
+            .font_cache()
+            .select_font(family_id, &Default::default())
+            .unwrap();
+        let font_size = 14.0;
+        let buffer = MultiBuffer::build_simple(input_text, cx);
+        let buffer_snapshot = buffer.read(cx).snapshot(cx);
+        let display_map =
+            cx.add_model(|cx| DisplayMap::new(buffer, font_id, font_size, None, 1, 1, cx));
+
+        // add all kinds of inlays between two word boundaries: we should be able to cross them all, when looking for another boundary
+        let mut id = 0;
+        let inlays = (0..buffer_snapshot.len())
+            .map(|offset| {
+                [
+                    Inlay {
+                        id: InlayId::Suggestion(post_inc(&mut id)),
+                        position: buffer_snapshot.anchor_at(offset, Bias::Left),
+                        text: format!("test").into(),
+                    },
+                    Inlay {
+                        id: InlayId::Suggestion(post_inc(&mut id)),
+                        position: buffer_snapshot.anchor_at(offset, Bias::Right),
+                        text: format!("test").into(),
+                    },
+                    Inlay {
+                        id: InlayId::Hint(post_inc(&mut id)),
+                        position: buffer_snapshot.anchor_at(offset, Bias::Left),
+                        text: format!("test").into(),
+                    },
+                    Inlay {
+                        id: InlayId::Hint(post_inc(&mut id)),
+                        position: buffer_snapshot.anchor_at(offset, Bias::Right),
+                        text: format!("test").into(),
+                    },
+                ]
+            })
+            .flatten()
+            .collect();
+        let snapshot = display_map.update(cx, |map, cx| {
+            map.splice_inlays(Vec::new(), inlays, cx);
+            map.snapshot(cx)
+        });
+
+        assert_eq!(
+            find_preceding_boundary(
+                &snapshot,
+                buffer_snapshot.len().to_display_point(&snapshot),
+                |left, _| left == 'a',
+            ),
+            0.to_display_point(&snapshot),
+            "Should not stop at inlays when looking for boundaries"
+        );
+
+        assert_eq!(
+            find_preceding_boundary_in_line(
+                &snapshot,
+                buffer_snapshot.len().to_display_point(&snapshot),
+                |left, _| left == 'a',
+            ),
+            0.to_display_point(&snapshot),
+            "Should not stop at inlays when looking for boundaries in line"
+        );
+    }
+
     #[gpui::test]
     fn test_next_word_end(cx: &mut gpui::AppContext) {
         init_test(cx);