diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 9d8187052..f24d672be 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -1717,13 +1717,25 @@ impl Session { self.note_depth = 1; self.batch_stash = None; } - // A selection-only command given `ids` selects them first, in the same undo step (#2130). + // A selection-only command given `ids` selects them first, in the same undo step (#2130); + // when the command then fails, the selection (and the current layer selecting them made + // current) goes back to how it was, as when the ids are refused. let select_first = SELECTS_GIVEN_IDS.contains(&id).then(|| params.get("ids")).flatten(); + let selected_before = select_first.and(self.active()).map(|d| (d.uid, d.selection.clone(), d.active_layer)); let run = |s: &mut Session| { if let Some(ids) = select_first { s.execute("select.set", &serde_json::json!({ "ids": ids }))?; } - (spec.run)(s, params) + let r = (spec.run)(s, params); + if r.is_err() + && let Some((uid, selection, layer)) = selected_before + && let Some(st) = s.docs.iter_mut().find(|d| d.uid == uid) + { + st.selection = selection; + st.active_layer = layer; + st.revision += 1; + } + r }; // The entries the command journals belong to the undo groups of the documents it edits, and to // the group of the document it starts in when it ends there too. diff --git a/crates/engine/src/tests_select_ids_first.rs b/crates/engine/src/tests_select_ids_first.rs index fbeb2c867..03d4ebd0b 100644 --- a/crates/engine/src/tests_select_ids_first.rs +++ b/crates/engine/src/tests_select_ids_first.rs @@ -143,6 +143,24 @@ fn bad_ids_are_refused_and_change_nothing() { } } +#[test] +fn a_command_that_fails_after_selecting_its_ids_puts_the_selection_back() { + let (mut s, ids) = doc(); + let far = s.execute("shape.rectangle", &json!({"x": 300, "y": 200, "width": 20, "height": 20})).unwrap()["id"].as_u64().unwrap(); + s.execute("select.set", &json!({"ids": [ids[3]]})).unwrap(); + let (before, layer) = (s.doc().unwrap().doc.clone(), s.doc().unwrap().active_layer); + // The ids are fine, so they are selected; the shapes don't overlap, so Intersect then fails. + let e = s.execute("object.pathfinder.intersect", &json!({"ids": [ids[0], far]})).unwrap_err().to_string(); + assert!(e.contains("empty result"), "{e}"); + assert!(s.doc().unwrap().doc == before); + assert_eq!(s.doc().unwrap().selection.objects, vec![NodeId(ids[3])], "the selection stays"); + assert_eq!(s.doc().unwrap().active_layer, layer); + // Without ids the failing command leaves the selection it ran on, as before. + s.execute("select.set", &json!({"ids": [ids[0], far]})).unwrap(); + assert!(s.execute("object.pathfinder.intersect", &json!({})).is_err()); + assert_eq!(s.doc().unwrap().selection.objects, vec![NodeId(ids[0]), NodeId(far)]); +} + #[test] fn without_ids_the_selection_is_used_as_before() { let (mut s, ids) = doc(); diff --git a/crates/engine/src/tests_trap.rs b/crates/engine/src/tests_trap.rs index fd7619329..87fb92a6f 100644 --- a/crates/engine/src/tests_trap.rs +++ b/crates/engine/src/tests_trap.rs @@ -125,6 +125,16 @@ fn no_adjoining_edges_is_a_clear_error_that_changes_nothing() { s.execute("select.set", &json!({"ids": [a.0]})).unwrap(); let e = s.execute("object.pathfinder.trap", &json!({})).unwrap_err().to_string(); assert!(e.contains("two or more objects"), "{e}"); + // Given as ids, the objects are selected for the command, and its failure puts the + // selection back. + s.execute("select.none", &json!({})).unwrap(); + let e = s.execute("object.pathfinder.trap", &json!({"ids": [a.0, b.0]})).unwrap_err().to_string(); + assert!(e.contains("no edges of different colours meet"), "{e}"); + assert!(s.doc().unwrap().selection.objects.is_empty(), "{:?}", s.doc().unwrap().selection.objects); + s.execute("select.set", &json!({"ids": [a.0]})).unwrap(); + assert!(s.execute("object.pathfinder.trap", &json!({"ids": [a.0, b.0]})).is_err()); + assert_eq!(s.doc().unwrap().selection.objects, vec![a]); + assert_eq!(s.doc().unwrap().doc, before); } #[test]