Fix handling of editor lines (#389)

editor-core/src/buffer/rope_text.rs

@@ -55,14 +55,37 @@ pub trait RopeText {
         (line, col)
     }
 
+    /// Get the offset for a specific line and column.  
+    /// This should be preferred over simply adding the column to the line offset, because it
+    /// validates better and avoids returning newlines.
+    /// ```rust
+    /// # use floem_editor_core::xi_rope::Rope;
+    /// # use floem_editor_core::buffer::rope_text::{RopeText, RopeTextRef};
+    /// let text = Rope::from("hello\nworld");
+    /// let text = RopeTextRef::new(&text);
+    /// assert_eq!(text.offset_of_line_col(0, 0), 0);  // "h"
+    /// assert_eq!(text.offset_of_line_col(0, 4), 4);  // "o"
+    /// assert_eq!(text.offset_of_line_col(0, 5), 5);  // "\n"
+    /// assert_eq!(text.offset_of_line_col(0, 6), 5);  // "\n", avoids newline
+    /// assert_eq!(text.offset_of_line_col(1, 0), 6);  // "w"
+    /// assert_eq!(text.offset_of_line_col(1, 4), 10); // "d"
+    /// let text = Rope::from("hello\r\nworld");
+    /// let text = RopeTextRef::new(&text);
+    /// assert_eq!(text.offset_of_line_col(0, 0), 0);  // "h"
+    /// assert_eq!(text.offset_of_line_col(0, 4), 4);  // "o"
+    /// assert_eq!(text.offset_of_line_col(0, 5), 5);  // "\r"
+    /// assert_eq!(text.offset_of_line_col(0, 6), 5);  // "\r", avoids being in the middle
+    /// assert_eq!(text.offset_of_line_col(1, 0), 7);  // "w"
+    /// assert_eq!(text.offset_of_line_col(1, 4), 11); // "d"
+    /// ````
     fn offset_of_line_col(&self, line: usize, col: usize) -> usize {
         let mut pos = 0;
         let mut offset = self.offset_of_line(line);
-        for c in self
-            .slice_to_cow(offset..self.offset_of_line(line + 1))
-            .chars()
-        {
-            if c == '\n' {
+        let text = self.slice_to_cow(offset..self.offset_of_line(line + 1));
+        let mut iter = text.chars().peekable();
+        while let Some(c) = iter.next() {
+            // Stop at the end of the line
+            if c == '\n' || (c == '\r' && iter.peek() == Some(&'\n')) {
                 return offset;
             }
 
@@ -84,10 +107,12 @@ pub trait RopeText {
 
     /// Get the offset of the end of the line. The caret decides whether it is after the last
     /// character, or before it.
-    /// If the line is out of bounds, then the last offset (the len) is returned.
-    /// ```rust,ignore
+    /// If the line is out of bounds, then the last offset (the len) is returned.  
+    /// ```rust
+    /// # use floem_editor_core::xi_rope::Rope;
+    /// # use floem_editor_core::buffer::rope_text::{RopeText, RopeTextRef};
     /// let text = Rope::from("hello\nworld");
-    /// let text = RopeText::new(&text);
+    /// let text = RopeTextRef::new(&text);
     /// assert_eq!(text.line_end_offset(0, false), 4);  // "hell|o"
     /// assert_eq!(text.line_end_offset(0, true), 5);   // "hello|"
     /// assert_eq!(text.line_end_offset(1, false), 10); // "worl|d"
@@ -509,6 +534,29 @@ mod tests {
         assert_eq!(text.offset_of_line(5), text.len());
     }
 
+    #[test]
+    fn test_offset_of_line_col() {
+        let text = Rope::from("abc\ndef\nghi");
+        let text = RopeTextVal::new(text);
+
+        assert_eq!(text.offset_of_line_col(0, 0), 0);
+        assert_eq!(text.offset_of_line_col(0, 1), 1);
+        assert_eq!(text.offset_of_line_col(0, 2), 2);
+        assert_eq!(text.offset_of_line_col(0, 3), 3);
+        assert_eq!(text.offset_of_line_col(0, 4), 3);
+        assert_eq!(text.offset_of_line_col(1, 0), 4);
+
+        let text = Rope::from("abc\r\ndef\r\nghi");
+        let text = RopeTextVal::new(text);
+
+        assert_eq!(text.offset_of_line_col(0, 0), 0);
+        assert_eq!(text.offset_of_line_col(0, 1), 1);
+        assert_eq!(text.offset_of_line_col(0, 2), 2);
+        assert_eq!(text.offset_of_line_col(0, 3), 3);
+        assert_eq!(text.offset_of_line_col(0, 4), 3);
+        assert_eq!(text.offset_of_line_col(1, 0), 5);
+    }
+
     #[test]
     fn test_line_end_offset() {
         let text = Rope::from("");
@@ -534,19 +582,6 @@ mod tests {
         assert_eq!(text.line_end_offset(3, true), text.len());
         assert_eq!(text.line_end_offset(4, false), text.len());
         assert_eq!(text.line_end_offset(4, true), text.len());
-
-        // This is equivalent to the doc test for RopeText::line_end_offset
-        // because you don't seem to be able to do a `use RopeText` in a doc test since it isn't
-        // public..
-        let text = Rope::from("hello\nworld");
-        let text = RopeTextVal::new(text);
-
-        assert_eq!(text.line_end_offset(0, false), 4); // "hell|o"
-        assert_eq!(text.line_end_offset(0, true), 5); // "hello|"
-        assert_eq!(text.line_end_offset(1, false), 10); // "worl|d"
-        assert_eq!(text.line_end_offset(1, true), 11); // "world|"
-                                                       // Out of bounds
-        assert_eq!(text.line_end_offset(2, false), 11); // "world|"
     }
 
     #[test]

editor-core/src/lib.rs

@@ -14,3 +14,5 @@ pub mod selection;
 pub mod soft_tab;
 pub mod util;
 pub mod word;
+
+pub use lapce_xi_rope as xi_rope;

editor-core/src/line_ending.rs

@@ -85,6 +85,7 @@ impl LineEnding {
         }
     }
 
+    /// Get the name of the line ending
     pub fn as_str(&self) -> &'static str {
         match self {
             LineEnding::CrLf => "CRLF",
@@ -218,17 +219,18 @@ impl<'a, I: Iterator<Item = &'a str>> Iterator for FullLeChunkSearch<'a, I> {
             }
             Some(x) => {
                 // Typically this only occurs for a lone `\r`.
-                // However, we need to handl the case where the `\r` is the last character in the
+                // However, we need to handle the case where the `\r` is the last character in the
                 // chunk whilst the next chunk starts with a `\n`.
-                assert!(bytes[x] == b'\r');
+                assert_eq!(bytes[x], b'\r');
 
                 let start = self.offset + self.chunk_pos + x;
                 self.chunk_pos += x + 1;
 
                 let v = if self.chunk_pos == chunk.len() {
                     if let Some(next_chunk) = self.get_chunk() {
-                        if next_chunk.starts_with('\n') {
-                            self.chunk_pos = 1;
+                        let next_chunk = &next_chunk.as_bytes()[self.chunk_pos..];
+                        if next_chunk.starts_with(b"\n") {
+                            self.chunk_pos += 1;
                             Some((start..start + 2, LeChunkKind::CrLf))
                         } else {
                             None
@@ -257,6 +259,7 @@ impl<'a, I: Iterator<Item = &'a str>> Iterator for FullLeChunkSearch<'a, I> {
 
 /// Iterator that searches for lone carriage returns ('\r') in chunks of text.
 struct LoneCrChunkSearch<'a, I: Iterator<Item = &'a str>> {
+    /// Offset of the start of the current chunk
     offset: usize,
     chunk_pos: usize,
     chunks: Peekable<I>,
@@ -271,6 +274,8 @@ impl<'a, I: Iterator<Item = &'a str>> LoneCrChunkSearch<'a, I> {
         }
     }
 
+    /// Get the current chunk, or if chunk pos is past the end of the chunk, then
+    /// advance to the next chunk and get it.
     fn get_chunk(&mut self) -> Option<&'a str> {
         let chunk = self.chunks.peek()?;
         if self.chunk_pos >= chunk.len() {
@@ -309,10 +314,11 @@ impl<'a, I: Iterator<Item = &'a str>> Iterator for LoneCrChunkSearch<'a, I> {
                     // Skip \r\n sequences
                     self.chunk_pos += 1;
                     self.next()
-                } else if let Some(next_chunk) = self.get_chunk() {
-                    if next_chunk.starts_with('\n') {
+                } else if let Some(chunk_b) = self.get_chunk() {
+                    let chunk_b = &chunk_b.as_bytes()[self.chunk_pos..];
+                    if chunk_b.starts_with(b"\n") {
                         // Skip \r\n sequences across chunks
-                        self.chunk_pos = 1;
+                        self.chunk_pos += 1;
                         self.next()
                     } else {
                         // Lone \r
@@ -407,5 +413,16 @@ mod tests {
 
         let multi_chunk = LoneCrChunkSearch::new(text.into_iter());
         assert_eq!(multi_chunk.collect::<Vec<_>>(), vec![13, 14]);
+
+        let text = ["\n\rb"];
+        let chunks = FullLeChunkSearch::new(text.into_iter());
+        assert_eq!(
+            chunks.collect::<Vec<_>>(),
+            vec![(0..1, LeChunkKind::Lf), (1..2, LeChunkKind::Cr)]
+        );
+
+        let text = ["\n\rb"];
+        let chunks = LoneCrChunkSearch::new(text.into_iter());
+        assert_eq!(chunks.collect::<Vec<_>>(), vec![1]);
     }
 }

src/views/editor/movement.rs

@@ -1,7 +1,7 @@
 //! Movement logic for the editor.
 
 use floem_editor_core::{
-    buffer::rope_text::RopeText,
+    buffer::rope_text::{RopeText, RopeTextVal},
     command::MultiSelectionCommand,
     cursor::{ColPosition, Cursor, CursorAffinity, CursorMode},
     mode::{Mode, MotionMode, VisualMode},
@@ -97,7 +97,7 @@ pub fn move_offset(
     movement: &Movement,
     mode: Mode,
 ) -> (usize, Option<ColPosition>) {
-    match movement {
+    let (new_offset, horiz) = match movement {
         Movement::Left => {
             let new_offset = move_left(view, offset, affinity, mode, count);
 
@@ -192,7 +192,28 @@ pub fn move_offset(
 
             (new_offset, None)
         }
+    };
+
+    let new_offset = correct_crlf(&view.rope_text(), new_offset);
+
+    (new_offset, horiz)
+}
+
+/// If the offset is at `\r|\n` then move it back.
+fn correct_crlf(text: &RopeTextVal, offset: usize) -> usize {
+    if offset == 0 || offset == text.len() {
+        return offset;
     }
+
+    let mut iter = text.char_indices_iter(offset - 1..=offset);
+
+    if let Some((_, '\r')) = iter.next() {
+        if let Some((_, '\n')) = iter.next() {
+            return offset - 1;
+        }
+    }
+
+    offset
 }
 
 fn atomic_soft_tab_width_for_offset(ed: &Editor, offset: usize) -> Option<usize> {
@@ -757,5 +778,87 @@ pub fn do_motion_mode(
     }
 }
 
-// TODO: Write tests for the various functions. We'll need a more easily swappable API than
-// `Editor` for that.
+#[cfg(test)]
+mod tests {
+    use std::rc::Rc;
+
+    use floem_editor_core::{
+        buffer::rope_text::{RopeText, RopeTextVal},
+        cursor::CursorAffinity,
+        mode::Mode,
+    };
+    use floem_reactive::Scope;
+    use lapce_xi_rope::Rope;
+
+    use crate::views::editor::{
+        movement::{correct_crlf, end_of_line},
+        text::SimpleStyling,
+        text_document::TextDocument,
+    };
+
+    use super::Editor;
+
+    fn make_ed(text: &str) -> Editor {
+        let cx = Scope::new();
+        let doc = Rc::new(TextDocument::new(cx, text));
+        let style = Rc::new(SimpleStyling::light());
+        Editor::new(cx, doc, style)
+    }
+
+    // Tests for movement logic.
+    // Many of the locations that use affinity are unsure of the specifics, and should only be
+    // assumed to be mostly kinda correct.
+
+    #[test]
+    fn test_correct_crlf() {
+        let text = Rope::from("hello\nworld");
+        let text = RopeTextVal::new(text);
+        assert_eq!(correct_crlf(&text, 0), 0);
+        assert_eq!(correct_crlf(&text, 5), 5);
+        assert_eq!(correct_crlf(&text, 6), 6);
+        assert_eq!(correct_crlf(&text, text.len()), text.len());
+
+        let text = Rope::from("hello\r\nworld");
+        let text = RopeTextVal::new(text);
+        assert_eq!(correct_crlf(&text, 0), 0);
+        assert_eq!(correct_crlf(&text, 5), 5);
+        assert_eq!(correct_crlf(&text, 6), 5);
+        assert_eq!(correct_crlf(&text, 7), 7);
+        assert_eq!(correct_crlf(&text, text.len()), text.len());
+    }
+
+    #[test]
+    fn test_end_of_line() {
+        let ed = make_ed("abc\ndef\nghi");
+        let mut aff = CursorAffinity::Backward;
+        assert_eq!(end_of_line(&ed, &mut aff, 0, Mode::Insert).0, 3);
+        assert_eq!(aff, CursorAffinity::Backward);
+        assert_eq!(end_of_line(&ed, &mut aff, 1, Mode::Insert).0, 3);
+        assert_eq!(aff, CursorAffinity::Backward);
+        assert_eq!(end_of_line(&ed, &mut aff, 3, Mode::Insert).0, 3);
+        assert_eq!(aff, CursorAffinity::Backward);
+
+        assert_eq!(end_of_line(&ed, &mut aff, 4, Mode::Insert).0, 7);
+        assert_eq!(end_of_line(&ed, &mut aff, 5, Mode::Insert).0, 7);
+        assert_eq!(end_of_line(&ed, &mut aff, 7, Mode::Insert).0, 7);
+
+        let ed = make_ed("abc\r\ndef\r\nghi");
+        let mut aff = CursorAffinity::Forward;
+        assert_eq!(end_of_line(&ed, &mut aff, 0, Mode::Insert).0, 3);
+        assert_eq!(aff, CursorAffinity::Backward);
+
+        assert_eq!(end_of_line(&ed, &mut aff, 1, Mode::Insert).0, 3);
+        assert_eq!(aff, CursorAffinity::Backward);
+        assert_eq!(end_of_line(&ed, &mut aff, 3, Mode::Insert).0, 3);
+        assert_eq!(aff, CursorAffinity::Backward);
+
+        assert_eq!(end_of_line(&ed, &mut aff, 5, Mode::Insert).0, 8);
+        assert_eq!(end_of_line(&ed, &mut aff, 6, Mode::Insert).0, 8);
+        assert_eq!(end_of_line(&ed, &mut aff, 7, Mode::Insert).0, 8);
+        assert_eq!(end_of_line(&ed, &mut aff, 8, Mode::Insert).0, 8);
+
+        let ed = make_ed("testing\r\nAbout\r\nblah");
+        let mut aff = CursorAffinity::Backward;
+        assert_eq!(end_of_line(&ed, &mut aff, 0, Mode::Insert).0, 7);
+    }
+}