FIX: diverse issues with set/delete column and row styles

base/src/test/user_model/mod.rs

@@ -5,6 +5,7 @@ mod test_border;
 mod test_clear_cells;
 mod test_column_style;
 mod test_defined_names;
+mod test_delete_row_column_formatting;
 mod test_diff_queue;
 mod test_evaluation;
 mod test_general;

base/src/test/user_model/test_column_style.rs

@@ -330,3 +330,76 @@ fn cell_row_undo() {
     let style = model.get_cell_style(0, 12, 7).unwrap();
     assert_eq!(style.fill.bg_color, Some("#333444".to_string()));
 }
+
+#[test]
+fn set_column_style_then_cell() {
+    // We check that if we set a cell style in a column that already has a style
+    // the styles compound
+    let mut model = UserModel::new_empty("model", "en", "UTC").unwrap();
+    let cell_g12 = Area {
+        sheet: 0,
+        row: 12,
+        column: 7,
+        width: 1,
+        height: 1,
+    };
+
+    let column_g_range = Area {
+        sheet: 0,
+        row: 1,
+        column: 7,
+        width: 1,
+        height: LAST_ROW,
+    };
+
+    // Set G12 background to red
+    model
+        .update_range_style(&column_g_range, "fill.bg_color", "#333444")
+        .unwrap();
+
+    model
+        .update_range_style(&cell_g12, "alignment.horizontal", "center")
+        .unwrap();
+
+    let style = model.get_cell_style(0, 12, 7).unwrap();
+    assert_eq!(style.fill.bg_color, Some("#333444".to_string()));
+
+    model.undo().unwrap();
+    model.undo().unwrap();
+    let style = model.get_cell_style(0, 12, 7).unwrap();
+    assert_eq!(style.fill.bg_color, None);
+}
+
+#[test]
+fn set_row_style_then_cell() {
+    // We check that if we set a cell style in a column that already has a style
+    // the styles compound
+    let mut model = UserModel::new_empty("model", "en", "UTC").unwrap();
+    let cell_g12 = Area {
+        sheet: 0,
+        row: 12,
+        column: 7,
+        width: 1,
+        height: 1,
+    };
+
+    let row_12_range = Area {
+        sheet: 0,
+        row: 12,
+        column: 1,
+        width: LAST_COLUMN,
+        height: 1,
+    };
+
+    // Set G12 background to red
+    model
+        .update_range_style(&row_12_range, "fill.bg_color", "#333444")
+        .unwrap();
+
+    model
+        .update_range_style(&cell_g12, "alignment.horizontal", "center")
+        .unwrap();
+
+    let style = model.get_cell_style(0, 12, 7).unwrap();
+    assert_eq!(style.fill.bg_color, Some("#333444".to_string()));
+}

base/src/test/user_model/test_delete_row_column_formatting.rs

@@ -0,0 +1,134 @@
+#![allow(clippy::unwrap_used)]
+
+use crate::{
+    constants::{DEFAULT_COLUMN_WIDTH, LAST_COLUMN, LAST_ROW},
+    expressions::types::Area,
+    UserModel,
+};
+
+#[test]
+fn delete_column_formatting() {
+    // We are going to delete formatting in column G (7)
+    // There are cells with their own styles
+    // There are rows with their own styles
+    let mut model = UserModel::new_empty("model", "en", "UTC").unwrap();
+    let cell_g123 = Area {
+        sheet: 0,
+        row: 123,
+        column: 7,
+        width: 1,
+        height: 1,
+    };
+
+    let column_g_range = Area {
+        sheet: 0,
+        row: 1,
+        column: 7,
+        width: 1,
+        height: LAST_ROW,
+    };
+
+    let row_3_range = Area {
+        sheet: 0,
+        row: 3,
+        column: 1,
+        width: LAST_COLUMN,
+        height: 1,
+    };
+
+    // Set the style of the whole column
+    model
+        .update_range_style(&column_g_range, "fill.bg_color", "#555666")
+        .unwrap();
+
+    // Set G123 background to red
+    model
+        .update_range_style(&cell_g123, "fill.bg_color", "#FF5533")
+        .unwrap();
+
+    // Set the style of the whole row
+    model
+        .update_range_style(&row_3_range, "fill.bg_color", "#333444")
+        .unwrap();
+
+    // Delete the column formatting
+    model.range_clear_formatting(&column_g_range).unwrap();
+
+    // Check the style of G123 is now what it was before
+    let style = model.get_cell_style(0, 123, 7).unwrap();
+    assert_eq!(style.fill.bg_color, None);
+
+    // Check the style of the whole row is still there
+    let style = model.get_cell_style(0, 3, 1).unwrap();
+    assert_eq!(style.fill.bg_color, Some("#333444".to_owned()));
+
+    // Check the style of the whole column is now gone
+    let style = model.get_cell_style(0, 3, 7).unwrap();
+    assert_eq!(style.fill.bg_color, None);
+
+    let style = model.get_cell_style(0, 40, 7).unwrap();
+    assert_eq!(style.fill.bg_color, None);
+
+    model.undo().unwrap();
+
+    // Check the style of G123 is now what it was before
+    let style = model.get_cell_style(0, 123, 7).unwrap();
+    assert_eq!(style.fill.bg_color, Some("#FF5533".to_owned()));
+
+    // Check G3 is the row style
+    let style = model.get_cell_style(0, 3, 7).unwrap();
+    assert_eq!(style.fill.bg_color, Some("#333444".to_owned()));
+
+    // Check G40 is the column style
+    let style = model.get_cell_style(0, 40, 7).unwrap();
+    assert_eq!(style.fill.bg_color, Some("#555666".to_owned()));
+
+    model.redo().unwrap();
+
+    // Check the style of G123 is now what it was before
+    let style = model.get_cell_style(0, 123, 7).unwrap();
+    assert_eq!(style.fill.bg_color, None);
+
+    // Check the style of the whole row is still there
+    let style = model.get_cell_style(0, 3, 1).unwrap();
+    assert_eq!(style.fill.bg_color, Some("#333444".to_owned()));
+
+    // Check the style of the whole column is now gone
+    let style = model.get_cell_style(0, 3, 7).unwrap();
+    assert_eq!(style.fill.bg_color, None);
+
+    let style = model.get_cell_style(0, 40, 7).unwrap();
+    assert_eq!(style.fill.bg_color, None);
+}
+
+#[test]
+fn column_width() {
+    let mut model = UserModel::new_empty("model", "en", "UTC").unwrap();
+    model
+        .set_column_width(0, 7, DEFAULT_COLUMN_WIDTH * 2.0)
+        .unwrap();
+
+    let column_g_range = Area {
+        sheet: 0,
+        row: 1,
+        column: 7,
+        width: 1,
+        height: LAST_ROW,
+    };
+
+    // Set the style of the whole column
+    model
+        .update_range_style(&column_g_range, "fill.bg_color", "#555666")
+        .unwrap();
+
+    model.range_clear_formatting(&column_g_range).unwrap();
+    assert_eq!(model.get_column_width(0, 7).unwrap(), DEFAULT_COLUMN_WIDTH);
+
+    model.undo().unwrap();
+    assert_eq!(
+        model.get_column_width(0, 7).unwrap(),
+        2.0 * DEFAULT_COLUMN_WIDTH
+    );
+    model.redo().unwrap();
+    assert_eq!(model.get_column_width(0, 7).unwrap(), DEFAULT_COLUMN_WIDTH);
+}

base/src/user_model/common.rs

@@ -6,7 +6,7 @@ use csv::{ReaderBuilder, WriterBuilder};
 use serde::{Deserialize, Serialize};
 
 use crate::{
-    constants::{self, DEFAULT_ROW_HEIGHT, LAST_COLUMN, LAST_ROW},
+    constants::{self, DEFAULT_COLUMN_WIDTH, DEFAULT_ROW_HEIGHT, LAST_COLUMN, LAST_ROW},
     expressions::{
         types::{Area, CellReferenceIndex},
         utils::{is_valid_column_number, is_valid_row},
@@ -648,13 +648,23 @@ impl UserModel {
         let mut diff_list = Vec::new();
         let old_value = self.model.get_column_style(sheet, column)?;
         self.model.delete_column_style(sheet, column)?;
-        self.model
-            .set_column_width(sheet, column, constants::DEFAULT_COLUMN_WIDTH)?;
         diff_list.push(Diff::DeleteColumnStyle {
             sheet,
             column,
             old_value: Box::new(old_value),
         });
+        let column_width = self.model.get_column_width(sheet, column)?;
+        if column_width != DEFAULT_COLUMN_WIDTH {
+            self.model
+                .set_column_width(sheet, column, DEFAULT_COLUMN_WIDTH)?;
+
+            diff_list.push(Diff::SetColumnWidth {
+                sheet,
+                column,
+                new_value: DEFAULT_COLUMN_WIDTH,
+                old_value: column_width,
+            });
+        }
 
         let data_rows: Vec<i32> = self
             .model
@@ -790,7 +800,6 @@ impl UserModel {
     /// * [UserModel::range_clear_contents]
     pub fn range_clear_formatting(&mut self, range: &Area) -> Result<(), String> {
         let sheet = range.sheet;
-
         if range.row == 1 && range.height == LAST_ROW {
             for column in range.column..range.column + range.width {
                 self.clear_column_formatting(sheet, column)?;
@@ -1084,6 +1093,7 @@ impl UserModel {
         Ok(())
     }
 
+    // Updates the style of a cell
     fn update_single_cell_style(
         &mut self,
         sheet: u32,
@@ -1093,12 +1103,11 @@ impl UserModel {
         value: &str,
         diff_list: &mut Vec<Diff>,
     ) -> Result<(), String> {
+        // This is the value in the cell itself
         let old_value = self.model.get_cell_style_or_none(sheet, row, column)?;
-        let style = match old_value.as_ref() {
-            Some(s) => s,
-            None => &Style::default(),
-        };
-        let new_style = update_style(style, style_path, value)?;
+        // This takes into account row or column styles
+        let style = self.get_cell_style(sheet, row, column)?;
+        let new_style = update_style(&style, style_path, value)?;
         self.model.set_cell_style(sheet, row, column, &new_style)?;
         diff_list.push(Diff::SetCellStyle {
             sheet,
@@ -2291,27 +2300,19 @@ impl UserModel {
                 Diff::DeleteColumnStyle {
                     sheet,
                     column,
-                    old_value,
+                    old_value: _,
                 } => {
-                    if let Some(s) = old_value.as_ref() {
-                        self.model.set_column_style(*sheet, *column, s)?;
-                    } else {
                     self.model.delete_column_style(*sheet, *column)?;
                 }
-                }
                 Diff::DeleteRowStyle {
                     sheet,
                     row,
-                    old_value,
+                    old_value: _,
                 } => {
-                    if let Some(s) = old_value.as_ref() {
-                        self.model.set_row_style(*sheet, *row, s)?;
-                    } else {
                     self.model.delete_row_style(*sheet, *row)?;
                 }
             }
         }
-        }
 
         if needs_evaluation {
             self.evaluate_if_not_paused();