diff --git a/src/command/dispatch.rs b/src/command/dispatch.rs index c8d5bf1..29eeb47 100644 --- a/src/command/dispatch.rs +++ b/src/command/dispatch.rs @@ -36,12 +36,16 @@ pub fn dispatch(model: &mut Model, cmd: &Command) -> CommandResult { let kv: Vec<(String, String)> = coords.iter() .map(|pair| (pair[0].clone(), pair[1].clone())) .collect(); - // Ensure items exist - for (cat_name, item_name) in &kv { - if let Some(cat) = model.category_mut(cat_name) { - cat.add_item(item_name); + // Validate all categories exist before mutating anything + for (cat_name, _) in &kv { + if model.category(cat_name).is_none() { + return CommandResult::err(format!("Category '{cat_name}' not found")); } } + // Ensure items exist within their categories + for (cat_name, item_name) in &kv { + model.category_mut(cat_name).unwrap().add_item(item_name); + } let key = CellKey::new(kv); let cell_value = match value { CellValueArg::Number { number } => CellValue::Number(*number), @@ -136,7 +140,8 @@ pub fn dispatch(model: &mut Model, cmd: &Command) -> CommandResult { Command::Load { path } => { match persistence::load(std::path::Path::new(path)) { - Ok(loaded) => { + Ok(mut loaded) => { + loaded.normalize_view_state(); *model = loaded; CommandResult::ok_msg(format!("Loaded from {path}")) } diff --git a/src/formula/parser.rs b/src/formula/parser.rs index 7d2d9fb..25ab00b 100644 --- a/src/formula/parser.rs +++ b/src/formula/parser.rs @@ -255,6 +255,8 @@ fn parse_primary(tokens: &[Token], pos: &mut usize) -> Result { // expect ) if *pos < tokens.len() && tokens[*pos] == Token::RParen { *pos += 1; + } else { + return Err(anyhow!("Expected ')' to close aggregate function")); } return Ok(Expr::Agg(func, Box::new(inner), filter)); } @@ -268,7 +270,11 @@ fn parse_primary(tokens: &[Token], pos: &mut usize) -> Result { let then = parse_add_sub(tokens, pos)?; if *pos < tokens.len() && tokens[*pos] == Token::Comma { *pos += 1; } let else_ = parse_add_sub(tokens, pos)?; - if *pos < tokens.len() && tokens[*pos] == Token::RParen { *pos += 1; } + if *pos < tokens.len() && tokens[*pos] == Token::RParen { + *pos += 1; + } else { + return Err(anyhow!("Expected ')' to close IF(...)")); + } return Ok(Expr::If(Box::new(cond), Box::new(then), Box::new(else_))); } Ok(Expr::Ref(name)) diff --git a/src/model/model.rs b/src/model/model.rs index 2e889d6..24441b2 100644 --- a/src/model/model.rs +++ b/src/model/model.rs @@ -123,6 +123,16 @@ impl Model { Ok(()) } + /// Reset all view scroll offsets to zero. + /// Call this after loading or replacing a model so stale offsets don't + /// cause the grid to render an empty area. + pub fn normalize_view_state(&mut self) { + for view in self.views.values_mut() { + view.row_offset = 0; + view.col_offset = 0; + } + } + /// Return all category names pub fn category_names(&self) -> Vec<&str> { self.categories.keys().map(|s| s.as_str()).collect() @@ -182,7 +192,7 @@ impl Model { "+" => lv + rv, "-" => lv - rv, "*" => lv * rv, - "/" => if rv == 0.0 { 0.0 } else { lv / rv }, + "/" => { if rv == 0.0 { return None; } lv / rv } "^" => lv.powf(rv), _ => return None, }) @@ -482,7 +492,7 @@ mod formula_tests { } #[test] - fn division_by_zero_yields_zero() { + fn division_by_zero_yields_empty() { let mut m = Model::new("Test"); m.add_category("Measure").unwrap(); m.add_category("Region").unwrap(); @@ -494,7 +504,8 @@ mod formula_tests { m.set_cell(coord(&[("Measure", "Revenue"), ("Region", "East")]), CellValue::Number(100.0)); m.set_cell(coord(&[("Measure", "Zero"), ("Region", "East")]), CellValue::Number(0.0)); m.add_formula(parse_formula("Result = Revenue / Zero", "Measure").unwrap()); - assert_eq!(m.evaluate(&coord(&[("Measure", "Result"), ("Region", "East")])), CellValue::Number(0.0)); + // Division by zero must yield Empty, not 0, so the user sees a blank not a misleading zero. + assert_eq!(m.evaluate(&coord(&[("Measure", "Result"), ("Region", "East")])), CellValue::Empty); } #[test]