Live-shape question: refused once the document changed under it (#12768)

The question also stores the document's revision: an edit while it is
open (an anchor added through the control channel, the question being
modal on screen) shifts the stored knot index, so Yes is refused rather
than deleting another anchor. Test: yes_after_the_path_changed_deletes_nothing
(red before: Yes deleted an anchor of the edited path).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Trevor BrownandClaude Opus 5.5 committed 2026-11-11 10:51:13 -05:00
1 parent f1e5a09f0b
commit a709ac2ef5
2 files changed
+20 -4

No files matched your search

@@ -32,7 +32,7 @@ fn is_live(app: &PhotocraftApp, r: PathRef) -> bool {
/// or "Don't show again" was ticked.
pub fn ask(app: &mut PhotocraftApp, r: PathRef, params: &Value) -> bool {
let PathRef::Layer(layer) = r else { return false };
let Some(document) = app.session.active().map(|st| st.doc.id.0) else { return false };
let Some((document, revision)) = app.session.active().map(|st| (st.doc.id.0, st.revision)) else { return false };
if !is_live(app, r) || dont_show_again::dismissed(app, PREF) {
return false;
}
@@ -44,8 +44,10 @@ pub fn ask(app: &mut PhotocraftApp, r: PathRef, params: &Value) -> bool {
f.insert("__command".into(), json!("path.deleteAnchor"));
f.insert("__label".into(), json!(tl!("Delete Anchor Point")));
f.insert("layer".into(), json!(layer));
// The layer id means something only in this document (another may become active meanwhile).
// The layer id means something only in this document (another may become active meanwhile),
// and the knot index only in this revision of it (an edit meanwhile can shift the anchors).
f.insert("__document".into(), json!(document));
f.insert("__revision".into(), json!(revision));
f.insert("params".into(), params.clone());
f.insert("__dontShowAgain".into(), json!(PREF));
f.insert("skip".into(), json!(false));
@@ -62,8 +64,8 @@ pub fn body(ui: &mut egui::Ui, fields: &mut Map<String, Value>) {
pub fn confirm(app: &mut PhotocraftApp, fields: &Map<String, Value>) -> Result<Value, String> {
let layer = fields.get("layer").and_then(Value::as_u64).ok_or(tl!("No anchor to delete"))?;
let params = fields.get("params").cloned().ok_or(tl!("No anchor to delete"))?;
let document = fields.get("__document").and_then(Value::as_u64);
if document.is_none() || app.session.active().map(|st| st.doc.id.0) != document {
let asked = fields.get("__document").and_then(Value::as_u64).zip(fields.get("__revision").and_then(Value::as_u64));
if asked.is_none() || app.session.active().map(|st| (st.doc.id.0, st.revision)) != asked {
return Err(tl!("No anchor to delete").into());
}
let result = app.run("path.deleteAnchor", params)?;
@@ -104,6 +104,20 @@ fn yes_after_another_document_became_active_deletes_nothing() {
assert_eq!(corners(&mut app, shape), 4, "the first document's shape keeps its corner");
}
/// The question's knot index belongs to the path as it was asked: an anchor added before the
/// corner meanwhile (through the control channel; the question is modal on screen) shifts it, so Yes
/// is refused instead of deleting another anchor.
#[test]
fn yes_after_the_path_changed_deletes_nothing() {
let (mut app, shape) = live(Tool::DeleteAnchorPoint);
click(&mut app, CORNER);
let id = question(&app).expect("the question");
app.run("path.addAnchor", json!({"name": "layer", "layer": shape, "subpath": 0, "knot": 0, "t": 0.5})).unwrap();
let (before, n) = (shape_path(&mut app, shape), steps(&app));
assert!(crate::dialogs::confirm(&mut app, id).is_err());
assert_eq!((shape_path(&mut app, shape), steps(&app)), (before, n));
}
#[test]
fn no_leaves_the_shape_its_parameters_and_history_unchanged() {
let (mut app, shape) = live(Tool::DeleteAnchorPoint);