Add ability to insert at line and column

core/src/editor_buffer/mod.rs

@@ -25,7 +25,7 @@ impl CommandResponse {
         match self {
             Self::Ok => true,
             Self::Success(_) => true,
-            Self::Failure(_) => false
+            Self::Failure(_) => false,
         }
     }
 }
@@ -64,8 +64,9 @@ impl EditorBuffer {
         todo!()
     }
 
-    fn insert_char(&mut self, _c: char) -> CommandResponse {
+    fn insert_char(&mut self, c: char) -> CommandResponse {
-        todo!()
+        self.buffer.insert_char(c, self.cursor);
+        CommandResponse::Ok
     }
 
     fn insert_string(&mut self, _s: String) -> CommandResponse {
@@ -137,11 +138,146 @@ mod tests {
         let test_file = create_simple_test_file();
         let mut target = EditorBuffer::new();
         target.execute(Command::OpenFile(test_file.path().into()));
-        assert!(target.execute(Command::MoveCursorTo(Point::LineColumn(0, 5))).is_ok());
+        assert!(
+            target
+                .execute(Command::MoveCursorTo(Point::LineColumn(0, 5)))
+                .is_ok()
+        );
         assert_eq!(Point::LineColumn(0, 5), target.get_cursor_position());
-        assert!(target.execute(Command::MoveCursorTo(Point::LineColumn(3, 11))).is_ok());
+        assert!(
+            target
+                .execute(Command::MoveCursorTo(Point::LineColumn(3, 11)))
+                .is_ok()
+        );
         assert_eq!(Point::LineColumn(3, 11), target.get_cursor_position());
-        assert!(target.execute(Command::MoveCursorTo(Point::LineColumn(3, 0))).is_ok());
+        assert!(
+            target
+                .execute(Command::MoveCursorTo(Point::LineColumn(3, 0)))
+                .is_ok()
+        );
         assert_eq!(Point::LineColumn(3, 0), target.get_cursor_position());
     }
+
+    #[test]
+    fn insert_character() {
+        let mut test_file = create_simple_test_file();
+        let mut file_contents = String::new();
+        test_file
+            .read_to_string(&mut file_contents)
+            .expect("Reading text file");
+        let test_file = test_file;
+        let mut expected_lines: Vec<_> = file_contents.lines().collect();
+        expected_lines[2] = "Xbíth a menmasam fri seilgg";
+        let expected_lines = expected_lines;
+        let mut target = EditorBuffer::new();
+        assert!(
+            target
+                .execute(Command::OpenFile(test_file.path().into()))
+                .is_ok()
+        );
+        assert!(
+            target
+                .execute(Command::MoveCursorTo(Point::LineColumn(3, 0)))
+                .is_ok()
+        );
+        assert!(target.execute(Command::InsertChar('X')).is_ok());
+        let found_lines: Vec<String> = target
+            .buffer
+            .iter_chars()
+            .collect::<String>()
+            .lines()
+            .map(|l| l.into())
+            .collect();
+        assert_eq!(expected_lines, found_lines);
+
+        let mut test_file = create_simple_test_file();
+        let mut file_contents = String::new();
+        test_file
+            .read_to_string(&mut file_contents)
+            .expect("Reading text file");
+        let test_file = test_file;
+        let mut expected_lines: Vec<_> = file_contents.lines().collect();
+        expected_lines[2] = "bXíth a menmasam fri seilgg";
+        let expected_lines = expected_lines;
+        let mut target = EditorBuffer::new();
+        assert!(
+            target
+                .execute(Command::OpenFile(test_file.path().into()))
+                .is_ok()
+        );
+        assert!(
+            target
+                .execute(Command::MoveCursorTo(Point::LineColumn(3, 1)))
+                .is_ok()
+        );
+        assert!(target.execute(Command::InsertChar('X')).is_ok());
+        let found_lines: Vec<String> = target
+            .buffer
+            .iter_chars()
+            .collect::<String>()
+            .lines()
+            .map(|l| l.into())
+            .collect();
+        assert_eq!(expected_lines, found_lines);
+
+        let mut test_file = create_simple_test_file();
+        let mut file_contents = String::new();
+        test_file
+            .read_to_string(&mut file_contents)
+            .expect("Reading text file");
+        let test_file = test_file;
+        let mut expected_lines: Vec<_> = file_contents.lines().collect();
+        expected_lines[2] = "bíXth a menmasam fri seilgg";
+        let expected_lines = expected_lines;
+        let mut target = EditorBuffer::new();
+        assert!(
+            target
+                .execute(Command::OpenFile(test_file.path().into()))
+                .is_ok()
+        );
+        assert!(
+            target
+                .execute(Command::MoveCursorTo(Point::LineColumn(3, 2)))
+                .is_ok()
+        );
+        assert!(target.execute(Command::InsertChar('X')).is_ok());
+        let found_lines: Vec<String> = target
+            .buffer
+            .iter_chars()
+            .collect::<String>()
+            .lines()
+            .map(|l| l.into())
+            .collect();
+        assert_eq!(expected_lines, found_lines);
+
+        let mut test_file = create_simple_test_file();
+        let mut file_contents = String::new();
+        test_file
+            .read_to_string(&mut file_contents)
+            .expect("Reading text file");
+        let test_file = test_file;
+        let mut expected_lines: Vec<_> = file_contents.lines().collect();
+        expected_lines[2] = "bíth a menmXasam fri seilgg";
+        let expected_lines = expected_lines;
+        let mut target = EditorBuffer::new();
+        assert!(
+            target
+                .execute(Command::OpenFile(test_file.path().into()))
+                .is_ok()
+        );
+        assert!(
+            target
+                .execute(Command::MoveCursorTo(Point::LineColumn(3, 11)))
+                .is_ok()
+        );
+        assert!(target.execute(Command::InsertChar('X')).is_ok());
+        let found_lines: Vec<String> = target
+            .buffer
+            .iter_chars()
+            .collect::<String>()
+            .lines()
+            .map(|l| l.into())
+            .collect();
+        assert_eq!(expected_lines, found_lines);
+    }
 }

core/src/text_buffer/mod.rs

@@ -50,15 +50,6 @@ impl TextBuffer {
             0
         } else {
             self.contents.total_lines()
-                + if self
-                    .contents
-                    .get_char_at_index(self.contents.total_chars() - 1)
-                    == '\n'
-                {
-                    0
-                } else {
-                    1
-                }
         }
     }
 
@@ -66,10 +57,8 @@ impl TextBuffer {
     pub fn insert_text(&mut self, text: impl Into<String>, point: Point) {
         match point {
             Point::Start => {
-                self.contents = self
+                self.contents = self.contents.insert_at_char_index(0, text);
-                    .contents
+            }
-                    .insert_at_char_index(0, text);
-            },
             Point::LineColumn(_, _) => {
                 todo!()
             }
@@ -82,25 +71,26 @@ impl TextBuffer {
     }
 
     pub fn insert_char(&mut self, c: char, point: Point) {
-        match point {
+        self.contents = match point {
-            Point::Start => {
+            Point::Start => self.contents.insert_at_char_index(0, c),
-                self.contents = self
+            Point::LineColumn(line_num, column_num) => {
+                self.contents
+                    .insert_at_line_and_column(line_num - 1, column_num, c)
+            }
+            Point::End => self
                 .contents
-                    .insert_at_char_index(0, c)
+                .insert_at_char_index(self.contents.total_chars(), c),
-            },
+        };
-            Point::LineColumn(_, _) => {
-                todo!()
-            }
-            Point::End => {
-                self.contents = self
-                    .contents
-                    .insert_at_char_index(self.contents.total_chars(), c)
-            }
-        }
     }
 
-    pub fn delete_at_char_index(&mut self, start: usize, length: usize) {
+    pub fn delete_at_char_index(&mut self, start: usize, length: usize) -> bool {
-        self.contents = self.contents.delete_at_char_index(start, length)
+        match self.contents.delete_at_char_index(start, length) {
+            Ok(r) => {
+                self.contents = r;
+                true
+            }
+            Err(_) => false,
+        }
     }
 
     pub fn iter_chars(&self) -> CharIterator {

core/src/text_buffer/reader.rs

@@ -27,7 +27,7 @@ impl std::io::Read for TextBufferReader {
     fn read(&mut self, buf: &mut [u8]) -> std::io::Result<usize> {
         let mut bytes_written = 0;
         while let Some(rope) = self.current_node.clone() {
-            if let Rope::Leaf { text } = rope.as_ref() {
+            if let Rope::Leaf { text, .. } = rope.as_ref() {
                 let bytes = text.as_bytes();
                 let length = bytes
                     .len()

core/src/text_buffer/rope/line_column.rs

@@ -0,0 +1,119 @@
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+/// Represents a block of text, where:
+///
+/// LineColumn.0 is the numbner of newlines in the text.
+/// LineColumn.1 is the number of characters after the last newline.
+///
+/// So a block of text with `n` lines, ending with a newline, would be
+/// represented by `LineColumn(n, 0)`. A block of text, *not* ending in a
+/// newline, with `m` characters on the last line, would be represented by
+/// `LineColumn(n-1, m)`.
+pub struct LineColumn(pub usize, pub usize);
+
+impl std::cmp::PartialOrd for LineColumn {
+    fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
+/// Assume the two values are substrings withing the same string. If the two
+/// values start on the same character, does the second one end before, after or
+/// at the same place as the first?
+impl std::cmp::Ord for LineColumn {
+    fn cmp(&self, other: &Self) -> std::cmp::Ordering {
+        if self.0 == other.0 {
+            self.1.cmp(&other.1)
+        } else {
+            self.0.cmp(&other.0)
+        }
+    }
+}
+
+/// Return the representation of a string formed by concatenating the two
+/// strings.  Note that, like string concatenation, this is *not*
+/// commutative. a+b and b+a are different things.
+impl std::ops::Add for LineColumn {
+    type Output = LineColumn;
+
+    fn add(self, rhs: Self) -> Self::Output {
+        if rhs.0 == 0 {
+            LineColumn(self.0, self.1 + rhs.1)
+        } else {
+            LineColumn(self.0 + rhs.0, rhs.1)
+        }
+    }
+}
+
+/// If a + b = c, then c - a = b
+/// The result of removing the second value from beginning of the first.
+impl std::ops::Sub for LineColumn {
+    type Output = LineColumn;
+
+    fn sub(self, rhs: Self) -> Self::Output {
+        if rhs.0 == 0 {
+            LineColumn(self.0, self.1 - rhs.1)
+        } else if self.0 == rhs.0 {
+            LineColumn(0, self.1 - rhs.1)
+        } else {
+            LineColumn(self.0 - rhs.0, self.1)
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    use rand::{Rng, SeedableRng, rngs::SmallRng};
+
+    #[test]
+    fn cmp_works() {
+        assert_eq!(LineColumn(0, 0), LineColumn(0, 0));
+        assert_eq!(LineColumn(1, 0), LineColumn(1, 0));
+        assert_eq!(LineColumn(0, 2), LineColumn(0, 2));
+        assert_eq!(LineColumn(4, 16), LineColumn(4, 16));
+
+        assert_ne!(LineColumn(0, 0), LineColumn(0, 1));
+        assert_ne!(LineColumn(0, 0), LineColumn(1, 0));
+        assert_ne!(LineColumn(3, 0), LineColumn(3, 2));
+        assert_ne!(LineColumn(16, 21), LineColumn(17, 21));
+
+        assert!(LineColumn(0, 0) < LineColumn(0, 1));
+        assert!(LineColumn(0, 1) > LineColumn(0, 0));
+        assert!(LineColumn(12, 34) < LineColumn(23, 45));
+        assert!(LineColumn(12, 34) > LineColumn(1, 23));
+        assert!(LineColumn(12, 34) < LineColumn(12, 35));
+        assert!(LineColumn(12, 34) > LineColumn(12, 33));
+    }
+
+    #[test]
+    fn add_works() {
+        assert_eq!(LineColumn(12, 34), LineColumn(0, 0) + LineColumn(12, 34));
+        assert_eq!(LineColumn(12, 34), LineColumn(0, 1) + LineColumn(12, 34));
+        assert_eq!(LineColumn(12, 34), LineColumn(0, 2) + LineColumn(12, 34));
+        assert_eq!(LineColumn(12, 34), LineColumn(0, 89) + LineColumn(12, 34));
+        assert_eq!(LineColumn(15, 34), LineColumn(3, 0) + LineColumn(12, 34));
+        assert_eq!(LineColumn(15, 34), LineColumn(3, 1) + LineColumn(12, 34));
+        assert_eq!(LineColumn(15, 34), LineColumn(3, 2) + LineColumn(12, 34));
+        assert_eq!(LineColumn(15, 34), LineColumn(3, 89) + LineColumn(12, 34));
+        assert_eq!(LineColumn(5, 35), LineColumn(5, 23) + LineColumn(0, 12));
+        assert_eq!(LineColumn(5, 23), LineColumn(5, 23) + LineColumn(0, 0));
+        assert_eq!(LineColumn(0, 34), LineColumn(0, 15) + LineColumn(0, 19));
+    }
+
+    #[test]
+    fn subtract_works() {
+        assert_eq!(LineColumn(12, 34), LineColumn(12, 34) - LineColumn(0, 0));
+        assert_eq!(LineColumn(12, 31), LineColumn(12, 34) - LineColumn(0, 3));
+        assert_eq!(LineColumn(2, 34), LineColumn(12, 34) - LineColumn(10, 0));
+        assert_eq!(LineColumn(2, 34), LineColumn(12, 34) - LineColumn(10, 5));
+
+        let mut rng = SmallRng::seed_from_u64(0x0123456789abcdef);
+        for _ in 0..100 {
+            let a = LineColumn(rng.random_range(..1000), rng.random_range(..300));
+            let b = LineColumn(rng.random_range(..1000), rng.random_range(..300));
+            let c = a + b;
+            assert_eq!(b, c - a);
+        }
+    }
+}

core/src/text_buffer/rope/mod.rs

@@ -3,6 +3,11 @@ use std::rc::Rc;
 mod fibbonacci;
 use fibbonacci::fibbonacci;
 
+mod line_column;
+use line_column::LineColumn;
+
+pub type RopeResult = Result<Rc<Rope>, Rc<Rope>>;
+
 /// [Rope](https://en.wikipedia.org/wiki/Rope_(data_structure)) data structure
 /// implementation.
 ///
@@ -19,9 +24,11 @@ pub enum Rope {
         /// or the number of characters in the string if this is a leaf.
         chars_weight: usize,
 
-        /// Total number of line endings in the string contained in the left
+        /// Total number of line endings and number of characters in last line
-        /// subtree, or the number of line endings in the string if this is a leaf.
+        /// in the string contained in the left subtree, or the number of line
-        lines_weight: usize,
+        /// endings and number of characters in last line in the string if this
+        /// is a leaf.
+        line_column_weight: LineColumn,
 
         /// The root of the left subtree
         left: Rc<Rope>,
@@ -29,6 +36,8 @@ pub enum Rope {
         right: Option<Rc<Rope>>,
     },
     Leaf {
+        chars_count: usize,
+        line_column_count: LineColumn,
         text: String,
     },
 }
@@ -37,13 +46,29 @@ impl Rope {
     /// Create a new Rope containing the passed text in a single node.
     pub fn new(contents: impl Into<String>) -> Rc<Self> {
         let text = contents.into();
-        Rc::new(Rope::Leaf { text })
+        let (chars_count, line_column_count) = text.chars().fold(
+            (0, LineColumn(0, 0)),
+            |(char_count, LineColumn(newline_count, chars_since_newline)), c| {
+                (
+                    char_count + 1,
+                    if c == '\n' {
+                        LineColumn(newline_count + 1, 0)
+                    } else {
+                        LineColumn(newline_count, chars_since_newline + 1)
+                    },
+                )
+            },
+        );
+        Rc::new(Rope::Leaf {
+            chars_count,
+            line_column_count,
+            text,
+        })
     }
 
     /// Create a new empty Rope
     pub fn empty() -> Rc<Self> {
-        let text = "".into();
+        Rope::new("")
-        Rc::new(Rope::Leaf { text })
     }
 
     /// Return the total number of bytes in the text.
@@ -59,7 +84,7 @@ impl Rope {
                 right: Some(right),
                 ..
             } => bytes_weight + right.total_bytes(),
-            Rope::Leaf { text } => text.len(),
+            Rope::Leaf { text, .. } => text.len(),
         }
     }
 
@@ -76,25 +101,14 @@ impl Rope {
                 right: Some(right),
                 ..
             } => chars_weight + right.total_chars(),
-            Rope::Leaf { text } => text.chars().count(),
+            Rope::Leaf { chars_count, .. } => *chars_count,
         }
     }
 
     /// Return the total number of lines in the text
     pub fn total_lines(&self) -> usize {
-        match self {
+        let LineColumn(l, c) = self.line_column_total();
-            Rope::Branch {
+        if c > 0 { l + 1 } else { l }
-                lines_weight,
-                right: None,
-                ..
-            } => *lines_weight,
-            Rope::Branch {
-                lines_weight,
-                right: Some(right),
-                ..
-            } => lines_weight + right.total_lines(),
-            Rope::Leaf { text } => text.chars().filter(|&c| c == '\n').count(),
-        }
     }
 
     /// Return the character as a given character index.
@@ -115,7 +129,7 @@ impl Rope {
                     right.get_char_at_index(index - chars_weight)
                 }
             }
-            Rope::Leaf { text } => text.chars().nth(index).unwrap(),
+            Rope::Leaf { text, .. } => text.chars().nth(index).unwrap(),
         }
     }
 
@@ -131,65 +145,176 @@ impl Rope {
         index: usize,
         text: impl Into<String>,
     ) -> Rc<Rope> {
+        let text: String = text.into();
         let new_node = Rope::new(text);
         let total_chars = self.total_chars();
-        if index == 0 {
+        if total_chars == 0 {
-            new_node.concat(self.clone())
+            new_node
-        } else if index < total_chars {
-            let (before, after) = self.split_at_char_index(index);
-            before.concat(new_node).concat(after)
-        } else if index == total_chars {
-            self.clone().concat(new_node)
         } else {
-            panic!("Attempt to insert past end of rope.")
+            match self.split_at_char_index(index) {
+                (Some(before), after) => {
+                    Rope::join(Rope::join(before, Some(new_node)), after).rebalance()
+                }
+                (None, after) => Rope::join(new_node, after).rebalance(),
+            }
         }
     }
 
+    /// Insert new text into the rope at a given line and column.
+    pub fn insert_at_line_and_column(
+        self: &Rc<Rope>,
+        line_num: usize,
+        column_num: usize,
+        text: impl Into<String>,
+    ) -> Rc<Rope> {
+        let new_node = Rope::new(text);
+        if line_num == 0 && column_num == 0 {
+            Rope::join(new_node, Some(Rc::clone(self))).rebalance()
+        } else {
+            match self.split_at_line_and_column(line_num, column_num) {
+                (Some(before), after) => {
+                    Rope::join(Rope::join(before, Some(new_node)), after).rebalance()
+                }
+                (None, after) => Rope::join(new_node, after).rebalance(),
+            }
+        }
+    }
+
+    pub fn split_at_line_and_column(
+        self: &Rc<Rope>,
+        line_num: usize,
+        column_num: usize,
+    ) -> (Option<Rc<Rope>>, Option<Rc<Rope>>) {
+        self.split_at_index(
+            LineColumn(line_num, column_num),
+            |rope| match rope {
+                Rope::Branch {
+                    line_column_weight, ..
+                } => *line_column_weight,
+                Rope::Leaf {
+                    line_column_count, ..
+                } => *line_column_count,
+            },
+            |text, LineColumn(line, column)| {
+                let mut current_line = 0;
+                let mut current_column = 0;
+                let mut byte_to_split_at = text.len();
+                for (i, c) in text.char_indices() {
+                    if c == '\n' {
+                        current_line += 1;
+                        current_column = 0;
+                    } else {
+                        current_column += 1;
+                    }
+                    if (current_line == line && current_column > column) || current_line > line {
+                        byte_to_split_at = i;
+                        break;
+                    }
+                }
+                if byte_to_split_at == 0 {
+                    (None, Some(Rope::new(text)))
+                } else if byte_to_split_at == text.len() {
+                    (Some(Rope::new(text)), None)
+                } else {
+                    let (first, second) = text.split_at(byte_to_split_at);
+                    (Some(Rope::new(first)), Some(Rope::new(second)))
+                }
+            },
+        )
+    }
+
     /// Return a new rope with `length` characters removed after `start`.
-    pub fn delete_at_char_index(self: &Rc<Rope>, start: usize, length: usize) -> Rc<Rope> {
+    pub fn delete_at_char_index(self: &Rc<Rope>, start: usize, length: usize) -> RopeResult {
-        let (beginning, rest) = self.split_at_char_index(start);
+        if let (beginning, Some(rest)) = self.split_at_char_index(start) {
             let (_, end) = rest.split_at_char_index(length);
-        beginning.concat(end)
+            if let Some(beginning) = beginning {
+                Ok(Rope::join(beginning, end).rebalance())
+            } else if let Some(end) = end {
+                Ok(end)
+            } else {
+                Ok(Rope::new(""))
+            }
+        } else {
+            Err(Rc::clone(self))
+        }
     }
 
     /// Split the rope in two at character `index`.
     ///
     /// The result is two Ropes—one containing the first 'i' characters of the
     /// text and the other containing the rest of the text.
-    pub fn split_at_char_index(self: &Rc<Self>, index: usize) -> (Rc<Self>, Rc<Self>) {
+    pub fn split_at_char_index(
+        self: &Rc<Self>,
+        index: usize,
+    ) -> (Option<Rc<Self>>, Option<Rc<Self>>) {
+        self.split_at_index(
+            index,
+            |rope| match rope {
+                Rope::Branch { chars_weight, .. } => *chars_weight,
+                Rope::Leaf { chars_count, .. } => *chars_count,
+            },
+            |text, i| {
+                if let Some((byte_index, _)) = text.char_indices().nth(i) {
+                    let (first, second) = text.split_at(byte_index);
+                    (Some(Rope::new(first)), Some(Rope::new(second)))
+                } else {
+                    panic!()
+                }
+            },
+        )
+    }
+
+    /// Split the rope in two at character `index`.
+    ///
+    /// The result is two Ropes—one containing the first 'i' characters of the
+    /// text and the other containing the rest of the text.
+    pub fn split_at_index<I, WeightF, SplitF>(
+        self: &Rc<Self>,
+        index: I,
+        weight: WeightF,
+        split_leaf: SplitF,
+    ) -> (Option<Rc<Self>>, Option<Rc<Self>>)
+    where
+        I: std::cmp::Ord + std::ops::Sub<Output = I> + std::fmt::Debug + Copy,
+        WeightF: Fn(&Self) -> I,
+        SplitF: Fn(&str, I) -> (Option<Rc<Self>>, Option<Rc<Self>>),
+    {
         match *self.as_ref() {
             Rope::Branch {
-                chars_weight,
                 ref left,
                 ref right,
                 ..
             } => {
-                if index < chars_weight {
+                let self_weight = weight(self);
-                    let (first, second) = left.split_at_char_index(index);
+                if index < self_weight {
+                    // Split left subtree
+                    let (first, second) = left.split_at_index(index, weight, split_leaf);
                     (
-                        first.rebalance(),
+                        first,
-                        Rope::join(second, right.as_ref().map(|r| r.clone())).rebalance(),
+                        second
+                            .map(|rope| Rope::join(rope, right.as_ref().map(Rc::clone)))
+                            .or_else(|| right.as_ref().map(Rc::clone)),
                     )
                 } else if let Some(right) = right {
-                    if index > chars_weight {
+                    if index > self_weight {
-                        let (first, second) = right.split_at_char_index(index - chars_weight);
+                        // Split right subtree
-                        (
+                        let (first, second) =
-                            Rope::join(left.clone(), Some(first)).rebalance(),
+                            right.split_at_index(index - self_weight, weight, split_leaf);
-                            second.rebalance(),
+                        (Some(Rope::join(Rc::clone(left), first)), second)
-                        )
                     } else {
-                        (left.clone(), right.clone())
+                        // Split is aleady exactly between left and right subtrees
+                        (Some(Rc::clone(left)), Some(Rc::clone(right)))
                     }
                 } else {
-                    (left.clone(), Rope::empty())
+                    // Split is at end
+                    (Some(Rc::clone(left)), None)
                 }
             }
-            Rope::Leaf { ref text } => {
+            Rope::Leaf { ref text, .. } => {
-                if let Some((byte_index, _)) = text.char_indices().nth(index) {
+                if index >= weight(self) {
-                    let (first, second) = text.split_at(byte_index);
+                    (Some(self.clone()), None)
-                    (Rope::new(first), Rope::new(second))
                 } else {
-                    (self.clone(), Rope::empty())
+                    split_leaf(text, index)
                 }
             }
         }
@@ -245,7 +370,7 @@ impl Rope {
         Rc::new(Rope::Branch {
             bytes_weight: left.total_bytes(),
             chars_weight: left.total_chars(),
-            lines_weight: left.total_lines(),
+            line_column_weight: left.line_column_total(),
             left,
             right,
         })
@@ -255,6 +380,32 @@ impl Rope {
     pub fn iter_nodes(self: Rc<Self>) -> NodeIterator {
         NodeIterator::new(self)
     }
+
+    /// Return the total number of lines in the text, and the number of characters on the last line
+    pub fn line_column_total(&self) -> LineColumn {
+        match self {
+            Rope::Branch {
+                line_column_weight,
+                right: None,
+                ..
+            } => *line_column_weight,
+            Rope::Branch {
+                line_column_weight,
+                right: Some(right),
+                ..
+            } => *line_column_weight + right.line_column_total(),
+            Rope::Leaf { text, .. } => {
+                text.chars()
+                    .fold(LineColumn(0, 0), |LineColumn(line, col), c| {
+                        if c == '\n' {
+                            LineColumn(line + 1, 0)
+                        } else {
+                            LineColumn(line, col + 1)
+                        }
+                    })
+            }
+        }
+    }
 }
 
 fn merge(leaf_nodes: &[Rc<Rope>]) -> Rc<Rope> {
@@ -349,7 +500,7 @@ impl CharWithPointIterator {
 
     fn next_string(node_iterator: &mut NodeIterator) -> Option<String> {
         node_iterator.next().map(|rope| {
-            if let Rope::Leaf { text } = rope.as_ref() {
+            if let Rope::Leaf { text, .. } = rope.as_ref() {
                 text.clone()
             } else {
                 panic!("Rope NodeIterator yielded non-leaf node.")

core/src/text_buffer/rope/tests/command_list.rs

@@ -1,8 +1,8 @@
 use super::super::{Rc, Rope};
 use rand::{
+    Rng, SeedableRng,
     distr::{Alphanumeric, SampleString},
     rngs::SmallRng,
-    Rng, SeedableRng,
 };
 
 #[derive(Clone, Debug)]
@@ -15,9 +15,9 @@ impl Command {
     pub fn run(&self, rope: &Rc<Rope>) -> Rc<Rope> {
         match self {
             Command::InsertAtCharIndex { index, text } => rope.insert_at_char_index(*index, text),
-            Command::DeleteAtCharIndex { index, length } => {
+            Command::DeleteAtCharIndex { index, length } => rope
-                rope.delete_at_char_index(*index, *length)
+                .delete_at_char_index(*index, *length)
-            }
+                .expect("delete was successful"),
         }
     }
 

core/src/text_buffer/rope/tests/mod.rs

@@ -74,7 +74,7 @@ fn node_iterator_for_single_node_returns_node_and_only_node() {
     assert_eq!(result.len(), 1);
     assert_eq!(result[0].total_bytes(), 3);
     assert_eq!(result[0].total_chars(), 3);
-    assert_eq!(result[0].total_lines(), 0);
+    assert_eq!(result[0].total_lines(), 1);
     match result[0].as_ref() {
         Rope::Leaf { text, .. } => assert_eq!(text, "The"),
         _ => panic!(),
@@ -91,7 +91,7 @@ fn node_iterator_returns_nodes_in_correct_order() {
     assert_eq!(result.len(), 10);
     for (node, string) in result.iter().zip(strings) {
         match &node.as_ref() {
-            Rope::Leaf { text } => assert_eq!(text, string),
+            Rope::Leaf { text, .. } => assert_eq!(text, string),
             _ => panic!(),
         }
     }
@@ -116,8 +116,14 @@ fn split_splits_at_correct_location() {
     let full_string = small_test_rope_full_string();
     for i in 0..(full_string.chars().count()) {
         let (first, second) = target.split_at_char_index(i);
-        let first_string: String = first.iter_chars().collect();
+        let first_string: String = first
-        let second_string: String = second.iter_chars().collect();
+            .expect("first part is not None")
+            .iter_chars()
+            .collect();
+        let second_string: String = second
+            .expect("second part is not None")
+            .iter_chars()
+            .collect();
         assert_eq!(first_string, full_string[0..i]);
         assert_eq!(second_string, full_string[i..]);
     }
@@ -132,6 +138,8 @@ fn split_splits_at_correct_location_with_multibyte_chars() {
         .collect();
     for i in 0..(expected_chars.len()) {
         let (first, second) = target.split_at_char_index(i);
+        let first = first.expect("First part is not None");
+        let second = second.expect("Second part is not None");
         let string: String = first.iter_chars().collect();
         let expected: String = expected_chars[0..i].iter().collect();
         assert_eq!(string, expected);
@@ -224,11 +232,11 @@ fn get_char_at_index() {
 fn insert_at_char_index() {
     let target = Rope::new("The brown dog");
     assert_eq!(target.iter_chars().collect::<String>(), "The brown dog");
-    assert_eq!(0, target.total_lines());
+    assert_eq!(1, target.total_lines());
     let rope1 = target.insert_at_char_index(4, "quick");
     assert_eq!(target.iter_chars().collect::<String>(), "The brown dog");
     assert_eq!(rope1.iter_chars().collect::<String>(), "The quickbrown dog");
-    assert_eq!(0, rope1.total_lines());
+    assert_eq!(1, rope1.total_lines());
     let rope2 = rope1.insert_at_char_index(9, " ");
     assert_eq!(target.iter_chars().collect::<String>(), "The brown dog");
     assert_eq!(rope1.iter_chars().collect::<String>(), "The quickbrown dog");
@@ -236,20 +244,22 @@ fn insert_at_char_index() {
         rope2.iter_chars().collect::<String>(),
         "The quick brown dog"
     );
-    assert_eq!(0, rope2.total_lines());
+    assert_eq!(1, rope2.total_lines());
     let rope3 =
         rope2.insert_at_char_index("The quick brown dog".len(), " jumps over the lazy fox.");
     assert_eq!(
         rope3.iter_chars().collect::<String>(),
         "The quick brown dog jumps over the lazy fox."
     );
-    assert_eq!(0, rope3.total_lines());
+    assert_eq!(1, rope3.total_lines());
 }
 
 #[test]
 fn delete_at_char_index() {
     let target = Rope::new("The quick brown fox jumps over the lazy dog.");
-    let test = target.delete_at_char_index(10, 6);
+    let test = target
+        .delete_at_char_index(10, 6)
+        .expect("Delete was successful");
     assert_eq!(
         target.iter_chars().collect::<String>(),
         "The quick brown fox jumps over the lazy dog."
@@ -258,7 +268,9 @@ fn delete_at_char_index() {
         test.iter_chars().collect::<String>(),
         "The quick fox jumps over the lazy dog."
     );
-    let test = target.delete_at_char_index(0, 4);
+    let test = target
+        .delete_at_char_index(0, 4)
+        .expect("Delete was successful");
     assert_eq!(
         target.iter_chars().collect::<String>(),
         "The quick brown fox jumps over the lazy dog."
@@ -267,8 +279,9 @@ fn delete_at_char_index() {
         test.iter_chars().collect::<String>(),
         "quick brown fox jumps over the lazy dog."
     );
-    let test =
+    let test = target
-        target.delete_at_char_index("The quick brown fox jumps over the lazy dog.".len() - 5, 5);
+        .delete_at_char_index("The quick brown fox jumps over the lazy dog.".len() - 5, 5)
+        .expect("Delete was successful");
     assert_eq!(
         target.iter_chars().collect::<String>(),
         "The quick brown fox jumps over the lazy dog."
@@ -486,7 +499,6 @@ fn random_insertions_and_deletions_with_seed(seed: u64) {
     let (start_text, edits) = command_list::generate_random_edit_sequence_with_seed(1000, seed);
     let mut target = Rope::new(start_text);
     for (command, expected_text) in edits {
-        println!("{:?}", command);
         target = command.run(&target);
         assert_eq!(expected_text, target.iter_chars().collect::<String>());
     }