Get cursor movement working with byte columns

Introduce `clip_point` and `clip_offset` methods on `Rope`, `Buffer`, and `FoldMap`.

Co-Authored-By: Antonio Scandurra <me@as-cii.com>

zed/src/editor/buffer/mod.rs

@@ -11,6 +11,7 @@ pub use selection::*;
 use similar::{ChangeTag, TextDiff};
 
 use crate::{
+    editor::Bias,
     operation_queue::{self, OperationQueue},
     sum_tree::{self, FilterCursor, SeekBias, SumTree},
     time::{self, ReplicaId},
@@ -1865,12 +1866,12 @@ impl Buffer {
         }
     }
 
-    pub fn next_char_boundary(&self, offset: usize) -> usize {
-        self.visible_text.next_char_boundary(offset)
+    pub fn clip_point(&self, point: Point, bias: Bias) -> Point {
+        self.visible_text.clip_point(point, bias)
     }
 
-    pub fn prev_char_boundary(&self, offset: usize) -> usize {
-        self.visible_text.prev_char_boundary(offset)
+    pub fn clip_offset(&self, offset: usize, bias: Bias) -> usize {
+        self.visible_text.clip_offset(offset, bias)
     }
 }
 
@@ -3232,8 +3233,8 @@ mod tests {
 
     impl Buffer {
         fn random_byte_range(&mut self, start_offset: usize, rng: &mut impl Rng) -> Range<usize> {
-            let end = self.next_char_boundary(rng.gen_range(start_offset..=self.len()));
-            let start = self.prev_char_boundary(rng.gen_range(start_offset..=end));
+            let end = self.clip_offset(rng.gen_range(start_offset..=self.len()), Bias::Right);
+            let start = self.clip_offset(rng.gen_range(start_offset..=end), Bias::Left);
             start..end
         }
 

zed/src/editor/buffer/rope.rs

@@ -1,5 +1,8 @@
 use super::Point;
-use crate::sum_tree::{self, SeekBias, SumTree};
+use crate::{
+    editor::Bias,
+    sum_tree::{self, SeekBias, SumTree},
+};
 use arrayvec::ArrayString;
 use smallvec::SmallVec;
 use std::{cmp, ops::Range, str};
@@ -142,32 +145,38 @@ impl Rope {
         cursor.start().bytes + cursor.item().map_or(0, |chunk| chunk.to_offset(overshoot))
     }
 
-    pub fn next_char_boundary(&self, mut offset: usize) -> usize {
-        assert!(offset <= self.summary().bytes);
+    pub fn clip_offset(&self, mut offset: usize, bias: Bias) -> usize {
         let mut cursor = self.chunks.cursor::<usize, usize>();
         cursor.seek(&offset, SeekBias::Left, &());
         if let Some(chunk) = cursor.item() {
             let mut ix = offset - cursor.start();
             while !chunk.0.is_char_boundary(ix) {
-                ix += 1;
-                offset += 1;
+                match bias {
+                    Bias::Left => {
+                        ix -= 1;
+                        offset -= 1;
+                    }
+                    Bias::Right => {
+                        ix += 1;
+                        offset += 1;
+                    }
+                }
             }
+            offset
+        } else {
+            self.summary().bytes
         }
-        offset
     }
 
-    pub fn prev_char_boundary(&self, mut offset: usize) -> usize {
-        assert!(offset <= self.summary().bytes);
-        let mut cursor = self.chunks.cursor::<usize, usize>();
-        cursor.seek(&offset, SeekBias::Left, &());
+    pub fn clip_point(&self, point: Point, bias: Bias) -> Point {
+        let mut cursor = self.chunks.cursor::<Point, Point>();
+        cursor.seek(&point, SeekBias::Right, &());
         if let Some(chunk) = cursor.item() {
-            let mut ix = offset - cursor.start();
-            while !chunk.0.is_char_boundary(ix) {
-                ix -= 1;
-                offset -= 1;
-            }
+            let overshoot = point - cursor.start();
+            *cursor.start() + chunk.clip_point(overshoot, bias)
+        } else {
+            self.summary().lines
         }
-        offset
     }
 }
 
@@ -351,6 +360,22 @@ impl Chunk {
         }
         offset
     }
+
+    fn clip_point(&self, target: Point, bias: Bias) -> Point {
+        for (row, line) in self.0.split('\n').enumerate() {
+            if row == target.row as usize {
+                let mut column = target.column.min(line.len() as u32);
+                while !line.is_char_boundary(column as usize) {
+                    match bias {
+                        Bias::Left => column -= 1,
+                        Bias::Right => column += 1,
+                    }
+                }
+                return Point::new(row as u32, column);
+            }
+        }
+        unreachable!()
+    }
 }
 
 impl sum_tree::Item for Chunk {
@@ -466,30 +491,13 @@ fn find_split_ix(text: &str) -> usize {
     ix
 }
 
-#[inline(always)]
-fn assert_char_boundary(s: &str, ix: usize) {
-    if !s.is_char_boundary(ix) {
-        let mut char_start = ix;
-        while !s.is_char_boundary(char_start) {
-            char_start -= 1;
-        }
-
-        let ch = s[char_start..].chars().next().unwrap();
-        let char_range = char_start..char_start + ch.len_utf8();
-        panic!(
-            "byte index {} is not a char boundary; it is inside {:?} (bytes {:?}) of `{}`",
-            ix, ch, char_range, s
-        );
-    }
-}
-
 #[cfg(test)]
 mod tests {
-    use crate::util::RandomCharIter;
-
     use super::*;
+    use crate::util::RandomCharIter;
     use rand::prelude::*;
     use std::env;
+    use Bias::{Left, Right};
 
     #[test]
     fn test_all_4_byte_chars() {
@@ -520,8 +528,8 @@ mod tests {
             let mut expected = String::new();
             let mut actual = Rope::new();
             for _ in 0..operations {
-                let end_ix = actual.next_char_boundary(rng.gen_range(0..=expected.len()));
-                let start_ix = actual.prev_char_boundary(rng.gen_range(0..=end_ix));
+                let end_ix = clip_offset(&expected, rng.gen_range(0..=expected.len()), Right);
+                let start_ix = clip_offset(&expected, rng.gen_range(0..=end_ix), Left);
                 let len = rng.gen_range(0..=64);
                 let new_text: String = RandomCharIter::new(&mut rng).take(len).collect();
 
@@ -539,8 +547,8 @@ mod tests {
                 log::info!("text: {:?}", expected);
 
                 for _ in 0..5 {
-                    let end_ix = actual.next_char_boundary(rng.gen_range(0..=expected.len()));
-                    let start_ix = actual.prev_char_boundary(rng.gen_range(0..=end_ix));
+                    let end_ix = clip_offset(&expected, rng.gen_range(0..=expected.len()), Right);
+                    let start_ix = clip_offset(&expected, rng.gen_range(0..=end_ix), Left);
                     assert_eq!(
                         actual.chunks_in_range(start_ix..end_ix).collect::<String>(),
                         &expected[start_ix..end_ix]
@@ -560,8 +568,8 @@ mod tests {
                 }
 
                 for _ in 0..5 {
-                    let end_ix = actual.next_char_boundary(rng.gen_range(0..=expected.len()));
-                    let start_ix = actual.prev_char_boundary(rng.gen_range(0..=end_ix));
+                    let end_ix = clip_offset(&expected, rng.gen_range(0..=expected.len()), Right);
+                    let start_ix = clip_offset(&expected, rng.gen_range(0..=end_ix), Left);
                     assert_eq!(
                         actual.cursor(start_ix).summary(end_ix),
                         TextSummary::from(&expected[start_ix..end_ix])
@@ -571,6 +579,16 @@ mod tests {
         }
     }
 
+    fn clip_offset(text: &str, mut offset: usize, bias: Bias) -> usize {
+        while !text.is_char_boundary(offset) {
+            match bias {
+                Bias::Left => offset -= 1,
+                Bias::Right => offset += 1,
+            }
+        }
+        offset
+    }
+
     impl Rope {
         fn text(&self) -> String {
             let mut text = String::new();

zed/src/editor/buffer/selection.rs

@@ -1,8 +1,8 @@
 use crate::{
     editor::{
         buffer::{Anchor, Buffer, Point, ToPoint},
-        display_map::{Bias, DisplayMap},
-        DisplayPoint,
+        display_map::DisplayMap,
+        Bias, DisplayPoint,
     },
     time,
 };

zed/src/editor/buffer_view.rs

@@ -650,16 +650,16 @@ impl BufferView {
             if let Err(error) = buffer.edit(edit_ranges, text.as_str(), Some(ctx)) {
                 log::error!("error inserting text: {}", error);
             };
-            let char_count = text.chars().count() as isize;
+            let text_len = text.len() as isize;
             let mut delta = 0_isize;
             new_selections = old_selections
                 .into_iter()
                 .map(|(id, range)| {
                     let start = range.start as isize;
                     let end = range.end as isize;
-                    let anchor = buffer.anchor_before((start + delta + char_count) as usize);
+                    let anchor = buffer.anchor_before((start + delta + text_len) as usize);
                     let deleted_count = end - start;
-                    delta += char_count - deleted_count;
+                    delta += text_len - deleted_count;
                     Selection {
                         id,
                         start: anchor.clone(),
@@ -2358,11 +2358,7 @@ impl workspace::ItemView for BufferView {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::{
-        editor::Point,
-        settings,
-        test::{multibyte_sample_text, sample_text},
-    };
+    use crate::{editor::Point, settings, test::sample_text};
     use unindent::Unindent;
 
     #[gpui::test]
@@ -2667,6 +2663,11 @@ mod tests {
         });
 
         view.update(app, |view, ctx| {
+            assert_eq!(
+                view.selection_ranges(ctx.as_ref()),
+                &[DisplayPoint::new(0, 0)..DisplayPoint::new(0, 0)]
+            );
+
             view.move_down(&(), ctx);
             assert_eq!(
                 view.selection_ranges(ctx.as_ref()),
@@ -2728,6 +2729,11 @@ mod tests {
         assert_eq!('ⓐ'.len_utf8(), 3);
         assert_eq!('α'.len_utf8(), 2);
 
+        fn empty_range(row: usize, column: usize) -> Range<DisplayPoint> {
+            let point = DisplayPoint::new(row as u32, column as u32);
+            point..point
+        }
+
         view.update(app, |view, ctx| {
             view.fold_ranges(
                 vec![
@@ -2742,43 +2748,80 @@ mod tests {
             view.move_right(&(), ctx);
             assert_eq!(
                 view.selection_ranges(ctx.as_ref()),
-                &[DisplayPoint::new(0, 3)..DisplayPoint::new(0, 3)]
+                &[empty_range(0, "ⓐ".len())]
             );
-
-            view.move_down(&(), ctx);
-            assert_eq!(
-                view.selection_ranges(ctx.as_ref()),
-                &[DisplayPoint::new(1, 1)..DisplayPoint::new(1, 1)]
-            );
-
             view.move_right(&(), ctx);
             assert_eq!(
                 view.selection_ranges(ctx.as_ref()),
-                &[DisplayPoint::new(1, 2)..DisplayPoint::new(1, 2)]
+                &[empty_range(0, "ⓐⓑ".len())]
+            );
+            view.move_right(&(), ctx);
+            assert_eq!(
+                view.selection_ranges(ctx.as_ref()),
+                &[empty_range(0, "ⓐⓑ…".len())]
             );
 
             view.move_down(&(), ctx);
             assert_eq!(
                 view.selection_ranges(ctx.as_ref()),
-                &[DisplayPoint::new(2, 4)..DisplayPoint::new(2, 4)]
+                &[empty_range(1, "ab…".len())]
             );
-
             view.move_left(&(), ctx);
             assert_eq!(
                 view.selection_ranges(ctx.as_ref()),
-                &[DisplayPoint::new(2, 2)..DisplayPoint::new(2, 2)]
+                &[empty_range(1, "ab".len())]
+            );
+            view.move_left(&(), ctx);
+            assert_eq!(
+                view.selection_ranges(ctx.as_ref()),
+                &[empty_range(1, "a".len())]
+            );
+
+            view.move_down(&(), ctx);
+            assert_eq!(
+                view.selection_ranges(ctx.as_ref()),
+                &[empty_range(2, "α".len())]
+            );
+            view.move_right(&(), ctx);
+            assert_eq!(
+                view.selection_ranges(ctx.as_ref()),
+                &[empty_range(2, "αβ".len())]
+            );
+            view.move_right(&(), ctx);
+            assert_eq!(
+                view.selection_ranges(ctx.as_ref()),
+                &[empty_range(2, "αβ…".len())]
+            );
+            view.move_right(&(), ctx);
+            assert_eq!(
+                view.selection_ranges(ctx.as_ref()),
+                &[empty_range(2, "αβ…ε".len())]
             );
 
             view.move_up(&(), ctx);
             assert_eq!(
                 view.selection_ranges(ctx.as_ref()),
-                &[DisplayPoint::new(1, 1)..DisplayPoint::new(1, 1)]
+                &[empty_range(1, "ab…e".len())]
             );
-
             view.move_up(&(), ctx);
             assert_eq!(
                 view.selection_ranges(ctx.as_ref()),
-                &[DisplayPoint::new(0, 3)..DisplayPoint::new(0, 3)]
+                &[empty_range(0, "ⓐⓑ…ⓔ".len())]
+            );
+            view.move_left(&(), ctx);
+            assert_eq!(
+                view.selection_ranges(ctx.as_ref()),
+                &[empty_range(0, "ⓐⓑ…".len())]
+            );
+            view.move_left(&(), ctx);
+            assert_eq!(
+                view.selection_ranges(ctx.as_ref()),
+                &[empty_range(0, "ⓐⓑ".len())]
+            );
+            view.move_left(&(), ctx);
+            assert_eq!(
+                view.selection_ranges(ctx.as_ref()),
+                &[empty_range(0, "ⓐ".len())]
             );
         });
     }

zed/src/editor/display_map/fold_map.rs

@@ -1,6 +1,6 @@
 use super::{
     buffer::{AnchorRangeExt, TextSummary},
-    Anchor, Buffer, DisplayPoint, Edit, Point, ToOffset,
+    Anchor, Bias, Buffer, DisplayPoint, Edit, Point, ToOffset,
 };
 use crate::{
     editor::buffer,
@@ -233,26 +233,6 @@ impl FoldMap {
         ))
     }
 
-    #[cfg(test)]
-    pub fn prev_char_boundary(&self, offset: DisplayOffset, ctx: &AppContext) -> DisplayOffset {
-        let transforms = self.sync(ctx);
-        let mut cursor = transforms.cursor::<DisplayOffset, TransformSummary>();
-        cursor.seek(&offset, SeekBias::Right, &());
-        if let Some(transform) = cursor.item() {
-            if transform.display_text.is_some() {
-                DisplayOffset(cursor.start().display.bytes)
-            } else {
-                let overshoot = offset.0 - cursor.start().display.bytes;
-                let buffer_offset = cursor.start().buffer.bytes + overshoot;
-                let buffer_boundary_offset =
-                    self.buffer.read(ctx).prev_char_boundary(buffer_offset);
-                DisplayOffset(offset.0 - (buffer_offset - buffer_boundary_offset))
-            }
-        } else {
-            offset
-        }
-    }
-
     fn sync(&self, ctx: &AppContext) -> MutexGuard<SumTree<Transform>> {
         let buffer = self.buffer.read(ctx);
         let mut edits = buffer.edits_since(self.last_sync.lock().clone()).peekable();
@@ -471,6 +451,64 @@ impl FoldMapSnapshot {
         let overshoot = point.0 - cursor.start().display.lines;
         (cursor.start().buffer.lines + overshoot).to_offset(self.buffer.read(ctx))
     }
+
+    #[cfg(test)]
+    pub fn clip_offset(
+        &self,
+        offset: DisplayOffset,
+        bias: Bias,
+        ctx: &AppContext,
+    ) -> DisplayOffset {
+        let mut cursor = self.transforms.cursor::<DisplayOffset, TransformSummary>();
+        cursor.seek(&offset, SeekBias::Right, &());
+        if let Some(transform) = cursor.item() {
+            let transform_start = cursor.start().display.bytes;
+            if transform.display_text.is_some() {
+                if offset.0 == transform_start || matches!(bias, Bias::Left) {
+                    DisplayOffset(transform_start)
+                } else {
+                    DisplayOffset(cursor.end().display.bytes)
+                }
+            } else {
+                let overshoot = offset.0 - transform_start;
+                let buffer_offset = cursor.start().buffer.bytes + overshoot;
+                let clipped_buffer_offset = self.buffer.read(ctx).clip_offset(buffer_offset, bias);
+                DisplayOffset(
+                    (offset.0 as isize + (clipped_buffer_offset as isize - buffer_offset as isize))
+                        as usize,
+                )
+            }
+        } else {
+            DisplayOffset(self.transforms.summary().display.bytes)
+        }
+    }
+
+    pub fn clip_point(&self, point: DisplayPoint, bias: Bias, ctx: &AppContext) -> DisplayPoint {
+        let mut cursor = self.transforms.cursor::<DisplayPoint, TransformSummary>();
+        cursor.seek(&point, SeekBias::Right, &());
+        if let Some(transform) = cursor.item() {
+            let transform_start = cursor.start().display.lines;
+            if transform.display_text.is_some() {
+                if point.0 == transform_start || matches!(bias, Bias::Left) {
+                    DisplayPoint(transform_start)
+                } else {
+                    DisplayPoint(cursor.end().display.lines)
+                }
+            } else {
+                let overshoot = point.0 - transform_start;
+                let buffer_position = cursor.start().buffer.lines + overshoot;
+                let clipped_buffer_position =
+                    self.buffer.read(ctx).clip_point(buffer_position, bias);
+                DisplayPoint::new(
+                    point.row(),
+                    ((point.column() as i32) + clipped_buffer_position.column as i32
+                        - buffer_position.column as i32) as u32,
+                )
+            }
+        } else {
+            DisplayPoint(self.transforms.summary().display.lines)
+        }
+    }
 }
 
 #[derive(Clone, Debug, Default, Eq, PartialEq)]
@@ -864,6 +902,7 @@ mod tests {
         use crate::util::RandomCharIter;
         use rand::prelude::*;
         use std::env;
+        use Bias::{Left, Right};
 
         let iterations = env::var("ITERATIONS")
             .map(|i| i.parse().expect("invalid `ITERATIONS` variable"))
@@ -896,8 +935,8 @@ mod tests {
                         let buffer = buffer.read(app);
                         let mut to_fold = Vec::new();
                         for _ in 0..rng.gen_range(1..=5) {
-                            let end = buffer.next_char_boundary(rng.gen_range(0..=buffer.len()));
-                            let start = buffer.prev_char_boundary(rng.gen_range(0..=end));
+                            let end = buffer.clip_offset(rng.gen_range(0..=buffer.len()), Right);
+                            let start = buffer.clip_offset(rng.gen_range(0..=end), Left);
                             to_fold.push(start..end);
                         }
                         log::info!("folding {:?}", to_fold);
@@ -907,8 +946,8 @@ mod tests {
                         let buffer = buffer.read(app);
                         let mut to_unfold = Vec::new();
                         for _ in 0..rng.gen_range(1..=3) {
-                            let end = buffer.next_char_boundary(rng.gen_range(0..=buffer.len()));
-                            let start = buffer.prev_char_boundary(rng.gen_range(0..=end));
+                            let end = buffer.clip_offset(rng.gen_range(0..=buffer.len()), Right);
+                            let start = buffer.clip_offset(rng.gen_range(0..=end), Left);
                             to_unfold.push(start..end);
                         }
                         log::info!("unfolding {:?}", to_unfold);
@@ -989,8 +1028,9 @@ mod tests {
                 }
 
                 for _ in 0..5 {
-                    let offset = map.prev_char_boundary(
+                    let offset = map.snapshot(app.as_ref()).clip_offset(
                         DisplayOffset(rng.gen_range(0..=map.len(app.as_ref()))),
+                        Bias::Right,
                         app.as_ref(),
                     );
                     assert_eq!(
@@ -1020,8 +1060,8 @@ mod tests {
                 }
 
                 for _ in 0..5 {
-                    let end = buffer.next_char_boundary(rng.gen_range(0..=buffer.len()));
-                    let start = buffer.prev_char_boundary(rng.gen_range(0..=end));
+                    let end = buffer.clip_offset(rng.gen_range(0..=buffer.len()), Right);
+                    let start = buffer.clip_offset(rng.gen_range(0..=end), Left);
                     let expected_folds = map
                         .folds
                         .items()

zed/src/editor/display_map/mod.rs

@@ -1,17 +1,11 @@
 mod fold_map;
 
-use super::{buffer, Anchor, Buffer, Edit, Point, ToOffset, ToPoint};
+use super::{buffer, Anchor, Bias, Buffer, Edit, Point, ToOffset, ToPoint};
 pub use fold_map::BufferRows;
 use fold_map::{FoldMap, FoldMapSnapshot};
 use gpui::{AppContext, ModelHandle};
 use std::ops::Range;
 
-#[derive(Copy, Clone)]
-pub enum Bias {
-    Left,
-    Right,
-}
-
 pub struct DisplayMap {
     buffer: ModelHandle<Buffer>,
     fold_map: FoldMap,
@@ -110,6 +104,7 @@ impl DisplayMap {
             .column()
     }
 
+    // TODO - make this delegate to the DisplayMapSnapshot
     pub fn max_point(&self, ctx: &AppContext) -> DisplayPoint {
         self.fold_map.max_point(ctx).expand_tabs(self, ctx)
     }
@@ -168,11 +163,11 @@ impl DisplayMapSnapshot {
         let mut count = 0;
         let mut column = 0;
         for c in self.chars_at(DisplayPoint::new(display_row, 0), ctx) {
-            count += 1;
-            column += c.len_utf8() as u32;
             if column >= target {
                 break;
             }
+            count += 1;
+            column += c.len_utf8() as u32;
         }
         count
     }
@@ -181,15 +176,23 @@ impl DisplayMapSnapshot {
         let mut count = 0;
         let mut column = 0;
         for c in self.chars_at(DisplayPoint::new(display_row, 0), ctx) {
-            count += 1;
-            column += c.len_utf8() as u32;
             if count >= char_count {
                 break;
             }
+            count += 1;
+            column += c.len_utf8() as u32;
         }
         column
     }
 
+    pub fn clip_point(&self, point: DisplayPoint, bias: Bias, ctx: &AppContext) -> DisplayPoint {
+        self.expand_tabs(
+            self.folds_snapshot
+                .clip_point(self.collapse_tabs(point, bias, ctx).0, bias, ctx),
+            ctx,
+        )
+    }
+
     fn expand_tabs(&self, mut point: DisplayPoint, ctx: &AppContext) -> DisplayPoint {
         let chars = self
             .folds_snapshot
@@ -364,7 +367,7 @@ pub fn collapse_tabs(
     let mut expanded_chars = 0;
     let mut collapsed_bytes = 0;
     while let Some(c) = chars.next() {
-        if expanded_bytes == column {
+        if expanded_bytes >= column {
             break;
         }
 
@@ -383,19 +386,19 @@ pub fn collapse_tabs(
             expanded_chars += 1;
             expanded_bytes += c.len_utf8();
         }
-        collapsed_bytes += c.len_utf8();
 
-        if expanded_bytes > column {
-            panic!("column {} is inside of character {:?}", column, c);
+        if expanded_bytes > column && matches!(bias, Bias::Left) {
+            expanded_chars -= 1;
+            break;
         }
+
+        collapsed_bytes += c.len_utf8();
     }
     (collapsed_bytes, expanded_chars, 0)
 }
 
 #[cfg(test)]
 mod tests {
-    use gpui::MutableAppContext;
-
     use super::*;
     use crate::test::*;
 
@@ -446,25 +449,25 @@ mod tests {
     }
 
     #[gpui::test]
-    fn test_tabs_with_multibyte_chars(app: &mut MutableAppContext) {
-        let text = "✅\t\tx\nα\t\n🏀α\t\ty";
+    fn test_tabs_with_multibyte_chars(app: &mut gpui::MutableAppContext) {
+        let text = "✅\t\tα\nβ\t\n🏀β\t\tγ";
         let buffer = app.add_model(|ctx| Buffer::new(0, text, ctx));
         let ctx = app.as_ref();
         let map = DisplayMap::new(buffer.clone(), 4, ctx);
-        assert_eq!(map.text(ctx), "✅       x\nα   \n🏀α      y");
+        assert_eq!(map.text(ctx), "✅       α\nβ   \n🏀β      γ");
 
         let point = Point::new(0, "✅\t\t".len() as u32);
         let display_point = DisplayPoint::new(0, "✅       ".len() as u32);
         assert_eq!(point.to_display_point(&map, ctx), display_point);
         assert_eq!(display_point.to_buffer_point(&map, Bias::Left, ctx), point,);
 
-        let point = Point::new(1, "α\t".len() as u32);
-        let display_point = DisplayPoint::new(1, "α   ".len() as u32);
+        let point = Point::new(1, "β\t".len() as u32);
+        let display_point = DisplayPoint::new(1, "β   ".len() as u32);
         assert_eq!(point.to_display_point(&map, ctx), display_point);
         assert_eq!(display_point.to_buffer_point(&map, Bias::Left, ctx), point,);
 
-        let point = Point::new(2, "🏀α\t\t".len() as u32);
-        let display_point = DisplayPoint::new(2, "🏀α      ".len() as u32);
+        let point = Point::new(2, "🏀β\t\t".len() as u32);
+        let display_point = DisplayPoint::new(2, "🏀β      ".len() as u32);
         assert_eq!(point.to_display_point(&map, ctx), display_point);
         assert_eq!(display_point.to_buffer_point(&map, Bias::Left, ctx), point,);
 
@@ -481,7 +484,7 @@ mod tests {
             map.snapshot(ctx)
                 .chunks_at(DisplayPoint::new(0, "✅      ".len() as u32), ctx)
                 .collect::<String>(),
-            " x\nα   \n🏀α      y"
+            " α\nβ   \n🏀β      γ"
         );
         assert_eq!(
             DisplayPoint::new(0, "✅ ".len() as u32).to_buffer_point(&map, Bias::Right, ctx),
@@ -495,7 +498,25 @@ mod tests {
             map.snapshot(ctx)
                 .chunks_at(DisplayPoint::new(0, "✅ ".len() as u32), ctx)
                 .collect::<String>(),
-            "      x\nα   \n🏀α      y"
+            "      α\nβ   \n🏀β      γ"
+        );
+
+        // Clipping display points inside of multi-byte characters
+        assert_eq!(
+            map.snapshot(ctx).clip_point(
+                DisplayPoint::new(0, "✅".len() as u32 - 1),
+                Bias::Left,
+                ctx
+            ),
+            DisplayPoint::new(0, 0)
+        );
+        assert_eq!(
+            map.snapshot(ctx).clip_point(
+                DisplayPoint::new(0, "✅".len() as u32 - 1),
+                Bias::Right,
+                ctx
+            ),
+            DisplayPoint::new(0, "✅".len() as u32)
         );
     }
 

zed/src/editor/mod.rs

@@ -11,6 +11,12 @@ pub use display_map::DisplayPoint;
 use display_map::*;
 use std::{cmp, ops::Range};
 
+#[derive(Copy, Clone)]
+pub enum Bias {
+    Left,
+    Right,
+}
+
 trait RangeExt<T> {
     fn sorted(&self) -> Range<T>;
 }

zed/src/editor/movement.rs

@@ -1,7 +1,6 @@
-use super::{DisplayMap, DisplayPoint, SelectionGoal};
+use super::{Bias, DisplayMap, DisplayPoint, SelectionGoal};
 use anyhow::Result;
 use gpui::AppContext;
-use std::cmp;
 
 pub fn left(map: &DisplayMap, mut point: DisplayPoint, app: &AppContext) -> Result<DisplayPoint> {
     if point.column() > 0 {
@@ -10,23 +9,18 @@ pub fn left(map: &DisplayMap, mut point: DisplayPoint, app: &AppContext) -> Resu
         *point.row_mut() -= 1;
         *point.column_mut() = map.line_len(point.row(), app);
     }
-    Ok(point)
+    Ok(map.snapshot(app).clip_point(point, Bias::Left, app))
 }
 
 pub fn right(map: &DisplayMap, mut point: DisplayPoint, app: &AppContext) -> Result<DisplayPoint> {
     let max_column = map.line_len(point.row(), app);
     if point.column() < max_column {
-        *point.column_mut() += map
-            .snapshot(app)
-            .chars_at(point, app)
-            .next()
-            .unwrap()
-            .len_utf8() as u32;
+        *point.column_mut() += 1;
     } else if point.row() < map.max_point(app).row() {
         *point.row_mut() += 1;
         *point.column_mut() = 0;
     }
-    Ok(point)
+    Ok(map.snapshot(app).clip_point(point, Bias::Right, app))
 }
 
 pub fn up(
@@ -35,18 +29,16 @@ pub fn up(
     goal: SelectionGoal,
     app: &AppContext,
 ) -> Result<(DisplayPoint, SelectionGoal)> {
+    let map = map.snapshot(app);
     let goal_column = if let SelectionGoal::Column(column) = goal {
         column
     } else {
-        point.column()
+        map.column_to_chars(point.row(), point.column(), app)
     };
 
-    let map = map.snapshot(app);
-    let char_column = map.column_to_chars(point.row(), goal_column, app);
-
     if point.row() > 0 {
         *point.row_mut() -= 1;
-        *point.column_mut() = map.column_from_chars(point.row(), char_column, app);
+        *point.column_mut() = map.column_from_chars(point.row(), goal_column, app);
     } else {
         point = DisplayPoint::new(0, 0);
     }
@@ -60,19 +52,17 @@ pub fn down(
     goal: SelectionGoal,
     app: &AppContext,
 ) -> Result<(DisplayPoint, SelectionGoal)> {
+    let max_point = map.max_point(app);
+    let map = map.snapshot(app);
     let goal_column = if let SelectionGoal::Column(column) = goal {
         column
     } else {
-        point.column()
+        map.column_to_chars(point.row(), point.column(), app)
     };
 
-    let max_point = map.max_point(app);
-    let map = map.snapshot(app);
-    let char_column = map.column_to_chars(point.row(), goal_column, app);
-
     if point.row() < max_point.row() {
         *point.row_mut() += 1;
-        *point.column_mut() = map.column_from_chars(point.row(), char_column, app);
+        *point.column_mut() = map.column_from_chars(point.row(), goal_column, app);
     } else {
         point = max_point;
     }