diff --git a/.github/workflows/benchmarks.yml b/.github/workflows/benchmarks.yml index f6327d5bb8..1279035120 100644 --- a/.github/workflows/benchmarks.yml +++ b/.github/workflows/benchmarks.yml @@ -31,13 +31,5 @@ jobs: - name: on current branch; prepare a self-contained benchmark folder run: ./ci/safe-earthly.sh +prep-bench-folder - - name: benchmark trunk - run: ulimit -s unlimited && cd bench-folder-trunk && ./target/release/deps/time_bench --bench - # ulimit to prevent stack overflow on cfold - - - name: move benchmark results so they can be compared later - run: cp -r bench-folder-trunk/target/criterion bench-folder-branch/target/ - - - name: benchmark current branch - run: ulimit -s unlimited && cd bench-folder-branch && ./target/release/deps/time_bench --bench - # ulimit to prevent stack overflow on cfold + - name: execute benchmarks with regression check + run: ./ci/bench-runner.sh diff --git a/ci/bench-runner.sh b/ci/bench-runner.sh new file mode 100755 index 0000000000..0038612c63 --- /dev/null +++ b/ci/bench-runner.sh @@ -0,0 +1,67 @@ +#!/usr/bin/env bash + +# script to return exit code 1 if benchmarks have regressed + +# benchmark trunk +ulimit -s unlimited +cd bench-folder-trunk +./target/release/deps/time_bench --bench +cd .. + +# move benchmark results so they can be compared later +cp -r bench-folder-trunk/target/criterion bench-folder-branch/target/ + +cd bench-folder-branch + +LOG_FILE="bench_log.txt" +touch $LOG_FILE + +FULL_CMD=" ./target/release/deps/time_bench --bench" +echo $FULL_CMD + +script -efq $LOG_FILE -c "$FULL_CMD" +EXIT_CODE=$? + +if grep -q "regressed" "$LOG_FILE"; then + echo "" + echo "" + echo "------<<<<<<>>>>>>------" + echo "Benchmark detected regression. Running benchmark again to confirm..." + echo "------<<<<<<>>>>>>------" + echo "" + echo "" + + # delete criterion folder to compare to trunk only + rm -rf ./target/criterion + # copy benchmark data from trunk again + cp -r ../bench-folder-trunk/target/criterion ./target + + rm $LOG_FILE + touch $LOG_FILE + + script -efq $LOG_FILE -c "$FULL_CMD" + EXIT_CODE=$? + + if grep -q "regressed" "$LOG_FILE"; then + echo "" + echo "" + echo "------<<<<<>>>>>------" + echo "Benchmarks were run twice and a regression was detected both times." + echo "------<<<<<>>>>>------" + echo "" + echo "" + exit 1 + else + echo "Benchmarks were run twice and a regression was detected on one run. We assume this was a fluke." + exit 0 + fi +else + echo "" + echo "" + echo "------<<<<<>>>>>------" + echo "Benchmark execution failed with exit code: $EXIT_CODE." + echo "------<<<<<>>>>>------" + echo "" + echo "" + exit $EXIT_CODE +fi \ No newline at end of file diff --git a/editor/src/editor/ed_error.rs b/editor/src/editor/ed_error.rs index a21cb2de86..19a6124606 100644 --- a/editor/src/editor/ed_error.rs +++ b/editor/src/editor/ed_error.rs @@ -1,5 +1,5 @@ -use crate::editor::slow_pool::MarkNodeId; use crate::ui::ui_error::UIResult; +use crate::{editor::slow_pool::MarkNodeId, ui::text::text_pos::TextPos}; use colored::*; use snafu::{Backtrace, ErrorCompat, NoneError, ResultExt, Snafu}; @@ -111,6 +111,15 @@ pub enum EdError { backtrace: Backtrace, }, + #[snafu(display( + "NoNodeAtCaretPosition: there was no node at the current caret position {:?}.", + caret_pos, + ))] + NoNodeAtCaretPosition { + caret_pos: TextPos, + backtrace: Backtrace, + }, + #[snafu(display( "UnexpectedASTNode: required a {} at this position, node was a {}.", required_node_type, diff --git a/editor/src/editor/markup/nodes.rs b/editor/src/editor/markup/nodes.rs index ed8c8586ec..1d1416bb3f 100644 --- a/editor/src/editor/markup/nodes.rs +++ b/editor/src/editor/markup/nodes.rs @@ -2,16 +2,19 @@ use super::attribute::Attributes; use crate::editor::ed_error::EdResult; use crate::editor::ed_error::ExpectedTextNode; use crate::editor::ed_error::GetContentOnNestedNode; -use crate::editor::ed_error::NestedNodeRequired; +use crate::editor::ed_error::{NestedNodeMissingChild, NestedNodeRequired}; use crate::editor::slow_pool::MarkNodeId; use crate::editor::slow_pool::SlowPool; use crate::editor::syntax_highlight::HighlightStyle; +use crate::editor::util::index_of; +use crate::lang::ast::ExprId; use crate::lang::ast::RecordField; use crate::lang::{ ast::Expr2, expr::Env, pool::{NodeId, PoolStr}, }; +use crate::ui::util::slice_get; use bumpalo::Bump; use std::fmt; @@ -72,6 +75,81 @@ impl MarkupNode { } } + // return (index of child in list of children, closest ast index of child corresponding to ast node) + pub fn get_child_indices( + &self, + child_id: MarkNodeId, + markup_node_pool: &SlowPool, + ) -> EdResult<(usize, usize)> { + match self { + MarkupNode::Nested { children_ids, .. } => { + let mark_position_opt = children_ids.iter().position(|&c_id| c_id == child_id); + + if let Some(child_index) = mark_position_opt { + let self_ast_id = self.get_ast_node_id(); + + let child_ids_with_ast = children_ids + .iter() + .filter(|c_id| { + let child_mark_node = markup_node_pool.get(**c_id); + // a node that points to the same ast_node as the parent is a ',', '[', ']' + // those are not "real" ast children + child_mark_node.get_ast_node_id() != self_ast_id + }) + .copied() + .collect::>(); + + if child_index == (children_ids.len() - 1) { + let ast_child_index = child_ids_with_ast.len(); + + Ok((child_index, ast_child_index)) + } else { + // we want to find the index of the closest ast mark node to child_index + let indices_in_mark_res: EdResult> = child_ids_with_ast + .iter() + .map(|c_id| index_of(*c_id, children_ids)) + .collect(); + + let indices_in_mark = indices_in_mark_res?; + + let mut last_diff = usize::MAX; + let mut best_index = 0; + + for index in indices_in_mark.iter() { + let curr_diff = + isize::abs((*index as isize) - (child_index as isize)) as usize; + + if curr_diff >= last_diff { + break; + } else { + last_diff = curr_diff; + best_index = *index; + } + } + + let closest_ast_child = slice_get(best_index, &children_ids)?; + + let closest_ast_child_index = + index_of(*closest_ast_child, &child_ids_with_ast)?; + + // +1 because we want to insert after ast_child + Ok((child_index, closest_ast_child_index + 1)) + } + } else { + NestedNodeMissingChild { + node_id: child_id, + children_ids: children_ids.clone(), + } + .fail() + } + } + _ => NestedNodeRequired { + node_type: self.node_type_as_string(), + } + .fail(), + } + } + // can't be &str, this creates borrowing issues pub fn get_content(&self) -> EdResult { match self { @@ -146,6 +224,7 @@ pub const RIGHT_ACCOLADE: &str = " }"; pub const LEFT_SQUARE_BR: &str = "[ "; pub const RIGHT_SQUARE_BR: &str = " ]"; pub const COLON: &str = ": "; +pub const COMMA: &str = ", "; pub const STRING_QUOTES: &str = "\"\""; fn new_markup_node( @@ -177,12 +256,16 @@ pub fn expr2_to_markup<'a, 'b>( Expr2::SmallInt { text, .. } | Expr2::I128 { text, .. } | Expr2::U128 { text, .. } - | Expr2::Float { text, .. } => new_markup_node( - get_string(env, &text), - expr2_node_id, - HighlightStyle::Number, - markup_node_pool, - ), + | Expr2::Float { text, .. } => { + let num_str = get_string(env, &text); + + new_markup_node( + num_str, + expr2_node_id, + HighlightStyle::Number, + markup_node_pool, + ) + } Expr2::Str(text) => new_markup_node( "\"".to_owned() + text.as_str(env.pool) + "\"", expr2_node_id, @@ -217,21 +300,24 @@ pub fn expr2_to_markup<'a, 'b>( markup_node_pool, )]; - for (idx, node_id) in elems.iter_node_ids().enumerate() { - let sub_expr2 = env.pool.get(node_id); + let indexed_node_ids: Vec<(usize, ExprId)> = + elems.iter(env.pool).copied().enumerate().collect(); + + for (idx, node_id) in indexed_node_ids.iter() { + let sub_expr2 = env.pool.get(*node_id); children_ids.push(expr2_to_markup( arena, env, sub_expr2, - node_id, + *node_id, markup_node_pool, )); if idx + 1 < elems.len() { children_ids.push(new_markup_node( ", ".to_string(), - node_id, + expr2_node_id, HighlightStyle::Operator, markup_node_pool, )); diff --git a/editor/src/editor/mvc/ed_model.rs b/editor/src/editor/mvc/ed_model.rs index 3700163417..b57a36df8a 100644 --- a/editor/src/editor/mvc/ed_model.rs +++ b/editor/src/editor/mvc/ed_model.rs @@ -4,7 +4,7 @@ use crate::editor::slow_pool::{MarkNodeId, SlowPool}; use crate::editor::syntax_highlight::HighlightStyle; use crate::editor::{ ed_error::EdError::ParseError, - ed_error::EdResult, + ed_error::{EdResult, MissingParent, NoNodeAtCaretPosition}, markup::attribute::Attributes, markup::nodes::{expr2_to_markup, set_parent_for_all, MarkupNode}, }; @@ -134,6 +134,29 @@ impl<'a> EdModel<'a> { pub fn node_exists_at_caret(&self) -> bool { self.grid_node_map.node_exists_at_pos(self.get_caret()) } + + // return (index of child in list of children, closest ast index of child corresponding to ast node) of MarkupNode at current caret position + pub fn get_curr_child_indices(&self) -> EdResult<(usize, usize)> { + if self.node_exists_at_caret() { + let curr_mark_node_id = self.get_curr_mark_node_id()?; + let curr_mark_node = self.markup_node_pool.get(curr_mark_node_id); + + if let Some(parent_id) = curr_mark_node.get_parent_id_opt() { + let parent = self.markup_node_pool.get(parent_id); + parent.get_child_indices(curr_mark_node_id, &self.markup_node_pool) + } else { + MissingParent { + node_id: curr_mark_node_id, + } + .fail() + } + } else { + NoNodeAtCaretPosition { + caret_pos: self.get_caret(), + } + .fail() + } + } } #[derive(Debug)] @@ -159,8 +182,7 @@ impl<'a> EdModule<'a> { let ast_root_id = env.pool.add(expr2); // for debugging - // let expr2_str = expr2_to_string(ast_root_id, env.pool); - // println!("expr2_string: {}", expr2_str); + // dbg!(expr2_to_string(ast_root_id, env.pool)); Ok(EdModule { env, ast_root_id }) } diff --git a/editor/src/editor/mvc/ed_update.rs b/editor/src/editor/mvc/ed_update.rs index ba615e28bf..cf510dfd7f 100644 --- a/editor/src/editor/mvc/ed_update.rs +++ b/editor/src/editor/mvc/ed_update.rs @@ -13,7 +13,7 @@ use crate::editor::mvc::ed_model::EdModel; use crate::editor::mvc::ed_model::SelectedExpression; use crate::editor::mvc::int_update::start_new_int; use crate::editor::mvc::int_update::update_int; -use crate::editor::mvc::list_update::{prep_empty_list, start_new_list}; +use crate::editor::mvc::list_update::{add_blank_child, start_new_list}; use crate::editor::mvc::lookup_update::update_invalid_lookup; use crate::editor::mvc::record_update::start_new_record; use crate::editor::mvc::record_update::update_empty_record; @@ -80,6 +80,15 @@ impl<'a> EdModel<'a> { } } + // 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_left(&mut self, repeat: usize) { + for caret_tup in self.caret_w_select_vec.iter_mut() { + caret_tup.0.caret_pos.column -= repeat; + caret_tup.1 = None; + } + } + pub fn build_node_map_from_markup( markup_root_id: MarkNodeId, markup_node_pool: &SlowPool, @@ -687,13 +696,13 @@ pub fn handle_new_char(received_char: &char, ed_model: &mut EdModel) -> EdResult Expr2::List{ elem_var: _, elems: _} => { let prev_mark_node = ed_model.markup_node_pool.get(prev_mark_node_id); - if prev_mark_node.get_content()? == nodes::LEFT_SQUARE_BR { - if curr_mark_node.get_content()? == nodes::RIGHT_SQUARE_BR { - prep_empty_list(ed_model)?; // insert a Blank first, this results in cleaner code - handle_new_char(received_char, ed_model)? - } else { - InputOutcome::Ignored - } + if prev_mark_node.get_content()? == nodes::LEFT_SQUARE_BR && curr_mark_node.get_content()? == nodes::RIGHT_SQUARE_BR { + // based on if, we are at the start of the list + let new_child_index = 1; + let new_ast_child_index = 0; + // insert a Blank first, this results in cleaner code + add_blank_child(new_child_index, new_ast_child_index, ed_model)?; + handle_new_char(received_char, ed_model)? } else { InputOutcome::Ignored } @@ -726,19 +735,54 @@ pub fn handle_new_char(received_char: &char, ed_model: &mut EdModel) -> EdResult } else { InputOutcome::Ignored } - } else if "\"{[".contains(*ch) { - let prev_mark_node = ed_model.markup_node_pool.get(prev_mark_node_id); + } else if *ch == ',' { + if curr_mark_node.get_content()? == nodes::LEFT_SQUARE_BR { + InputOutcome::Ignored + } else { + let mark_parent_id_opt = curr_mark_node.get_parent_id_opt(); - if prev_mark_node.get_content()? == nodes::LEFT_SQUARE_BR { - if curr_mark_node.get_content()? == nodes::RIGHT_SQUARE_BR { - prep_empty_list(ed_model)?; // insert a Blank first, this results in cleaner code - handle_new_char(received_char, ed_model)? + if let Some(mark_parent_id) = mark_parent_id_opt { + let parent_ast_id = ed_model.markup_node_pool.get(mark_parent_id).get_ast_node_id(); + let parent_expr2 = ed_model.module.env.pool.get(parent_ast_id); + + match parent_expr2 { + Expr2::List { elem_var:_, elems:_} => { + + let (new_child_index, new_ast_child_index) = ed_model.get_curr_child_indices()?; + // insert a Blank first, this results in cleaner code + add_blank_child( + new_child_index, + new_ast_child_index, + ed_model + )? + } + Expr2::Record { record_var:_, fields:_ } => { + todo!("multiple record fields") + } + _ => { + InputOutcome::Ignored + } + } } else { InputOutcome::Ignored } + } + } else if "\"{[".contains(*ch) { + let prev_mark_node = ed_model.markup_node_pool.get(prev_mark_node_id); + + if prev_mark_node.get_content()? == nodes::LEFT_SQUARE_BR && curr_mark_node.get_content()? == nodes::RIGHT_SQUARE_BR { + let (new_child_index, new_ast_child_index) = ed_model.get_curr_child_indices()?; + // insert a Blank first, this results in cleaner code + add_blank_child( + new_child_index, + new_ast_child_index, + ed_model + )?; + handle_new_char(received_char, ed_model)? } else { InputOutcome::Ignored } + } else { InputOutcome::Ignored } @@ -849,7 +893,13 @@ pub mod test_ed_update { let mut ed_model = ed_model_from_dsl(&code_str, pre_lines, &mut model_refs)?; for input_char in new_char_seq.chars() { - ed_res_to_res(handle_new_char(&input_char, &mut ed_model))?; + if input_char == '👲' { + ed_model.simple_move_carets_right(1); + } else if input_char == '👰' { + ed_model.simple_move_carets_left(1); + } else { + ed_res_to_res(handle_new_char(&input_char, &mut ed_model))?; + } } let post_lines = ui_res_to_res(ed_model_to_dsl(&ed_model))?; @@ -1523,7 +1573,7 @@ pub mod test_ed_update { } #[test] - fn test_list() -> Result<(), String> { + fn test_single_elt_list() -> Result<(), String> { assert_insert(&["┃"], &["[ ┃ ]"], '[')?; assert_insert_seq(&["┃"], &["[ 0┃ ]"], "[0")?; @@ -1559,7 +1609,48 @@ pub mod test_ed_update { } #[test] - fn test_ignore_list() -> Result<(), String> { + fn test_multi_elt_list() -> Result<(), String> { + assert_insert_seq(&["┃"], &["[ 0, 1┃ ]"], "[0,1")?; + assert_insert_seq(&["┃"], &["[ 987, 6543, 210┃ ]"], "[987,6543,210")?; + + assert_insert_seq( + &["┃"], + &["[ \"a\", \"bcd\", \"EFGH┃\" ]"], + "[\"a👲,\"bcd👲,\"EFGH", + )?; + + assert_insert_seq( + &["┃"], + &["[ { a: 1 }, { b: 23 }, { c: 456┃ } ]"], + "[{a:1👲👲,{b:23👲👲,{c:456", + )?; + + assert_insert_seq(&["┃"], &["[ [ 1 ], [ 23 ], [ 456┃ ] ]"], "[[1👲👲,[23👲👲,[456")?; + + // insert element in between + assert_insert_seq(&["┃"], &["[ 0, 2┃, 1 ]"], "[0,1👰👰👰,2")?; + assert_insert_seq(&["┃"], &["[ 0, 2, 3┃, 1 ]"], "[0,1👰👰👰,2,3")?; + assert_insert_seq(&["┃"], &["[ 0, 3┃, 2, 1 ]"], "[0,1👰👰👰,2👰👰👰,3")?; + + assert_insert_seq( + &["┃"], + &["[ \"abc\", \"f┃\", \"de\" ]"], + "[\"abc👲,\"de👰👰👰👰👰,\"f", + )?; + + assert_insert_seq(&["┃"], &["[ [ 0 ], [ 2┃ ], [ 1 ] ]"], "[[0👲👲,[1👰👰👰👰👰,[2")?; + + assert_insert_seq( + &["┃"], + &["[ { a: 0 }, { a: 2┃ }, { a: 1 } ]"], + "[{a:0👲👲,{a:1👰👰👰👰👰👰👰👰,{a:2", + )?; + + Ok(()) + } + + #[test] + fn test_ignore_single_elt_list() -> Result<(), String> { assert_insert_seq_ignore(&["┃[ ]"], IGNORE_CHARS)?; assert_insert_seq_ignore(&["[ ]┃"], IGNORE_CHARS)?; assert_insert_seq_ignore(&["[┃ ]"], IGNORE_CHARS)?; @@ -1606,6 +1697,51 @@ pub mod test_ed_update { Ok(()) } + #[test] + fn test_ignore_multi_elt_list() -> Result<(), String> { + assert_insert_seq_ignore(&["┃[ 0, 1 ]"], IGNORE_CHARS)?; + assert_insert_seq_ignore(&["[ 0, 1 ]┃"], IGNORE_CHARS)?; + assert_insert_seq_ignore(&["[┃ 0, 1 ]"], IGNORE_CHARS)?; + assert_insert_seq_ignore(&["[ 0, 1 ┃]"], IGNORE_CHARS)?; + assert_insert_seq_ignore(&["[ 0,┃ 1 ]"], IGNORE_CHARS)?; + + assert_insert_seq_ignore(&["┃[ 123, 56, 7 ]"], IGNORE_CHARS)?; + assert_insert_seq_ignore(&["[ 123, 56, 7 ]┃"], IGNORE_CHARS)?; + assert_insert_seq_ignore(&["[┃ 123, 56, 7 ]"], IGNORE_CHARS)?; + assert_insert_seq_ignore(&["[ 123, 56, 7 ┃]"], IGNORE_CHARS)?; + assert_insert_seq_ignore(&["[ 123,┃ 56, 7 ]"], IGNORE_CHARS)?; + assert_insert_seq_ignore(&["[ 123, 56,┃ 7 ]"], IGNORE_CHARS)?; + + assert_insert_seq_ignore(&["┃[ \"123\", \"56\", \"7\" ]"], IGNORE_CHARS)?; + assert_insert_seq_ignore(&["[ \"123\", \"56\", \"7\" ]┃"], IGNORE_CHARS)?; + assert_insert_seq_ignore(&["[┃ \"123\", \"56\", \"7\" ]"], IGNORE_CHARS)?; + assert_insert_seq_ignore(&["[ \"123\", \"56\", \"7\" ┃]"], IGNORE_CHARS)?; + assert_insert_seq_ignore(&["[ \"123\",┃ \"56\", \"7\" ]"], IGNORE_CHARS)?; + assert_insert_seq_ignore(&["[ \"123\", \"56\",┃ \"7\" ]"], IGNORE_CHARS)?; + + assert_insert_seq_ignore(&["┃[ { a: 0 }, { a: 1 } ]"], IGNORE_CHARS)?; + assert_insert_seq_ignore(&["[ { a: 0 }, { a: 1 } ]┃"], IGNORE_CHARS)?; + assert_insert_seq_ignore(&["[┃ { a: 0 }, { a: 1 } ]"], IGNORE_CHARS)?; + assert_insert_seq_ignore(&["[ { a: 0 }, { a: 1 } ┃]"], IGNORE_CHARS)?; + assert_insert_seq_ignore(&["[ { a: 0 },┃ { a: 1 } ]"], IGNORE_CHARS)?; + + assert_insert_seq_ignore(&["┃[ [ 0 ], [ 1 ] ]"], IGNORE_CHARS)?; + assert_insert_seq_ignore(&["[ [ 0 ], [ 1 ] ]┃"], IGNORE_CHARS)?; + assert_insert_seq_ignore(&["[┃ [ 0 ], [ 1 ] ]"], IGNORE_CHARS)?; + assert_insert_seq_ignore(&["[ [ 0 ], [ 1 ] ┃]"], IGNORE_CHARS)?; + assert_insert_seq_ignore(&["[ [ 0 ],┃ [ 1 ] ]"], IGNORE_CHARS)?; + assert_insert_seq_ignore(&["[ ┃[ 0 ], [ 1 ] ]"], IGNORE_CHARS)?; + assert_insert_seq_ignore(&["[ [ 0 ]┃, [ 1 ] ]"], IGNORE_CHARS)?; + assert_insert_seq_ignore(&["[ [┃ 0 ], [ 1 ] ]"], IGNORE_CHARS)?; + assert_insert_seq_ignore(&["[ [ 0 ┃], [ 1 ] ]"], IGNORE_CHARS)?; + assert_insert_seq_ignore(&["[ [ 0 ], ┃[ 1 ] ]"], IGNORE_CHARS)?; + assert_insert_seq_ignore(&["[ [ 0 ], [┃ 1 ] ]"], IGNORE_CHARS)?; + assert_insert_seq_ignore(&["[ [ 0 ], [ 1 ]┃ ]"], IGNORE_CHARS)?; + assert_insert_seq_ignore(&["[ [ 0 ], [ 1 ┃] ]"], IGNORE_CHARS)?; + + Ok(()) + } + // Create ed_model from pre_lines DSL, do ctrl+shift+up as many times as repeat. // check if modified ed_model has expected string representation of code, caret position and active selection. pub fn assert_ctrl_shift_up_repeat( @@ -1879,7 +2015,11 @@ pub mod test_ed_update { let mut ed_model = ed_model_from_dsl(&code_str, pre_lines, &mut model_refs)?; for input_char in new_char_seq.chars() { - ed_res_to_res(handle_new_char(&input_char, &mut ed_model))?; + if input_char == '👲' { + ed_model.simple_move_carets_right(1); + } else { + ed_res_to_res(handle_new_char(&input_char, &mut ed_model))?; + } } for expected_tooltip in expected_tooltips.iter() { @@ -1936,16 +2076,13 @@ pub mod test_ed_update { assert_type_tooltip_clean(&["{ ┃z: { } }"], "{ z : {} }")?; assert_type_tooltip_clean(&["{ camelCase: ┃0 }"], "Num *")?; - assert_type_tooltips_seq(&["┃"], &vec!["*"], "")?; - assert_type_tooltips_seq(&["┃"], &vec!["*", "{ a : * }"], "{a:")?; + assert_type_tooltips_seq(&["┃"], &["*"], "")?; + assert_type_tooltips_seq(&["┃"], &["*", "{ a : * }"], "{a:")?; - assert_type_tooltips_clean( - &["{ camelCase: ┃0 }"], - &vec!["Num *", "{ camelCase : Num * }"], - )?; + assert_type_tooltips_clean(&["{ camelCase: ┃0 }"], &["Num *", "{ camelCase : Num * }"])?; assert_type_tooltips_clean( &["{ a: { b: { c: \"hello┃, hello.0123456789ZXY{}[]-><-\" } } }"], - &vec![ + &[ "Str", "{ c : Str }", "{ b : { c : Str } }", @@ -1953,13 +2090,18 @@ pub mod test_ed_update { ], )?; + Ok(()) + } + + #[test] + fn test_type_tooltip_list() -> Result<(), String> { assert_type_tooltip(&["┃"], "List *", '[')?; - assert_type_tooltips_seq(&["┃"], &vec!["List (Num *)"], "[0")?; - assert_type_tooltips_seq(&["┃"], &vec!["List (Num *)", "List (List (Num *))"], "[[0")?; - assert_type_tooltips_seq(&["┃"], &vec!["Str", "List Str"], "[\"a")?; + assert_type_tooltips_seq(&["┃"], &["List (Num *)"], "[0")?; + assert_type_tooltips_seq(&["┃"], &["List (Num *)", "List (List (Num *))"], "[[0")?; + assert_type_tooltips_seq(&["┃"], &["Str", "List Str"], "[\"a")?; assert_type_tooltips_seq( &["┃"], - &vec![ + &[ "Str", "List Str", "List (List Str)", @@ -1969,7 +2111,7 @@ pub mod test_ed_update { )?; assert_type_tooltips_seq( &["┃"], - &vec![ + &[ "{ a : Num * }", "List { a : Num * }", "List (List { a : Num * })", @@ -1977,6 +2119,34 @@ pub mod test_ed_update { "[[{a:1", )?; + // multi element lists + assert_type_tooltips_seq(&["┃"], &["List (Num *)"], "[1,2,3")?; + assert_type_tooltips_seq(&["┃"], &["Str", "List Str"], "[\"abc👲,\"de👲,\"f")?; + assert_type_tooltips_seq( + &["┃"], + &["{ a : Num * }", "List { a : Num * }"], + "[{a:0👲👲,{a:12👲👲,{a:444", + )?; + Ok(()) + } + + #[test] + fn test_type_tooltip_mismatch() -> Result<(), String> { + assert_type_tooltips_seq(&["┃"], &["Str", "List "], "[1,\"abc")?; + assert_type_tooltips_seq(&["┃"], &["List "], "[\"abc👲,50")?; + + assert_type_tooltips_seq( + &["┃"], + &["Str", "{ a : Str }", "List "], + "[{a:0👲👲,{a:\"0", + )?; + + assert_type_tooltips_seq( + &["┃"], + &["List (Num *)", "List (List )"], + "[[0,1,\"2👲👲👲,[3, 4, 5", + )?; + Ok(()) } diff --git a/editor/src/editor/mvc/list_update.rs b/editor/src/editor/mvc/list_update.rs index 8533967209..24cc7fab32 100644 --- a/editor/src/editor/mvc/list_update.rs +++ b/editor/src/editor/mvc/list_update.rs @@ -1,5 +1,5 @@ use crate::editor::ed_error::EdResult; -use crate::editor::ed_error::{MissingParent, UnexpectedASTNode, UnexpectedEmptyPoolVec}; +use crate::editor::ed_error::{MissingParent, UnexpectedASTNode}; use crate::editor::markup::attribute::Attributes; use crate::editor::markup::nodes; use crate::editor::markup::nodes::MarkupNode; @@ -7,11 +7,12 @@ use crate::editor::mvc::app_update::InputOutcome; use crate::editor::mvc::ed_model::EdModel; use crate::editor::mvc::ed_update::get_node_context; use crate::editor::mvc::ed_update::NodeContext; +use crate::editor::slow_pool::MarkNodeId; use crate::editor::syntax_highlight::HighlightStyle; -use crate::lang::ast::expr2_to_string; use crate::lang::ast::Expr2; +use crate::lang::ast::{expr2_to_string, ExprId}; use crate::lang::pool::PoolVec; -use snafu::OptionExt; +use crate::ui::text::text_pos::TextPos; pub fn start_new_list(ed_model: &mut EdModel) -> EdResult { let NodeContext { @@ -89,7 +90,11 @@ pub fn start_new_list(ed_model: &mut EdModel) -> EdResult { } // insert Blank at current position for easy code reuse -pub fn prep_empty_list(ed_model: &mut EdModel) -> EdResult { +pub fn add_blank_child( + new_child_index: usize, + new_ast_child_index: usize, + ed_model: &mut EdModel, +) -> EdResult { let NodeContext { old_caret_pos, curr_mark_node_id, @@ -98,67 +103,143 @@ pub fn prep_empty_list(ed_model: &mut EdModel) -> EdResult { ast_node_id, } = get_node_context(&ed_model)?; - let blank_elt = Expr2::Blank; + let trip_result: EdResult<(ExprId, ExprId, MarkNodeId)> = if let Some(parent_id) = parent_id_opt + { + let parent = ed_model.markup_node_pool.get(parent_id); - let list_ast_node = ed_model.module.env.pool.get(ast_node_id); + let list_ast_node_id = parent.get_ast_node_id(); + let list_ast_node = ed_model.module.env.pool.get(list_ast_node_id); + + match list_ast_node { + Expr2::List { + elem_var: _, + elems: _, + } => { + let blank_elt = Expr2::Blank; + let blank_elt_id = ed_model.module.env.pool.add(blank_elt); + + Ok((blank_elt_id, list_ast_node_id, parent_id)) + } + _ => UnexpectedASTNode { + required_node_type: "List".to_string(), + encountered_node_type: expr2_to_string(ast_node_id, ed_model.module.env.pool), + } + .fail(), + } + } else { + MissingParent { + node_id: curr_mark_node_id, + } + .fail() + }; + + let (blank_elt_id, list_ast_node_id, parent_id) = trip_result?; + + let list_ast_node = ed_model.module.env.pool.get(list_ast_node_id); match list_ast_node { - Expr2::List { elem_var, elems: _ } => { - let children: Vec = vec![blank_elt]; - let children_pool_vec = PoolVec::new(children.into_iter(), ed_model.module.env.pool); + Expr2::List { elem_var, elems } => { + let mut new_elems: Vec = + elems.iter(ed_model.module.env.pool).copied().collect(); - let blank_elt_id = - children_pool_vec - .iter_node_ids() - .next() - .context(UnexpectedEmptyPoolVec { - descriptive_vec_name: "\"children of List AST node\"", - })?; + new_elems.insert(new_ast_child_index, blank_elt_id); let new_list_node = Expr2::List { elem_var: *elem_var, - elems: children_pool_vec, + elems: PoolVec::new(new_elems.into_iter(), ed_model.module.env.pool), }; - ed_model.module.env.pool.set(ast_node_id, new_list_node); + ed_model + .module + .env + .pool + .set(list_ast_node_id, new_list_node); - let blank_mark_node = MarkupNode::Blank { - ast_node_id: blank_elt_id, - syn_high_style: HighlightStyle::Blank, - attributes: Attributes::new(), - parent_id_opt, - }; - - let blank_mark_node_id = ed_model.markup_node_pool.add(blank_mark_node); - - // add blank mark node to nested mark node from list - if let Some(parent_id) = parent_id_opt { - let parent = ed_model.markup_node_pool.get_mut(parent_id); - - let new_child_index = 1; // 1 because left bracket is first element - - parent.add_child_at_index(new_child_index, blank_mark_node_id)?; - } else { - MissingParent { - node_id: curr_mark_node_id, - } - .fail()? - } - - // update GridNodeMap and CodeLines - ed_model.insert_between_line( - old_caret_pos.line, - old_caret_pos.column, - nodes::BLANK_PLACEHOLDER, - blank_mark_node_id, - )?; - - Ok(InputOutcome::Accepted) + Ok(()) } _ => UnexpectedASTNode { required_node_type: "List".to_string(), encountered_node_type: expr2_to_string(ast_node_id, ed_model.module.env.pool), } - .fail()?, + .fail(), + }?; + + let new_mark_children = update_mark_children( + new_child_index, + blank_elt_id, + list_ast_node_id, + old_caret_pos, + parent_id_opt, + ed_model, + )?; + + let parent = ed_model.markup_node_pool.get_mut(parent_id); + + for (indx, child) in new_mark_children.iter().enumerate() { + parent.add_child_at_index(new_child_index + indx, *child)?; } + + Ok(InputOutcome::Accepted) +} + +// add a Blank child to the Nested mark node and update the caret +pub fn update_mark_children( + new_child_index: usize, + blank_elt_id: ExprId, + list_ast_node_id: ExprId, + old_caret_pos: TextPos, + parent_id_opt: Option, + ed_model: &mut EdModel, +) -> EdResult> { + let blank_mark_node = MarkupNode::Blank { + ast_node_id: blank_elt_id, + syn_high_style: HighlightStyle::Blank, + attributes: Attributes::new(), + parent_id_opt, + }; + + let blank_mark_node_id = ed_model.markup_node_pool.add(blank_mark_node); + + let mut children: Vec = vec![]; + + if new_child_index > 1 { + let comma_mark_node = MarkupNode::Text { + content: nodes::COMMA.to_owned(), + ast_node_id: list_ast_node_id, + syn_high_style: HighlightStyle::Blank, + attributes: Attributes::new(), + parent_id_opt, + }; + + let comma_mark_node_id = ed_model.markup_node_pool.add(comma_mark_node); + + ed_model.simple_move_carets_right(nodes::COMMA.len()); + + ed_model.insert_between_line( + old_caret_pos.line, + old_caret_pos.column, + nodes::COMMA, + comma_mark_node_id, + )?; + + children.push(comma_mark_node_id); + } + + children.push(blank_mark_node_id); + + let comma_shift = if new_child_index == 1 { + 0 + } else { + nodes::COMMA.len() + }; + + // update GridNodeMap and CodeLines + ed_model.insert_between_line( + old_caret_pos.line, + old_caret_pos.column + comma_shift, + nodes::BLANK_PLACEHOLDER, + blank_mark_node_id, + )?; + + Ok(children) } diff --git a/editor/src/editor/render_debug.rs b/editor/src/editor/render_debug.rs index c265f42914..ea3ca106b1 100644 --- a/editor/src/editor/render_debug.rs +++ b/editor/src/editor/render_debug.rs @@ -19,7 +19,7 @@ pub fn build_debug_graphics( let area_bounds = (size.width as f32, size.height as f32); let layout = wgpu_glyph::Layout::default().h_align(wgpu_glyph::HorizontalAlign::Left); - let debug_txt_coords: Vector2 = (txt_coords.x, txt_coords.y * 6.0).into(); + let debug_txt_coords: Vector2 = (txt_coords.x, txt_coords.y * 3.0).into(); let grid_node_map_text = glyph_brush::OwnedText::new(format!("{}", ed_model.grid_node_map)) .with_color(colors::to_slice(from_hsb(20, 41, 100))) diff --git a/editor/src/editor/slow_pool.rs b/editor/src/editor/slow_pool.rs index 9ee925a369..d6ee4c85b9 100644 --- a/editor/src/editor/slow_pool.rs +++ b/editor/src/editor/slow_pool.rs @@ -44,12 +44,28 @@ impl fmt::Display for SlowPool { write!(f, "\n\n(mark_node_pool)\n")?; for (index, node) in self.nodes.iter().enumerate() { + let ast_node_id_str = format!("{:?}", node.get_ast_node_id()); + let ast_node_id: String = ast_node_id_str + .chars() + .filter(|c| c.is_ascii_digit()) + .collect(); + + let mut child_str = String::new(); + + let node_children = node.get_children_ids(); + + if !node_children.is_empty() { + child_str = format!("children: {:?}", node_children); + } + writeln!( f, - "{}: {} ({})", + "{}: {} ({}) ast_id {:?} {}", index, node.node_type_as_string(), node.get_content().unwrap_or_else(|_| "".to_string()), + ast_node_id.parse::().unwrap(), + child_str )?; } diff --git a/editor/src/lang/ast.rs b/editor/src/lang/ast.rs index d0d42ad1b6..9f421b252b 100644 --- a/editor/src/lang/ast.rs +++ b/editor/src/lang/ast.rs @@ -118,8 +118,8 @@ pub enum Expr2 { InvalidLookup(PoolStr), // 8B List { - elem_var: Variable, // 4B - elems: PoolVec, // 8B + elem_var: Variable, // 4B + elems: PoolVec, // 8B }, If { cond_var: Variable, // 4B @@ -527,13 +527,15 @@ fn expr2_to_string_helper( let mut first_elt = true; - for elem_expr2 in elems.iter(pool) { + for elem_expr2_id in elems.iter(pool) { if !first_elt { out_string.push_str(", ") } else { first_elt = false; } + let elem_expr2 = pool.get(*elem_expr2_id); + expr2_to_string_helper(elem_expr2, indent_level + 2, pool, out_string) } diff --git a/editor/src/lang/constrain.rs b/editor/src/lang/constrain.rs index b813d76153..cb1ed061bc 100644 --- a/editor/src/lang/constrain.rs +++ b/editor/src/lang/constrain.rs @@ -1,7 +1,7 @@ use bumpalo::{collections::Vec as BumpVec, Bump}; use crate::lang::{ - ast::{Expr2, RecordField, ValueDef, WhenBranch}, + ast::{Expr2, ExprId, RecordField, ValueDef, WhenBranch}, expr::Env, pattern::{DestructType, Pattern2, PatternState2, RecordDestruct}, pool::{Pool, PoolStr, PoolVec, ShallowClone}, @@ -134,7 +134,10 @@ pub fn constrain_expr<'a>( let list_elem_type = Type2::Variable(*elem_var); - for (index, elem_node_id) in elems.iter_node_ids().enumerate() { + let indexed_node_ids: Vec<(usize, ExprId)> = + elems.iter(env.pool).copied().enumerate().collect(); + + for (index, elem_node_id) in indexed_node_ids { let elem_expr = env.pool.get(elem_node_id); let elem_expected = Expected::ForReason( diff --git a/editor/src/lang/expr.rs b/editor/src/lang/expr.rs index 936da9242d..3a0662b9f1 100644 --- a/editor/src/lang/expr.rs +++ b/editor/src/lang/expr.rs @@ -28,6 +28,7 @@ use roc_module::low_level::LowLevel; use roc_module::operator::CalledVia; use roc_module::symbol::{IdentIds, ModuleId, ModuleIds, Symbol}; use roc_parse::ast; +use roc_parse::ast::Expr; use roc_parse::ast::StrLiteral; use roc_parse::parser::{loc, Parser, State, SyntaxError}; use roc_parse::pattern::PatternType; @@ -318,6 +319,7 @@ pub fn to_expr2<'a>( region: Region, ) -> (Expr2, self::Output) { use roc_parse::ast::Expr::*; + match parse_expr { Float(string) => { match finish_parsing_float(string) { @@ -405,14 +407,16 @@ pub fn to_expr2<'a>( let mut output = Output::default(); let output_ref = &mut output; - let elems = PoolVec::with_capacity(items.len() as u32, env.pool); + let elems: PoolVec> = + PoolVec::with_capacity(items.len() as u32, env.pool); for (node_id, item) in elems.iter_node_ids().zip(items.iter()) { let (expr, sub_output) = to_expr2(env, scope, &item.value, item.region); output_ref.union(sub_output); - env.pool[node_id] = expr; + let expr_id = env.pool.add(expr); + env.pool[node_id] = expr_id; } let expr = Expr2::List { diff --git a/editor/src/lang/pool.rs b/editor/src/lang/pool.rs index 1d77814b46..e68199e339 100644 --- a/editor/src/lang/pool.rs +++ b/editor/src/lang/pool.rs @@ -352,11 +352,11 @@ impl<'a, T: 'a + Sized> PoolVec { let index = first_node_id.index as isize; let mut next_node_ptr = unsafe { pool.nodes.offset(index) } as *mut T; - for node in nodes { + for (indx_inc, node) in nodes.enumerate() { unsafe { *next_node_ptr = node; - next_node_ptr = next_node_ptr.offset(1); + next_node_ptr = pool.nodes.offset(index + (indx_inc as isize) + 1) as *mut T; } } @@ -603,6 +603,17 @@ impl Iterator for PoolVecIterNodeIds { } } +#[test] +fn pool_vec_iter_test() { + let expected_vec: Vec = vec![2, 4, 8, 16]; + + let mut test_pool = Pool::with_capacity(1024); + let pool_vec = PoolVec::new(expected_vec.clone().into_iter(), &mut test_pool); + + let current_vec: Vec = pool_vec.iter(&test_pool).copied().collect(); + + assert_eq!(current_vec, expected_vec); +} /// Clones the outer node, but does not clone any nodeids pub trait ShallowClone { fn shallow_clone(&self) -> Self;