Merge pull request #1659

crates/engine/src/commands/prop.rs

@@ -987,6 +987,7 @@ fn delete_keys(s: &mut Session, _: &Value) -> Result<Value> {
     let sel = s.state.selected_keys.clone();
     s.edit("Delete Keyframes", None, |proj, st| {
         let comp = proj.comp_mut(cid).ok_or(EngineError::NoComp)?;
+        let mut removed = 0usize;
         for k in &sel {
             if let Some(pr) = comp.layer_mut(k.layer).and_then(|l| l.props.find_mut(k.prop))
                 && let Some(i) = key_at(&pr.keys, k.time)
@@ -995,10 +996,13 @@ fn delete_keys(s: &mut Session, _: &Value) -> Result<Value> {
                     pr.value = pr.keys[0].value.clone();
                 }
                 pr.keys.remove(i);
+                removed += 1;
             }
         }
         st.selected_keys.clear();
-        Ok(json!(sel.len()))
+        // The keys actually removed: a selected key that is gone (or listed twice) isn't counted,
+        // so a caller can tell that a delete changed nothing.
+        Ok(json!(removed))
     })
 }
 

crates/engine/src/lib.rs

@@ -1232,6 +1232,8 @@ mod tests_codec_options;
 #[cfg(test)]
 mod tests_color_view;
 #[cfg(test)]
+mod tests_delete_keys;
+#[cfg(test)]
 mod tests_disk_cache;
 #[cfg(test)]
 mod tests_effects;

crates/engine/src/tests_delete_keys.rs

@@ -0,0 +1,45 @@
+//! `keys.delete` reports the keys it removed, not the keys it was asked to remove.
+
+use effectcraft_project::LayerId;
+use serde_json::json;
+
+use crate::{KeyRef, Session};
+
+fn setup() -> (Session, u64, u64) {
+    let mut s = Session::default();
+    s.execute_checked("comp.new", json!({"width": 64, "height": 64, "frameRate": 30, "duration": 4})).unwrap();
+    let layer = s.execute_checked("layer.newSolid", json!({"name": "Box", "color": "#fff"})).unwrap()["layer"].as_u64().unwrap();
+    for t in 0..3 {
+        s.execute_checked("prop.addKey", json!({"layer": layer, "path": "transform/opacity", "time": t, "value": t * 25})).unwrap();
+    }
+    let prop = s.active_comp().unwrap().layer(LayerId(layer)).unwrap().props.prop("transform/opacity").unwrap().uid;
+    (s, layer, prop)
+}
+
+fn keys_of(s: &Session, layer: u64, prop: u64) -> Vec<KeyRef> {
+    let keys = &s.active_comp().unwrap().layer(LayerId(layer)).unwrap().props.find(prop).unwrap().keys;
+    keys.iter().map(|k| KeyRef { layer: LayerId(layer), prop, time: k.time }).collect()
+}
+
+#[test]
+fn delete_keys_counts_only_the_keys_it_removed() {
+    let (mut s, layer, prop) = setup();
+    let keys = keys_of(&s, layer, prop);
+    assert_eq!(keys.len(), 3);
+    // Two real keys, one of them listed twice, and a key of a property that does not exist.
+    let missing = KeyRef { layer: LayerId(layer), prop: u64::MAX, time: keys[1].time };
+    s.state.selected_keys = vec![keys[0], keys[2], keys[0], missing];
+    let removed = s.execute_checked("keys.delete", json!({})).unwrap();
+    assert_eq!(removed, json!(2));
+    assert_eq!(keys_of(&s, layer, prop), vec![keys[1]]);
+}
+
+#[test]
+fn delete_keys_reports_zero_when_no_selected_key_exists() {
+    let (mut s, layer, prop) = setup();
+    let keys = keys_of(&s, layer, prop);
+    s.state.selected_keys = vec![KeyRef { layer: LayerId(layer), prop: u64::MAX, time: keys[0].time }];
+    let removed = s.execute_checked("keys.delete", json!({})).unwrap();
+    assert_eq!(removed, json!(0));
+    assert_eq!(keys_of(&s, layer, prop), keys);
+}