fix(textwrap): reject malformed explicit object targets atomically
This commit is contained in:
parent
c3e0ca3dfa
commit
ec41d72e0d
|
|
@ -36,9 +36,11 @@ pub fn specs() -> Vec<CommandSpec> {
|
|||
]
|
||||
}
|
||||
|
||||
fn targets(s: &Session, p: &Value) -> Result<Vec<NodeId>> {
|
||||
match p.get("ids").and_then(Value::as_array) {
|
||||
Some(a) => Ok(a.iter().filter_map(Value::as_u64).map(NodeId).collect()),
|
||||
/// An explicit target list must be all valid ids: a partially applied text wrap is
|
||||
/// particularly surprising when some of the chosen objects are silently skipped.
|
||||
fn targets(s: &Session, p: &Value, command: &str) -> Result<Vec<NodeId>> {
|
||||
match p.get("ids") {
|
||||
Some(_) => checked_ids_param(s, p, "ids", command),
|
||||
None => selected_roots(s),
|
||||
}
|
||||
}
|
||||
|
|
@ -48,7 +50,7 @@ fn wrap_param(p: &Value, base: TextWrap) -> TextWrap {
|
|||
}
|
||||
|
||||
fn make(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
let ids = targets(s, p)?;
|
||||
let ids = targets(s, p, "object.textWrap.make")?;
|
||||
let w = wrap_param(p, TextWrap::default());
|
||||
s.edit("Make Text Wrap", |d, _| {
|
||||
for id in &ids {
|
||||
|
|
@ -66,7 +68,7 @@ fn make(s: &mut Session, p: &Value) -> Result<Value> {
|
|||
fn release(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
let ids: Vec<NodeId> = {
|
||||
let d = &s.doc()?.doc;
|
||||
targets(s, p)?.into_iter().filter(|i| d.node(*i).is_some_and(|n| n.wrap.is_some())).collect()
|
||||
targets(s, p, "object.textWrap.release")?.into_iter().filter(|i| d.node(*i).is_some_and(|n| n.wrap.is_some())).collect()
|
||||
};
|
||||
if ids.is_empty() {
|
||||
return Err(EngineError::Other("Text Wrap: select a wrap object".into()));
|
||||
|
|
@ -83,7 +85,7 @@ fn release(s: &mut Session, p: &Value) -> Result<Value> {
|
|||
}
|
||||
|
||||
fn options(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
let ids = targets(s, p)?;
|
||||
let ids = targets(s, p, "object.textWrap.options")?;
|
||||
let current = {
|
||||
let d = &s.doc()?.doc;
|
||||
ids.iter().find_map(|i| d.node(*i).and_then(|n| n.wrap)).unwrap_or_default()
|
||||
|
|
@ -212,6 +214,35 @@ mod tests {
|
|||
vectorcraft_text::layout(vectorcraft_text::FontDb::global(), &t).lines.iter().map(|l| l.avail).collect()
|
||||
}
|
||||
|
||||
/// Invalid explicit ids must not turn a requested multi-object edit into a partial one.
|
||||
#[test]
|
||||
fn text_wrap_commands_reject_malformed_target_lists_without_editing() {
|
||||
let mut s = Session::new();
|
||||
s.execute("file.new", &json!({"width": 300, "height": 300})).unwrap();
|
||||
let id = s.execute("shape.rectangle", &json!({"x": 10, "y": 10, "width": 80, "height": 80})).unwrap()["id"].as_u64().unwrap();
|
||||
let other = s.execute("shape.rectangle", &json!({"x": 120, "y": 10, "width": 80, "height": 80})).unwrap()["id"].as_u64().unwrap();
|
||||
s.execute("select.set", &json!({"ids": [id]})).unwrap();
|
||||
|
||||
let before = s.doc().unwrap().history.undo.len();
|
||||
for command in ["object.textWrap.make", "object.textWrap.release", "object.textWrap.options"] {
|
||||
for ids in [json!([id, "invalid"]), json!([id, 99999]), json!({"not": "an array"})] {
|
||||
let e = s.execute(command, &json!({"ids": ids, "offset": 12})).unwrap_err().to_string();
|
||||
assert!(e.contains("id") || e.contains("array"), "{command}: {e}");
|
||||
assert_eq!(s.doc().unwrap().history.undo.len(), before, "{command} added history on error");
|
||||
assert!(s.doc().unwrap().doc.node(NodeId(id)).unwrap().wrap.is_none());
|
||||
assert!(s.doc().unwrap().doc.node(NodeId(other)).unwrap().wrap.is_none());
|
||||
}
|
||||
}
|
||||
|
||||
s.execute("object.textWrap.make", &json!({"ids": [id]})).unwrap();
|
||||
assert!(s.doc().unwrap().doc.node(NodeId(id)).unwrap().wrap.is_some());
|
||||
assert!(s.doc().unwrap().doc.node(NodeId(other)).unwrap().wrap.is_none());
|
||||
s.execute("object.textWrap.options", &json!({"ids": [id], "offset": 12})).unwrap();
|
||||
assert_eq!(s.doc().unwrap().doc.node(NodeId(id)).unwrap().wrap.unwrap().offset, 12.0);
|
||||
s.execute("object.textWrap.release", &json!({"ids": [id]})).unwrap();
|
||||
assert!(s.doc().unwrap().doc.node(NodeId(id)).unwrap().wrap.is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn area_type_wraps_around_objects_above_and_follows_edits() {
|
||||
let mut s = Session::new();
|
||||
|
|
|
|||
Loading…
Reference in New Issue