FIX: Minor fixes in column/row styles

Most notably deleting the formatting does not change width/height

base/src/test/user_model/test_column_style.rs

@@ -403,3 +403,55 @@ fn set_row_style_then_cell() {
     let style = model.get_cell_style(0, 12, 7).unwrap();
     assert_eq!(style.fill.bg_color, Some("#333444".to_string()));
 }
+
+#[test]
+fn column_style_then_row_alignment() {
+    let mut model = UserModel::new_empty("model", "en", "UTC").unwrap();
+    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,
+    };
+    model
+        .update_range_style(&column_g_range, "fill.bg_color", "#555666")
+        .unwrap();
+    model
+        .update_range_style(&row_3_range, "alignment.horizontal", "center")
+        .unwrap();
+    // check the row alignment does not affect the column style
+    let style = model.get_cell_style(0, 3, 7).unwrap();
+    assert_eq!(style.fill.bg_color, Some("#555666".to_string()));
+}
+
+#[test]
+fn column_style_then_width() {
+    let mut model = UserModel::new_empty("model", "en", "UTC").unwrap();
+    let column_g_range = Area {
+        sheet: 0,
+        row: 1,
+        column: 7,
+        width: 1,
+        height: LAST_ROW,
+    };
+    model
+        .update_range_style(&column_g_range, "fill.bg_color", "#555666")
+        .unwrap();
+    model
+        .set_column_width(0, 7, DEFAULT_COLUMN_WIDTH * 2.0)
+        .unwrap();
+
+    // Check column width worked:
+    assert_eq!(
+        model.get_column_width(0, 7).unwrap(),
+        DEFAULT_COLUMN_WIDTH * 2.0
+    );
+}

base/src/test/user_model/test_delete_row_column_formatting.rs

@@ -1,7 +1,7 @@
 #![allow(clippy::unwrap_used)]
 
 use crate::{
-    constants::{DEFAULT_COLUMN_WIDTH, LAST_COLUMN, LAST_ROW},
+    constants::{DEFAULT_COLUMN_WIDTH, DEFAULT_ROW_HEIGHT, LAST_COLUMN, LAST_ROW},
     expressions::types::Area,
     UserModel,
 };
@@ -121,8 +121,13 @@ fn column_width() {
         .update_range_style(&column_g_range, "fill.bg_color", "#555666")
         .unwrap();
 
+    // Delete the column formatting
     model.range_clear_formatting(&column_g_range).unwrap();
-    assert_eq!(model.get_column_width(0, 7).unwrap(), DEFAULT_COLUMN_WIDTH);
+    // This does not change the column width
+    assert_eq!(
+        model.get_column_width(0, 7).unwrap(),
+        2.0 * DEFAULT_COLUMN_WIDTH
+    );
 
     model.undo().unwrap();
     assert_eq!(
@@ -130,5 +135,109 @@ fn column_width() {
         2.0 * DEFAULT_COLUMN_WIDTH
     );
     model.redo().unwrap();
-    assert_eq!(model.get_column_width(0, 7).unwrap(), DEFAULT_COLUMN_WIDTH);
+    assert_eq!(
+        model.get_column_width(0, 7).unwrap(),
+        2.0 * DEFAULT_COLUMN_WIDTH
+    );
+}
+
+#[test]
+fn column_row_style_undo() {
+    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,
+    };
+
+    let row_123_range = Area {
+        sheet: 0,
+        row: 123,
+        column: 1,
+        width: LAST_COLUMN,
+        height: 1,
+    };
+
+    let delete_range = Area {
+        sheet: 0,
+        row: 120,
+        column: 5,
+        width: 20,
+        height: 20,
+    };
+
+    // Set the style of the whole column
+    model
+        .update_range_style(&column_g_range, "fill.bg_color", "#555666")
+        .unwrap();
+
+    model
+        .update_range_style(&row_123_range, "fill.bg_color", "#111222")
+        .unwrap();
+
+    model.range_clear_formatting(&delete_range).unwrap();
+
+    // check G123 is empty
+    let style = model.get_cell_style(0, 123, 7).unwrap();
+    assert_eq!(style.fill.bg_color, None);
+
+    // uno clear formatting
+    model.undo().unwrap();
+
+    // G123 has the row style
+    let style = model.get_cell_style(0, 123, 7).unwrap();
+    assert_eq!(style.fill.bg_color, Some("#111222".to_owned()));
+
+    // undo twice
+    model.undo().unwrap();
+    model.undo().unwrap();
+
+    // check G123 is empty
+    let style = model.get_cell_style(0, 123, 7).unwrap();
+    assert_eq!(style.fill.bg_color, None);
+}
+
+#[test]
+fn column_row_row_height_undo() {
+    let mut model = UserModel::new_empty("model", "en", "UTC").unwrap();
+
+    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,
+    };
+
+    model
+        .update_range_style(&column_g_range, "fill.bg_color", "#555666")
+        .unwrap();
+
+    model
+        .set_row_height(0, 3, DEFAULT_ROW_HEIGHT * 2.0)
+        .unwrap();
+
+    model
+        .update_range_style(&row_3_range, "fill.bg_color", "#111222")
+        .unwrap();
+
+    model.undo().unwrap();
+
+    // check G3 has the column style
+    let style = model.get_cell_style(0, 3, 7).unwrap();
+    assert_eq!(style.fill.bg_color, Some("#555666".to_string()));
 }

base/src/user_model/common.rs

@@ -6,7 +6,7 @@ use csv::{ReaderBuilder, WriterBuilder};
 use serde::{Deserialize, Serialize};
 
 use crate::{
-    constants::{self, DEFAULT_COLUMN_WIDTH, DEFAULT_ROW_HEIGHT, LAST_COLUMN, LAST_ROW},
+    constants::{self, DEFAULT_ROW_HEIGHT, LAST_COLUMN, LAST_ROW},
     expressions::{
         types::{Area, CellReferenceIndex},
         utils::{is_valid_column_number, is_valid_row},
@@ -653,18 +653,6 @@ impl UserModel {
             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
@@ -688,7 +676,7 @@ impl UserModel {
                     sheet,
                     row,
                     column,
-                    old_style: Box::new(old_style),
+                    old_style: Box::new(Some(old_style)),
                 });
             } else {
                 let old_style = self.model.get_style_for_cell(sheet, row, column)?;
@@ -701,7 +689,7 @@ impl UserModel {
                         sheet,
                         row,
                         column,
-                        old_style: Box::new(old_style),
+                        old_style: Box::new(None),
                     });
                 }
             }
@@ -718,7 +706,7 @@ impl UserModel {
                     sheet,
                     row: row.r,
                     column,
-                    old_style: Box::new(old_style),
+                    old_style: Box::new(Some(old_style)),
                 });
             } else {
                 let old_style = self.model.get_style_for_cell(sheet, row.r, column)?;
@@ -731,7 +719,7 @@ impl UserModel {
                         sheet,
                         row: row.r,
                         column,
-                        old_style: Box::new(old_style),
+                        old_style: Box::new(None),
                     });
                 }
             }
@@ -770,7 +758,7 @@ impl UserModel {
                     sheet,
                     row,
                     column,
-                    old_style: Box::new(old_style),
+                    old_style: Box::new(Some(old_style)),
                 });
             } else {
                 let old_style = self.model.get_style_for_cell(sheet, row, column)?;
@@ -783,7 +771,7 @@ impl UserModel {
                         sheet,
                         row,
                         column,
-                        old_style: Box::new(old_style),
+                        old_style: Box::new(None),
                     });
                 }
             }
@@ -825,7 +813,7 @@ impl UserModel {
                         sheet,
                         row,
                         column,
-                        old_style: Box::new(old_style),
+                        old_style: Box::new(Some(old_style)),
                     });
                 } else {
                     let old_style = self.model.get_style_for_cell(sheet, row, column)?;
@@ -838,7 +826,7 @@ impl UserModel {
                             sheet,
                             row,
                             column,
-                            old_style: Box::new(old_style),
+                            old_style: Box::new(None),
                         });
                     }
                 }
@@ -1191,22 +1179,8 @@ impl UserModel {
             }
         } else if range.column == 1 && range.width == LAST_COLUMN {
             // Full rows
+            let styled_columns = &self.model.workbook.worksheet(sheet)?.cols.clone();
             for row in range.row..range.row + range.height {
-                // First update the style of the row
-                let old_style = self.model.get_row_style(sheet, row)?;
-                let style = match old_style.as_ref() {
-                    Some(s) => s,
-                    None => &Style::default(),
-                };
-                let style = update_style(style, style_path, value)?;
-                self.model.set_row_style(sheet, row, &style)?;
-                diff_list.push(Diff::SetRowStyle {
-                    sheet,
-                    row,
-                    old_value: Box::new(old_style),
-                    new_value: Box::new(style),
-                });
-
                 // Now update style in all cells that are not empty
                 let columns: Vec<i32> = self
                     .model
@@ -1226,8 +1200,35 @@ impl UserModel {
                         &mut diff_list,
                     )?;
                 }
-                // since rows take precedence over columns we do not need
+
-                // to update the styles in all cells that have a column style
+                // We need to go through all the cells that have a column style and merge them
+                for col in styled_columns.iter() {
+                    for column in col.min..col.max + 1 {
+                        self.update_single_cell_style(
+                            sheet,
+                            row,
+                            column,
+                            style_path,
+                            value,
+                            &mut diff_list,
+                        )?;
+                    }
+                }
+
+                // Finally update the style of the row
+                let old_style = self.model.get_row_style(sheet, row)?;
+                let style = match old_style.as_ref() {
+                    Some(s) => s,
+                    None => &Style::default(),
+                };
+                let style = update_style(style, style_path, value)?;
+                self.model.set_row_style(sheet, row, &style)?;
+                diff_list.push(Diff::SetRowStyle {
+                    sheet,
+                    row,
+                    old_value: Box::new(old_style),
+                    new_value: Box::new(style),
+                });
             }
         } else {
             for row in range.row..range.row + range.height {
@@ -2036,8 +2037,11 @@ impl UserModel {
                     column,
                     old_style,
                 } => {
-                    self.model
+                    if let Some(value) = old_style.as_ref() {
-                        .set_cell_style(*sheet, *row, *column, old_style)?;
+                        self.model.set_cell_style(*sheet, *row, *column, value)?;
+                    } else {
+                        self.model.cell_clear_all(*sheet, *row, *column)?;
+                    }
                 }
                 Diff::DeleteSheet { sheet, old_data } => {
                     needs_evaluation = true;

base/src/user_model/history.rs

@@ -43,7 +43,7 @@ pub(crate) enum Diff {
         sheet: u32,
         row: i32,
         column: i32,
-        old_style: Box<Style>,
+        old_style: Box<Option<Style>>,
     },
     SetCellStyle {
         sheet: u32,

base/src/worksheet.rs

@@ -123,18 +123,20 @@ impl Worksheet {
     }
 
     pub fn set_row_style(&mut self, row: i32, style_index: i32) -> Result<(), String> {
+        // FIXME: This is a HACK
+        let custom_format = style_index != 0;
         for r in self.rows.iter_mut() {
             if r.r == row {
                 r.s = style_index;
-                r.custom_format = true;
+                r.custom_format = custom_format;
                 return Ok(());
             }
         }
         self.rows.push(Row {
             height: constants::DEFAULT_ROW_HEIGHT / constants::ROW_HEIGHT_FACTOR,
             r: row,
-            custom_format: true,
+            custom_format,
-            custom_height: true,
+            custom_height: false,
             s: style_index,
             hidden: false,
         });
@@ -150,7 +152,10 @@ impl Worksheet {
             }
         }
         if let Some(i) = index {
-            self.rows.remove(i);
+            if let Some(r) = self.rows.get_mut(i) {
+                r.s = 0;
+                r.custom_format = false;
+            }
         }
         Ok(())
     }
@@ -400,6 +405,7 @@ impl Worksheet {
                 if min == column && max == column {
                     c.style = style;
                     c.width = width / constants::COLUMN_WIDTH_FACTOR;
+                    c.custom_width = width != constants::DEFAULT_COLUMN_WIDTH;
                     return Ok(());
                 }
                 split = true;