From fdb0aa9eb7e78ad548eebbb5cd24ae8473e5e14c Mon Sep 17 00:00:00 2001 From: Anton-4 <17049058+Anton-4@users.noreply.github.com> Date: Thu, 1 Jul 2021 19:08:39 +0200 Subject: [PATCH 01/14] multi element lists without updating ast nodes --- compiler/mono/src/alias_analysis.rs | 4 +- editor/src/editor/markup/nodes.rs | 13 ++- editor/src/editor/mvc/ed_update.rs | 27 ++++- editor/src/editor/mvc/list_update.rs | 167 +++++++++++++++++---------- editor/src/lang/ast.rs | 8 +- editor/src/lang/constrain.rs | 7 +- editor/src/lang/expr.rs | 11 +- 7 files changed, 163 insertions(+), 74 deletions(-) diff --git a/compiler/mono/src/alias_analysis.rs b/compiler/mono/src/alias_analysis.rs index 2b4354d26e..ebd467954d 100644 --- a/compiler/mono/src/alias_analysis.rs +++ b/compiler/mono/src/alias_analysis.rs @@ -166,9 +166,9 @@ where p.build()? }; - if DEBUG { + /*if DEBUG { eprintln!("{}", program.to_source_string()); - } + }*/ morphic_lib::solve(program) } diff --git a/editor/src/editor/markup/nodes.rs b/editor/src/editor/markup/nodes.rs index 8faf0cbf4b..590fc05c5b 100644 --- a/editor/src/editor/markup/nodes.rs +++ b/editor/src/editor/markup/nodes.rs @@ -6,6 +6,7 @@ use crate::editor::ed_error::NestedNodeRequired; use crate::editor::slow_pool::MarkNodeId; use crate::editor::slow_pool::SlowPool; use crate::editor::syntax_highlight::HighlightStyle; +use crate::lang::ast::ExprId; use crate::lang::ast::RecordField; use crate::lang::{ ast::Expr2, @@ -146,6 +147,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( @@ -217,21 +219,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, + *node_id, HighlightStyle::Operator, markup_node_pool, )); diff --git a/editor/src/editor/mvc/ed_update.rs b/editor/src/editor/mvc/ed_update.rs index 859f01cece..4ea9faac9d 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; @@ -689,7 +689,7 @@ pub fn handle_new_char(received_char: &char, ed_model: &mut EdModel) -> EdResult 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 + add_blank_child(ed_model)?; // insert a Blank first, this results in cleaner code handle_new_char(received_char, ed_model)? } else { InputOutcome::Ignored @@ -726,12 +726,33 @@ pub fn handle_new_char(received_char: &char, ed_model: &mut EdModel) -> EdResult } else { InputOutcome::Ignored } + } else if *ch == ',' { + let mark_parent_id_opt = curr_mark_node.get_parent_id_opt(); + + 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:_} => { + add_blank_child(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 { if curr_mark_node.get_content()? == nodes::RIGHT_SQUARE_BR { - prep_empty_list(ed_model)?; // insert a Blank first, this results in cleaner code + add_blank_child(ed_model)?; // insert a Blank first, this results in cleaner code handle_new_char(received_char, ed_model)? } else { InputOutcome::Ignored diff --git a/editor/src/editor/mvc/list_update.rs b/editor/src/editor/mvc/list_update.rs index 8533967209..ee9366f7bd 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,7 @@ 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(ed_model: &mut EdModel) -> EdResult { let NodeContext { old_caret_pos, curr_mark_node_id, @@ -98,67 +99,117 @@ 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, usize, 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 new_child_index = parent.get_children_ids().len() - 1; // TODO support adding child at place other than end - 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); + let list_ast_node_id = parent.get_ast_node_id(); + let list_ast_node = ed_model.module.env.pool.get(list_ast_node_id); - let blank_elt_id = - children_pool_vec - .iter_node_ids() - .next() - .context(UnexpectedEmptyPoolVec { - descriptive_vec_name: "\"children of List AST node\"", - })?; + 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); - let new_list_node = Expr2::List { - elem_var: *elem_var, - elems: children_pool_vec, - }; - - ed_model.module.env.pool.set(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, + Ok((blank_elt_id, new_child_index, list_ast_node_id, parent_id)) } - .fail()? + _ => 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() + }; - // update GridNodeMap and CodeLines - ed_model.insert_between_line( - old_caret_pos.line, - old_caret_pos.column, - nodes::BLANK_PLACEHOLDER, - blank_mark_node_id, - )?; + let (blank_elt_id, new_child_index, list_ast_node_id, parent_id) = trip_result?; - Ok(InputOutcome::Accepted) - } - _ => UnexpectedASTNode { - required_node_type: "List".to_string(), - encountered_node_type: expr2_to_string(ast_node_id, ed_model.module.env.pool), - } - .fail()?, + let new_mark_children = make_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)?; } + + //TODO add ast children + + Ok(InputOutcome::Accepted) +} + +pub fn make_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/lang/ast.rs b/editor/src/lang/ast.rs index 4b95b1dc62..1e3cba1495 100644 --- a/editor/src/lang/ast.rs +++ b/editor/src/lang/ast.rs @@ -114,8 +114,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 @@ -438,13 +438,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 106eea2f1f..f2ad95ed81 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, WhenBranch}, + ast::{Expr2, ExprId, RecordField, WhenBranch}, expr::Env, pattern::{DestructType, Pattern2, PatternState2, RecordDestruct}, pool::{Pool, PoolStr, PoolVec, ShallowClone}, @@ -131,7 +131,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 2a9626bd5b..54843e2576 100644 --- a/editor/src/lang/expr.rs +++ b/editor/src/lang/expr.rs @@ -19,6 +19,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_problem::can::{Problem, RuntimeError}; @@ -337,12 +338,18 @@ pub fn to_expr2<'a>( let elems = PoolVec::with_capacity(items.len() as u32, env.pool); - for (node_id, item) in elems.iter_node_ids().zip(items.iter()) { + let node_id_item_tups: Vec<(ExprId, &Located)> = elems + .iter(env.pool) + .map(|node_ref| *node_ref) + .zip(items.iter().map(|item_ref| *item_ref)) + .collect(); + + for (node_id, item) in node_id_item_tups.iter() { let (expr, sub_output) = to_expr2(env, scope, &item.value, item.region); output_ref.union(sub_output); - env.pool[node_id] = expr; + env.pool.set(*node_id, expr); } let expr = Expr2::List { From 544ac8634329fea6d5fd670b02f2271cff2f4c36 Mon Sep 17 00:00:00 2001 From: Anton-4 <17049058+Anton-4@users.noreply.github.com> Date: Fri, 2 Jul 2021 16:27:01 +0200 Subject: [PATCH 02/14] fixed poolvec bug, ast nodes are added to list --- editor/src/editor/mvc/list_update.rs | 31 ++++++++++++++++++++++++++-- editor/src/lang/pool.rs | 16 ++++++++++++-- 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/editor/src/editor/mvc/list_update.rs b/editor/src/editor/mvc/list_update.rs index ee9366f7bd..421a193c17 100644 --- a/editor/src/editor/mvc/list_update.rs +++ b/editor/src/editor/mvc/list_update.rs @@ -133,6 +133,35 @@ pub fn add_blank_child(ed_model: &mut EdModel) -> EdResult { let (blank_elt_id, new_child_index, 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 mut new_elems: Vec = + elems.iter(ed_model.module.env.pool).copied().collect(); + + new_elems.push(blank_elt_id); + + let new_list_node = + Expr2::List { + elem_var: *elem_var, + elems: PoolVec::new(new_elems.into_iter(), ed_model.module.env.pool), + }; + + ed_model.module.env.pool.set(list_ast_node_id, new_list_node); + + Ok(()) + } + _ => UnexpectedASTNode { + required_node_type: "List".to_string(), + encountered_node_type: expr2_to_string(ast_node_id, ed_model.module.env.pool), + } + .fail(), + }?; + let new_mark_children = make_mark_children( new_child_index, blank_elt_id, @@ -148,8 +177,6 @@ pub fn add_blank_child(ed_model: &mut EdModel) -> EdResult { parent.add_child_at_index(new_child_index + indx, *child)?; } - //TODO add ast children - Ok(InputOutcome::Accepted) } diff --git a/editor/src/lang/pool.rs b/editor/src/lang/pool.rs index 1d77814b46..e47bd79107 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,18 @@ 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; From 659f33c44b3bacd4f800cd56d5362eb8261a67cf Mon Sep 17 00:00:00 2001 From: Anton-4 <17049058+Anton-4@users.noreply.github.com> Date: Fri, 2 Jul 2021 19:08:05 +0200 Subject: [PATCH 03/14] some multi element list tests --- editor/src/editor/mvc/ed_update.rs | 32 ++++++++++++++++++++++++++-- editor/src/editor/mvc/list_update.rs | 20 ++++++++--------- 2 files changed, 40 insertions(+), 12 deletions(-) diff --git a/editor/src/editor/mvc/ed_update.rs b/editor/src/editor/mvc/ed_update.rs index 4ea9faac9d..a23565550a 100644 --- a/editor/src/editor/mvc/ed_update.rs +++ b/editor/src/editor/mvc/ed_update.rs @@ -870,7 +870,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))?; + } } let post_lines = ui_res_to_res(ed_model_to_dsl(&ed_model))?; @@ -1520,7 +1524,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")?; @@ -1555,6 +1559,30 @@ pub mod test_ed_update { Ok(()) } + #[test] + 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")?; + + // TODO issue #1448: assert_insert_seq(&["┃"], &["[ 0, ┃ ]"], "[0,\"")?; + // assert_insert_seq(&["┃"], &["[ [ [ 0 ], [ 1 ] ], ┃"], "[[[0➔➔,[1➔➔➔➔,[[\"")? + Ok(()) + } + #[test] fn test_ignore_list() -> Result<(), String> { assert_insert_seq_ignore(&["┃[ ]"], IGNORE_CHARS)?; diff --git a/editor/src/editor/mvc/list_update.rs b/editor/src/editor/mvc/list_update.rs index 421a193c17..67ed064e0a 100644 --- a/editor/src/editor/mvc/list_update.rs +++ b/editor/src/editor/mvc/list_update.rs @@ -136,22 +136,22 @@ pub fn add_blank_child(ed_model: &mut EdModel) -> EdResult { let list_ast_node = ed_model.module.env.pool.get(list_ast_node_id); match list_ast_node { - Expr2::List { - elem_var, - elems, - } => { + Expr2::List { elem_var, elems } => { let mut new_elems: Vec = elems.iter(ed_model.module.env.pool).copied().collect(); new_elems.push(blank_elt_id); - let new_list_node = - Expr2::List { - elem_var: *elem_var, - elems: PoolVec::new(new_elems.into_iter(), ed_model.module.env.pool), - }; + let new_list_node = Expr2::List { + elem_var: *elem_var, + elems: PoolVec::new(new_elems.into_iter(), ed_model.module.env.pool), + }; - ed_model.module.env.pool.set(list_ast_node_id, new_list_node); + ed_model + .module + .env + .pool + .set(list_ast_node_id, new_list_node); Ok(()) } From 3598e2dd7fb0df665ec9470b64a1fc7dd80e5c0d Mon Sep 17 00:00:00 2001 From: Anton-4 <17049058+Anton-4@users.noreply.github.com> Date: Mon, 5 Jul 2021 19:24:49 +0200 Subject: [PATCH 04/14] multi elt list tests and bug fixes --- editor/src/editor/markup/nodes.rs | 18 +++++++----- editor/src/editor/mvc/ed_model.rs | 3 +- editor/src/editor/mvc/ed_update.rs | 47 +++++++++++++++++++++++++++++- editor/src/editor/slow_pool.rs | 9 +++++- editor/src/lang/expr.rs | 15 ++++------ editor/src/lang/pool.rs | 1 - 6 files changed, 72 insertions(+), 21 deletions(-) diff --git a/editor/src/editor/markup/nodes.rs b/editor/src/editor/markup/nodes.rs index 590fc05c5b..c6d1f74384 100644 --- a/editor/src/editor/markup/nodes.rs +++ b/editor/src/editor/markup/nodes.rs @@ -179,12 +179,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, @@ -236,7 +240,7 @@ pub fn expr2_to_markup<'a, 'b>( 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..bdd38e0e46 100644 --- a/editor/src/editor/mvc/ed_model.rs +++ b/editor/src/editor/mvc/ed_model.rs @@ -159,8 +159,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 a23565550a..12c904ba37 100644 --- a/editor/src/editor/mvc/ed_update.rs +++ b/editor/src/editor/mvc/ed_update.rs @@ -1584,7 +1584,7 @@ pub mod test_ed_update { } #[test] - fn test_ignore_list() -> Result<(), String> { + 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)?; @@ -1631,6 +1631,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( diff --git a/editor/src/editor/slow_pool.rs b/editor/src/editor/slow_pool.rs index 9ee925a369..543b6ed2a1 100644 --- a/editor/src/editor/slow_pool.rs +++ b/editor/src/editor/slow_pool.rs @@ -44,12 +44,19 @@ 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(); + writeln!( f, - "{}: {} ({})", + "{}: {} ({}) ast_n_id {:?}", index, node.node_type_as_string(), node.get_content().unwrap_or_else(|_| "".to_string()), + ast_node_id.parse::().unwrap(), )?; } diff --git a/editor/src/lang/expr.rs b/editor/src/lang/expr.rs index 54843e2576..b974bb292e 100644 --- a/editor/src/lang/expr.rs +++ b/editor/src/lang/expr.rs @@ -249,6 +249,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) { @@ -336,20 +337,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); - let node_id_item_tups: Vec<(ExprId, &Located)> = elems - .iter(env.pool) - .map(|node_ref| *node_ref) - .zip(items.iter().map(|item_ref| *item_ref)) - .collect(); - - for (node_id, item) in node_id_item_tups.iter() { + 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.set(*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 e47bd79107..e68199e339 100644 --- a/editor/src/lang/pool.rs +++ b/editor/src/lang/pool.rs @@ -614,7 +614,6 @@ fn pool_vec_iter_test() { assert_eq!(current_vec, expected_vec); } - /// Clones the outer node, but does not clone any nodeids pub trait ShallowClone { fn shallow_clone(&self) -> Self; From 7c1dc4a9751bfd5326aa0d07cad5d6d7c44115bb Mon Sep 17 00:00:00 2001 From: Anton-4 <17049058+Anton-4@users.noreply.github.com> Date: Tue, 6 Jul 2021 19:35:53 +0200 Subject: [PATCH 05/14] list insertion at any position, not yet correct on ast list node --- editor/src/editor/ed_error.rs | 11 ++++- editor/src/editor/markup/nodes.rs | 22 +++++++++- editor/src/editor/mvc/ed_model.rs | 20 ++++++++- editor/src/editor/mvc/ed_update.rs | 61 ++++++++++++++++++++++------ editor/src/editor/mvc/list_update.rs | 7 ++-- editor/src/editor/render_debug.rs | 2 +- 6 files changed, 102 insertions(+), 21 deletions(-) diff --git a/editor/src/editor/ed_error.rs b/editor/src/editor/ed_error.rs index a21cb2de86..3d1c47390d 100644 --- a/editor/src/editor/ed_error.rs +++ b/editor/src/editor/ed_error.rs @@ -1,4 +1,4 @@ -use crate::editor::slow_pool::MarkNodeId; +use crate::{editor::slow_pool::MarkNodeId, ui::text::text_pos::TextPos}; use crate::ui::ui_error::UIResult; 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 c6d1f74384..50ade40ebd 100644 --- a/editor/src/editor/markup/nodes.rs +++ b/editor/src/editor/markup/nodes.rs @@ -2,7 +2,7 @@ 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::{NestedNodeRequired, NestedNodeMissingChild}; use crate::editor::slow_pool::MarkNodeId; use crate::editor::slow_pool::SlowPool; use crate::editor::syntax_highlight::HighlightStyle; @@ -73,6 +73,26 @@ impl MarkupNode { } } + pub fn get_child_index(&self, child_id: MarkNodeId) -> EdResult { + match self { + MarkupNode::Nested { children_ids, .. } => { + let position_opt = children_ids.iter().position(|&c_id| c_id == child_id); + + if let Some(child_index) = position_opt { + Ok(child_index) + } 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 { diff --git a/editor/src/editor/mvc/ed_model.rs b/editor/src/editor/mvc/ed_model.rs index bdd38e0e46..b3127947d8 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, NoNodeAtCaretPosition, MissingParent}, markup::attribute::Attributes, markup::nodes::{expr2_to_markup, set_parent_for_all, MarkupNode}, }; @@ -134,6 +134,24 @@ impl<'a> EdModel<'a> { pub fn node_exists_at_caret(&self) -> bool { self.grid_node_map.node_exists_at_pos(self.get_caret()) } + + pub fn get_curr_child_index(&self) -> EdResult { + 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_index(curr_mark_node_id) + } else { + MissingParent{ node_id: curr_mark_node_id }.fail() + } + } else { + NoNodeAtCaretPosition{ caret_pos: self.get_caret()}.fail() + } + + + } } #[derive(Debug)] diff --git a/editor/src/editor/mvc/ed_update.rs b/editor/src/editor/mvc/ed_update.rs index 12c904ba37..2fb9cff1de 100644 --- a/editor/src/editor/mvc/ed_update.rs +++ b/editor/src/editor/mvc/ed_update.rs @@ -689,7 +689,9 @@ pub fn handle_new_char(received_char: &char, ed_model: &mut EdModel) -> EdResult if prev_mark_node.get_content()? == nodes::LEFT_SQUARE_BR { if curr_mark_node.get_content()? == nodes::RIGHT_SQUARE_BR { - add_blank_child(ed_model)?; // insert a Blank first, this results in cleaner code + // based on if, we are at the start of the list + let new_child_index = 1; + add_blank_child(new_child_index, ed_model)?; // insert a Blank first, this results in cleaner code handle_new_char(received_char, ed_model)? } else { InputOutcome::Ignored @@ -735,7 +737,11 @@ pub fn handle_new_char(received_char: &char, ed_model: &mut EdModel) -> EdResult match parent_expr2 { Expr2::List { elem_var:_, elems:_} => { - add_blank_child(ed_model)? + // insert a Blank first, this results in cleaner code + add_blank_child( + ed_model.get_curr_child_index()?, + ed_model + )? } Expr2::Record { record_var:_, fields:_ } => { todo!("multiple record fields") @@ -752,7 +758,11 @@ pub fn handle_new_char(received_char: &char, ed_model: &mut EdModel) -> EdResult if prev_mark_node.get_content()? == nodes::LEFT_SQUARE_BR { if curr_mark_node.get_content()? == nodes::RIGHT_SQUARE_BR { - add_blank_child(ed_model)?; // insert a Blank first, this results in cleaner code + // insert a Blank first, this results in cleaner code + add_blank_child( + ed_model.get_curr_child_index()?, + ed_model + )?; handle_new_char(received_char, ed_model)? } else { InputOutcome::Ignored @@ -1949,7 +1959,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() { @@ -2006,16 +2020,16 @@ 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 * }"], + &["Num *", "{ camelCase : Num * }"], )?; assert_type_tooltips_clean( &["{ a: { b: { c: \"hello┃, hello.0123456789ZXY{}[]-><-\" } } }"], - &vec![ + &[ "Str", "{ c : Str }", "{ b : { c : Str } }", @@ -2023,13 +2037,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)", @@ -2039,7 +2058,7 @@ pub mod test_ed_update { )?; assert_type_tooltips_seq( &["┃"], - &vec![ + &[ "{ a : Num * }", "List { a : Num * }", "List (List { a : Num * })", @@ -2047,6 +2066,22 @@ 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 67ed064e0a..ef8ab083dd 100644 --- a/editor/src/editor/mvc/list_update.rs +++ b/editor/src/editor/mvc/list_update.rs @@ -90,7 +90,7 @@ pub fn start_new_list(ed_model: &mut EdModel) -> EdResult { } // insert Blank at current position for easy code reuse -pub fn add_blank_child(ed_model: &mut EdModel) -> EdResult { +pub fn add_blank_child(new_child_index:usize, ed_model: &mut EdModel) -> EdResult { let NodeContext { old_caret_pos, curr_mark_node_id, @@ -103,8 +103,6 @@ pub fn add_blank_child(ed_model: &mut EdModel) -> EdResult { if let Some(parent_id) = parent_id_opt { let parent = ed_model.markup_node_pool.get(parent_id); - let new_child_index = parent.get_children_ids().len() - 1; // TODO support adding child at place other than end - let list_ast_node_id = parent.get_ast_node_id(); let list_ast_node = ed_model.module.env.pool.get(list_ast_node_id); @@ -140,7 +138,8 @@ pub fn add_blank_child(ed_model: &mut EdModel) -> EdResult { let mut new_elems: Vec = elems.iter(ed_model.module.env.pool).copied().collect(); - new_elems.push(blank_elt_id); + let new_ast_child_index = // TODO filter everything from nested marknode that has list_ast_node as ast_node and count that; + new_elems.insert(new_ast_child_index ,blank_elt_id); let new_list_node = Expr2::List { elem_var: *elem_var, 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))) From 82828d3bd8ddc28ad11ad2771c68d686d7321c4d Mon Sep 17 00:00:00 2001 From: Anton-4 <17049058+Anton-4@users.noreply.github.com> Date: Wed, 7 Jul 2021 15:24:22 +0200 Subject: [PATCH 06/14] in between element insert working --- editor/src/editor/ed_error.rs | 2 +- editor/src/editor/markup/nodes.rs | 73 +++++++++++++++-- editor/src/editor/mvc/ed_model.rs | 19 +++-- editor/src/editor/mvc/ed_update.rs | 113 +++++++++++++++++---------- editor/src/editor/mvc/list_update.rs | 59 +++++++------- editor/src/editor/slow_pool.rs | 11 ++- 6 files changed, 191 insertions(+), 86 deletions(-) diff --git a/editor/src/editor/ed_error.rs b/editor/src/editor/ed_error.rs index 3d1c47390d..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, ui::text::text_pos::TextPos}; 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}; diff --git a/editor/src/editor/markup/nodes.rs b/editor/src/editor/markup/nodes.rs index 50ade40ebd..ff31d26673 100644 --- a/editor/src/editor/markup/nodes.rs +++ b/editor/src/editor/markup/nodes.rs @@ -2,10 +2,11 @@ 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, NestedNodeMissingChild}; +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::{ @@ -13,6 +14,7 @@ use crate::lang::{ expr::Env, pool::{NodeId, PoolStr}, }; +use crate::ui::util::slice_get; use bumpalo::Bump; use std::fmt; @@ -73,23 +75,78 @@ impl MarkupNode { } } - pub fn get_child_index(&self, child_id: MarkNodeId) -> EdResult { + // return (index of child in list of children, index of child in list of children of 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 position_opt = children_ids.iter().position(|&c_id| c_id == child_id); + let mark_position_opt = children_ids.iter().position(|&c_id| c_id == child_id); - if let Some(child_index) = position_opt { - Ok(child_index) + 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() + } + .fail() } - }, + } _ => NestedNodeRequired { node_type: self.node_type_as_string(), - }.fail(), + } + .fail(), } } diff --git a/editor/src/editor/mvc/ed_model.rs b/editor/src/editor/mvc/ed_model.rs index b3127947d8..3a9bc4faa4 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, NoNodeAtCaretPosition, MissingParent}, + ed_error::{EdResult, MissingParent, NoNodeAtCaretPosition}, markup::attribute::Attributes, markup::nodes::{expr2_to_markup, set_parent_for_all, MarkupNode}, }; @@ -135,22 +135,27 @@ impl<'a> EdModel<'a> { self.grid_node_map.node_exists_at_pos(self.get_caret()) } - pub fn get_curr_child_index(&self) -> EdResult { + // return (index of child in list of children, index of child in list of children of 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_index(curr_mark_node_id) + parent.get_child_indices(curr_mark_node_id, &self.markup_node_pool) } else { - MissingParent{ node_id: curr_mark_node_id }.fail() + MissingParent { + node_id: curr_mark_node_id, + } + .fail() } } else { - NoNodeAtCaretPosition{ caret_pos: self.get_caret()}.fail() + NoNodeAtCaretPosition { + caret_pos: self.get_caret(), + } + .fail() } - - } } diff --git a/editor/src/editor/mvc/ed_update.rs b/editor/src/editor/mvc/ed_update.rs index 2fb9cff1de..6c11409c57 100644 --- a/editor/src/editor/mvc/ed_update.rs +++ b/editor/src/editor/mvc/ed_update.rs @@ -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, @@ -691,7 +700,8 @@ pub fn handle_new_char(received_char: &char, ed_model: &mut EdModel) -> EdResult if 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; - add_blank_child(new_child_index, ed_model)?; // insert a Blank first, this results in cleaner code + let new_ast_child_index = 0; + add_blank_child(new_child_index, new_ast_child_index, ed_model)?; // insert a Blank first, this results in cleaner code handle_new_char(received_char, ed_model)? } else { InputOutcome::Ignored @@ -729,38 +739,47 @@ pub fn handle_new_char(received_char: &char, ed_model: &mut EdModel) -> EdResult InputOutcome::Ignored } } else if *ch == ',' { - let mark_parent_id_opt = curr_mark_node.get_parent_id_opt(); - - 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:_} => { - // insert a Blank first, this results in cleaner code - add_blank_child( - ed_model.get_curr_child_index()?, - ed_model - )? - } - Expr2::Record { record_var:_, fields:_ } => { - todo!("multiple record fields") - } - _ => { - InputOutcome::Ignored - } - } - } else { + 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 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 { if 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( - ed_model.get_curr_child_index()?, + new_child_index, + new_ast_child_index, ed_model )?; handle_new_char(received_char, ed_model)? @@ -880,8 +899,10 @@ 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() { - if input_char == '➔' { + 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))?; } @@ -1571,25 +1592,26 @@ pub mod test_ed_update { #[test] fn test_multi_elt_list() -> Result<(), String> { - assert_insert_seq(&["┃"], &["[ 0, 1┃ ]"], "[0,1")?; + /*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", + "[\"a🡲,\"bcd🡲,\"EFGH", )?; assert_insert_seq( &["┃"], &["[ { a: 1 }, { b: 23 }, { c: 456┃ } ]"], - "[{a:1➔➔,{b:23➔➔,{c:456", + "[{a:1🡲🡲,{b:23🡲🡲,{c:456", )?; - assert_insert_seq(&["┃"], &["[ [ 1 ], [ 23 ], [ 456┃ ] ]"], "[[1➔➔,[23➔➔,[456")?; + assert_insert_seq(&["┃"], &["[ [ 1 ], [ 23 ], [ 456┃ ] ]"], "[[1🡲🡲,[23🡲🡲,[456")?;*/ + // insert element in between - // TODO issue #1448: assert_insert_seq(&["┃"], &["[ 0, ┃ ]"], "[0,\"")?; - // assert_insert_seq(&["┃"], &["[ [ [ 0 ], [ 1 ] ], ┃"], "[[[0➔➔,[1➔➔➔➔,[[\"")? + + assert_insert_seq(&["┃"], &["[ 0, 1┃, 2 ]"], "[0,2🡰🡰🡰,1")?; Ok(()) } @@ -1959,7 +1981,7 @@ 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() { - if input_char == '➔' { + if input_char == '🡲' { ed_model.simple_move_carets_right(1); } else { ed_res_to_res(handle_new_char(&input_char, &mut ed_model))?; @@ -2023,10 +2045,7 @@ pub mod test_ed_update { assert_type_tooltips_seq(&["┃"], &["*"], "")?; assert_type_tooltips_seq(&["┃"], &["*", "{ a : * }"], "{a:")?; - assert_type_tooltips_clean( - &["{ camelCase: ┃0 }"], - &["Num *", "{ camelCase : Num * }"], - )?; + assert_type_tooltips_clean(&["{ camelCase: ┃0 }"], &["Num *", "{ camelCase : Num * }"])?; assert_type_tooltips_clean( &["{ a: { b: { c: \"hello┃, hello.0123456789ZXY{}[]-><-\" } } }"], &[ @@ -2068,19 +2087,31 @@ pub mod test_ed_update { // 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")?; + 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(&["┃"], &["List "], "[\"abc🡲,50")?; - assert_type_tooltips_seq(&["┃"], &["Str", "{ a : Str }", "List "], "[{a:0➔➔,{a:\"0")?; + 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")?; + 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 ef8ab083dd..efc3eca5a3 100644 --- a/editor/src/editor/mvc/list_update.rs +++ b/editor/src/editor/mvc/list_update.rs @@ -90,7 +90,11 @@ pub fn start_new_list(ed_model: &mut EdModel) -> EdResult { } // insert Blank at current position for easy code reuse -pub fn add_blank_child(new_child_index:usize, 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, @@ -99,37 +103,37 @@ pub fn add_blank_child(new_child_index:usize, ed_model: &mut EdModel) -> EdResul ast_node_id, } = get_node_context(&ed_model)?; - let trip_result: EdResult<(ExprId, usize, ExprId, MarkNodeId)> = - if let Some(parent_id) = parent_id_opt { - let parent = ed_model.markup_node_pool.get(parent_id); + 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_id = parent.get_ast_node_id(); - let list_ast_node = ed_model.module.env.pool.get(list_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); + 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, new_child_index, 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(), + Ok((blank_elt_id, list_ast_node_id, parent_id)) } - } else { - MissingParent { - node_id: curr_mark_node_id, + _ => UnexpectedASTNode { + required_node_type: "List".to_string(), + encountered_node_type: expr2_to_string(ast_node_id, ed_model.module.env.pool), } - .fail() - }; + .fail(), + } + } else { + MissingParent { + node_id: curr_mark_node_id, + } + .fail() + }; - let (blank_elt_id, new_child_index, list_ast_node_id, parent_id) = trip_result?; + 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); @@ -138,8 +142,7 @@ pub fn add_blank_child(new_child_index:usize, ed_model: &mut EdModel) -> EdResul let mut new_elems: Vec = elems.iter(ed_model.module.env.pool).copied().collect(); - let new_ast_child_index = // TODO filter everything from nested marknode that has list_ast_node as ast_node and count that; - new_elems.insert(new_ast_child_index ,blank_elt_id); + new_elems.insert(new_ast_child_index, blank_elt_id); let new_list_node = Expr2::List { elem_var: *elem_var, diff --git a/editor/src/editor/slow_pool.rs b/editor/src/editor/slow_pool.rs index 543b6ed2a1..d6ee4c85b9 100644 --- a/editor/src/editor/slow_pool.rs +++ b/editor/src/editor/slow_pool.rs @@ -50,13 +50,22 @@ impl fmt::Display for SlowPool { .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_n_id {:?}", + "{}: {} ({}) ast_id {:?} {}", index, node.node_type_as_string(), node.get_content().unwrap_or_else(|_| "".to_string()), ast_node_id.parse::().unwrap(), + child_str )?; } From e7c5cc5664519fbc24140844fb935a404c2dbf8c Mon Sep 17 00:00:00 2001 From: Anton-4 <17049058+Anton-4@users.noreply.github.com> Date: Wed, 7 Jul 2021 17:13:25 +0200 Subject: [PATCH 07/14] more tests --- editor/src/editor/mvc/ed_update.rs | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/editor/src/editor/mvc/ed_update.rs b/editor/src/editor/mvc/ed_update.rs index 6c11409c57..e16fd306e7 100644 --- a/editor/src/editor/mvc/ed_update.rs +++ b/editor/src/editor/mvc/ed_update.rs @@ -1592,7 +1592,7 @@ pub mod test_ed_update { #[test] fn test_multi_elt_list() -> Result<(), String> { - /*assert_insert_seq(&["┃"], &["[ 0, 1┃ ]"], "[0,1")?; + assert_insert_seq(&["┃"], &["[ 0, 1┃ ]"], "[0,1")?; assert_insert_seq(&["┃"], &["[ 987, 6543, 210┃ ]"], "[987,6543,210")?; assert_insert_seq( @@ -1607,11 +1607,27 @@ pub mod test_ed_update { "[{a:1🡲🡲,{b:23🡲🡲,{c:456", )?; - assert_insert_seq(&["┃"], &["[ [ 1 ], [ 23 ], [ 456┃ ] ]"], "[[1🡲🡲,[23🡲🡲,[456")?;*/ - // insert element in between + 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", + )?; - - assert_insert_seq(&["┃"], &["[ 0, 1┃, 2 ]"], "[0,2🡰🡰🡰,1")?; Ok(()) } From e156fbef030264a28edbe418fce05f61e76f8e3f Mon Sep 17 00:00:00 2001 From: Anton-4 <17049058+Anton-4@users.noreply.github.com> Date: Wed, 7 Jul 2021 18:08:34 +0200 Subject: [PATCH 08/14] minor cleanup --- compiler/mono/src/alias_analysis.rs | 4 +-- editor/src/editor/markup/nodes.rs | 2 +- editor/src/editor/mvc/ed_model.rs | 2 +- editor/src/editor/mvc/ed_update.rs | 40 ++++++++++++---------------- editor/src/editor/mvc/list_update.rs | 5 ++-- 5 files changed, 24 insertions(+), 29 deletions(-) diff --git a/compiler/mono/src/alias_analysis.rs b/compiler/mono/src/alias_analysis.rs index ebd467954d..2b4354d26e 100644 --- a/compiler/mono/src/alias_analysis.rs +++ b/compiler/mono/src/alias_analysis.rs @@ -166,9 +166,9 @@ where p.build()? }; - /*if DEBUG { + if DEBUG { eprintln!("{}", program.to_source_string()); - }*/ + } morphic_lib::solve(program) } diff --git a/editor/src/editor/markup/nodes.rs b/editor/src/editor/markup/nodes.rs index ff31d26673..dd30826d2e 100644 --- a/editor/src/editor/markup/nodes.rs +++ b/editor/src/editor/markup/nodes.rs @@ -75,7 +75,7 @@ impl MarkupNode { } } - // return (index of child in list of children, index of child in list of children of ast node) + // 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, diff --git a/editor/src/editor/mvc/ed_model.rs b/editor/src/editor/mvc/ed_model.rs index 3a9bc4faa4..b57a36df8a 100644 --- a/editor/src/editor/mvc/ed_model.rs +++ b/editor/src/editor/mvc/ed_model.rs @@ -135,7 +135,7 @@ impl<'a> EdModel<'a> { self.grid_node_map.node_exists_at_pos(self.get_caret()) } - // return (index of child in list of children, index of child in list of children of ast node) of MarkupNode at current caret position + // 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()?; diff --git a/editor/src/editor/mvc/ed_update.rs b/editor/src/editor/mvc/ed_update.rs index e16fd306e7..1802854f76 100644 --- a/editor/src/editor/mvc/ed_update.rs +++ b/editor/src/editor/mvc/ed_update.rs @@ -696,16 +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 { - // based on if, we are at the start of the list - let new_child_index = 1; - let new_ast_child_index = 0; - add_blank_child(new_child_index, new_ast_child_index, 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 } @@ -773,22 +770,19 @@ pub fn handle_new_char(received_char: &char, ed_model: &mut EdModel) -> EdResult } 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 { - if 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 - } + 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 } diff --git a/editor/src/editor/mvc/list_update.rs b/editor/src/editor/mvc/list_update.rs index efc3eca5a3..24cc7fab32 100644 --- a/editor/src/editor/mvc/list_update.rs +++ b/editor/src/editor/mvc/list_update.rs @@ -164,7 +164,7 @@ pub fn add_blank_child( .fail(), }?; - let new_mark_children = make_mark_children( + let new_mark_children = update_mark_children( new_child_index, blank_elt_id, list_ast_node_id, @@ -182,7 +182,8 @@ pub fn add_blank_child( Ok(InputOutcome::Accepted) } -pub fn make_mark_children( +// 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, From 83ee9a0cdac4cbcc5168a9791aaaaf83a979d835 Mon Sep 17 00:00:00 2001 From: Anton-4 <17049058+Anton-4@users.noreply.github.com> Date: Fri, 9 Jul 2021 10:47:26 +0200 Subject: [PATCH 09/14] added benchmark regression check script --- .github/workflows/benchmarks.yml | 12 ++------ ci/bench-runner.sh | 51 ++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 10 deletions(-) create mode 100644 ci/bench-runner.sh 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 100644 index 0000000000..0c35b5643c --- /dev/null +++ b/ci/bench-runner.sh @@ -0,0 +1,51 @@ +#!/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 [ $EXIT_CODE -ne 0 ]; then + +if grep -q "regressed" "$LOG_FILE"; then + echo "Benchmark detected regression. Running benchmark again to confirm..." + + # 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 "Benchmarks were run twice and a regression was detected both times." + 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 "Benchmark execution failed with exit code: $EXIT_CODE" + exit $EXIT_CODE +fi \ No newline at end of file From 7ce04f24be0ba6eeafd7895ee960c610d7006f9d Mon Sep 17 00:00:00 2001 From: Anton-4 <17049058+Anton-4@users.noreply.github.com> Date: Fri, 9 Jul 2021 11:14:38 +0200 Subject: [PATCH 10/14] added permission to execute bench script --- ci/bench-runner.sh | 0 1 file changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 ci/bench-runner.sh diff --git a/ci/bench-runner.sh b/ci/bench-runner.sh old mode 100644 new mode 100755 From c9c073f79a29041471ffa17fdce6384ffb2572e4 Mon Sep 17 00:00:00 2001 From: Anton-4 <17049058+Anton-4@users.noreply.github.com> Date: Fri, 9 Jul 2021 11:58:34 +0200 Subject: [PATCH 11/14] removed useless line of code --- ci/bench-runner.sh | 2 -- 1 file changed, 2 deletions(-) diff --git a/ci/bench-runner.sh b/ci/bench-runner.sh index 0c35b5643c..f0f3c13afe 100755 --- a/ci/bench-runner.sh +++ b/ci/bench-runner.sh @@ -21,8 +21,6 @@ echo $FULL_CMD script -efq $LOG_FILE -c "$FULL_CMD" EXIT_CODE=$? - -if [ $EXIT_CODE -ne 0 ]; then if grep -q "regressed" "$LOG_FILE"; then echo "Benchmark detected regression. Running benchmark again to confirm..." From 56b19a59f50fd5e9cd488284313a2b272aeecbda Mon Sep 17 00:00:00 2001 From: Anton-4 <17049058+Anton-4@users.noreply.github.com> Date: Fri, 9 Jul 2021 12:21:36 +0200 Subject: [PATCH 12/14] regression testing --- cli/cli_utils/src/bench_utils.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/cli_utils/src/bench_utils.rs b/cli/cli_utils/src/bench_utils.rs index d74e3f5bad..3695e9eed5 100644 --- a/cli/cli_utils/src/bench_utils.rs +++ b/cli/cli_utils/src/bench_utils.rs @@ -77,9 +77,9 @@ fn bench_cmd( pub fn bench_nqueens(bench_group_opt: Option<&mut BenchmarkGroup>) { exec_bench_w_input( &example_file("benchmarks", "NQueens.roc"), - "11", + "12", "nqueens", - "2680\n", + "14200\n", bench_group_opt, ); } From 5e1d89d9d5411d6864f5d5dca94efb18feded219 Mon Sep 17 00:00:00 2001 From: Anton-4 <17049058+Anton-4@users.noreply.github.com> Date: Fri, 9 Jul 2021 12:39:24 +0200 Subject: [PATCH 13/14] made bench regression msg more visible --- ci/bench-runner.sh | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/ci/bench-runner.sh b/ci/bench-runner.sh index f0f3c13afe..0038612c63 100755 --- a/ci/bench-runner.sh +++ b/ci/bench-runner.sh @@ -23,7 +23,13 @@ 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 @@ -37,13 +43,25 @@ if grep -q "regressed" "$LOG_FILE"; then 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 "Benchmark execution failed with exit code: $EXIT_CODE" + 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 From 29ad384cba05dd61e6f39b10eec7671446ab07b3 Mon Sep 17 00:00:00 2001 From: Anton-4 <17049058+Anton-4@users.noreply.github.com> Date: Fri, 9 Jul 2021 13:37:27 +0200 Subject: [PATCH 14/14] back to original nqueen arg --- cli/cli_utils/src/bench_utils.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/cli_utils/src/bench_utils.rs b/cli/cli_utils/src/bench_utils.rs index 3695e9eed5..d74e3f5bad 100644 --- a/cli/cli_utils/src/bench_utils.rs +++ b/cli/cli_utils/src/bench_utils.rs @@ -77,9 +77,9 @@ fn bench_cmd( pub fn bench_nqueens(bench_group_opt: Option<&mut BenchmarkGroup>) { exec_bench_w_input( &example_file("benchmarks", "NQueens.roc"), - "12", + "11", "nqueens", - "14200\n", + "2680\n", bench_group_opt, ); }