Styles: name the attributes in the params docs, and a wrong one in the command's error (#777)

- describe style.paragraph.create (and .edit, style.character.create,
  .edit) gave chars?: {…} and para?: {…}. The descriptions now say they
  take type.char's and type.para's attributes and name the common ones.
- A wrong attribute gave "invalid parameters for `style`: unknown
  attribute `font`". The error now names the command and the group,
  and the attribute meant (nearest name, or one the key starts or ends)
  or else lists them all: "invalid parameters for
  `style.paragraph.create`: chars: unknown attribute `font`; did you
  mean `fontFamily`?". The suggestion comes from set_json, so
  type.char, type.para and find.change get it too.
- chars or para that isn't an object is an error instead of ignored.

Fixes #777
This commit is contained in:
Gert Pellin
2026-10-13 16:48:27 +02:00
parent e862e0a613
commit 2a3d1dc183
2 changed files with 132 additions and 16 deletions
+69 -1
View File
@@ -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::<Vec<_>>();
let (a, b) = (norm(a), norm(b));
let mut prev: Vec<usize> = (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::<Vec<_>>().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 {
+63 -15
View File
@@ -86,7 +86,7 @@ pub fn specs() -> Vec<CommandSpec> {
"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<CommandSpec> {
"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<CommandSpec> {
"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<CommandSpec> {
"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<CommandSpec> {
]
}
fn attrs<T: Default>(v: Option<&Value>, set: impl Fn(&mut T, &str, &Value) -> std::result::Result<(), String>) -> Result<T> {
/// 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<T: Default>(cmd: &str, group: &str, p: &Value, set: impl Fn(&mut T, &str, &Value) -> std::result::Result<(), String>) -> Result<T> {
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<Value> {
fn create_para(s: &mut Session, p: &Value) -> Result<Value> {
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<Value> {
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<Value> {
}
// 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, &current))?;
let para: ParaAttrs = attrs("style.paragraph.edit", "para", p, |a: &mut ParaAttrs, k, v| a.set_json_over(k, v, &current))?;
let st = d.styles_mut().para_mut(&name).ok_or_else(|| bad("style.paragraph.edit", format!("no style `{name}`")))?;
st.para.merge(&para);
st.chars.merge(&chars);
@@ -631,7 +637,7 @@ fn delete_para(s: &mut Session, p: &Value) -> Result<Value> {
fn create_char(s: &mut Session, p: &Value) -> Result<Value> {
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<Value> {
// 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<Value> {
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<String> = 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::*;