Merge pull request #1125 from rtfeldman/fix_caret_move

editor/src/editor/code_lines.rs

@@ -1,6 +1,7 @@
 use crate::ui::text::lines::Lines;
 use crate::ui::ui_error::UIResult;
 use crate::ui::util::slice_get;
+use crate::ui::util::slice_get_mut;
 use bumpalo::collections::String as BumpString;
 use bumpalo::Bump;
 
@@ -17,6 +18,34 @@ impl CodeLines {
             nr_of_chars: code_str.len(),
         }
     }
+
+    pub fn add_to_line(&mut self, line_nr: usize, new_str: &str) -> UIResult<()> {
+        let line_ref = slice_get_mut(line_nr, &mut self.lines)?;
+        line_ref.push_str(new_str);
+
+        Ok(())
+    }
+
+    pub fn insert_between_line(
+        &mut self,
+        line_nr: usize,
+        index: usize,
+        new_str: &str,
+    ) -> UIResult<()> {
+        let line_ref = slice_get_mut(line_nr, &mut self.lines)?;
+
+        line_ref.insert_str(index, new_str);
+
+        Ok(())
+    }
+
+    pub fn del_at_line(&mut self, line_nr: usize, index: usize) -> UIResult<()> {
+        let line_ref = slice_get_mut(line_nr, &mut self.lines)?;
+
+        line_ref.remove(index);
+
+        Ok(())
+    }
 }
 
 //TODO use rust's split_inclusive once it's no longer unstable
@@ -70,6 +99,10 @@ impl Lines for CodeLines {
         lines
     }
 
+    fn is_last_line(&self, line_nr: usize) -> bool {
+        line_nr == self.nr_of_lines() - 1
+    }
+
     fn last_char(&self, line_nr: usize) -> UIResult<Option<char>> {
         Ok(self.get_line(line_nr)?.chars().last())
     }

editor/src/editor/grid_node_map.rs

@@ -1,4 +1,7 @@
+use crate::editor::code_lines::CodeLines;
+use crate::editor::ed_error::EdResult;
 use crate::editor::slow_pool::MarkNodeId;
+use crate::editor::util::index_of;
 use crate::ui::text::text_pos::TextPos;
 use crate::ui::ui_error::UIResult;
 use crate::ui::util::{slice_get, slice_get_mut};
@@ -15,8 +18,8 @@ impl GridNodeMap {
         }
     }
 
-    pub fn add_to_line(&mut self, line: usize, len: usize, node_id: MarkNodeId) -> UIResult<()> {
+    pub fn add_to_line(&mut self, line_nr: usize, len: usize, node_id: MarkNodeId) -> UIResult<()> {
-        let line_ref = slice_get_mut(line, &mut self.lines)?;
+        let line_ref = slice_get_mut(line_nr, &mut self.lines)?;
         let mut new_cols_vec: Vec<MarkNodeId> = std::iter::repeat(node_id).take(len).collect();
 
         line_ref.append(&mut new_cols_vec);
@@ -26,12 +29,12 @@ impl GridNodeMap {
 
     pub fn insert_between_line(
         &mut self,
-        line: usize,
+        line_nr: usize,
         index: usize,
         len: usize,
         node_id: MarkNodeId,
     ) -> UIResult<()> {
-        let line_ref = slice_get_mut(line, &mut self.lines)?;
+        let line_ref = slice_get_mut(line_nr, &mut self.lines)?;
         let new_cols_vec: Vec<MarkNodeId> = std::iter::repeat(node_id).take(len).collect();
 
         line_ref.splice(index..index, new_cols_vec);
@@ -39,6 +42,14 @@ impl GridNodeMap {
         Ok(())
     }
 
+    pub fn del_at_line(&mut self, line_nr: usize, index: usize) -> UIResult<()> {
+        let line_ref = slice_get_mut(line_nr, &mut self.lines)?;
+
+        line_ref.remove(index);
+
+        Ok(())
+    }
+
     /*pub fn new_line(&mut self) {
         self.lines.push(vec![])
     }*/
@@ -49,4 +60,50 @@ impl GridNodeMap {
 
         Ok(*node_id)
     }
+
+    pub fn get_offset_to_node_id(
+        &self,
+        caret_pos: TextPos,
+        node_id: MarkNodeId,
+    ) -> EdResult<usize> {
+        let line = slice_get(caret_pos.line, &self.lines)?;
+
+        let first_node_index = index_of(node_id, line)?;
+
+        Ok(caret_pos.column - first_node_index)
+    }
+}
+
+// perform updates for both GridNodeMap and CodeLines
+pub fn add_to_line_both(
+    grid_node_map: &mut GridNodeMap,
+    code_lines: &mut CodeLines,
+    line_nr: usize,
+    new_str: &str,
+    node_id: MarkNodeId,
+) -> UIResult<()> {
+    grid_node_map.add_to_line(line_nr, new_str.len(), node_id)?;
+    code_lines.add_to_line(line_nr, new_str)
+}
+
+pub fn insert_between_line_both(
+    grid_node_map: &mut GridNodeMap,
+    code_lines: &mut CodeLines,
+    line_nr: usize,
+    index: usize,
+    new_str: &str,
+    node_id: MarkNodeId,
+) -> UIResult<()> {
+    grid_node_map.insert_between_line(line_nr, index, new_str.len(), node_id)?;
+    code_lines.insert_between_line(line_nr, index, new_str)
+}
+
+pub fn del_at_line_both(
+    grid_node_map: &mut GridNodeMap,
+    code_lines: &mut CodeLines,
+    line_nr: usize,
+    index: usize,
+) -> UIResult<()> {
+    grid_node_map.del_at_line(line_nr, index)?;
+    code_lines.del_at_line(line_nr, index)
 }

editor/src/editor/mvc/ed_update.rs

@@ -1,5 +1,6 @@
 use crate::editor::code_lines::CodeLines;
 use crate::editor::ed_error::EdResult;
+use crate::editor::grid_node_map::insert_between_line_both;
 use crate::editor::grid_node_map::GridNodeMap;
 use crate::editor::markup::attribute::Attributes;
 use crate::editor::markup::nodes;
@@ -33,6 +34,8 @@ impl<'a> EdModel<'a> {
         move_fun: MoveCaretFun<CodeLines>,
         modifiers: &Modifiers,
     ) -> UIResult<()> {
+        self.dirty = true;
+
         for caret_tup in self.caret_w_select_vec.iter_mut() {
             caret_tup.0 = move_fun(&self.code_lines, caret_tup.0, modifiers)?;
             caret_tup.1 = None;
@@ -41,7 +44,6 @@ impl<'a> EdModel<'a> {
         Ok(())
     }
 
-    // TODO delete once we use `GridNodeMap` for everything that now uses `CodeLines`.
     // disregards EdModel.code_lines because the caller knows the resulting caret position will be valid.
     // allows us to prevent multiple updates to EdModel.code_lines
     pub fn simple_move_carets_right(&mut self) {
@@ -328,8 +330,27 @@ pub fn handle_new_char(received_char: &char, ed_model: &mut EdModel) -> EdResult
         }
         ch => {
             let old_caret_pos = ed_model.get_caret();
-            let curr_mark_node_id = ed_model.grid_node_map.get_id_at_row_col(ed_model.get_caret())?;
+            let curr_mark_node_id = ed_model.grid_node_map.get_id_at_row_col(old_caret_pos)?;
             let curr_mark_node = ed_model.markup_node_pool.get(curr_mark_node_id);
+
+            let prev_mark_node_id_opt =
+                if old_caret_pos.column > 0 {
+                    let prev_mark_node_id = ed_model.grid_node_map.get_id_at_row_col(
+                        TextPos {
+                            line: old_caret_pos.line,
+                            column: old_caret_pos.column - 1,
+                        }
+                    )?;
+
+                    Some(prev_mark_node_id)
+                } else {
+                    None
+                };
+
+            let prev_mark_node_opt = prev_mark_node_id_opt.map(
+                |prev_mark_node_id| ed_model.markup_node_pool.get(prev_mark_node_id)
+            );
+
             let parent_id_opt = curr_mark_node.get_parent_id_opt();
             let ast_node_id = curr_mark_node.get_ast_node_id();
 
@@ -338,75 +359,88 @@ pub fn handle_new_char(received_char: &char, ed_model: &mut EdModel) -> EdResult
             if let Some(parent_id) = parent_id_opt {
                 let sibling_ids = curr_mark_node.get_sibling_ids(&ed_model.markup_node_pool);
 
-                let new_child_index = index_of(curr_mark_node_id, &sibling_ids)? + 1;
+                let new_child_index = index_of(curr_mark_node_id, &sibling_ids)?;
 
                 match ast_node_ref {
                     Expr2::EmptyRecord => {
-                        if curr_mark_node.get_content()? == nodes::LEFT_ACCOLADE {
+                        if let Some(prev_mark_node) = prev_mark_node_opt {
+                            if prev_mark_node.get_content()? == nodes::LEFT_ACCOLADE {
+                                // update Markup
+                                let record_field_str = &ch.to_string();
 
-                            // update Markup
+                                let record_field_node = MarkupNode::Text {
-                            let record_field_str = &ch.to_string();
+                                    content: record_field_str.to_owned(),
-
+                                    ast_node_id,
-                            let record_field_node = MarkupNode::Text {
+                                    syn_high_style: HighlightStyle::RecordField,
-                                content: record_field_str.to_owned(),
+                                    attributes: Attributes::new(),
-                                ast_node_id,
+                                    parent_id_opt: Some(parent_id),
-                                syn_high_style: HighlightStyle::RecordField,
-                                attributes: Attributes::new(),
-                                parent_id_opt: Some(parent_id),
-                            };
-
-                            let record_field_node_id = ed_model.markup_node_pool.add(record_field_node);
-
-                            let parent = ed_model.markup_node_pool.get_mut(parent_id);
-                            parent.add_child_at_index(new_child_index, record_field_node_id)?;
-
-                            // update caret
-                            ed_model.simple_move_carets_right();
-
-                            // update GridNodeMap
-                            ed_model.grid_node_map.insert_between_line(
-                                old_caret_pos.line,
-                                old_caret_pos.column + 1,
-                                record_field_str.len(),
-                                record_field_node_id,
-                            )?;
-
-                            // update AST
-                            let record_var = ed_model.module.env.var_store.fresh();
-                            let field_name = PoolStr::new(record_field_str, &mut ed_model.module.env.pool);
-                            let field_var = ed_model.module.env.var_store.fresh();
-                            //TODO actually check if field_str belongs to a previously defined variable
-                            let field_val = Expr2::InvalidLookup(
-                                                PoolStr::new(record_field_str, ed_model.module.env.pool)
-                                            );
-                            let field_val_id = ed_model.module.env.pool.add(field_val);
-                            let first_field = (field_name, field_var, field_val_id);
-
-                            let fields = PoolVec::new(
-                                vec![first_field].into_iter(),
-                                &mut ed_model.module.env.pool,
-                            );
-
-                            let new_ast_node =
-                                Expr2::Record {
-                                    record_var,
-                                    fields,
                                 };
 
-                            ed_model.module.env.pool.set(ast_node_id, new_ast_node);
+                                let record_field_node_id = ed_model.markup_node_pool.add(record_field_node);
-                        } else {
+
-                            unimplemented!("TODO handle this else")
+                                let parent = ed_model.markup_node_pool.get_mut(parent_id);
+                                parent.add_child_at_index(new_child_index, record_field_node_id)?;
+
+                                // update caret
+                                ed_model.simple_move_carets_right();
+
+                                // update GridNodeMap and CodeLines
+                                insert_between_line_both(
+                                    &mut ed_model.grid_node_map,
+                                    &mut ed_model.code_lines,
+                                    old_caret_pos.line,
+                                    old_caret_pos.column,
+                                    record_field_str,
+                                    record_field_node_id,
+                                )?;
+
+                                // update AST
+                                let record_var = ed_model.module.env.var_store.fresh();
+                                let field_name = PoolStr::new(record_field_str, &mut ed_model.module.env.pool);
+                                let field_var = ed_model.module.env.var_store.fresh();
+                                //TODO actually check if field_str belongs to a previously defined variable
+                                let field_val = Expr2::InvalidLookup(
+                                                    PoolStr::new(record_field_str, ed_model.module.env.pool)
+                                                );
+                                let field_val_id = ed_model.module.env.pool.add(field_val);
+                                let first_field = (field_name, field_var, field_val_id);
+
+                                let fields = PoolVec::new(
+                                    vec![first_field].into_iter(),
+                                    &mut ed_model.module.env.pool,
+                                );
+
+                                let new_ast_node =
+                                    Expr2::Record {
+                                        record_var,
+                                        fields,
+                                    };
+
+                                ed_model.module.env.pool.set(ast_node_id, new_ast_node);
+                            }
                         }
                     },
                     Expr2::Record { record_var:_, fields } => {
-                        update_record_field(
+                        if let Some(prev_mark_node_id) = prev_mark_node_id_opt {
-                            &ch.to_string(),
+                            let prev_mark_node = ed_model.markup_node_pool.get(prev_mark_node_id);
-                            old_caret_pos,
+                            let prev_node_content = prev_mark_node.get_content()?;
-                            curr_mark_node_id,
+
-                            new_child_index,
+                            let node_to_update_id =
-                            fields,
+                                if prev_node_content == nodes::LEFT_ACCOLADE {
-                            ed_model,
+                                    curr_mark_node_id
-                        )?;
+                                } else {
+                                    // caret is already past field so we need the previous MarkupNode
+                                    prev_mark_node_id
+                                };
+
+                            update_record_field(
+                                &ch.to_string(),
+                                old_caret_pos,
+                                node_to_update_id,
+                                fields,
+                                ed_model,
+                            )?;
+                        }
                     },
                     other => {
                         unimplemented!("TODO implement updating of Expr2 {:?}.", other)

editor/src/editor/mvc/record_update.rs

@@ -1,6 +1,9 @@
 use crate::editor::ed_error::EdResult;
 use crate::editor::ed_error::MissingParent;
 use crate::editor::ed_error::RecordWithoutFields;
+use crate::editor::grid_node_map::add_to_line_both;
+use crate::editor::grid_node_map::del_at_line_both;
+use crate::editor::grid_node_map::insert_between_line_both;
 use crate::editor::markup::attribute::Attributes;
 use crate::editor::markup::nodes;
 use crate::editor::markup::nodes::MarkupNode;
@@ -65,20 +68,32 @@ pub fn start_new_record(ed_model: &mut EdModel) -> EdResult<()> {
     if is_blank_node {
         mark_node_pool.replace_node(curr_mark_node_id, nested_node);
 
+        // remove data corresponding to Blank node
+        del_at_line_both(
+            &mut ed_model.grid_node_map,
+            &mut ed_model.code_lines,
+            old_caret_pos.line,
+            old_caret_pos.column,
+        )?;
+
         for _ in 0..nodes::LEFT_ACCOLADE.len() {
             ed_model.simple_move_carets_right();
         }
 
-        // update GridNodeMap
+        // update GridNodeMap and CodeLines
-        ed_model.grid_node_map.add_to_line(
+        add_to_line_both(
+            &mut ed_model.grid_node_map,
+            &mut ed_model.code_lines,
             old_caret_pos.line,
-            nodes::LEFT_ACCOLADE.len(),
+            nodes::LEFT_ACCOLADE,
             left_bracket_node_id,
         )?;
 
-        ed_model.grid_node_map.add_to_line(
+        add_to_line_both(
+            &mut ed_model.grid_node_map,
+            &mut ed_model.code_lines,
             old_caret_pos.line,
-            nodes::RIGHT_ACCOLADE.len(),
+            nodes::RIGHT_ACCOLADE,
             right_bracket_node_id,
         )?;
     }
@@ -90,54 +105,54 @@ pub fn update_record_field(
     new_input: &str,
     old_caret_pos: TextPos,
     curr_mark_node_id: MarkNodeId,
-    new_child_index: usize,
     record_fields: &PoolVec<(PoolStr, Variable, NodeId<Expr2>)>,
     ed_model: &mut EdModel,
 ) -> EdResult<()> {
-    if new_child_index == 2 {
+    // update MarkupNode
-        // update MarkupNode
+    let curr_mark_node_mut = ed_model.markup_node_pool.get_mut(curr_mark_node_id);
-        let curr_mark_node_mut = ed_model.markup_node_pool.get_mut(curr_mark_node_id);
+    let content_str_mut = curr_mark_node_mut.get_content_mut()?;
-        let content_str_mut = curr_mark_node_mut.get_content_mut()?;
+    let node_caret_offset = ed_model
-        content_str_mut.push_str(new_input);
+        .grid_node_map
+        .get_offset_to_node_id(old_caret_pos, curr_mark_node_id)?;
+    content_str_mut.insert_str(node_caret_offset, new_input);
 
-        // update caret
+    // update caret
-        for _ in 0..new_input.len() {
+    for _ in 0..new_input.len() {
-            ed_model.simple_move_carets_right();
+        ed_model.simple_move_carets_right();
-        }
-
-        // update GridNodeMap
-        ed_model.grid_node_map.insert_between_line(
-            old_caret_pos.line,
-            old_caret_pos.column,
-            new_input.len(),
-            curr_mark_node_id,
-        )?;
-
-        // update AST Node
-        let first_field = record_fields
-            .iter(ed_model.module.env.pool)
-            .next()
-            .with_context(|| RecordWithoutFields {})?;
-
-        let mut new_field_name = String::new();
-
-        first_field.0.as_str(ed_model.module.env.pool).to_string();
-
-        new_field_name.push_str(new_input);
-
-        let new_pool_str = PoolStr::new(&new_field_name, &mut ed_model.module.env.pool);
-
-        let first_field_mut = record_fields
-            .iter_mut(ed_model.module.env.pool)
-            .next()
-            .with_context(|| RecordWithoutFields {})?;
-
-        first_field_mut.0 = new_pool_str;
-
-        Ok(())
-    } else {
-        unimplemented!("TODO implement updating of other fields of record.")
     }
+
+    // update GridNodeMap and CodeLines
+    insert_between_line_both(
+        &mut ed_model.grid_node_map,
+        &mut ed_model.code_lines,
+        old_caret_pos.line,
+        old_caret_pos.column,
+        new_input,
+        curr_mark_node_id,
+    )?;
+
+    // update AST Node
+    let first_field = record_fields
+        .iter(ed_model.module.env.pool)
+        .next()
+        .with_context(|| RecordWithoutFields {})?;
+
+    let mut new_field_name = String::new();
+
+    first_field.0.as_str(ed_model.module.env.pool).to_string();
+
+    new_field_name.push_str(new_input);
+
+    let new_pool_str = PoolStr::new(&new_field_name, &mut ed_model.module.env.pool);
+
+    let first_field_mut = record_fields
+        .iter_mut(ed_model.module.env.pool)
+        .next()
+        .with_context(|| RecordWithoutFields {})?;
+
+    first_field_mut.0 = new_pool_str;
+
+    Ok(())
 }
 
 pub fn update_record_colon(ed_model: &mut EdModel) -> EdResult<()> {
@@ -152,7 +167,7 @@ pub fn update_record_colon(ed_model: &mut EdModel) -> EdResult<()> {
     if let Some(parent_id) = parent_id_opt {
         let sibling_ids = curr_mark_node.get_sibling_ids(&ed_model.markup_node_pool);
 
-        let new_child_index = index_of(curr_mark_node_id, &sibling_ids)? + 1;
+        let new_child_index = index_of(curr_mark_node_id, &sibling_ids)?;
 
         let ast_node_ref = ed_model.module.env.pool.get(ast_node_id);
 
@@ -196,16 +211,20 @@ pub fn update_record_colon(ed_model: &mut EdModel) -> EdResult<()> {
                     ed_model.simple_move_carets_right();
                 }
 
-                // update GridNodeMap
+                // update GridNodeMap and CodeLines
-                ed_model.grid_node_map.add_to_line(
+                add_to_line_both(
+                    &mut ed_model.grid_node_map,
+                    &mut ed_model.code_lines,
                     old_caret_pos.line,
-                    nodes::COLON.len(),
+                    nodes::COLON,
                     record_colon_node_id,
                 )?;
 
-                ed_model.grid_node_map.add_to_line(
+                add_to_line_both(
+                    &mut ed_model.grid_node_map,
+                    &mut ed_model.code_lines,
                     old_caret_pos.line,
-                    nodes::BLANK_PLACEHOLDER.len(),
+                    nodes::BLANK_PLACEHOLDER,
                     record_blank_node_id,
                 )?;
 

editor/src/ui/text/big_text_area.rs

@@ -113,6 +113,10 @@ impl Lines for BigTextArea {
         lines
     }
 
+    fn is_last_line(&self, line_nr: usize) -> bool {
+        line_nr == self.nr_of_lines() - 1
+    }
+
     fn last_char(&self, line_nr: usize) -> UIResult<Option<char>> {
         Ok(self.get_line(line_nr)?.chars().last())
     }

editor/src/ui/text/lines.rs

@@ -26,6 +26,8 @@ pub trait Lines {
 
     fn all_lines<'a>(&self, arena: &'a Bump) -> BumpString<'a>;
 
+    fn is_last_line(&self, line_nr: usize) -> bool;
+
     fn last_char(&self, line_nr: usize) -> UIResult<Option<char>>;
 }
 
@@ -175,20 +177,17 @@ pub fn move_caret_right<T: Lines>(
             None => unreachable!(),
         }
     } else {
-        let curr_line = lines.get_line(old_line_nr)?;
+        let curr_line_len = lines.line_len(old_line_nr)?;
+        let is_last_line = lines.is_last_line(old_line_nr);
 
-        if let Some(last_char) = curr_line.chars().last() {
+        if !is_last_line {
-            if is_newline(&last_char) {
+            if old_col_nr + 1 > curr_line_len - 1 {
-                if old_col_nr + 1 > curr_line.len() - 1 {
+                (old_line_nr + 1, 0)
-                    (old_line_nr + 1, 0)
-                } else {
-                    (old_line_nr, old_col_nr + 1)
-                }
-            } else if old_col_nr < curr_line.len() {
-                (old_line_nr, old_col_nr + 1)
             } else {
-                (old_line_nr, old_col_nr)
+                (old_line_nr, old_col_nr + 1)
             }
+        } else if old_col_nr < curr_line_len {
+            (old_line_nr, old_col_nr + 1)
         } else {
             (old_line_nr, old_col_nr)
         }
@@ -322,17 +321,15 @@ pub fn move_caret_down<T: Lines>(
 
         (old_line_nr, curr_line_len)
     } else {
-        let next_line = lines.get_line(old_line_nr + 1)?;
+        let next_line_index = old_line_nr + 1;
+        let next_line_len = lines.line_len(next_line_index)?;
+        let is_last_line = lines.is_last_line(next_line_index);
 
-        if next_line.len() <= old_col_nr {
+        if next_line_len <= old_col_nr {
-            if let Some(last_char) = next_line.chars().last() {
+            if !is_last_line {
-                if is_newline(&last_char) {
+                (old_line_nr + 1, next_line_len - 1)
-                    (old_line_nr + 1, next_line.len() - 1)
-                } else {
-                    (old_line_nr + 1, next_line.len())
-                }
             } else {
-                (old_line_nr + 1, 0)
+                (old_line_nr + 1, next_line_len)
             }
         } else {
             (old_line_nr + 1, old_col_nr)