Expressions: the pick whip inserts at the cursor while editing (#284)

Dragging an expression's pick whip always replaced the whole
expression, also while it was being edited (and the editor then wrote
its old text back over the reference). As in After Effects, a pick
whip pressed while the expression is being edited now puts the
reference at the cursor, replacing the selected text, and the editing
goes on with the cursor after the reference. Without an edit in
progress it replaces the expression as before.

`prop.pickWhip` takes the text being edited (`expression`) and the
selection in it (`range`, in characters) to insert there.

Tests: the_expression_pick_whip_inserts_at_the_cursor_while_editing
(ui_inline_edit), pick_whip_inserts_into_the_expression_being_edited
(engine).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

crates/engine/src/commands/link.rs

@@ -6,7 +6,7 @@ use effectcraft_time::{TICKS_PER_SECOND, Tick};
 use serde_json::{Value, json};
 
 use super::prop::prop_ref;
-use super::{CommandSpec, b_p, bad, has_layers, layer_mut, layer_p};
+use super::{CommandSpec, b_p, bad, has_layers, layer_mut, layer_p, str_p};
 use crate::{EngineError, Result, Session, cmd};
 
 const AXES: [(&str, &str); 3] = [("positionX", "X Position"), ("positionY", "Y Position"), ("positionZ", "Z Position")];
@@ -164,6 +164,23 @@ fn pick_whip(s: &mut Session, p: &Value) -> Result<Value> {
     let r = reference(comp, from, target, tu, s.prefs.general.expression_pick_whip_compact).ok_or_else(|| bad("prop.pickWhip", "no such target property"))?;
     let (me, them) = (from.props.find(uid).ok_or(EngineError::NoComp)?, target.props.find(tu).ok_or(EngineError::NoComp)?);
     let (dm, dt) = (dims(from, me), dims(target, them));
+    // While the expression is being edited the reference goes in at the cursor, replacing the
+    // selected text (After Effects): `expression` is the text being edited, `range` the
+    // selection [start, end] in characters.
+    if let Some(base) = str_p(p, "expression") {
+        let range: Option<Vec<u64>> = p.get("range").and_then(Value::as_array).map(|a| a.iter().filter_map(Value::as_u64).collect());
+        let n = base.chars().count() as u64;
+        let (a, b) = match range.as_deref() {
+            Some(&[a, b]) => (a.min(b).min(n), a.max(b).min(n)),
+            None => (n, n),
+            Some(_) => return Err(bad("prop.pickWhip", "`range` must be [start, end] (characters)")),
+        };
+        let head: String = base.chars().take(a as usize).collect();
+        let tail: String = base.chars().skip(b as usize).collect();
+        let text = format!("{head}{r}{tail}");
+        s.execute("prop.setExpression", json!({"layer": lid.0, "prop": uid, "expression": text}))?;
+        return Ok(json!(text));
+    }
     let text = if dt == 1 && dm > 1 {
         format!("temp = {r};\n[{}]", vec!["temp"; dm].join(", "))
     } else if dt > 1 && dm == 1 {
@@ -183,7 +200,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Pick Whip (Link Property)",
             [],
             None,
-            "{layer?, path|prop, target: {layer, path|prop}} → sets an AE reference expression",
+            "{layer?, path|prop, target: {layer, path|prop}, expression?, range?: [start, end]} → sets an AE reference expression (with `expression`: inserts it there, replacing the characters in `range`, default the end)",
             has_layers,
             pick_whip
         ),

crates/engine/src/tests_timeline.rs

@@ -466,6 +466,33 @@ fn pick_whip_generates_ae_reference_expressions() {
     assert!(s.execute("prop.pickWhip", json!({"layer": l, "path": "transform/opacity", "target": {"layer": l, "path": "transform/opacity"}})).is_err());
 }
 
+/// #284: with the expression being edited, the pick whip puts the reference at the cursor,
+/// replacing the selected characters (After Effects), in one undo step.
+#[test]
+fn pick_whip_inserts_into_the_expression_being_edited() {
+    let (mut s, l) = setup();
+    let o = s.execute("layer.newSolid", json!({"name": "Ö", "color": "#00ff00"})).unwrap()["layer"].as_u64().unwrap();
+    let whip = |s: &mut Session, base: &str, range: Value| {
+        let mut p = json!({"layer": l, "path": "transform/opacity", "target": {"layer": o, "path": "transform/rotation"}, "expression": base});
+        if !range.is_null() {
+            p["range"] = range;
+        }
+        s.execute("prop.pickWhip", p)
+    };
+    let r = "thisComp.layer(\"Ö\").transform.rotation";
+    // Characters, not bytes: "é" before the cursor.
+    assert_eq!(whip(&mut s, "é + 5", json!([1, 1])).unwrap(), json!(format!("é{r} + 5")));
+    assert_eq!(prop(&s, l, "transform/opacity").expr.unwrap().text, format!("é{r} + 5"));
+    // A selection (either way round) is replaced; no range: at the end.
+    assert_eq!(whip(&mut s, "a*2+b", json!([4, 2])).unwrap(), json!(format!("a*{r}b")));
+    assert_eq!(whip(&mut s, "x + ", Value::Null).unwrap(), json!(format!("x + {r}")));
+    // Past the end clamps; a malformed range is an error.
+    assert_eq!(whip(&mut s, "7", json!([9, 99])).unwrap(), json!(format!("7{r}")));
+    assert!(whip(&mut s, "7", json!([1])).is_err());
+    s.execute("edit.undo", json!({})).unwrap();
+    assert_eq!(prop(&s, l, "transform/opacity").expr.unwrap().text, format!("x + {r}"));
+}
+
 /// Outer comp (4 s) containing a precomp whose 10×10 white solid moves x = 10 + 20·t (t in s).
 /// Returns (session, precomp layer id in the outer comp).
 fn precomp_setup() -> (Session, u64) {

crates/ui-egui/src/panels/timeline.rs

@@ -115,6 +115,20 @@ fn pick_whip_id() -> egui::Id {
     egui::Id::new("tl-pickwhip")
 }
 
+/// The text of an expression being edited when its pick whip was pressed, and the selection in
+/// it (characters): the pick whip's reference goes in there.
+type WhipInto = (String, [usize; 2]);
+
+fn whip_into_id(prop: u64) -> egui::Id {
+    egui::Id::new(("tl-pickwhip-into", prop))
+}
+
+/// After a pick whip into the expression being edited: its editor takes the focus back, the
+/// cursor this many characters before the end (just after the reference).
+fn expr_refocus_id(prop: u64) -> egui::Id {
+    egui::Id::new(("tl-expr-refocus", prop))
+}
+
 #[derive(Clone, Debug)]
 struct Row {
     layer: LayerId,
@@ -2036,6 +2050,25 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) {
                 let pw = ui.interact(pw_rect, egui::Id::new(("expr-whip", uid)), Sense::drag()).on_hover_text("Expression pick whip: drag onto a property");
                 icons::paint(&lp, pw_rect.shrink(1.0), Icon::PickWhip, if pw.hovered() || pw.dragged() { t.text } else { t.text_dim });
                 app.auto.add(&format!("timeline.prop.{uid}.pickWhip"), pw_rect, "Expression pick whip");
+                if pw.is_pointer_button_down_on() && ui.input(|i| i.pointer.primary_pressed()) {
+                    // Pressed while the expression is being edited (its text is buffered while
+                    // the editor has the focus): the reference will go in at the cursor,
+                    // replacing the selection (#284).
+                    let into = ctx.data(|d| d.get_temp::<String>(egui::Id::new(("expr-buf", uid)))).map(|buf| {
+                        let n = buf.chars().count();
+                        let sel = egui::text_edit::TextEditState::load(&ctx, egui::Id::new(("expr-edit", uid)))
+                            .and_then(|st| st.cursor.char_range())
+                            .map_or([n, n], |r| [r.primary.index.0.min(r.secondary.index.0).min(n), r.primary.index.0.max(r.secondary.index.0).min(n)]);
+                        (buf, sel)
+                    });
+                    ctx.data_mut(|d| {
+                        if let Some(into) = into {
+                            d.insert_temp(whip_into_id(*uid), into);
+                        } else {
+                            d.remove::<WhipInto>(whip_into_id(*uid));
+                        }
+                    });
+                }
                 if pw.drag_started() {
                     let pw_state: PickWhip = (1, layer.id.0, *uid, pw_rect.center());
                     ctx.data_mut(|d| d.insert_temp(pick_whip_id(), pw_state));
@@ -2089,19 +2122,20 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) {
                 gp.rect_filled(Rect::from_min_max(pos2(graph_x0, r.min.y), r.max), 0.0, Color32::from_rgb(0x1a, 0x1a, 0x1a));
                 let er = Rect::from_min_max(pos2(graph_x0 + 8.0, r.min.y + 3.0), pos2(rect.max.x - 14.0, r.max.y - 3.0));
                 let buf_id = egui::Id::new(("expr-buf", uid));
+                let edit_id = egui::Id::new(("expr-edit", uid));
                 let mut buf: String = ctx.data(|d| d.get_temp(buf_id)).unwrap_or_else(|| ex.text.clone());
+                if let Some(after) = ctx.data_mut(|d| d.remove_temp::<usize>(expr_refocus_id(*uid))) {
+                    // A pick whip just put a reference in: editing goes on in the new text,
+                    // the cursor after the reference.
+                    buf = ex.text.clone();
+                    let mut st = egui::text_edit::TextEditState::load(&ctx, edit_id).unwrap_or_default();
+                    st.cursor.set_char_range(Some(egui::text::CCursorRange::one(egui::text::CCursor::new(buf.chars().count().saturating_sub(after)))));
+                    st.store(&ctx, edit_id);
+                    ctx.memory_mut(|m| m.request_focus(edit_id));
+                }
                 // Settings ▸ Scripting & Expressions ▸ Expressions Editor.
                 let sp = app.session.prefs.scripting.clone();
-                let resp = super::expr_editor::editor(
-                    ui,
-                    egui::Id::new(("expr-edit", uid)),
-                    &mut buf,
-                    er,
-                    &sp,
-                    if ex.enabled { expr_col } else { t.text_dim },
-                    (*lines).clamp(1, 8),
-                    &t,
-                );
+                let resp = super::expr_editor::editor(ui, edit_id, &mut buf, er, &sp, if ex.enabled { expr_col } else { t.text_dim }, (*lines).clamp(1, 8), &t);
                 app.auto.add(&format!("timeline.prop.{uid}.expression"), er, &prop.name);
                 let commit = resp.has_focus() && ui.input(|i| i.key_pressed(egui::Key::Enter) && (i.modifiers.command || i.modifiers.ctrl));
                 if commit {
@@ -2443,12 +2477,21 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) {
         }
         if ctx.input(|i| !i.pointer.any_down()) {
             ctx.data_mut(|d| d.remove::<PickWhip>(pick_whip_id()));
+            let into = (kind == 1).then(|| ctx.data_mut(|d| d.remove_temp::<WhipInto>(whip_into_id(src_prop)))).flatten();
             match (kind, target) {
                 (0, Some((_, row))) if row.layer.0 != src_layer => {
                     actions.push(("layer.setParent".into(), json!({"layers": [src_layer], "parent": row.layer.0})));
                 }
                 (1, Some((_, Row { layer, kind: RowKind::Prop { uid }, .. }))) if uid != src_prop => {
-                    actions.push(("prop.pickWhip".into(), json!({"layer": src_layer, "prop": src_prop, "target": {"layer": layer.0, "prop": uid}})));
+                    let mut params = json!({"layer": src_layer, "prop": src_prop, "target": {"layer": layer.0, "prop": uid}});
+                    if let Some((text, [a, b])) = into {
+                        // Into the expression being edited, which goes on being edited.
+                        params["expression"] = json!(text);
+                        params["range"] = json!([a, b]);
+                        let after = text.chars().count().saturating_sub(b);
+                        ctx.data_mut(|d| d.insert_temp(expr_refocus_id(src_prop), after));
+                    }
+                    actions.push(("prop.pickWhip".into(), params));
                 }
                 _ => {}
             }

crates/ui-egui/tests/ui_inline_edit.rs

@@ -73,3 +73,57 @@ fn timecode_field_replaces_commits_and_cancels() {
     assert_eq!(frame(&h), 24, "Escape cancels");
     assert!(!h.ctx.egui_wants_keyboard_input(), "Escape releases the keyboard");
 }
+
+/// #284: dragging the expression pick whip while the expression is being edited puts the
+/// reference at the cursor (After Effects), and the editing goes on after it; without an edit
+/// in progress it replaces the expression.
+#[test]
+fn the_expression_pick_whip_inserts_at_the_cursor_while_editing() {
+    let (mut h, _) = harness();
+    let (layer, opacity, rotation, transform) = {
+        let s = &mut h.state_mut().session;
+        let l = s.execute("layer.newSolid", json!({"name": "Plate", "color": "#406080"})).unwrap()["layer"].as_u64().unwrap();
+        s.execute("prop.setExpression", json!({"layer": l, "path": "transform/opacity", "expression": "50 + 0"})).unwrap();
+        let tr = s.active_comp().unwrap().layer(effectcraft_engine::project::LayerId(l)).unwrap().props.sub("transform").unwrap().clone();
+        (l, tr.get("opacity").unwrap().uid, tr.get("rotation").unwrap().uid, tr.uid)
+    };
+    h.state_mut().ui.timeline.open_layers.insert(layer);
+    h.state_mut().ui.timeline.open_groups.insert(transform);
+    h.run_steps(3);
+    let expr = |h: &Harness<'_, EffectcraftApp>| {
+        let comp = h.state().session.active_comp().unwrap();
+        comp.layer(effectcraft_engine::project::LayerId(layer)).unwrap().props.find(opacity).unwrap().expr.as_ref().unwrap().text.clone()
+    };
+    let whip = |h: &mut Harness<'_, EffectcraftApp>| {
+        let (from, to) = (center(h, &format!("timeline.prop.{opacity}.pickWhip")), center(h, &format!("timeline.prop.{rotation}.name")));
+        h.input_mut().events.push(Event::PointerMoved(from));
+        h.step();
+        h.input_mut().events.push(Event::PointerButton { pos: from, button: PointerButton::Primary, pressed: true, modifiers: Modifiers::NONE });
+        h.step();
+        for k in 1..=8 {
+            h.input_mut().events.push(Event::PointerMoved(from + (to - from) * (k as f32 / 8.0)));
+            h.step();
+        }
+        h.input_mut().events.push(Event::PointerButton { pos: to, button: PointerButton::Primary, pressed: false, modifiers: Modifiers::NONE });
+        h.run_steps(3);
+    };
+    // Editing: the cursor at the end after typing " * ".
+    let p = center(&h, &format!("timeline.prop.{opacity}.expression"));
+    click(&mut h, p, 1);
+    key(&mut h, Key::End);
+    type_text(&mut h, " * ");
+    whip(&mut h);
+    assert_eq!(expr(&h), "50 + 0 * transform.rotation");
+    // The editor has the keyboard again, its cursor after the reference.
+    type_text(&mut h, " + 1");
+    // Ctrl/Cmd+Enter commits.
+    h.input_mut().events.push(Event::ModifiersChanged(Modifiers::COMMAND));
+    h.input_mut().events.push(Event::Key { key: Key::Enter, physical_key: None, pressed: true, repeat: false, modifiers: Modifiers::COMMAND });
+    h.step();
+    h.input_mut().events.push(Event::ModifiersChanged(Modifiers::NONE));
+    h.run_steps(3);
+    assert_eq!(expr(&h), "50 + 0 * transform.rotation + 1");
+    // Not editing: the reference replaces the expression.
+    whip(&mut h);
+    assert_eq!(expr(&h), "transform.rotation");
+}