Rework code/formula translation & name-updating

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)

quadratic-core/src/a1/a1_selection/create.rs

@@ -159,18 +159,18 @@ 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)]
     #[track_caller]
     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()
     }
 }

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",

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"

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, A1Error> {
         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<Pos>,
     ) -> Result<Self, A1Error> {
@@ -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(&[("Sheet 1", 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(),

quadratic-core/src/a1/js_selection/create.rs

@@ -79,7 +79,7 @@
 ) -> Result<JsSelection, String> {
     let default_sheet_id = SheetId::from_str(default_sheet_id).map_err(|e| e.to_string())?;
     let context = serde_json::from_str::<A1Context>(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 })
 }

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();
                 }
             }

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<i64>,
-        row: Option<i64>,
-        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,

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,
                             );
                         }
                     }
@@ -210,12 +210,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