diff --git a/crates/doc/src/props.rs b/crates/doc/src/props.rs index 97d653572..768625d4c 100644 --- a/crates/doc/src/props.rs +++ b/crates/doc/src/props.rs @@ -31,13 +31,42 @@ impl Rgb { } /// A text colour: automatic (black on light backgrounds, white on dark) or a fixed colour. -#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Serialize, Deserialize, Default)] +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Serialize, Default)] pub enum TextColor { #[default] Auto, Rgb(Rgb), } +impl TextColor { + /// `auto` or `automatic` (any case), or a colour [`Rgb::parse`] reads. + pub fn parse(s: &str) -> Option { + if s.eq_ignore_ascii_case("auto") || s.eq_ignore_ascii_case("automatic") { Some(TextColor::Auto) } else { Rgb::parse(s).map(TextColor::Rgb) } + } +} + +impl<'de> Deserialize<'de> for TextColor { + /// Reads what it writes (`"Auto"`, `{"Rgb": [r, g, b]}`) and also a string [`TextColor::parse`] + /// reads, so `styles.modify` and `format.set` take the colours `format.color` takes. + fn deserialize>(d: D) -> Result { + use serde::de::{Error, Unexpected}; + #[derive(Deserialize)] + enum Written { + Auto, + Rgb(Rgb), + } + match serde_json::Value::deserialize(d)? { + serde_json::Value::String(s) => { + TextColor::parse(&s).ok_or_else(|| D::Error::invalid_value(Unexpected::Str(&s), &"`auto`, `RRGGBB` or `#RRGGBB`")) + } + v => Ok(match serde_json::from_value(v).map_err(D::Error::custom)? { + Written::Auto => TextColor::Auto, + Written::Rgb(c) => TextColor::Rgb(c), + }), + } + } +} + #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Serialize, Deserialize, Default)] #[serde(rename_all = "camelCase")] pub enum Underline { @@ -1125,6 +1154,27 @@ mod tests { assert_eq!(TextDirection::from_ooxml("é?"), TextDirection::Horizontal); } + /// #865: a colour reads as `format.color` takes it (`RRGGBB`, `#RRGGBB`, `auto`), while what + /// documents and settings already hold reads and writes exactly as before. + #[test] + fn text_color_reads_hex_and_keeps_its_serialized_forms() { + let read = |j: &str| serde_json::from_str::(j); + let ink = TextColor::Rgb(Rgb(0x1B, 0x16, 0x11)); + assert_eq!(read(r#""1B1611""#).unwrap(), ink); + assert_eq!(read(r##""#1b1611""##).unwrap(), ink); + assert_eq!(read(r#""auto""#).unwrap(), TextColor::Auto); + assert_eq!(read(r#""Automatic""#).unwrap(), TextColor::Auto); + for (c, j) in [(TextColor::Auto, r#""Auto""#), (ink, r#"{"Rgb":[27,22,17]}"#)] { + assert_eq!(serde_json::to_string(&c).unwrap(), j, "written as before"); + assert_eq!(read(j).unwrap(), c); + } + for bad in [r#""1B161""#, r#""zzzzzz""#, r#""""#, r#"{"Rgb":"1B1611"}"#, r#"{"Hex":[1,2,3]}"#, "7", "[27,22,17]"] { + assert!(read(bad).is_err(), "{bad}"); + } + let p: CharProps = serde_json::from_value(serde_json::json!({"color": "#1B1611", "bold": true})).unwrap(); + assert_eq!((p.color, p.bold), (Some(ink), Some(true))); + } + #[test] fn props_serde_skip_none() { let p = CharProps { bold: Some(true), ..Default::default() }; diff --git a/crates/engine/src/cmd/format.rs b/crates/engine/src/cmd/format.rs index 85ca49e13..dc5503d22 100644 --- a/crates/engine/src/cmd/format.rs +++ b/crates/engine/src/cmd/format.rs @@ -361,11 +361,7 @@ fn nudge_size(s: &mut Session, d: f32) -> CmdResult { fn color(s: &mut Session, v: &Value) -> CmdResult { let c = p::req_str(v, "color")?; - let tc = if c.eq_ignore_ascii_case("auto") || c.eq_ignore_ascii_case("automatic") { - TextColor::Auto - } else { - TextColor::Rgb(Rgb::parse(c).ok_or_else(|| CmdError::Params("color must be RRGGBB or auto".into()))?) - }; + let tc = TextColor::parse(c).ok_or_else(|| CmdError::Params("color must be RRGGBB or auto".into()))?; apply(s, &|x| x.color = Some(tc)) } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 785c11d25..624b3aa76 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -2471,6 +2471,35 @@ fn styles_apply_multi_paragraph_selection() { } } +/// #865: `styles.modify` takes a colour the way `format.color` does (`RRGGBB`, `#RRGGBB`, `auto`), +/// still reads the serialized forms, and rejects a bad colour without touching the style. +#[test] +fn modify_style_takes_hex_colours_like_format_color() { + use wordcraft_doc::{CharProps, Rgb, TextColor}; + let mut s = s(); + let color = |s: &Session| s.doc.styles.get("Heading1").and_then(|st| st.chr.color); + let modify = |s: &mut Session, c: serde_json::Value| s.run("styles.modify", &json!({"style": "Heading1", "chr": {"color": c}})); + + modify(&mut s, json!("1B1611")).unwrap(); + assert_eq!(color(&s), Some(TextColor::Rgb(Rgb(0x1B, 0x16, 0x11)))); + modify(&mut s, json!("auto")).unwrap(); + assert_eq!(color(&s), Some(TextColor::Auto)); + modify(&mut s, json!("#1B1611")).unwrap(); + assert_eq!(color(&s), Some(TextColor::Rgb(Rgb(0x1B, 0x16, 0x11)))); + modify(&mut s, json!("Auto")).unwrap(); + assert_eq!(color(&s), Some(TextColor::Auto)); + modify(&mut s, json!({"Rgb": [192, 0, 0]})).unwrap(); + let red = TextColor::Rgb(Rgb(0xC0, 0, 0)); + assert_eq!(color(&s), Some(red)); + assert_eq!(s.doc.styles.resolve_char(Some("Heading1"), &CharProps::default()).color, red); + + for bad in [json!("1B16"), json!("not a colour"), json!({"Rgb": "1B1611"}), json!(7)] { + let r = modify(&mut s, bad.clone()); + assert!(matches!(r, Err(crate::CmdError::Params(_))), "{bad}: {r:?}"); + assert_eq!(color(&s), Some(red), "{bad} leaves the style alone"); + } +} + /// #555: Update Heading 2 to Match Selection (the styles pane's context menu) makes Heading 2 /// look like the selected text — its paragraph style and direct formatting — so every Heading 2 /// paragraph changes; the selected paragraph takes Heading 2, and it is one undo step.