From db935e2090c2a652d270d75d9d9074e393209c38 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sat, 24 Oct 2026 09:05:30 -0700 Subject: [PATCH] Footnotes: deleting a footnote or endnote keeps the caret on the same text footnote.delete and endnote.delete removed the reference character from the story but left a caret or selection in the story text at its old byte offsets. Everything after the reference moved back by the reference's length, so the caret ended up three bytes further along the text. A caret or selection after the reference now moves back with the text, and one before it stays where it is. footnote.setText left a caret in the replaced footnote at its old offset, which could fall past the end of the new text or inside a character. Backspace there then failed with an internal error. The caret now goes to the end of the new text. --- crates/engine/src/cmd/endnotes.rs | 20 ++++++++++++- crates/engine/src/cmd/notes.rs | 48 ++++++++++++++++++++++++++++++- 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/endnotes.rs b/crates/engine/src/cmd/endnotes.rs index fb3bf64ce..9b02d2c87 100644 --- a/crates/engine/src/cmd/endnotes.rs +++ b/crates/engine/src/cmd/endnotes.rs @@ -31,7 +31,7 @@ pub fn specs() -> Vec { }), cmd!("endnote.delete", "Delete Endnote", [], None, "{story, id} — removes the reference and its text", has_doc, |s, p| { let (sid, id) = target(p)?; - s.edit(|d, _| { + s.edit(|d, sel| { let st = d.story_mut(sid).ok_or_else(|| bad("endnote.delete", "no such story"))?; let k = st.endnotes.iter().position(|n| n.id == id).ok_or_else(|| bad("endnote.delete", format!("no endnote {id}")))?; let at = st @@ -40,6 +40,7 @@ pub fn specs() -> Vec { .nth(k) .map(|(i, _)| i) .ok_or_else(|| bad("endnote.delete", format!("endnote {id} has no reference in the text")))?; + super::notes::follow_delete(sel, sid, at..at + designcraft_doc::ENDNOTE_REF.len_utf8()); st.delete(at..at + designcraft_doc::ENDNOTE_REF.len_utf8()); ok() }) @@ -172,4 +173,21 @@ mod tests { s.execute("edit.undo", &json!({})).unwrap(); assert_eq!(s.doc().unwrap().doc.stories[&esid].text, "Notes\ni\tEarlier.\nii\tEdited."); } + + #[test] + fn deleting_an_endnote_keeps_the_caret_on_its_character() { + let mut s = Session::new(); + s.execute("file.new", &json!({"pages": 1})).unwrap(); + let a = s.execute("frame.create", &json!({"rect": [72, 72, 400, 300], "content": "text", "text": "Alpha beta."})).unwrap(); + let sid = designcraft_doc::StoryId(a["story"].as_u64().unwrap()); + s.execute("text.select", &json!({"story": a["story"], "anchor": 5, "focus": 5})).unwrap(); + let r = s.execute("endnote.insert", &json!({"text": "Source."})).unwrap(); + let n = designcraft_doc::ENDNOTE_REF.len_utf8(); + // "beta" is selected after the reference. + s.execute("text.select", &json!({"story": a["story"], "anchor": 6 + n, "focus": 10 + n})).unwrap(); + s.execute("endnote.delete", &json!({"story": r["story"], "id": r["id"]})).unwrap(); + let t = s.doc().unwrap().selection.text.unwrap(); + assert_eq!((t.anchor, t.focus), (6, 10)); + assert_eq!(&s.doc().unwrap().doc.stories[&sid].text[t.anchor..t.focus], "beta"); + } } diff --git a/crates/engine/src/cmd/notes.rs b/crates/engine/src/cmd/notes.rs index 03dde5c40..fd08e9314 100644 --- a/crates/engine/src/cmd/notes.rs +++ b/crates/engine/src/cmd/notes.rs @@ -115,6 +115,16 @@ fn note_id(p: &Value, cmd: &str) -> Result { p.get("id").and_then(Value::as_u64).ok_or_else(|| bad(cmd, "missing id")) } +/// Moves a caret or selection in story `sid`'s own text (not a cell or note) back by the length of `range` when it lies +/// after `range.start`, or to `range.start` when it lies inside `range`. +pub(super) fn follow_delete(sel: &mut designcraft_doc::Selection, sid: StoryId, range: std::ops::Range) { + if let Some(t) = sel.text.as_mut().filter(|t| t.story == sid && t.cell.is_none()) { + let back = |i: usize| if i > range.start { i - (i - range.start).min(range.len()) } else { i }; + t.anchor = back(t.anchor); + t.focus = back(t.focus); + } +} + fn list(s: &mut Session, p: &Value) -> Result { let d = &s.doc()?.doc; let only = p.get("story").and_then(Value::as_u64).map(StoryId); @@ -131,11 +141,15 @@ fn set_text(s: &mut Session, p: &Value) -> Result { let sid = story_id(p, "footnote.setText")?; let id = note_id(p, "footnote.setText")?; let text = super::text_param(p, "text"); - s.edit(|d, _| { + s.edit(|d, sel| { let st = d.story_mut(sid).ok_or(designcraft_doc::DocError::NoStory(sid))?; let n = st.note_mut(id).ok_or_else(|| bad("footnote.setText", format!("no footnote {id}")))?; let len = n.text.len(); n.text.replace(0..len, &text); + // A caret in the replaced text goes to its end. + if let Some(t) = sel.text.as_mut().filter(|t| t.story == sid && t.cell == Some(CellAddr::footnote(id))) { + (t.anchor, t.focus) = (n.text.len(), n.text.len()); + } Ok(Value::Null) }) } @@ -146,6 +160,7 @@ fn delete(s: &mut Session, p: &Value) -> Result { s.edit(|d, sel| { let st = d.story_mut(sid).ok_or(designcraft_doc::DocError::NoStory(sid))?; let at = st.note_anchor(id).ok_or_else(|| bad("footnote.delete", format!("no footnote {id}")))?; + follow_delete(sel, sid, at..at + designcraft_doc::FOOTNOTE_REF.len_utf8()); st.delete(at..at + designcraft_doc::FOOTNOTE_REF.len_utf8()); if sel.text.is_some_and(|t| t.story == sid && t.cell == Some(CellAddr::footnote(id))) { sel.text = Some(TextSel::caret(sid, at)); @@ -232,4 +247,35 @@ mod tests { let cs = s.cache.get(&d, sid, None); assert_eq!(cs.frames[0].notes.len(), 1); } + + #[test] + fn deleting_a_footnote_keeps_the_caret_on_its_character() { + let n = designcraft_doc::FOOTNOTE_REF.len_utf8(); + // (anchor, focus) in the story text before the delete, then after it. + for (before, after) in [((17 + n, 21 + n), (17, 21)), ((5, 21 + n), (5, 21)), ((5, 5), (5, 5)), ((16 + n, 16 + n), (16, 16))] { + let (mut s, sid) = session_with_text(); + s.execute("text.select", &json!({"story": sid.0, "anchor": 16, "focus": 16})).unwrap(); + let id = s.execute("footnote.insert", &json!({"text": "x"})).unwrap()["id"].as_u64().unwrap(); + s.execute("text.select", &json!({"story": sid.0, "anchor": before.0, "focus": before.1})).unwrap(); + s.execute("footnote.delete", &json!({"story": sid.0, "id": id})).unwrap(); + let t = s.doc().unwrap().selection.text.unwrap(); + assert_eq!((t.anchor, t.focus), after, "selected {before:?}"); + } + } + + #[test] + fn replacing_footnote_text_puts_its_caret_at_the_end() { + let (mut s, sid) = session_with_text(); + s.execute("text.select", &json!({"story": sid.0, "anchor": 16, "focus": 16})).unwrap(); + let id = s.execute("footnote.insert", &json!({"text": "abcd"})).unwrap()["id"].as_u64().unwrap(); + // The caret between "c" and "d" is inside the second "é" of the new text. + let t = s.doc().unwrap().selection.text.unwrap(); + assert_eq!(t.cell, Some(CellAddr::footnote(id))); + s.doc_mut().unwrap().selection.text = Some(TextSel { anchor: 3, focus: 3, ..t }); + s.execute("footnote.setText", &json!({"story": sid.0, "id": id, "text": "éé"})).unwrap(); + let t = s.doc().unwrap().selection.text.unwrap(); + assert_eq!((t.anchor, t.focus), (4, 4)); + s.execute("text.delete", &json!({})).unwrap(); + assert_eq!(s.doc().unwrap().doc.story(sid).unwrap().notes[0].text.text, "é"); + } }