diff --git a/crates/doc/src/attrs.rs b/crates/doc/src/attrs.rs index b99740e2c..6dc9b8efd 100644 --- a/crates/doc/src/attrs.rs +++ b/crates/doc/src/attrs.rs @@ -62,7 +62,7 @@ macro_rules! attr_set { return Ok(()); } )* - Err(format!("unknown attribute `{key}`")) + Err(unknown_attribute(key, &[ $( stringify!($field), )* ])) } /// Fields where `self` differs from `base`'s resolved value (used for "clear overrides"). pub fn diff_from(&self, base: &$props) -> $attrs { @@ -104,6 +104,57 @@ fn attr_key(snake: &'static str) -> &'static str { snake } +/// `snake_case` → `camelCase`, as commands name attributes. +fn camel(snake: &str) -> String { + let mut out = String::with_capacity(snake.len()); + let mut up = false; + for c in snake.chars() { + if c == '_' { + up = true; + } else if up { + out.push(c.to_ascii_uppercase()); + up = false; + } else { + out.push(c); + } + } + out +} + +/// Edits (insert, delete, replace) that turn `a` into `b`, ignoring case and underscores. +fn edit_distance(a: &str, b: &str) -> usize { + let norm = |s: &str| s.chars().filter(|c| *c != '_').map(|c| c.to_ascii_lowercase()).collect::>(); + let (a, b) = (norm(a), norm(b)); + let mut prev: Vec = (0..=b.len()).collect(); + for (i, ca) in a.iter().enumerate() { + let mut row = vec![i + 1]; + for (j, cb) in b.iter().enumerate() { + let d = (prev[j] + usize::from(ca != cb)).min(prev[j + 1] + 1).min(row[j] + 1); + row.push(d); + } + prev = row; + } + prev.last().copied().unwrap_or(0) +} + +/// The error for an attribute `key` that isn't one of `fields` (snake_case): the nearest one +/// (a typo, or a name it starts or ends: `font` → `fontFamily`), or else all of them. +fn unknown_attribute(key: &str, fields: &[&str]) -> String { + let k = key.to_ascii_lowercase().replace('_', ""); + let flat = |f: &str| f.replace('_', ""); + let near = fields + .iter() + .map(|f| (edit_distance(key, f), *f)) + .filter(|(d, _)| *d <= (k.len() / 3).max(1)) + .min() + .map(|(_, f)| f) + .or_else(|| fields.iter().copied().find(|f| k.len() >= 3 && (flat(f).starts_with(&k) || flat(f).ends_with(&k)))); + match near { + Some(f) => format!("unknown attribute `{key}`; did you mean `{}`?", camel(f)), + None => format!("unknown attribute `{key}`; the attributes are: {}", fields.iter().map(|f| camel(f)).collect::>().join(", ")), + } +} + // ---------- value types ---------- #[derive(Clone, Copy, Debug, Default, PartialEq, Serialize, Deserialize)] @@ -946,6 +997,23 @@ impl ParaProps { mod tests { use super::*; + /// #777: an unknown attribute names the nearest one, or lists them all. + #[test] + fn unknown_attributes_suggest_the_right_name() { + let mut c = CharAttrs::default(); + assert_eq!(c.set_json("font", &serde_json::json!("X")).unwrap_err(), "unknown attribute `font`; did you mean `fontFamily`?"); + assert_eq!(c.set_json("fontFamly", &serde_json::json!("X")).unwrap_err(), "unknown attribute `fontFamly`; did you mean `fontFamily`?"); + let all = c.set_json("zzzzzz", &serde_json::json!(1)).unwrap_err(); + assert!( + all.starts_with("unknown attribute `zzzzzz`; the attributes are: ") && all.contains("fontFamily") && all.contains("fontStyle"), + "{all}" + ); + assert!(c.set_json("font_family", &serde_json::json!("X")).is_ok() && c.set_json("fontFamily", &serde_json::json!("X")).is_ok()); + let mut p = ParaAttrs::default(); + assert_eq!(p.set_json("spaceBefor", &serde_json::json!(3)).unwrap_err(), "unknown attribute `spaceBefor`; did you mean `spaceBefore`?"); + assert_eq!(camel("first_line_indent"), "firstLineIndent"); + } + #[test] fn a_partial_rule_keeps_the_other_fields() { let current = ParaProps { diff --git a/crates/engine/src/cmd/style.rs b/crates/engine/src/cmd/style.rs index c4d20f099..3c854ad87 100644 --- a/crates/engine/src/cmd/style.rs +++ b/crates/engine/src/cmd/style.rs @@ -86,7 +86,7 @@ pub fn specs() -> Vec { "New Paragraph Style…", [], None, - "{name, basedOn?, nextStyle?, para?: {…}, chars?: {…}, fromSelection?: bool}", + "{name, basedOn?, nextStyle?, para?: {paragraph attributes, as type.para names them: align, leftIndent, firstLineIndent, spaceBefore, spaceAfter, …}, chars?: {character attributes, as type.char names them: fontFamily, fontStyle, size, leading, tracking, fill, …}, fromSelection?: bool}", has_doc, create_para ), @@ -95,7 +95,7 @@ pub fn specs() -> Vec { "Paragraph Style Options…", [], None, - "{name, rename?, basedOn?, nextStyle?, para?: {… ruleAbove?/ruleBelow?: only the named rule fields change}, chars?: {…}}", + "{name, rename?, basedOn?, nextStyle?, para?: {paragraph attributes, as type.para names them: align, leftIndent, spaceBefore, …; ruleAbove?/ruleBelow?: only the named rule fields change}, chars?: {character attributes, as type.char names them: fontFamily, fontStyle, size, leading, tracking, fill, …}}", has_doc, edit_para ), @@ -105,7 +105,7 @@ pub fn specs() -> Vec { "New Character Style…", [], None, - "{name, basedOn?: style | \"[None]\", chars?: {…}, fromSelection?: bool (default false)} — if true, capture the first selected character's style/local overrides (caret: typing format), then overlay chars (null unsets); only explicit fill names are validated; paragraph formatting stays inherited; no text target: chars only → {name} (made unique)", + "{name, basedOn?: style | \"[None]\", chars?: {character attributes, as type.char names them: fontFamily, fontStyle, size, leading, tracking, fill, …}, fromSelection?: bool (default false)} — if true, capture the first selected character's style/local overrides (caret: typing format), then overlay chars (null unsets); only explicit fill names are validated; paragraph formatting stays inherited; no text target: chars only → {name} (made unique)", has_doc, create_char ), @@ -114,7 +114,7 @@ pub fn specs() -> Vec { "Character Style Options…", [], None, - "{name, rename?, basedOn?: style | \"[None]\" | null, chars?: {attr: value | null (no longer set)}} — renaming updates every use", + "{name, rename?, basedOn?: style | \"[None]\" | null, chars?: {character attribute (as type.char names them: fontFamily, fontStyle, size, …): value | null (no longer set)}} — renaming updates every use", has_doc, edit_char ), @@ -380,12 +380,18 @@ pub fn specs() -> Vec { ] } -fn attrs(v: Option<&Value>, set: impl Fn(&mut T, &str, &Value) -> std::result::Result<(), String>) -> Result { +/// The `group` (`chars`, `para`) object of command `cmd`'s params as attributes; a bad attribute +/// is that command's parameter error, naming the group. +fn attrs(cmd: &str, group: &str, p: &Value, set: impl Fn(&mut T, &str, &Value) -> std::result::Result<(), String>) -> Result { let mut a = T::default(); - if let Some(o) = v.and_then(Value::as_object) { - for (k, v) in o { - set(&mut a, k, v).map_err(|e| bad("style", e))?; + match p.get(group) { + None | Some(Value::Null) => {} + Some(Value::Object(o)) => { + for (k, v) in o { + set(&mut a, k, v).map_err(|e| bad(cmd, format!("{group}: {e}")))?; + } } + Some(_) => return Err(bad(cmd, format!("{group} must be an object of attributes"))), } Ok(a) } @@ -436,8 +442,8 @@ fn apply_char(s: &mut Session, p: &Value) -> Result { fn create_para(s: &mut Session, p: &Value) -> Result { let base = str_param(p, "name").unwrap_or("Paragraph Style 1").to_string(); - let mut para: ParaAttrs = attrs(p.get("para"), |a: &mut ParaAttrs, k, v| a.set_json(k, v))?; - let mut chars: CharAttrs = attrs(p.get("chars"), |a: &mut CharAttrs, k, v| a.set_json(k, v))?; + let mut para: ParaAttrs = attrs("style.paragraph.create", "para", p, |a: &mut ParaAttrs, k, v| a.set_json(k, v))?; + let mut chars: CharAttrs = attrs("style.paragraph.create", "chars", p, |a: &mut CharAttrs, k, v| a.set_json(k, v))?; if p.get("fromSelection").and_then(Value::as_bool).unwrap_or(false) && let Ok(cur) = s.execute("type.selectionAttrs", &json!({})) && !cur.is_null() @@ -479,7 +485,7 @@ fn CharProps_to_attrs(p: &designcraft_doc::CharProps) -> CharAttrs { fn edit_para(s: &mut Session, p: &Value) -> Result { let name = str_param(p, "name").ok_or_else(|| bad("style.paragraph.edit", "missing name"))?.to_string(); - let chars: CharAttrs = attrs(p.get("chars"), |a: &mut CharAttrs, k, v| a.set_json(k, v))?; + let chars: CharAttrs = attrs("style.paragraph.edit", "chars", p, |a: &mut CharAttrs, k, v| a.set_json(k, v))?; super::text::validate_char_fill(&s.doc()?.doc, &chars, "style.paragraph.edit")?; let rename = str_param(p, "rename").map(str::to_string); let based = p.get("basedOn").cloned(); @@ -491,7 +497,7 @@ fn edit_para(s: &mut Session, p: &Value) -> Result { } // A rule object changes only the rule fields it names, over the style's resolved rule. let (current, _) = d.styles.resolve_para_style(&name); - let para: ParaAttrs = attrs(p.get("para"), |a: &mut ParaAttrs, k, v| a.set_json_over(k, v, ¤t))?; + let para: ParaAttrs = attrs("style.paragraph.edit", "para", p, |a: &mut ParaAttrs, k, v| a.set_json_over(k, v, ¤t))?; let st = d.styles_mut().para_mut(&name).ok_or_else(|| bad("style.paragraph.edit", format!("no style `{name}`")))?; st.para.merge(¶); st.chars.merge(&chars); @@ -631,7 +637,7 @@ fn delete_para(s: &mut Session, p: &Value) -> Result { fn create_char(s: &mut Session, p: &Value) -> Result { let base = str_param(p, "name").unwrap_or("Character Style 1").to_string(); - let mut chars: CharAttrs = attrs(p.get("chars"), |a: &mut CharAttrs, k, v| a.set_json(k, v))?; + let mut chars: CharAttrs = attrs("style.character.create", "chars", p, |a: &mut CharAttrs, k, v| a.set_json(k, v))?; // Validate caller-supplied fills; captured formatting may retain a deleted swatch name. super::text::validate_char_fill(&s.doc()?.doc, &chars, "style.character.create")?; if p.get("fromSelection").and_then(Value::as_bool).unwrap_or(false) @@ -647,7 +653,7 @@ fn create_char(s: &mut Session, p: &Value) -> Result { // Replay the explicit fields so null clears a captured value rather than looking absent. if let Some(overrides) = p.get("chars").and_then(Value::as_object) { for (key, value) in overrides { - captured.set_json(key, value).map_err(|e| bad("style", e))?; + captured.set_json(key, value).map_err(|e| bad("style.character.create", format!("chars: {e}")))?; } } chars = captured; @@ -671,7 +677,7 @@ fn edit_char(s: &mut Session, p: &Value) -> Result { if name == designcraft_doc::NO_CHAR_STYLE { return Err(bad("style.character.edit", "[None] can't be edited")); } - let chars: CharAttrs = attrs(p.get("chars"), |a: &mut CharAttrs, k, v| a.set_json(k, v))?; + let chars: CharAttrs = attrs("style.character.edit", "chars", p, |a: &mut CharAttrs, k, v| a.set_json(k, v))?; super::text::validate_char_fill(&s.doc()?.doc, &chars, "style.character.edit")?; // `null` unsets an attribute (the style no longer sets it). let unset: Vec = p @@ -1199,6 +1205,48 @@ fn rename_in(it: &mut designcraft_doc::Item, from: &str, to: &str) { } } +/// #777: the params descriptions name the attributes, and a wrong one is the command's error. +#[cfg(test)] +mod attribute_error_tests { + use super::*; + use crate::cmd::find_command; + + #[test] + fn a_wrong_attribute_names_the_command_and_the_right_attribute() { + let mut s = Session::new(); + s.execute("file.new", &json!({})).unwrap(); + let err = |s: &mut Session, id: &str, p: Value| s.execute(id, &p).unwrap_err().to_string(); + assert_eq!( + err(&mut s, "style.paragraph.create", json!({"name": "H", "chars": {"font": "Source Sans 3"}})), + "invalid parameters for `style.paragraph.create`: chars: unknown attribute `font`; did you mean `fontFamily`?" + ); + assert_eq!( + err(&mut s, "style.paragraph.create", json!({"name": "H", "para": {"spaceBefor": 6}})), + "invalid parameters for `style.paragraph.create`: para: unknown attribute `spaceBefor`; did you mean `spaceBefore`?" + ); + assert!( + err(&mut s, "style.character.create", json!({"name": "C", "chars": {"font": "X"}})) + .starts_with("invalid parameters for `style.character.create`: chars:") + ); + assert!(err(&mut s, "style.paragraph.create", json!({"name": "H", "chars": "bold"})).contains("chars must be an object of attributes")); + s.execute("style.paragraph.create", &json!({"name": "H", "chars": {"fontFamily": "Source Sans 3", "size": 14}})).unwrap(); + assert!( + err(&mut s, "style.paragraph.edit", json!({"name": "H", "chars": {"font": "X"}})) + .starts_with("invalid parameters for `style.paragraph.edit`: chars:") + ); + s.execute("style.character.create", &json!({"name": "C", "chars": {"fontStyle": "Bold"}})).unwrap(); + assert!( + err(&mut s, "style.character.edit", json!({"name": "C", "chars": {"font": "X"}})) + .starts_with("invalid parameters for `style.character.edit`: chars:") + ); + for id in ["style.paragraph.create", "style.paragraph.edit", "style.character.create", "style.character.edit"] { + let doc = find_command(id).unwrap().params; + assert!(doc.contains("as type.char names them: fontFamily"), "{id}: {doc}"); + } + assert!(find_command("style.paragraph.create").unwrap().params.contains("as type.para names them: align")); + } +} + #[cfg(test)] mod color_tests { use super::*;