diff --git a/crates/engine/src/cmd/insert.rs b/crates/engine/src/cmd/insert.rs index 9f044613d..236222247 100644 --- a/crates/engine/src/cmd/insert.rs +++ b/crates/engine/src/cmd/insert.rs @@ -188,7 +188,7 @@ pub fn specs() -> Vec { "Conditional Formatting", ["Home", "Styles"], None, - "{range?, rule: {type: cellIs|expression|containsText|beginsWith|endsWith|blanks|noBlanks|errors|noErrors|duplicate|unique|top10|aboveAverage|timePeriod|colorScale|dataBar|iconSet, operator?, value?, value2?, formula?, text?, rank?, bottom?, percent?, below?, period?, colors?: [..], color?, set?, style?: {font..fill..}|preset: lightRedFill|redText|yellowFill|greenFill|redBorder|lightRedFillDarkRedText…}}", + "{range?, rule: {type: cellIs|expression|containsText|beginsWith|endsWith|blanks|noBlanks|errors|noErrors|duplicate|unique|top10|aboveAverage|timePeriod|colorScale|dataBar|iconSet, operator?, value?, value2?, formula?, text?, rank?, bottom?, percent?, below?, period?, colors?: [..], color?, set?, style?: {font?: {bold?, italic?, color?}, fill?: \"#FFC7CE\" | {pattern?, fg?, bg?}, borders?, num_fmt?} (colours \"#RRGGBB\") | preset?: lightRedFill|redText|yellowFill|greenFill|redBorder|lightRedFillDarkRedText…}; a text `value` is matched as text, or kept when already a quoted literal}", has_doc, add_cf ), @@ -1335,6 +1335,67 @@ fn preset_style(name: &str) -> Style { s } +/// Whether `v` is one formula string literal: `"…"` with every inner quote doubled. +fn is_string_literal(v: &str) -> bool { + v.len() >= 2 && v.starts_with('"') && v.ends_with('"') && v.get(1..v.len() - 1).is_some_and(|inner| !inner.replace("\"\"", "").contains('"')) +} + +/// A conditional format's `style` as the model's `Style` JSON (`font`, `fill`, `borders`, `align`, +/// `num_fmt`, `protection`, merged over the default by the caller), where a colour may also be +/// written as `format_range` takes it (`"#RRGGBB"`, `{theme, tint}`, `"auto"`) and `fill` may be a +/// colour alone (a solid fill) or `"none"` (no fill). An unknown key is an error rather than +/// dropped: dropped, it left a rule with an empty format that matches and shows nothing. +fn cf_style_json(st: &Json) -> Result { + const KEYS: [&str; 6] = ["font", "fill", "borders", "align", "num_fmt", "protection"]; + let err = |m: String| bad("home.conditionalFormat", m); + let Some(obj) = st.as_object() else { return Err(err("`style` must be an object, e.g. {\"fill\": \"#FFC7CE\"}".into())) }; + let mut st = obj.clone(); + if let Some(k) = st.keys().find(|k| !KEYS.contains(&k.as_str())) { + let hint = if k == "preset" { "; a preset goes in `rule.preset`" } else { "" }; + return Err(err(format!("unknown key `{k}` in `style` (known: {}){hint}", KEYS.join(", ")))); + } + // A colour written the way the other formatting commands take it, as the model stores it. + let color = |v: &mut Json, what: &str| -> Result<()> { + if v.is_string() || v.get("theme").is_some() { + let c = super::format::color_param(Some(v)).ok_or_else(|| err(format!("invalid colour {v} for `{what}`")))?; + *v = serde_json::to_value(c).map_err(|e| EngineError::Other(e.to_string()))?; + } + Ok(()) + }; + if let Some(f) = st.get_mut("font").and_then(Json::as_object_mut) + && let Some(c) = f.get_mut("color") + { + color(c, "font.color")?; + } + if let Some(fill) = st.get_mut("fill") { + // `"none"` (or `"auto"`) is no fill, as `home.fillColor` reads it, not a solid automatic one. + if fill.is_string() && super::format::color_param(Some(fill)) == Some(Color::Auto) { + *fill = serde_json::to_value(Fill::default()).map_err(|e| EngineError::Other(e.to_string()))?; + } else if fill.is_string() || fill.get("theme").is_some() { + *fill = json!({"pattern": "Solid", "fg": fill.clone()}); + } + if let Some(f) = fill.as_object_mut() { + for k in ["fg", "bg"] { + if let Some(c) = f.get_mut(k) { + color(c, &format!("fill.{k}"))?; + } + } + // A colour with no pattern is a solid fill, not an invisible one. + if f.contains_key("fg") && !f.contains_key("pattern") { + f.insert("pattern".into(), json!("Solid")); + } + } + } + if let Some(b) = st.get_mut("borders").and_then(Json::as_object_mut) { + for (side, line) in b.iter_mut() { + if let Some(c) = line.get_mut("color") { + color(c, &format!("borders.{side}.color"))?; + } + } + } + Ok(Json::Object(st)) +} + fn add_cf(s: &mut Session, p: &Json) -> Result { let ranges = target_ranges(s, p)?; let sheet = target_sheet(s, p)?; @@ -1344,9 +1405,10 @@ fn add_cf(s: &mut Session, p: &Json) -> Result { }; let g = |k: &str| rule.get(k).and_then(Json::as_str).map(str::to_string); let style = Box::new(match rule.get("style") { - Some(st) if st.is_object() => { + Some(st) if !st.is_null() => { + let st = cf_style_json(st)?; let base = serde_json::to_value(Style::default()).map_err(|e| EngineError::Other(e.to_string()))?; - serde_json::from_value(super::format::merge_json(base, st)).map_err(|e| bad("home.conditionalFormat", e.to_string()))? + serde_json::from_value(super::format::merge_json(base, &st)).map_err(|e| bad("home.conditionalFormat", e.to_string()))? } _ => preset_style(g("preset").as_deref().unwrap_or("lightRedFillDarkRedText")), }); @@ -1385,8 +1447,12 @@ fn add_cf(s: &mut Session, p: &Json) -> Result { let a = rule.get("value").map(super::edit::json_to_input).unwrap_or_default(); let b = rule.get("value2").map(super::edit::json_to_input); let quote = |v: String| { + // A number, a formula, or text already written as a string literal (`"Closed"`, as + // in the file and in openpyxl's `CellIsRule`) is kept; other text is quoted. if gridcraft_core::parse::parse_number_text(&v).is_some() || v.starts_with('=') { v.trim_start_matches('=').to_string() + } else if is_string_literal(&v) { + v } else { format!("\"{}\"", v.replace('"', "\"\"")) } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 2ac290654..1aacf1a61 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -3074,6 +3074,72 @@ fn icon_set_thresholds_from_files_pick_the_icon() { assert_eq!([icon(&s, "A1"), icon(&s, "A2"), icon(&s, "A3"), icon(&s, "A4")], [Some(0), Some(1), Some(1), Some(2)]); } +/// `home.conditionalFormat` styles take colours the way the other formatting commands do, refuse +/// keys they would drop, and keep a text criterion already written as a string literal. +#[test] +fn conditional_format_styles_take_hex_colours_and_quoted_values() { + use gridcraft_model::{CfRule, Color, Fill, PatternType}; + let mut s = s(); + s.execute("range.setValues", json!({"range": "A1", "values": [["Open"], ["Closed"], ["Blocked"]]})).unwrap(); + let rule = |s: &Session, i: usize| s.doc().unwrap().wb.active().unwrap().cond_formats[i].rule.clone(); + let style = |r: &CfRule| match r { + CfRule::Expression { style, .. } | CfRule::CellIs { style, .. } => (**style).clone(), + other => panic!("{other:?}"), + }; + s.execute( + "home.conditionalFormat", + json!({"range": "A1:A3", "rule": {"type": "expression", "formula": "=A1=\"Open\"", "style": {"fill": "#FFF2CC", "font": {"bold": true, "color": "#9C0006"}}}}), + ) + .unwrap(); + let st = style(&rule(&s, 0)); + assert_eq!(st.fill, Fill::solid(Color::Rgb(0xFFF2CC))); + assert_eq!((st.font.bold, st.font.color), (true, Color::Rgb(0x9C0006))); + // A fill colour without a pattern is solid; the model's own shape still works. + s.execute( + "home.conditionalFormat", + json!({"range": "A1", "rule": {"type": "expression", "formula": "=TRUE", "style": {"fill": {"fg": "C6EFCE"}}}}), + ) + .unwrap(); + assert_eq!(style(&rule(&s, 0)).fill, Fill { pattern: PatternType::Solid, fg: Color::Rgb(0xC6EFCE), bg: Color::Auto }); + let internal = json!({"fill": {"pattern": "Solid", "fg": {"Rgb": 16773836}}, "borders": {"left": {"style": "Thin", "color": "#C00000"}}}); + s.execute("home.conditionalFormat", json!({"range": "A1", "rule": {"type": "expression", "formula": "=TRUE", "style": internal}})).unwrap(); + let st = style(&rule(&s, 0)); + assert_eq!((st.fill.fg, st.borders.left.color), (Color::Rgb(0xFFF2CC), Color::Rgb(0xC00000))); + // `"none"` is no fill (the format only sets the font), as `home.fillColor` reads it. + s.execute( + "home.conditionalFormat", + json!({"range": "A1", "rule": {"type": "expression", "formula": "=TRUE", "style": {"fill": "none", "font": {"bold": true}}}}), + ) + .unwrap(); + let st = style(&rule(&s, 0)); + assert_eq!((st.fill, st.font.bold), (Fill::default(), true)); + // Keys that would be dropped, leaving a rule that shows nothing, are refused. + let n = s.doc().unwrap().wb.active().unwrap().cond_formats.len(); + for bad in [json!({"preset": "greenFill"}), json!({"fillColor": "#FFF2CC"}), json!({"fill": "#nothex"}), json!("bold")] { + let e = s.execute("home.conditionalFormat", json!({"range": "A1", "rule": {"type": "expression", "formula": "=TRUE", "style": bad}})); + assert!(e.is_err(), "{bad} should be refused"); + } + assert_eq!(s.doc().unwrap().wb.active().unwrap().cond_formats.len(), n); + + // `"Closed"` (a string literal, as in the file) and `Closed` both match the text Closed. + for value in ["\"Closed\"", "Closed"] { + s.execute( + "home.conditionalFormat", + json!({"range": "A1:A3", "rule": {"type": "cellIs", "operator": "equal", "value": value, "preset": "greenFill"}}), + ) + .unwrap(); + assert!(matches!(rule(&s, 0), CfRule::CellIs { ref a, .. } if a == "\"Closed\""), "{value}"); + let wb = s.doc().unwrap().wb.clone(); + let mut cache = crate::cf::CfCache::new(); + let mut fill = |c: &str| cache.look(&wb, 0, CellRef::parse(c).unwrap()).and_then(|l| l.style).map(|st| st.fill.fg); + assert_eq!(fill("A2"), Some(Color::Rgb(0xC6EFCE)), "{value}"); + s.execute("edit.undo", json!({})).unwrap(); + } + // Text with quotes inside that isn't one literal is still quoted as text. + s.execute("home.conditionalFormat", json!({"range": "A1", "rule": {"type": "cellIs", "operator": "equal", "value": "say \"hi\""}})).unwrap(); + assert!(matches!(rule(&s, 0), CfRule::CellIs { ref a, .. } if a == "\"say \"\"hi\"\"\"")); +} + #[test] fn row_height_and_column_width_take_a_list_of_areas() { // Issue #341: header drags on a multi-area selection size every selected line in one step.