From bf09a4fcad68f7dbb7f69879c05ad808e7f7d276 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Thu, 8 Oct 2026 23:16:46 +0700 Subject: [PATCH] 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 --- crates/engine/src/commands/link.rs | 21 ++++++++- crates/engine/src/tests_timeline.rs | 27 +++++++++++ crates/ui-egui/src/panels/timeline.rs | 65 +++++++++++++++++++++----- crates/ui-egui/tests/ui_inline_edit.rs | 54 +++++++++++++++++++++ 4 files changed, 154 insertions(+), 13 deletions(-) diff --git a/crates/engine/src/commands/link.rs b/crates/engine/src/commands/link.rs index 3892bd06..73697698 100644 --- a/crates/engine/src/commands/link.rs +++ b/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 { 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> = 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 { "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 ), diff --git a/crates/engine/src/tests_timeline.rs b/crates/engine/src/tests_timeline.rs index 03b91c28..8356aec7 100644 --- a/crates/engine/src/tests_timeline.rs +++ b/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) { diff --git a/crates/ui-egui/src/panels/timeline.rs b/crates/ui-egui/src/panels/timeline.rs index bdedd202..662efdb5 100644 --- a/crates/ui-egui/src/panels/timeline.rs +++ b/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::(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::(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::(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::(pick_whip_id())); + let into = (kind == 1).then(|| ctx.data_mut(|d| d.remove_temp::(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)); } _ => {} } diff --git a/crates/ui-egui/tests/ui_inline_edit.rs b/crates/ui-egui/tests/ui_inline_edit.rs index a8232de3..47b77471 100644 --- a/crates/ui-egui/tests/ui_inline_edit.rs +++ b/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"); +}