From b9de3703a3650e9671b1b6d4270d7e94e3377786 Mon Sep 17 00:00:00 2001 From: HactarCE <6060305+HactarCE@users.noreply.github.com> Date: Fri, 28 Feb 2025 11:36:26 -0500 Subject: [PATCH 1/4] Pass `SheetId` by value when possible --- quadratic-core/src/a1/a1_selection/create.rs | 6 ++-- quadratic-core/src/a1/a1_selection/display.rs | 7 ++-- .../src/a1/a1_selection/intersects.rs | 8 ++--- quadratic-core/src/a1/a1_selection/parse.rs | 32 +++++++++---------- quadratic-core/src/a1/js_selection/create.rs | 2 +- .../execute_operation/execute_validation.rs | 4 +-- .../execution/run_code/get_cells.rs | 2 +- .../execution/run_code/run_connection.rs | 2 +- .../src/controller/operations/clipboard.rs | 10 +++--- quadratic-core/src/controller/thumbnail.rs | 8 ++--- .../src/controller/user_actions/clipboard.rs | 10 +++--- .../controller/user_actions/validations.rs | 6 ++-- quadratic-core/src/grid/data_table/formats.rs | 12 +++---- quadratic-core/src/grid/selection.rs | 6 ++-- quadratic-core/src/grid/sheet/bounds/mod.rs | 2 +- quadratic-core/src/grid/sheet/cell_values.rs | 2 +- .../src/grid/sheet/rendering/fills.rs | 8 ++--- .../sheet/validations/validation_col_row.rs | 28 ++++++++-------- 18 files changed, 77 insertions(+), 78 deletions(-) diff --git a/quadratic-core/src/a1/a1_selection/create.rs b/quadratic-core/src/a1/a1_selection/create.rs index 5c3d71c368..ab7774403e 100644 --- a/quadratic-core/src/a1/a1_selection/create.rs +++ b/quadratic-core/src/a1/a1_selection/create.rs @@ -159,17 +159,17 @@ impl A1Selection { /// Returns a test selection from the A1-string with SheetId::TEST. #[cfg(test)] pub fn test_a1(a1: &str) -> Self { - Self::parse(a1, &SheetId::TEST, &A1Context::default(), None).unwrap() + Self::parse(a1, SheetId::TEST, &A1Context::default(), None).unwrap() } /// Returns a test selection from the A1-string with the given sheet ID. #[cfg(test)] - pub fn test_a1_sheet_id(a1: &str, sheet_id: &SheetId) -> Self { + pub fn test_a1_sheet_id(a1: &str, sheet_id: SheetId) -> Self { Self::parse(a1, sheet_id, &A1Context::default(), None).unwrap() } #[cfg(test)] pub fn test_a1_context(a1: &str, context: &A1Context) -> Self { - Self::parse(a1, &SheetId::TEST, context, None).unwrap() + Self::parse(a1, SheetId::TEST, context, None).unwrap() } } diff --git a/quadratic-core/src/a1/a1_selection/display.rs b/quadratic-core/src/a1/a1_selection/display.rs index db72c5fca3..ea35606240 100644 --- a/quadratic-core/src/a1/a1_selection/display.rs +++ b/quadratic-core/src/a1/a1_selection/display.rs @@ -142,7 +142,7 @@ mod tests { #[test] fn test_extra_comma() { let sheet_id = SheetId::TEST; - let selection = A1Selection::parse_a1("1,", &sheet_id, &A1Context::default()).unwrap(); + let selection = A1Selection::parse_a1("1,", sheet_id, &A1Context::default()).unwrap(); assert_eq!( selection.to_string(Some(sheet_id), &A1Context::default()), "1:1", @@ -152,8 +152,7 @@ mod tests { #[test] fn test_multiple_one_sized_rects() { let sheet_id = SheetId::TEST; - let selection = - A1Selection::parse_a1("A1,B1,C1", &sheet_id, &A1Context::default()).unwrap(); + let selection = A1Selection::parse_a1("A1,B1,C1", sheet_id, &A1Context::default()).unwrap(); assert_eq!( selection.to_string(Some(sheet_id), &A1Context::default()), "A1,B1,C1", @@ -166,7 +165,7 @@ mod tests { let sheet_second = SheetId::new(); let context = A1Context::test(&[("First", sheet_id), ("Second", sheet_second)], &[]); let selection = - A1Selection::parse_a1("second!A1,second!B1,second!C1", &sheet_id, &context).unwrap(); + A1Selection::parse_a1("second!A1,second!B1,second!C1", sheet_id, &context).unwrap(); assert_eq!( selection.to_string(Some(sheet_id), &context), "Second!A1,Second!B1,Second!C1", diff --git a/quadratic-core/src/a1/a1_selection/intersects.rs b/quadratic-core/src/a1/a1_selection/intersects.rs index 578a3e8331..21ef9a345a 100644 --- a/quadratic-core/src/a1/a1_selection/intersects.rs +++ b/quadratic-core/src/a1/a1_selection/intersects.rs @@ -142,8 +142,8 @@ mod tests { fn test_intersection() { let context = A1Context::default(); // Test different sheets return None - let sel1 = A1Selection::test_a1_sheet_id("A1:B2", &SheetId::new()); - let sel2 = A1Selection::test_a1_sheet_id("B2:C3", &SheetId::new()); + let sel1 = A1Selection::test_a1_sheet_id("A1:B2", SheetId::new()); + let sel2 = A1Selection::test_a1_sheet_id("B2:C3", SheetId::new()); assert_eq!( sel1.intersection(&sel2, &context), None, @@ -272,8 +272,8 @@ mod tests { fn test_overlaps_a1_selection() { let context = A1Context::test(&[], &[]); // Different sheets don't overlap - let sel1 = A1Selection::test_a1_sheet_id("A1:B2", &SheetId::new()); - let sel2 = A1Selection::test_a1_sheet_id("B2:C3", &SheetId::new()); + let sel1 = A1Selection::test_a1_sheet_id("A1:B2", SheetId::new()); + let sel2 = A1Selection::test_a1_sheet_id("B2:C3", SheetId::new()); assert!( !sel1.overlaps_a1_selection(&sel2, &context), "Different sheets should not overlap" diff --git a/quadratic-core/src/a1/a1_selection/parse.rs b/quadratic-core/src/a1/a1_selection/parse.rs index 7b3a8da24f..95625e711d 100644 --- a/quadratic-core/src/a1/a1_selection/parse.rs +++ b/quadratic-core/src/a1/a1_selection/parse.rs @@ -10,7 +10,7 @@ impl A1Selection { /// explicit sheet use `default_sheet_id`. pub fn parse_a1( a1: &str, - default_sheet_id: &SheetId, + default_sheet_id: SheetId, context: &A1Context, ) -> Result { Self::parse(a1, default_sheet_id, context, None) @@ -25,7 +25,7 @@ impl A1Selection { /// `Some`, then A1 and RC notation are both accepted. pub fn parse( s: &str, - default_sheet_id: &SheetId, + default_sheet_id: SheetId, context: &A1Context, base_pos: Option, ) -> Result { @@ -78,7 +78,7 @@ impl A1Selection { let mut sheet_id = None; for segment in segments { let range = - SheetCellRefRange::parse(segment.trim(), *default_sheet_id, context, base_pos)?; + SheetCellRefRange::parse(segment.trim(), default_sheet_id, context, base_pos)?; if *sheet.get_or_insert(range.sheet_id) != range.sheet_id { return Err(A1Error::TooManySheets(s.to_string())); } @@ -110,7 +110,7 @@ mod tests { let sheet_id = SheetId::TEST; let context = A1Context::default(); assert_eq!( - A1Selection::parse_a1("A1", &sheet_id, &context), + A1Selection::parse_a1("A1", sheet_id, &context), Ok(A1Selection::from_xy(1, 1, sheet_id)), ); } @@ -120,7 +120,7 @@ mod tests { let sheet_id = SheetId::TEST; let context = A1Context::default(); assert_eq!( - A1Selection::parse_a1("*", &sheet_id, &context), + A1Selection::parse_a1("*", sheet_id, &context), Ok(A1Selection::from_range( CellRefRange::ALL, sheet_id, @@ -134,15 +134,15 @@ mod tests { let sheet_id = SheetId::TEST; let context = A1Context::default(); assert_eq!( - A1Selection::parse_a1("A1:B2", &sheet_id, &context), + A1Selection::parse_a1("A1:B2", sheet_id, &context), Ok(A1Selection::test_a1("A1:B2")), ); assert_eq!( - A1Selection::parse_a1("D1:A5", &sheet_id, &context), + A1Selection::parse_a1("D1:A5", sheet_id, &context), Ok(A1Selection::test_a1("D1:A5")), ); assert_eq!( - A1Selection::parse_a1("A1:B2,D1:A5", &sheet_id, &context), + A1Selection::parse_a1("A1:B2,D1:A5", sheet_id, &context), Ok(A1Selection::test_a1("A1:B2,D1:A5")), ); } @@ -152,7 +152,7 @@ mod tests { let sheet_id = SheetId::TEST; let context = A1Context::default(); let selection = - A1Selection::parse_a1("A1,B1:D2,E:G,2:3,5:7,F6:G8,4", &sheet_id, &context).unwrap(); + A1Selection::parse_a1("A1,B1:D2,E:G,2:3,5:7,F6:G8,4", sheet_id, &context).unwrap(); assert_eq!(selection.sheet_id, sheet_id); assert_eq!(selection.cursor, pos![A4]); @@ -175,7 +175,7 @@ mod tests { let sheet_id = SheetId::TEST; let context = A1Context::default(); assert_eq!( - A1Selection::parse_a1("1", &sheet_id, &context), + A1Selection::parse_a1("1", sheet_id, &context), Ok(A1Selection::from_range( CellRefRange::new_relative_row(1), sheet_id, @@ -184,12 +184,12 @@ mod tests { ); assert_eq!( - A1Selection::parse_a1("1:3", &sheet_id, &context), + A1Selection::parse_a1("1:3", sheet_id, &context), Ok(A1Selection::test_a1("1:3")), ); assert_eq!( - A1Selection::parse_a1("1:", &sheet_id, &context), + A1Selection::parse_a1("1:", sheet_id, &context), Ok(A1Selection::test_a1("*")), ); } @@ -200,7 +200,7 @@ mod tests { let sheet_id2 = SheetId::new(); let context = A1Context::test(&[("Sheet1", sheet_id), ("Second", sheet_id2)], &[]); assert_eq!( - A1Selection::parse_a1("'Second'!A1", &sheet_id, &context), + A1Selection::parse_a1("'Second'!A1", sheet_id, &context), Ok(A1Selection::from_xy(1, 1, sheet_id2)), ); } @@ -210,7 +210,7 @@ mod tests { let sheet_id = SheetId::TEST; let context = A1Context::default(); assert_eq!( - A1Selection::parse_a1("Sheet' 1'!A1", &sheet_id, &context), + A1Selection::parse_a1("Sheet' 1'!A1", sheet_id, &context), Err(A1Error::InvalidSheetName("Sheet' 1'!A1".to_string())), ); } @@ -225,7 +225,7 @@ mod tests { assert_eq!( A1Selection::parse_a1( "test_table[[#DATA],[#HEADERS],[Col1]],A1", - &sheet_id, + sheet_id, &context ) .unwrap() @@ -238,7 +238,7 @@ mod tests { &[("test_table-2.csv", &["Col1"], Rect::test_a1("A1:C3"))], ); assert_eq!( - A1Selection::parse_a1("test_table-2.csv[Col1]", &sheet_id, &context) + A1Selection::parse_a1("test_table-2.csv[Col1]", sheet_id, &context) .unwrap() .to_string(Some(sheet_id), &context), "test_table-2.csv[Col1]".to_string(), diff --git a/quadratic-core/src/a1/js_selection/create.rs b/quadratic-core/src/a1/js_selection/create.rs index 85f79c9b08..7e313a81f3 100644 --- a/quadratic-core/src/a1/js_selection/create.rs +++ b/quadratic-core/src/a1/js_selection/create.rs @@ -79,7 +79,7 @@ pub fn to_selection( ) -> Result { let default_sheet_id = SheetId::from_str(default_sheet_id).map_err(|e| e.to_string())?; let context = serde_json::from_str::(context).map_err(|e| e.to_string())?; - let selection = A1Selection::parse_a1(a1, &default_sheet_id, &context) + let selection = A1Selection::parse_a1(a1, default_sheet_id, &context) .map_err(|e| serde_json::to_string(&e).unwrap_or(e.to_string()))?; Ok(JsSelection { selection, context }) } diff --git a/quadratic-core/src/controller/execution/execute_operation/execute_validation.rs b/quadratic-core/src/controller/execution/execute_operation/execute_validation.rs index 619bfd6cbc..a197978f28 100644 --- a/quadratic-core/src/controller/execution/execute_operation/execute_validation.rs +++ b/quadratic-core/src/controller/execution/execute_operation/execute_validation.rs @@ -251,7 +251,7 @@ mod tests { let validation = Validation { id: Uuid::new_v4(), - selection: A1Selection::test_a1_sheet_id("A1", &sheet_id), + selection: A1Selection::test_a1_sheet_id("A1", sheet_id), rule: ValidationRule::Logical(Default::default()), message: Default::default(), error: Default::default(), @@ -323,7 +323,7 @@ mod tests { // set validation let validation = Validation { id: Uuid::new_v4(), - selection: A1Selection::test_a1_sheet_id("A1", &sheet_id), + selection: A1Selection::test_a1_sheet_id("A1", sheet_id), rule: ValidationRule::Logical(Default::default()), message: Default::default(), error: Default::default(), diff --git a/quadratic-core/src/controller/execution/run_code/get_cells.rs b/quadratic-core/src/controller/execution/run_code/get_cells.rs index 5c257821f5..df9e43d40a 100644 --- a/quadratic-core/src/controller/execution/run_code/get_cells.rs +++ b/quadratic-core/src/controller/execution/run_code/get_cells.rs @@ -67,7 +67,7 @@ impl GridController { msg: RunErrorMsg::CodeRunError(msg.into()), } }; - let selection = match self.a1_selection_from_string(&a1, &code_sheet_pos.sheet_id) { + let selection = match self.a1_selection_from_string(&a1, code_sheet_pos.sheet_id) { Ok(selection) => selection, Err(e) => { // unable to parse A1 string diff --git a/quadratic-core/src/controller/execution/run_code/run_connection.rs b/quadratic-core/src/controller/execution/run_code/run_connection.rs index 0d241d598b..4c4c79b242 100644 --- a/quadratic-core/src/controller/execution/run_code/run_connection.rs +++ b/quadratic-core/src/controller/execution/run_code/run_connection.rs @@ -39,7 +39,7 @@ impl GridController { result.push_str(&code[last_match_end..whole_match.start()]); let content = cap.get(1).map(|m| m.as_str().trim()).unwrap_or(""); - let selection = A1Selection::parse_a1(content, &default_sheet_id, context)?; + let selection = A1Selection::parse_a1(content, default_sheet_id, context)?; let Some(pos) = selection.try_to_pos(context) else { return Err(A1Error::WrongCellCount( diff --git a/quadratic-core/src/controller/operations/clipboard.rs b/quadratic-core/src/controller/operations/clipboard.rs index 76f748551e..f56cd15a53 100644 --- a/quadratic-core/src/controller/operations/clipboard.rs +++ b/quadratic-core/src/controller/operations/clipboard.rs @@ -1166,7 +1166,7 @@ mod test { assert!(gc .paste_html_operations( - &A1Selection::test_a1_sheet_id("B2", &sheet_id), + &A1Selection::test_a1_sheet_id("B2", sheet_id), html, PasteSpecial::None, ) @@ -1222,7 +1222,7 @@ mod test { let JsClipboard { html, .. } = gc .sheet(sheet_id) .copy_to_clipboard( - &A1Selection::test_a1_sheet_id("B2:C3", &sheet_id), + &A1Selection::test_a1_sheet_id("B2:C3", sheet_id), gc.a1_context(), ClipboardOperation::Copy, false, @@ -1230,7 +1230,7 @@ mod test { .unwrap(); gc.paste_from_clipboard( - &A1Selection::test_a1_sheet_id("E13", &sheet_id), + &A1Selection::test_a1_sheet_id("E13", sheet_id), None, Some(html), PasteSpecial::None, @@ -1363,7 +1363,7 @@ mod test { let JsClipboard { plain_text, .. } = gc .sheet(sheet_id) .copy_to_clipboard( - &A1Selection::test_a1_sheet_id("B2:C3", &sheet_id), + &A1Selection::test_a1_sheet_id("B2:C3", sheet_id), gc.a1_context(), ClipboardOperation::Copy, true, @@ -1371,7 +1371,7 @@ mod test { .unwrap(); gc.paste_from_clipboard( - &A1Selection::test_a1_sheet_id("E13", &sheet_id), + &A1Selection::test_a1_sheet_id("E13", sheet_id), Some(plain_text), None, PasteSpecial::None, diff --git a/quadratic-core/src/controller/thumbnail.rs b/quadratic-core/src/controller/thumbnail.rs index c5620d93bb..bfce75eab9 100644 --- a/quadratic-core/src/controller/thumbnail.rs +++ b/quadratic-core/src/controller/thumbnail.rs @@ -117,11 +117,11 @@ mod test { let sheet_id = gc.sheet_ids()[0]; let wrong_sheet_id = SheetId::new(); - assert!(gc.thumbnail_dirty_a1(&A1Selection::test_a1_sheet_id("A1", &sheet_id))); - assert!(!gc.thumbnail_dirty_a1(&A1Selection::test_a1_sheet_id("A1", &wrong_sheet_id))); + assert!(gc.thumbnail_dirty_a1(&A1Selection::test_a1_sheet_id("A1", sheet_id))); + assert!(!gc.thumbnail_dirty_a1(&A1Selection::test_a1_sheet_id("A1", wrong_sheet_id))); - assert!(!gc.thumbnail_dirty_a1(&A1Selection::test_a1_sheet_id("O1:O", &sheet_id))); - assert!(!gc.thumbnail_dirty_a1(&A1Selection::test_a1_sheet_id("A75:75", &sheet_id))); + assert!(!gc.thumbnail_dirty_a1(&A1Selection::test_a1_sheet_id("O1:O", sheet_id))); + assert!(!gc.thumbnail_dirty_a1(&A1Selection::test_a1_sheet_id("A75:75", sheet_id))); } #[test] diff --git a/quadratic-core/src/controller/user_actions/clipboard.rs b/quadratic-core/src/controller/user_actions/clipboard.rs index 5fb035ee69..903cc0af1a 100644 --- a/quadratic-core/src/controller/user_actions/clipboard.rs +++ b/quadratic-core/src/controller/user_actions/clipboard.rs @@ -1229,7 +1229,7 @@ mod test { .unwrap(); gc.paste_from_clipboard( - &A1Selection::test_a1_sheet_id("F5", &sheet_id), + &A1Selection::test_a1_sheet_id("F5", sheet_id), Some(plain_text.clone()), Some(html.clone()), PasteSpecial::None, @@ -1260,7 +1260,7 @@ mod test { data_table.header_is_first_row = false; gc.paste_from_clipboard( - &A1Selection::test_a1_sheet_id("G10", &sheet_id), + &A1Selection::test_a1_sheet_id("G10", sheet_id), Some(plain_text.clone()), Some(html.clone()), PasteSpecial::None, @@ -1291,7 +1291,7 @@ mod test { data_table.show_ui = false; gc.paste_from_clipboard( - &A1Selection::test_a1_sheet_id("E11", &sheet_id), + &A1Selection::test_a1_sheet_id("E11", sheet_id), Some(plain_text), Some(html), PasteSpecial::None, @@ -1340,7 +1340,7 @@ mod test { .unwrap(); gc.paste_from_clipboard( - &A1Selection::test_a1_sheet_id("F5", &sheet_id), + &A1Selection::test_a1_sheet_id("F5", sheet_id), Some(plain_text), Some(html), PasteSpecial::None, @@ -1388,7 +1388,7 @@ mod test { .unwrap(); gc.paste_from_clipboard( - &A1Selection::test_a1_sheet_id("F5", &sheet_id), + &A1Selection::test_a1_sheet_id("F5", sheet_id), Some(plain_text), Some(html), PasteSpecial::None, diff --git a/quadratic-core/src/controller/user_actions/validations.rs b/quadratic-core/src/controller/user_actions/validations.rs index 885f73a2f1..b4af2fee0c 100644 --- a/quadratic-core/src/controller/user_actions/validations.rs +++ b/quadratic-core/src/controller/user_actions/validations.rs @@ -141,7 +141,7 @@ mod tests { let mut gc = GridController::test(); let sheet_id = gc.sheet_ids()[0]; - let selection = A1Selection::test_a1_sheet_id("*", &sheet_id); + let selection = A1Selection::test_a1_sheet_id("*", sheet_id); let validation = Validation { id: Uuid::new_v4(), selection: selection.clone(), @@ -173,7 +173,7 @@ mod tests { let mut gc = GridController::test(); let sheet_id = gc.sheet_ids()[0]; - let selection = A1Selection::test_a1_sheet_id("*", &sheet_id); + let selection = A1Selection::test_a1_sheet_id("*", sheet_id); let validation1 = Validation { id: Uuid::new_v4(), selection: selection.clone(), @@ -290,7 +290,7 @@ mod tests { let list = ValidationList { source: ValidationListSource::Selection(A1Selection::test_a1_sheet_id( - "A1:A4", &sheet_id, + "A1:A4", sheet_id, )), ignore_blank: true, drop_down: true, diff --git a/quadratic-core/src/grid/data_table/formats.rs b/quadratic-core/src/grid/data_table/formats.rs index b2d7c5a3b3..9d1becc5c7 100644 --- a/quadratic-core/src/grid/data_table/formats.rs +++ b/quadratic-core/src/grid/data_table/formats.rs @@ -117,7 +117,7 @@ pub mod test { gc.test_data_table_first_row_as_header(pos.to_sheet_pos(sheet_id), false); gc.set_bold( - &A1Selection::test_a1_sheet_id("E4,G5:J5", &sheet_id), + &A1Selection::test_a1_sheet_id("E4,G5:J5", sheet_id), Some(true), None, ) @@ -148,7 +148,7 @@ pub mod test { gc.test_data_table_first_row_as_header(pos.to_sheet_pos(sheet_id), false); gc.set_bold( - &A1Selection::test_a1_sheet_id("E4,G5:J5", &sheet_id), + &A1Selection::test_a1_sheet_id("E4,G5:J5", sheet_id), Some(true), None, ) @@ -244,7 +244,7 @@ pub mod test { gc.test_data_table_first_row_as_header(pos.to_sheet_pos(sheet_id), false); gc.set_bold( - &A1Selection::test_a1_sheet_id("E4,G5:J5", &sheet_id), + &A1Selection::test_a1_sheet_id("E4,G5:J5", sheet_id), Some(true), None, ) @@ -287,7 +287,7 @@ pub mod test { // add new bold formats with first column hidden gc.test_data_table_first_row_as_header(pos.to_sheet_pos(sheet_id), false); gc.set_bold( - &A1Selection::test_a1_sheet_id("F10,G12:J12", &sheet_id), + &A1Selection::test_a1_sheet_id("F10,G12:J12", sheet_id), Some(true), None, ) @@ -355,7 +355,7 @@ pub mod test { let (mut gc, sheet_id, pos, _) = simple_csv_at(pos!(E2)); gc.set_bold( - &A1Selection::test_a1_sheet_id("E4,G5:J5", &sheet_id), + &A1Selection::test_a1_sheet_id("E4,G5:J5", sheet_id), Some(true), None, ) @@ -408,7 +408,7 @@ pub mod test { // add new bold formats with sort gc.set_bold( - &A1Selection::test_a1_sheet_id("E4,G5:J5", &sheet_id), + &A1Selection::test_a1_sheet_id("E4,G5:J5", sheet_id), Some(true), None, ) diff --git a/quadratic-core/src/grid/selection.rs b/quadratic-core/src/grid/selection.rs index ee450b206e..8bd9a5ef13 100644 --- a/quadratic-core/src/grid/selection.rs +++ b/quadratic-core/src/grid/selection.rs @@ -7,7 +7,7 @@ impl GridController { pub fn a1_selection_from_string( &self, a1: &str, - default_sheet_id: &SheetId, + default_sheet_id: SheetId, ) -> Result { let context = self.grid().a1_context(); A1Selection::parse_a1(a1, default_sheet_id, &context) @@ -24,7 +24,7 @@ mod test { let gc = GridController::test(); let sheet_id = gc.sheet_ids()[0]; let selection = gc - .a1_selection_from_string("'Sheet1'!A1:B2", &sheet_id) + .a1_selection_from_string("'Sheet1'!A1:B2", sheet_id) .unwrap(); assert_eq!(selection.sheet_id, sheet_id); assert_eq!(selection.cursor, pos![A1]); @@ -37,7 +37,7 @@ mod test { gc.add_sheet_with_name("Types: sequences, mapping, sets".to_string(), None); let sheet_id = gc.sheet_ids()[1]; let selection = gc - .a1_selection_from_string("'Types: sequences, mapping, sets'!A1:B2", &sheet_id) + .a1_selection_from_string("'Types: sequences, mapping, sets'!A1:B2", sheet_id) .unwrap(); assert_eq!(selection.sheet_id, sheet_id); assert_eq!(selection.cursor, pos![A1]); diff --git a/quadratic-core/src/grid/sheet/bounds/mod.rs b/quadratic-core/src/grid/sheet/bounds/mod.rs index 6816803041..72e8ed1078 100644 --- a/quadratic-core/src/grid/sheet/bounds/mod.rs +++ b/quadratic-core/src/grid/sheet/bounds/mod.rs @@ -839,7 +839,7 @@ mod test { show_checkbox: true, ignore_blank: true, }), - selection: A1Selection::test_a1_sheet_id("A1", &sheet_id), + selection: A1Selection::test_a1_sheet_id("A1", sheet_id), message: Default::default(), error: Default::default(), }, diff --git a/quadratic-core/src/grid/sheet/cell_values.rs b/quadratic-core/src/grid/sheet/cell_values.rs index 83bd299e5b..65e5d49ea8 100644 --- a/quadratic-core/src/grid/sheet/cell_values.rs +++ b/quadratic-core/src/grid/sheet/cell_values.rs @@ -214,7 +214,7 @@ mod test { let validation = Validation { id: Uuid::new_v4(), - selection: A1Selection::test_a1_sheet_id("A1:C1", &sheet.id), + selection: A1Selection::test_a1_sheet_id("A1:C1", sheet.id), rule: ValidationRule::Logical(Default::default()), message: Default::default(), error: Default::default(), diff --git a/quadratic-core/src/grid/sheet/rendering/fills.rs b/quadratic-core/src/grid/sheet/rendering/fills.rs index 43b2ef38f2..b5a0947189 100644 --- a/quadratic-core/src/grid/sheet/rendering/fills.rs +++ b/quadratic-core/src/grid/sheet/rendering/fills.rs @@ -265,7 +265,7 @@ mod tests { // set a data table at E2 that's 3x3 and show_header is true gc.test_set_data_table(pos!(E2).to_sheet_pos(sheet_id), 3, 3, false, true); gc.set_fill_color( - &A1Selection::test_a1_sheet_id("E5:I5", &sheet_id), + &A1Selection::test_a1_sheet_id("E5:I5", sheet_id), Some("red".to_string()), None, ) @@ -339,7 +339,7 @@ mod tests { fn test_get_all_render_fills_table_with_sort() { let (mut gc, sheet_id, pos, file_name) = simple_csv_at(pos!(E2)); gc.set_fill_color( - &A1Selection::test_a1_sheet_id("E4:I4", &sheet_id), + &A1Selection::test_a1_sheet_id("E4:I4", sheet_id), Some("red".to_string()), None, ) @@ -391,7 +391,7 @@ mod tests { let (mut gc, sheet_id, pos, file_name) = simple_csv_at(pos!(E2)); gc.set_fill_color( - &A1Selection::test_a1_sheet_id("E4:I4", &sheet_id), + &A1Selection::test_a1_sheet_id("E4:I4", sheet_id), Some("red".to_string()), None, ) @@ -438,7 +438,7 @@ mod tests { assert_fill_eq(&fills[2], 6, 4, 2, 1, "red"); gc.set_fill_color( - &A1Selection::test_a1_sheet_id("E10:I10", &sheet_id), + &A1Selection::test_a1_sheet_id("E10:I10", sheet_id), Some("green".to_string()), None, ) diff --git a/quadratic-core/src/grid/sheet/validations/validation_col_row.rs b/quadratic-core/src/grid/sheet/validations/validation_col_row.rs index 0739aee9c7..77d9fa1d25 100644 --- a/quadratic-core/src/grid/sheet/validations/validation_col_row.rs +++ b/quadratic-core/src/grid/sheet/validations/validation_col_row.rs @@ -277,7 +277,7 @@ mod tests { // rect and columns to be updated let validation_rect_columns = Validation { id: Uuid::new_v4(), - selection: A1Selection::test_a1_sheet_id("A1:C3,A:C", &sheet_id), + selection: A1Selection::test_a1_sheet_id("A1:C3,A:C", sheet_id), rule: ValidationRule::Logical(ValidationLogical::default()), message: Default::default(), error: Default::default(), @@ -286,7 +286,7 @@ mod tests { // to be removed let validation_removed = Validation { id: Uuid::new_v4(), - selection: A1Selection::test_a1_sheet_id("B2:B3,B", &sheet_id), + selection: A1Selection::test_a1_sheet_id("B2:B3,B", sheet_id), rule: ValidationRule::Logical(ValidationLogical::default()), message: Default::default(), error: Default::default(), @@ -295,7 +295,7 @@ mod tests { // nothing to do with this one let validation_not_changed = Validation { id: Uuid::new_v4(), - selection: A1Selection::test_a1_sheet_id("A1:A1,A,5:10", &sheet_id), + selection: A1Selection::test_a1_sheet_id("A1:A1,A,5:10", sheet_id), rule: ValidationRule::Logical(ValidationLogical::default()), message: Default::default(), error: Default::default(), @@ -352,7 +352,7 @@ mod tests { let sheet = gc.sheet_mut(sheet_id); assert_eq!(sheet.validations.validations.len(), 2); let new_validation_rect_column = Validation { - selection: A1Selection::test_a1_sheet_id("A1:B3,A:B", &sheet_id), + selection: A1Selection::test_a1_sheet_id("A1:B3,A:B", sheet_id), ..validation_rect_columns.clone() }; assert_eq!(sheet.validations.validations[0], new_validation_rect_column); @@ -431,7 +431,7 @@ mod tests { // rect and rows to be updated let validation_rect_rows = Validation { id: Uuid::new_v4(), - selection: A1Selection::test_a1_sheet_id("A1:C3,1:3", &sheet_id), + selection: A1Selection::test_a1_sheet_id("A1:C3,1:3", sheet_id), rule: ValidationRule::Logical(ValidationLogical::default()), message: Default::default(), error: Default::default(), @@ -440,7 +440,7 @@ mod tests { // to be removed let validation_removed = Validation { id: Uuid::new_v4(), - selection: A1Selection::test_a1_sheet_id("A2:C2,2", &sheet_id), + selection: A1Selection::test_a1_sheet_id("A2:C2,2", sheet_id), rule: ValidationRule::Logical(ValidationLogical::default()), message: Default::default(), error: Default::default(), @@ -449,7 +449,7 @@ mod tests { // nothing to do with this one let validation_not_changed = Validation { id: Uuid::new_v4(), - selection: A1Selection::test_a1_sheet_id("A1:A1,A1:D1,1", &sheet_id), + selection: A1Selection::test_a1_sheet_id("A1:A1,A1:D1,1", sheet_id), rule: ValidationRule::Logical(ValidationLogical::default()), message: Default::default(), error: Default::default(), @@ -503,7 +503,7 @@ mod tests { let sheet = gc.sheet_mut(sheet_id); assert_eq!(sheet.validations.validations.len(), 2); let new_validation_rect_row = Validation { - selection: A1Selection::test_a1_sheet_id("A1:C2,1:2", &sheet_id), + selection: A1Selection::test_a1_sheet_id("A1:C2,1:2", sheet_id), ..validation_rect_rows.clone() }; assert_eq!(sheet.validations.validations[0], new_validation_rect_row); @@ -582,7 +582,7 @@ mod tests { // rect and rows to be updated let validation_rect_cols = Validation { id: Uuid::new_v4(), - selection: A1Selection::test_a1_sheet_id("A1:C3,A,B,C", &sheet_id), + selection: A1Selection::test_a1_sheet_id("A1:C3,A,B,C", sheet_id), rule: ValidationRule::Logical(ValidationLogical::default()), message: Default::default(), error: Default::default(), @@ -591,7 +591,7 @@ mod tests { // nothing to do with this one let validation_not_changed = Validation { id: Uuid::new_v4(), - selection: A1Selection::test_a1_sheet_id("A1:A1,A", &sheet_id), + selection: A1Selection::test_a1_sheet_id("A1:A1,A", sheet_id), rule: ValidationRule::Logical(ValidationLogical::default()), message: Default::default(), error: Default::default(), @@ -644,7 +644,7 @@ mod tests { let sheet = gc.sheet_mut(sheet_id); assert_eq!(sheet.validations.validations.len(), 2); let new_validation_rect_col = Validation { - selection: A1Selection::test_a1_sheet_id("A1:D3,A,C,D", &sheet_id), + selection: A1Selection::test_a1_sheet_id("A1:D3,A,C,D", sheet_id), ..validation_rect_cols.clone() }; assert_eq!(sheet.validations.validations[0], new_validation_rect_col); @@ -718,7 +718,7 @@ mod tests { // rect and columns to be updated let validation_rect_rows = Validation { id: Uuid::new_v4(), - selection: A1Selection::test_a1_sheet_id("A1:C3,1,2,3", &sheet_id), + selection: A1Selection::test_a1_sheet_id("A1:C3,1,2,3", sheet_id), rule: ValidationRule::Logical(ValidationLogical::default()), message: Default::default(), error: Default::default(), @@ -727,7 +727,7 @@ mod tests { // nothing to do with this one let validation_not_changed = Validation { id: Uuid::new_v4(), - selection: A1Selection::test_a1_sheet_id("A1:A1,1", &sheet_id), + selection: A1Selection::test_a1_sheet_id("A1:A1,1", sheet_id), rule: ValidationRule::Logical(ValidationLogical::default()), message: Default::default(), error: Default::default(), @@ -781,7 +781,7 @@ mod tests { let sheet = gc.sheet_mut(sheet_id); assert_eq!(sheet.validations.validations.len(), 2); let new_validation_rect_row = Validation { - selection: A1Selection::test_a1_sheet_id("A1:C4,1,3,4", &sheet_id), + selection: A1Selection::test_a1_sheet_id("A1:C4,1,3,4", sheet_id), ..validation_rect_rows.clone() }; assert_eq!(sheet.validations.validations[0], new_validation_rect_row); From 12ac920ad7cd13a77975562a047e8ca67cb09843 Mon Sep 17 00:00:00 2001 From: HactarCE <6060305+HactarCE@users.noreply.github.com> Date: Fri, 28 Feb 2025 11:37:01 -0500 Subject: [PATCH 2/4] Use case-insensitive comparison for column names --- quadratic-core/src/a1/table_ref/range.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/quadratic-core/src/a1/table_ref/range.rs b/quadratic-core/src/a1/table_ref/range.rs index abb981cecd..49d85869d6 100644 --- a/quadratic-core/src/a1/table_ref/range.rs +++ b/quadratic-core/src/a1/table_ref/range.rs @@ -47,20 +47,20 @@ impl ColRange { pub fn replace_column_name(&mut self, old_name: &str, new_name: &str) { match self { ColRange::Col(col) => { - if col == old_name { + if col.eq_ignore_ascii_case(old_name) { *col = new_name.to_string(); } } ColRange::ColRange(col1, col2) => { - if col1 == old_name { + if col1.eq_ignore_ascii_case(old_name) { *col1 = new_name.to_string(); } - if col2 == old_name { + if col2.eq_ignore_ascii_case(old_name) { *col2 = new_name.to_string(); } } ColRange::ColToEnd(col) => { - if col == old_name { + if col.eq_ignore_ascii_case(old_name) { *col = new_name.to_string(); } } From 9942fd16231b82725ce054455557414349cd5d5d Mon Sep 17 00:00:00 2001 From: HactarCE <6060305+HactarCE@users.noreply.github.com> Date: Fri, 28 Feb 2025 11:39:48 -0500 Subject: [PATCH 3/4] Rework code/formula translation & name-updating --- quadratic-core/src/a1/a1_context/sheet_map.rs | 1 + .../execute_operation/execute_col_rows.rs | 95 ++----- .../execute_operation/execute_sheets.rs | 7 +- .../src/controller/operations/autocomplete.rs | 12 +- .../src/controller/operations/clipboard.rs | 47 ++-- .../src/controller/user_actions/clipboard.rs | 4 +- quadratic-core/src/formulas/parser/mod.rs | 122 ++++++++- quadratic-core/src/grid/code_cell.rs | 237 +++++++++++------- quadratic-core/src/grid/code_run.rs | 13 +- quadratic-core/src/grid/sheet.rs | 17 +- quadratic-core/src/grid/sheet/data_table.rs | 25 +- quadratic-core/src/grid/sheets.rs | 22 +- 12 files changed, 352 insertions(+), 250 deletions(-) diff --git a/quadratic-core/src/a1/a1_context/sheet_map.rs b/quadratic-core/src/a1/a1_context/sheet_map.rs index b83e85e57a..d1fc9d2e0f 100644 --- a/quadratic-core/src/a1/a1_context/sheet_map.rs +++ b/quadratic-core/src/a1/a1_context/sheet_map.rs @@ -32,6 +32,7 @@ impl SheetMap { } pub fn try_sheet_id(&self, sheet_id: SheetId) -> Option<&String> { + // TODO(ajf): optimize this by making the hashmap go both ways self.sheet_map .iter() .find(|(_, id)| **id == sheet_id) diff --git a/quadratic-core/src/controller/execution/execute_operation/execute_col_rows.rs b/quadratic-core/src/controller/execution/execute_operation/execute_col_rows.rs index 74521f699b..7be5befd63 100644 --- a/quadratic-core/src/controller/execution/execute_operation/execute_col_rows.rs +++ b/quadratic-core/src/controller/execution/execute_operation/execute_col_rows.rs @@ -1,66 +1,14 @@ use crate::{ - a1::{A1Context, CellRefRange, CellRefRangeEnd, UNBOUNDED}, + a1::{CellRefRange, UNBOUNDED}, controller::{ active_transactions::pending_transaction::PendingTransaction, operations::operation::Operation, GridController, }, - formulas::replace_cell_references_with, - grid::{CodeCellLanguage, CodeCellValue, GridBounds, SheetId}, - CellValue, SheetPos, + grid::{GridBounds, SheetId}, + CellValue, }; impl GridController { - pub fn adjust_formula_column_row( - code_cell: &CodeCellValue, - parse_ctx: &A1Context, - code_cell_pos: SheetPos, - column: Option, - row: Option, - delta: i64, - ) -> String { - if let Some(column) = column { - replace_cell_references_with( - &code_cell.code, - parse_ctx, - code_cell_pos, - |sheet_id, cell_ref| { - Ok(CellRefRangeEnd { - col: if sheet_id == code_cell_pos.sheet_id - && cell_ref.col.coord >= column - && cell_ref.col.coord < UNBOUNDED - { - cell_ref.col.translate(delta)? - } else { - cell_ref.col - }, - row: cell_ref.row, - }) - }, - ) - } else if let Some(row) = row { - replace_cell_references_with( - &code_cell.code, - parse_ctx, - code_cell_pos, - |sheet_id, cell_ref| { - Ok(CellRefRangeEnd { - col: cell_ref.col, - row: if sheet_id == code_cell_pos.sheet_id - && cell_ref.row.coord >= row - && cell_ref.row.coord < UNBOUNDED - { - cell_ref.row.translate(delta)? - } else { - cell_ref.row - }, - }) - }, - ) - } else { - code_cell.code.clone() - } - } - fn adjust_code_cells_column_row( &self, transaction: &mut PendingTransaction, @@ -90,32 +38,17 @@ impl GridController { } if let Some(CellValue::Code(code)) = sheet.cell_value_ref(pos) { - let new_code = match code.language { - CodeCellLanguage::Formula => { - let pos = pos.to_sheet_pos(sheet.id); - GridController::adjust_formula_column_row( - code, - self.a1_context(), - pos, - column, - row, - delta, - ) - } - _ => { - let mut new_code = code.clone(); - let context = self.a1_context(); - new_code.adjust_code_cell_column_row( - column, row, delta, &sheet_id, context, - ); - new_code.code - } - }; - if new_code != code.code { - let code_cell_value = CellValue::Code(CodeCellValue { - code: new_code, - ..code.clone() - }); + let context = self.a1_context(); + let mut new_code = code.clone(); + new_code.adjust_code_cell_column_row( + context, + pos.to_sheet_pos(sheet_id), + column, + row, + delta, + ); + if new_code != *code { + let code_cell_value = CellValue::Code(new_code); let sheet_pos = pos.to_sheet_pos(sheet.id); transaction.operations.push_back(Operation::SetCellValues { sheet_pos, diff --git a/quadratic-core/src/controller/execution/execute_operation/execute_sheets.rs b/quadratic-core/src/controller/execution/execute_operation/execute_sheets.rs index f6e43975f9..a8c2752a9e 100644 --- a/quadratic-core/src/controller/execution/execute_operation/execute_sheets.rs +++ b/quadratic-core/src/controller/execution/execute_operation/execute_sheets.rs @@ -180,12 +180,7 @@ impl GridController { bail!(e); } - let context = self.a1_context().to_owned(); - - let sheet = self.try_sheet_result(sheet_id)?; - let old_name = sheet.name.to_owned(); - - self.grid.update_sheet_name(&old_name, &name, &context); + let old_name = self.grid.update_sheet_name(sheet_id, name.clone())?; transaction .forward_operations diff --git a/quadratic-core/src/controller/operations/autocomplete.rs b/quadratic-core/src/controller/operations/autocomplete.rs index 73c4d92035..88eef5b053 100644 --- a/quadratic-core/src/controller/operations/autocomplete.rs +++ b/quadratic-core/src/controller/operations/autocomplete.rs @@ -662,9 +662,15 @@ impl GridController { if let Some((CellValue::Code(code_cell), original_pos)) = series.get_mut(i) { let mut data_table_ops = vec![]; if let Some(original_pos) = original_pos { - let new_x = x - original_pos.x; - let new_y = y - original_pos.y; - code_cell.translate_cell_references(new_x, new_y, &sheet_id, context); + let delta_x = x - original_pos.x; + let delta_y = y - original_pos.y; + code_cell.translate_cell_references( + context, + original_pos.to_sheet_pos(sheet_id), + sheet_id, + delta_x, + delta_y, + ); let source_pos = original_pos.to_owned(); original_pos.x = x; diff --git a/quadratic-core/src/controller/operations/clipboard.rs b/quadratic-core/src/controller/operations/clipboard.rs index f56cd15a53..2369cbeb99 100644 --- a/quadratic-core/src/controller/operations/clipboard.rs +++ b/quadratic-core/src/controller/operations/clipboard.rs @@ -9,7 +9,6 @@ use uuid::Uuid; use super::operation::Operation; use crate::cell_values::CellValues; use crate::controller::GridController; -use crate::formulas::convert_a1_to_rc; use crate::grid::formats::Format; use crate::grid::formats::FormatUpdate; use crate::grid::formats::SheetFormatUpdates; @@ -17,7 +16,6 @@ use crate::grid::js_types::JsClipboard; use crate::grid::js_types::JsSnackbarSeverity; use crate::grid::sheet::borders::BordersUpdates; use crate::grid::sheet::validations::validation::Validation; -use crate::grid::CodeCellLanguage; use crate::grid::DataTable; use crate::grid::DataTableKind; use crate::grid::SheetId; @@ -42,6 +40,7 @@ pub enum PasteSpecial { pub struct ClipboardOrigin { pub x: i64, pub y: i64, + pub sheet_id: SheetId, pub column: Option, pub row: Option, pub all: Option<(i64, i64)>, @@ -668,33 +667,23 @@ impl GridController { // loop through the clipboard and replace cell references in formulas, translate cell references in other languages for (x, col) in clipboard.cells.columns.iter_mut().enumerate() { - for (&y, cell) in col.iter_mut() { - match cell { - CellValue::Code(code_cell) => match code_cell.language { - CodeCellLanguage::Formula => { - code_cell.code = convert_a1_to_rc( - &code_cell.code, - context, - Pos { - x: insert_at.x + x as i64, - y: insert_at.y + y as i64, - } - .to_sheet_pos(selection.sheet_id), - ); - } - _ => { - if clipboard.operation == ClipboardOperation::Copy { - code_cell.translate_cell_references( - delta_x, - delta_y, - &selection.sheet_id, - context, - ); - } - } - }, - _ => { /* noop */ } - }; + for (&y, cell) in col { + if let CellValue::Code(code_cell) = cell { + let original_pos = SheetPos { + x: clipboard.origin.x + x as i64, + y: clipboard.origin.y + y as i64, + sheet_id: clipboard.origin.sheet_id, + }; + if clipboard.operation == ClipboardOperation::Copy { + code_cell.translate_cell_references( + context, + original_pos, + selection.sheet_id, + delta_x, + delta_y, + ); + } + } } } diff --git a/quadratic-core/src/controller/user_actions/clipboard.rs b/quadratic-core/src/controller/user_actions/clipboard.rs index 903cc0af1a..a47cb5a067 100644 --- a/quadratic-core/src/controller/user_actions/clipboard.rs +++ b/quadratic-core/src/controller/user_actions/clipboard.rs @@ -709,11 +709,11 @@ mod test { // paste code cell (2,1) from the clipboard to (2,2) let dest_pos: SheetPos = (2, 2, sheet_id).into(); - assert_code_cell(&mut gc, dest_pos, "SUM(R[0]C[-1])", 2); + assert_code_cell(&mut gc, dest_pos, "SUM(A2)", 2); // paste code cell (2,1) from the clipboard to (2,3) let dest_pos: SheetPos = (2, 3, sheet_id).into(); - assert_code_cell(&mut gc, dest_pos, "SUM(R[0]C[-1])", 3); + assert_code_cell(&mut gc, dest_pos, "SUM(A3)", 3); } #[test] diff --git a/quadratic-core/src/formulas/parser/mod.rs b/quadratic-core/src/formulas/parser/mod.rs index e88ec63855..d2d11122cc 100644 --- a/quadratic-core/src/formulas/parser/mod.rs +++ b/quadratic-core/src/formulas/parser/mod.rs @@ -15,7 +15,7 @@ use super::*; use crate::{ a1::{A1Context, CellRefRange, CellRefRangeEnd, RefRangeBounds, SheetCellRefRange}, grid::{Grid, SheetId}, - CodeResult, CoerceInto, RefError, RunError, RunErrorMsg, SheetPos, Span, Spanned, + CodeResult, CoerceInto, RefError, RunError, RunErrorMsg, SheetPos, Span, Spanned, TableRef, }; /// Parses a formula. @@ -104,6 +104,7 @@ fn simple_parse_and_check_formula(formula_string: &str) -> bool { /// let replaced = convert_rc_to_a1("SUM(R{3}C[1])", &g.a1_context(), pos); /// assert_eq!(replaced, "SUM(B$3)"); /// ``` +#[must_use] pub fn convert_rc_to_a1(source: &str, ctx: &A1Context, pos: SheetPos) -> String { let replace_fn = |range_ref: SheetCellRefRange| Ok(range_ref.to_a1_string(None, ctx, false)); replace_cell_range_references(source, ctx, pos, replace_fn) @@ -122,6 +123,7 @@ pub fn convert_rc_to_a1(source: &str, ctx: &A1Context, pos: SheetPos) -> String /// let replaced = convert_a1_to_rc("SUM(B$3)", &g.a1_context(), pos); /// assert_eq!(replaced, "SUM(R{3}C[1])"); /// ``` +#[must_use] pub fn convert_a1_to_rc(source: &str, ctx: &A1Context, pos: SheetPos) -> String { let replace_fn = |range_ref: SheetCellRefRange| { Ok(range_ref.to_rc_string(Some(pos.sheet_id), ctx, false, pos.into())) @@ -129,9 +131,57 @@ pub fn convert_a1_to_rc(source: &str, ctx: &A1Context, pos: SheetPos) -> String replace_cell_range_references(source, ctx, pos, replace_fn) } -/// Replace all cell references with internal cell references (RC notation) by -/// applying the function `replace_x_fn` to X coordinates and `replace_y_fn` to -/// Y coordinates. +/// Translates all relative cell references that are within the same sheet as +/// the formula by `(delta_x, delta_y)`. +#[must_use] +pub fn translate_cell_references_within_sheet( + source: &str, + ctx: &A1Context, + pos: SheetPos, + delta_x: i64, + delta_y: i64, +) -> String { + replace_cell_references_with(source, ctx, pos, |sheet_id, mut cell_ref_range_end| { + if sheet_id == pos.sheet_id { + cell_ref_range_end.col.translate_in_place(delta_x)?; + cell_ref_range_end.row.translate_in_place(delta_y)?; + } + Ok(cell_ref_range_end) + }) +} + +/// Translates all relative cell references that are within the same sheet as +/// the formula _and_ past the given column or row. +/// +/// - If a column is supplied, then all X coordinates greater than or equal to +/// it are adjusted by `delta`. +/// - If a row is supplied, then all Y coordinates greater than or equal to it +/// are adjusted by `delta`. +#[must_use] +pub fn adjust_cell_references_within_sheet( + source: &str, + ctx: &A1Context, + pos: SheetPos, + column: Option, + row: Option, + delta: i64, +) -> String { + replace_cell_references_with(source, ctx, pos, |sheet_id, mut cell_ref_range_end| { + if sheet_id == pos.sheet_id { + if column.is_some_and(|c| c <= cell_ref_range_end.col.coord) { + cell_ref_range_end.col.translate_in_place(delta)?; + } + if row.is_some_and(|r| r <= cell_ref_range_end.row.coord) { + cell_ref_range_end.row.translate_in_place(delta)?; + } + } + Ok(cell_ref_range_end) + }) +} + +/// Replace all cell references by applying the function `replace_xy_fn` to each +/// end of the range. +#[must_use] pub fn replace_cell_references_with( source: &str, ctx: &A1Context, @@ -154,6 +204,70 @@ pub fn replace_cell_references_with( }) } +#[must_use] +pub fn replace_table_name( + source: &str, + ctx: &A1Context, + pos: SheetPos, + old_name: &str, + new_name: &str, +) -> String { + replace_table_references(source, ctx, pos, |mut table_ref| { + if table_ref.table_name.eq_ignore_ascii_case(old_name) { + table_ref.table_name = new_name.to_string(); + } + Ok(table_ref) + }) +} + +#[must_use] +pub fn replace_column_name( + source: &str, + ctx: &A1Context, + pos: SheetPos, + table_name: &str, + old_name: &str, + new_name: &str, +) -> String { + replace_table_references(source, ctx, pos, |mut table_ref| { + if table_ref.table_name.eq_ignore_ascii_case(table_name) { + table_ref.col_range.replace_column_name(old_name, new_name); + } + Ok(table_ref) + }) +} + +#[must_use] +fn replace_table_references( + source: &str, + ctx: &A1Context, + pos: SheetPos, + replace_fn: impl Fn(TableRef) -> Result, +) -> String { + replace_cell_range_references(source, ctx, pos, |range_ref| { + Ok(match range_ref.cells { + CellRefRange::Table { range } => CellRefRange::Table { + range: replace_fn(range)?, + }, + other @ CellRefRange::Sheet { .. } => other, + } + .to_string()) + }) +} + +#[must_use] +pub fn replace_sheet_name( + source: &str, + pos: SheetPos, + old_ctx: &A1Context, + new_ctx: &A1Context, +) -> String { + replace_cell_range_references(source, old_ctx, pos, |sheet_cell_ref_range| { + Ok(sheet_cell_ref_range.to_a1_string(Some(pos.sheet_id), new_ctx, false)) + }) +} + +#[must_use] fn replace_cell_range_references( source: &str, ctx: &A1Context, diff --git a/quadratic-core/src/grid/code_cell.rs b/quadratic-core/src/grid/code_cell.rs index 5e5ed9808f..5555c61264 100644 --- a/quadratic-core/src/grid/code_cell.rs +++ b/quadratic-core/src/grid/code_cell.rs @@ -4,7 +4,7 @@ use serde::{Deserialize, Serialize}; use crate::a1::{A1Context, A1Selection}; use crate::grid::CodeCellLanguage; -use crate::RefError; +use crate::{RefError, SheetPos}; use super::SheetId; @@ -22,10 +22,13 @@ pub struct CodeCellValue { } impl CodeCellValue { + /// Constructs a code cell. pub fn new(language: CodeCellLanguage, code: String) -> Self { Self { language, code } } + /// Constructs a new Python code cell. + #[cfg(test)] pub fn new_python(code: String) -> Self { Self { language: CodeCellLanguage::Python, @@ -33,13 +36,12 @@ impl CodeCellValue { } } - pub fn is_code_cell(&self) -> bool { - self.language == CodeCellLanguage::Python || self.language == CodeCellLanguage::Javascript - } - + /// Replaces `q.cells()` calls in Python and Javascript. + /// + /// Do not call this function unless `self.is_code_cell()`. fn replace_q_cells_a1_selection( &mut self, - default_sheet_id: &SheetId, + default_sheet_id: SheetId, a1_context: &A1Context, mut func: impl FnMut(A1Selection) -> Result, ) { @@ -66,21 +68,31 @@ impl CodeCellValue { /// Updates the cell references in the code by translating them by the given /// delta. Updates only relative cell references. + /// + /// `pos` should be the original position of the code cell, not the new + /// position. pub fn translate_cell_references( &mut self, + a1_context: &A1Context, + pos: SheetPos, + new_sheet_id: SheetId, delta_x: i64, delta_y: i64, - default_sheet_id: &SheetId, - a1_context: &A1Context, ) { - if delta_x == 0 && delta_y == 0 || !self.is_code_cell() { + if delta_x == 0 && delta_y == 0 { return; } - self.replace_q_cells_a1_selection(default_sheet_id, a1_context, |mut a1_selection| { - a1_selection.translate_in_place(delta_x, delta_y)?; - Ok(a1_selection.to_string(Some(*default_sheet_id), a1_context)) - }); + if self.language == CodeCellLanguage::Formula { + self.code = crate::formulas::translate_cell_references_within_sheet( + &self.code, a1_context, pos, delta_x, delta_y, + ); + } else if self.language.has_q_cells() { + self.replace_q_cells_a1_selection(new_sheet_id, a1_context, |mut a1_selection| { + a1_selection.translate_in_place(delta_x, delta_y)?; + Ok(a1_selection.to_string(Some(new_sheet_id), a1_context)) + }); + } } /// Adjusts the code cell references by the given delta. Updates only relative @@ -88,75 +100,89 @@ impl CodeCellValue { /// or row is inserted or deleted. pub fn adjust_code_cell_column_row( &mut self, + a1_context: &A1Context, + pos: SheetPos, column: Option, row: Option, delta: i64, - default_sheet_id: &SheetId, - a1_context: &A1Context, ) { - if delta != 0 && self.is_code_cell() { - self.replace_q_cells_a1_selection(default_sheet_id, a1_context, |mut a1_selection| { - a1_selection.adjust_column_row_in_place(column, row, delta); - Ok(a1_selection.to_string(Some(*default_sheet_id), a1_context)) - }); + if delta != 0 { + if self.language == CodeCellLanguage::Formula { + self.code = crate::formulas::adjust_cell_references_within_sheet( + &self.code, a1_context, pos, column, row, delta, + ); + } else if self.language.has_q_cells() { + self.replace_q_cells_a1_selection(pos.sheet_id, a1_context, |mut a1_selection| { + a1_selection.adjust_column_row_in_place(column, row, delta); + Ok(a1_selection.to_string(Some(pos.sheet_id), a1_context)) + }); + } } } /// Replaces the sheet name in the code cell references. pub fn replace_sheet_name_in_cell_references( &mut self, - old_name: &str, - new_name: &str, - default_sheet_id: &SheetId, - a1_context: &A1Context, + old_a1_context: &A1Context, + new_a1_context: &A1Context, + pos: SheetPos, ) { - if old_name != new_name && self.is_code_cell() { - let mut old_a1_context = a1_context.clone(); - - if let Some(sheet_id) = a1_context.try_sheet_name(old_name) { - old_a1_context.sheet_map.insert_parts(new_name, sheet_id); - } - - // create a copy of the a1_context so that we can send it to the to_string() function - let mut new_a1_context = old_a1_context.clone(); - new_a1_context.sheet_map.remove_name(old_name); - - self.replace_q_cells_a1_selection(default_sheet_id, &old_a1_context, |a1_selection| { - Ok(a1_selection.to_string(Some(*default_sheet_id), &new_a1_context)) + if self.language == CodeCellLanguage::Formula { + self.code = crate::formulas::replace_sheet_name( + &self.code, + pos, + old_a1_context, + new_a1_context, + ); + } else if self.language.has_q_cells() { + self.replace_q_cells_a1_selection(pos.sheet_id, old_a1_context, |a1_selection| { + Ok(a1_selection.to_string(Some(pos.sheet_id), new_a1_context)) }); } } - /// Replaces the table name in the code cell references. + /// Replaces a table name in the code cell references. pub fn replace_table_name_in_cell_references( &mut self, + a1_context: &A1Context, + pos: SheetPos, old_name: &str, new_name: &str, - default_sheet_id: &SheetId, - a1_context: &A1Context, ) { - if old_name != new_name && self.is_code_cell() { - self.replace_q_cells_a1_selection(default_sheet_id, a1_context, |mut a1_selection| { - a1_selection.replace_table_name(old_name, new_name); - Ok(a1_selection.to_string(Some(*default_sheet_id), a1_context)) - }); + if old_name != new_name { + if self.language == CodeCellLanguage::Formula { + self.code = crate::formulas::replace_table_name( + &self.code, a1_context, pos, old_name, new_name, + ); + } else if self.language.has_q_cells() { + self.replace_q_cells_a1_selection(pos.sheet_id, a1_context, |mut a1_selection| { + a1_selection.replace_table_name(old_name, new_name); + Ok(a1_selection.to_string(Some(pos.sheet_id), a1_context)) + }); + } } } /// Replaces column names in the code cell references. pub fn replace_column_name_in_cell_references( &mut self, + a1_context: &A1Context, + pos: SheetPos, table_name: &str, old_name: &str, new_name: &str, - default_sheet_id: &SheetId, - a1_context: &A1Context, ) { - if old_name != new_name && self.is_code_cell() { - self.replace_q_cells_a1_selection(default_sheet_id, a1_context, |mut a1_selection| { - a1_selection.replace_column_name(table_name, old_name, new_name); - Ok(a1_selection.to_string(Some(*default_sheet_id), a1_context)) - }); + if old_name != new_name { + if self.language == CodeCellLanguage::Formula { + self.code = crate::formulas::replace_column_name( + &self.code, a1_context, pos, table_name, old_name, new_name, + ); + } else if self.language.has_q_cells() { + self.replace_q_cells_a1_selection(pos.sheet_id, a1_context, |mut a1_selection| { + a1_selection.replace_column_name(table_name, old_name, new_name); + Ok(a1_selection.to_string(Some(pos.sheet_id), a1_context)) + }); + } } } } @@ -171,13 +197,18 @@ mod tests { fn test_update_cell_references() { let sheet_id = SheetId::new(); let a1_context = A1Context::default(); + let pos = SheetPos { + x: 100, + y: 100, + sheet_id, + }; // Basic single reference let mut code = CodeCellValue { language: CodeCellLanguage::Python, code: "q.cells('A1:B2')".to_string(), }; - code.translate_cell_references(1, 1, &sheet_id, &a1_context); + code.translate_cell_references(&a1_context, pos, sheet_id, 1, 1); assert_eq!(code.code, r#"q.cells("B2:C3")"#, "Basic reference failed"); // Multiple references in one line @@ -185,7 +216,7 @@ mod tests { language: CodeCellLanguage::Python, code: "x = q.cells('A1:B2') + q.cells('C3:D4')".to_string(), }; - code.translate_cell_references(1, 1, &sheet_id, &a1_context); + code.translate_cell_references(&a1_context, pos, sheet_id, 1, 1); assert_eq!( code.code, r#"x = q.cells("B2:C3") + q.cells("D4:E5")"#, "Multiple references failed" @@ -196,7 +227,7 @@ mod tests { language: CodeCellLanguage::Python, code: r#"q.cells("A1:B2"); q.cells("C3:D4"); q.cells("E5:F6");"#.to_string(), }; - code.translate_cell_references(1, 1, &sheet_id, &a1_context); + code.translate_cell_references(&a1_context, pos, sheet_id, 1, 1); assert_eq!( code.code, r#"q.cells("B2:C3"); q.cells("D4:E5"); q.cells("F6:G7");"#, "Quote types failed" @@ -207,7 +238,7 @@ mod tests { language: CodeCellLanguage::Python, code: r#"q.cells("A1:B2'); q.cells('C3:D4")"#.to_string(), }; - code.translate_cell_references(1, 1, &sheet_id, &a1_context); + code.translate_cell_references(&a1_context, pos, sheet_id, 1, 1); assert_eq!( code.code, r#"q.cells("A1:B2'); q.cells('C3:D4")"#, "Mismatched quotes failed" @@ -218,7 +249,7 @@ mod tests { language: CodeCellLanguage::Python, code: "q.cells('A1:B2')".to_string(), }; - code.translate_cell_references(0, 0, &sheet_id, &a1_context); + code.translate_cell_references(&a1_context, pos, sheet_id, 0, 0); assert_eq!(code.code, r#"q.cells('A1:B2')"#, "Zero delta failed"); // Negative delta @@ -226,7 +257,7 @@ mod tests { language: CodeCellLanguage::Python, code: "q.cells('C3:D4')".to_string(), }; - code.translate_cell_references(-1, -1, &sheet_id, &a1_context); + code.translate_cell_references(&a1_context, pos, sheet_id, -1, -1); assert_eq!(code.code, r#"q.cells("B2:C3")"#, "Negative delta failed"); // Whitespace variations @@ -234,26 +265,26 @@ mod tests { language: CodeCellLanguage::Python, code: "q.cells ( 'A1:B2' )".to_string(), }; - code.translate_cell_references(1, 1, &sheet_id, &a1_context); + code.translate_cell_references(&a1_context, pos, sheet_id, 1, 1); assert_eq!( code.code, r#"q.cells("B2:C3" )"#, "Whitespace variations failed" ); - // Non Python/JS languages should remain unchanged + // Formulas should get translated too let mut code = CodeCellValue { language: CodeCellLanguage::Formula, code: "A1".to_string(), }; - code.translate_cell_references(1, 1, &sheet_id, &a1_context); - assert_eq!(code.code, "A1", "Non Python/JS failed"); + code.translate_cell_references(&a1_context, pos, sheet_id, 1, 1); + assert_eq!(code.code, "B2", "Formula failed"); // Python first_row_header=True let mut code = CodeCellValue { language: CodeCellLanguage::Python, code: r#"q.cells("A1:B2", first_row_header=True)"#.to_string(), }; - code.translate_cell_references(1, 1, &sheet_id, &a1_context); + code.translate_cell_references(&a1_context, pos, sheet_id, 1, 1); assert_eq!( code.code, r#"q.cells("B2:C3", first_row_header=True)"#, "first_row_header=True failed" @@ -268,13 +299,18 @@ mod tests { a1_context .sheet_map .insert_test("Sheet1 (1)", SheetId::new()); + let pos = SheetPos { + x: 100, + y: 100, + sheet_id, + }; // Basic single reference let mut code = CodeCellValue { language: CodeCellLanguage::Python, code: r#"q.cells("'Sheet1 (1)'!A1:B2")"#.to_string(), }; - code.translate_cell_references(1, 1, &sheet_id, &a1_context); + code.translate_cell_references(&a1_context, pos, sheet_id, 1, 1); assert_eq!( code.code, r#"q.cells("'Sheet1 (1)'!B2:C3")"#, "Basic reference failed" @@ -285,7 +321,7 @@ mod tests { language: CodeCellLanguage::Python, code: r#"x = q.cells("'Sheet1'!A1:B2") + q.cells("'Sheet1 (1)'!C3:D4")"#.to_string(), }; - code.translate_cell_references(1, 1, &sheet_id, &a1_context); + code.translate_cell_references(&a1_context, pos, sheet_id, 1, 1); assert_eq!( code.code, r#"x = q.cells("B2:C3") + q.cells("'Sheet1 (1)'!D4:E5")"#, "Multiple references failed" @@ -296,7 +332,7 @@ mod tests { language: CodeCellLanguage::Python, code: r#"q.cells("'Sheet1'!A1:B2"); q.cells("'Sheet1 (1)'!C3:D4"); q.cells("'Sheet1'!E5:F6");"#.to_string(), }; - code.translate_cell_references(1, 1, &sheet_id, &a1_context); + code.translate_cell_references(&a1_context, pos, sheet_id, 1, 1); assert_eq!( code.code, r#"q.cells("B2:C3"); q.cells("'Sheet1 (1)'!D4:E5"); q.cells("F6:G7");"#, "Quote types failed" @@ -307,7 +343,7 @@ mod tests { language: CodeCellLanguage::Python, code: r#"q.cells("'Sheet1'!A1:B2'); q.cells(''Sheet1 (1)'!C3:D4")"#.to_string(), }; - code.translate_cell_references(1, 1, &sheet_id, &a1_context); + code.translate_cell_references(&a1_context, pos, sheet_id, 1, 1); assert_eq!( code.code, r#"q.cells("'Sheet1'!A1:B2'); q.cells(''Sheet1 (1)'!C3:D4")"#, "Mismatched quotes failed" @@ -318,7 +354,7 @@ mod tests { language: CodeCellLanguage::Python, code: r#"q.cells("'Sheet1 (1)'!A1:B2")"#.to_string(), }; - code.translate_cell_references(0, 0, &sheet_id, &a1_context); + code.translate_cell_references(&a1_context, pos, sheet_id, 0, 0); assert_eq!( code.code, r#"q.cells("'Sheet1 (1)'!A1:B2")"#, "Zero delta failed" @@ -329,7 +365,7 @@ mod tests { language: CodeCellLanguage::Python, code: r#"q.cells("'Sheet1 (1)'!C3:D4")"#.to_string(), }; - code.translate_cell_references(-1, -1, &sheet_id, &a1_context); + code.translate_cell_references(&a1_context, pos, sheet_id, -1, -1); assert_eq!( code.code, r#"q.cells("'Sheet1 (1)'!B2:C3")"#, "Negative delta failed" @@ -340,26 +376,26 @@ mod tests { language: CodeCellLanguage::Python, code: r#"q.cells ( "'Sheet1 (1)'!A1:B2" )"#.to_string(), }; - code.translate_cell_references(1, 1, &sheet_id, &a1_context); + code.translate_cell_references(&a1_context, pos, sheet_id, 1, 1); assert_eq!( code.code, r#"q.cells("'Sheet1 (1)'!B2:C3" )"#, "Whitespace variations failed" ); - // Non Python/JS languages should remain unchanged + // Formulas should get translated too let mut code = CodeCellValue { language: CodeCellLanguage::Formula, code: r#"A1"#.to_string(), }; - code.translate_cell_references(1, 1, &sheet_id, &a1_context); - assert_eq!(code.code, r#"A1"#, "Non Python/JS failed"); + code.translate_cell_references(&a1_context, pos, sheet_id, 1, 1); + assert_eq!(code.code, r#"B2"#, "Formula failed"); // Python first_row_header=True let mut code = CodeCellValue { language: CodeCellLanguage::Python, code: r#"q.cells("'Sheet1 (1)'!A1:B2", first_row_header=True)"#.to_string(), }; - code.translate_cell_references(1, 1, &sheet_id, &a1_context); + code.translate_cell_references(&a1_context, pos, sheet_id, 1, 1); assert_eq!( code.code, r#"q.cells("'Sheet1 (1)'!B2:C3", first_row_header=True)"#, "first_row_header=True failed" @@ -368,19 +404,24 @@ mod tests { #[test] fn test_replace_sheet_name_in_cell_references() { - let sheet_id = SheetId::TEST; - let a1_context = A1Context::test( - &[("Sheet1", sheet_id)], + let changed_sheet_id = SheetId::new(); + let old_a1_context = A1Context::test( + &[("Sheet1", changed_sheet_id)], &[("test.csv", &["city"], Rect::test_a1("A1:C3"))], ); + let mut new_a1_context = old_a1_context.clone(); + new_a1_context.sheet_map.remove_name("Sheet1"); + new_a1_context + .sheet_map + .insert_parts("Sheet1_new", changed_sheet_id); + let pos = SheetPos { + x: 100, + y: 100, + sheet_id: SheetId::new(), + }; let mut code = CodeCellValue::new_python("q.cells('Sheet1!A1:B2')".to_string()); - code.replace_sheet_name_in_cell_references( - "Sheet1", - "Sheet1_new", - &SheetId::new(), - &a1_context, - ); + code.replace_sheet_name_in_cell_references(&old_a1_context, &new_a1_context, pos); assert_eq!(code.code, r#"q.cells("'Sheet1_new'!A1:B2")"#); } @@ -391,17 +432,18 @@ mod tests { &[("Sheet1", sheet_id)], &[("simple", &["city"], Rect::test_a1("A1:C3"))], ); + let pos = SheetPos { + x: 100, + y: 100, + sheet_id, + }; let mut code = CodeCellValue { language: CodeCellLanguage::Python, code: "q.cells('simple[city]')".to_string(), }; - code.replace_table_name_in_cell_references( - "simple", - "test_new.csv", - &sheet_id, - &a1_context, - ); + + code.replace_table_name_in_cell_references(&a1_context, pos, "simple", "test_new.csv"); assert_eq!(code.code, r#"q.cells("test_new.csv[city]")"#); } @@ -412,37 +454,42 @@ mod tests { &[("Sheet1", sheet_id)], &[("test.csv", &["city", "state"], Rect::test_a1("A1:C3"))], ); + let pos = SheetPos { + x: 100, + y: 100, + sheet_id, + }; // ColRange::Col let mut code = CodeCellValue::new_python("q.cells('test.csv[city]')".to_string()); code.replace_column_name_in_cell_references( + &a1_context, + pos, "test.csv", "city", "city_new", - &sheet_id, - &a1_context, ); assert_eq!(code.code, r#"q.cells("test.csv[city_new]")"#); // ColRange::ColRange let mut code = CodeCellValue::new_python("q.cells('test.csv[[city]:[state]]')".to_string()); code.replace_column_name_in_cell_references( + &a1_context, + pos, "test.csv", "state", "state_new", - &sheet_id, - &a1_context, ); assert_eq!(code.code, r#"q.cells("test.csv[[city]:[state_new]]")"#); // ColRange::ColToEnd let mut code = CodeCellValue::new_python("q.cells('test.csv[[city]:]')".to_string()); code.replace_column_name_in_cell_references( + &a1_context, + pos, "test.csv", "city", "city_new", - &sheet_id, - &a1_context, ); assert_eq!(code.code, r#"q.cells("test.csv[[city_new]:]")"#); } diff --git a/quadratic-core/src/grid/code_run.rs b/quadratic-core/src/grid/code_run.rs index 95ada72370..db14795622 100644 --- a/quadratic-core/src/grid/code_run.rs +++ b/quadratic-core/src/grid/code_run.rs @@ -116,8 +116,13 @@ impl CodeRun { pub enum CodeCellLanguage { Python, Formula, - Connection { kind: ConnectionKind, id: String }, + /// Database connection. + Connection { + kind: ConnectionKind, + id: String, + }, Javascript, + /// CSV or other file import. Import, } @@ -128,6 +133,12 @@ impl CodeCellLanguage { CodeCellLanguage::Python | CodeCellLanguage::Javascript ) } + + /// Returns whether this language that uses `q.cells()` syntax (either + /// Python or Javascript). + pub fn has_q_cells(&self) -> bool { + *self == CodeCellLanguage::Python || *self == CodeCellLanguage::Javascript + } } #[derive(Serialize, Deserialize, Display, Copy, Debug, Clone, PartialEq, Eq, Hash)] diff --git a/quadratic-core/src/grid/sheet.rs b/quadratic-core/src/grid/sheet.rs index b594f666ad..5d322a24a4 100644 --- a/quadratic-core/src/grid/sheet.rs +++ b/quadratic-core/src/grid/sheet.rs @@ -128,15 +128,20 @@ impl Sheet { Ok(true) } + /// Replaces the sheet name when referenced in code cells. + /// + /// This must be called _before_ the sheet is actually renamed. pub fn replace_sheet_name_in_code_cells( &mut self, - old_name: &str, - new_name: &str, - context: &A1Context, + old_a1_context: &A1Context, + new_a1_context: &A1Context, ) { - self.replace_in_code_cells(context, |code_cell_value, a1_context, id| { - code_cell_value - .replace_sheet_name_in_cell_references(old_name, new_name, id, a1_context); + self.update_code_cells(|code_cell_value, pos| { + code_cell_value.replace_sheet_name_in_cell_references( + old_a1_context, + new_a1_context, + pos, + ); }); } diff --git a/quadratic-core/src/grid/sheet/data_table.rs b/quadratic-core/src/grid/sheet/data_table.rs index 65351f13ab..772837cd3b 100644 --- a/quadratic-core/src/grid/sheet/data_table.rs +++ b/quadratic-core/src/grid/sheet/data_table.rs @@ -2,8 +2,8 @@ use super::Sheet; use crate::{ a1::{A1Context, A1Selection}, cell_values::CellValues, - grid::{data_table::DataTable, CodeCellValue, DataTableKind, SheetId}, - Pos, Rect, + grid::{data_table::DataTable, CodeCellValue, DataTableKind}, + Pos, Rect, SheetPos, }; use anyhow::{anyhow, bail, Result}; @@ -139,18 +139,15 @@ impl Sheet { }) } - pub fn replace_in_code_cells( - &mut self, - context: &A1Context, - func: impl Fn(&mut CodeCellValue, &A1Context, &SheetId), - ) { + /// Calls a function to mutate all code cells. + pub fn update_code_cells(&mut self, func: impl Fn(&mut CodeCellValue, SheetPos)) { let positions = self.data_tables.keys().cloned().collect::>(); let sheet_id = self.id; for pos in positions { if let Some(cell_value) = self.cell_value_mut(pos) { if let Some(code_cell_value) = cell_value.code_cell_value_mut() { - func(code_cell_value, context, &sheet_id); + func(code_cell_value, pos.to_sheet_pos(sheet_id)); } } } @@ -161,11 +158,11 @@ impl Sheet { &mut self, old_name: &str, new_name: &str, - context: &A1Context, + a1_context: &A1Context, ) { - self.replace_in_code_cells(context, |code_cell_value, a1_context, id| { + self.update_code_cells(|code_cell_value, pos| { code_cell_value - .replace_table_name_in_cell_references(old_name, new_name, id, a1_context); + .replace_table_name_in_cell_references(a1_context, pos, old_name, new_name); }); } @@ -175,11 +172,11 @@ impl Sheet { table_name: &str, old_name: &str, new_name: &str, - context: &A1Context, + a1_context: &A1Context, ) { - self.replace_in_code_cells(context, |code_cell_value, a1_context, id| { + self.update_code_cells(|code_cell_value, pos| { code_cell_value.replace_column_name_in_cell_references( - table_name, old_name, new_name, id, a1_context, + a1_context, pos, table_name, old_name, new_name, ); }); } diff --git a/quadratic-core/src/grid/sheets.rs b/quadratic-core/src/grid/sheets.rs index 44ed965422..57183bbef4 100644 --- a/quadratic-core/src/grid/sheets.rs +++ b/quadratic-core/src/grid/sheets.rs @@ -1,9 +1,7 @@ use std::str::FromStr; -use crate::a1::A1Context; - use super::{Grid, Sheet, SheetId}; -use anyhow::{anyhow, Result}; +use anyhow::{anyhow, Context, Result}; use lexicon_fractional_index::key_between; impl Grid { @@ -166,14 +164,20 @@ impl Grid { self.sheets.iter_mut().find(|s| s.id == sheet_id) } - pub fn update_sheet_name(&mut self, old_name: &str, new_name: &str, context: &A1Context) { - for sheet in self.sheets.iter_mut() { - sheet.replace_sheet_name_in_code_cells(old_name, new_name, context); + /// Updates a sheet's name and returns the old name. + pub fn update_sheet_name(&mut self, sheet_id: SheetId, new_name: String) -> Result { + let old_a1_context = self.a1_context(); - if sheet.name == old_name { - sheet.name = new_name.to_string(); - } + let sheet = self.try_sheet_mut(sheet_id).context("missing sheet")?; + let old_name = std::mem::replace(&mut sheet.name, new_name); + + let new_a1_context = self.a1_context(); + + for sheet in &mut self.sheets { + sheet.replace_sheet_name_in_code_cells(&old_a1_context, &new_a1_context); } + + Ok(old_name) } #[cfg(test)] From 72fd2cbd852cfc8ce88368f4b0ec681dbe6d0829 Mon Sep 17 00:00:00 2001 From: HactarCE <6060305+HactarCE@users.noreply.github.com> Date: Fri, 28 Feb 2025 11:50:03 -0500 Subject: [PATCH 4/4] update another function call site --- .../controller/execution/execute_operation/execute_sheets.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/quadratic-core/src/controller/execution/execute_operation/execute_sheets.rs b/quadratic-core/src/controller/execution/execute_operation/execute_sheets.rs index 0ae9a6abf3..79cd9f72fe 100644 --- a/quadratic-core/src/controller/execution/execute_operation/execute_sheets.rs +++ b/quadratic-core/src/controller/execution/execute_operation/execute_sheets.rs @@ -86,10 +86,10 @@ impl GridController { { for (old_name, unique_name) in table_names_to_update_in_cell_ref.iter() { code_cell_value.replace_table_name_in_cell_references( + &context, + pos.to_sheet_pos(sheet_id), old_name, unique_name, - &sheet_id, - &context, ); } }