From e108408c31cffb7ec86aa359cf2628d8ba873582 Mon Sep 17 00:00:00 2001 From: moppeeer Date: Wed, 21 Oct 2026 23:58:07 +0200 Subject: [PATCH] Insert Pages: a placement given must be a page index; the dialog's page is clamped MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `layout.pages.insert` refuses `after` and `before` given together (null counts as given) and a placement that isn't a page index, instead of inserting at the default position; the default applies only when neither is given. The dialog sends the page its menu shows (a page past the end is the last). The dialog title is in the shared table, and Georgian "Before Page" reads "გვერდის წინ". Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/layout.rs | 36 ++++++++++++++++++++++++++------- crates/ui-egui/src/dialogs.rs | 21 ++++++++++++++++++- crates/ui-egui/src/i18n.rs | 1 + crates/ui-egui/src/i18n/fr.rs | 1 - crates/ui-egui/src/i18n/ka.rs | 2 +- 5 files changed, 51 insertions(+), 10 deletions(-) diff --git a/crates/engine/src/cmd/layout.rs b/crates/engine/src/cmd/layout.rs index 3ddaaf4c6..5e1879573 100644 --- a/crates/engine/src/cmd/layout.rs +++ b/crates/engine/src/cmd/layout.rs @@ -118,11 +118,21 @@ pub fn specs() -> Vec { let count = p.get("count").and_then(Value::as_u64).unwrap_or(1).clamp(1, 9999) as usize; let st = s.doc()?; let n = st.doc.page_count(); - let index = |k: &str| p.get(k).and_then(Value::as_u64).map(|v| usize::try_from(v).unwrap_or(usize::MAX)); + // A placement given (null included) must be a page index; only when neither is + // given do the pages go after the last. + if p.get("after").is_some() && p.get("before").is_some() { + return Err(bad("layout.pages.insert", "give `after` or `before`, not both")); + } + let index = |k: &str| match p.get(k) { + None => Ok(None), + Some(v) => v + .as_u64() + .map(|v| Some(usize::try_from(v).unwrap_or(usize::MAX))) + .ok_or_else(|| bad("layout.pages.insert", format!("`{k}` must be a page index, not {v}"))), + }; // The page the new ones follow; none: they go first. - let after = match (index("after"), index("before")) { - (Some(_), Some(_)) => return Err(bad("layout.pages.insert", "give `after` or `before`, not both")), - (None, Some(b)) => b.checked_sub(1), + let after = match (index("after")?, index("before")?) { + (_, Some(b)) => b.checked_sub(1), (a, None) => Some(a.unwrap_or(n.saturating_sub(1))), }; let parent = parent_ref("layout.pages.insert", &st.doc, p.get("parent"))?.unwrap_or_else(|| st.doc.parents.first().map(|x| x.id)); @@ -1884,10 +1894,22 @@ mod parent_margin_tests { assert_eq!(then[..3], now[..3]); assert_eq!(then[5], old[2]); assert_eq!((parent(&s, 3), parent(&s, 4)), (None, None)); + // Refused without a change or an undo step: both placements, even one of them null, and a + // placement that isn't a page index. let undo = s.doc().unwrap().history.undo.len(); - assert!(s.execute("layout.pages.insert", &json!({"after": 0, "before": 1})).is_err()); - assert_eq!(s.doc().unwrap().history.undo.len(), undo); - assert_eq!(ids(&s), then); + for bad in [ + json!({"after": 0, "before": 1}), + json!({"after": 0, "before": null}), + json!({"after": null, "before": 1}), + json!({"before": -1}), + json!({"before": "0"}), + json!({"before": null}), + json!({"after": 1.5}), + ] { + assert!(s.execute("layout.pages.insert", &bad).is_err(), "{bad}"); + assert_eq!(s.doc().unwrap().history.undo.len(), undo, "{bad}"); + assert_eq!(ids(&s), then, "{bad}"); + } } #[test] diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 13ce41a6e..640888f24 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -3132,7 +3132,8 @@ pub fn confirm(app: &mut DesignApp) -> Result { "ruby" => app.run("type.ruby", style_cjk::ruby_params(&d)), "insertPages" => { let n = app.session.active().map_or(1, |st| st.doc.page_count()); - let page = d.n("page").unwrap_or(0.0).max(0.0) as usize; + // The page the menu shows: a page past the end is the last. + let page = (d.n("page").unwrap_or(0.0).max(0.0) as usize).min(n.saturating_sub(1)); let mut p = json!({"count": d.n("count").unwrap_or(1.0).clamp(1.0, 9999.0) as u64}); match d.s("insert").as_str() { "before" => p["before"] = json!(page), @@ -7485,6 +7486,24 @@ mod tests { assert!(matches!(fill(&app, id), designcraft_color::Color::Rgb { .. }), "{:?}", fill(&app, id)); } + /// A page past the end shows as the last page in the dialog, and Before Page puts the new + /// page before that one rather than at the end. + #[test] + fn insert_pages_before_a_page_past_the_end_goes_before_the_last() { + let mut app = DesignApp::new(designcraft_engine::Session::new(), crate::Services::default()); + app.run("file.new", json!({"pages": 4, "facingPages": false})).unwrap(); + let ids = |app: &DesignApp| { + let d = &app.session.doc().unwrap().doc; + (0..d.page_count()).map(|i| d.page(i).unwrap().id).collect::>() + }; + let old = ids(&app); + app.ui.dialog = Some(Dialog::new("insertPages", json!({"count": 1, "insert": "before", "page": 100, "parent": ""}))); + confirm(&mut app).unwrap(); + let now = ids(&app); + assert_eq!(now.len(), 5); + assert_eq!((now[..3].to_vec(), now[4]), (old[..3].to_vec(), old[3]), "the new page is fourth, before the old last page"); + } + /// The course's chapter 25: Insert Pages as InDesign's. It opens after the page in view with /// that page's parent; At Start of Document with B-Cover puts a B page first, and Before Page /// with [None] puts a page without a parent before the page chosen. diff --git a/crates/ui-egui/src/i18n.rs b/crates/ui-egui/src/i18n.rs index ddc27e2eb..ee60b7d27 100644 --- a/crates/ui-egui/src/i18n.rs +++ b/crates/ui-egui/src/i18n.rs @@ -1846,6 +1846,7 @@ const TABLE: &[(&str, [&str; 5])] = &[ ["Insert Math Expression…", "Insérer une expression mathématique…", "Insert Math Expression…", "Insert Math Expression…", "插入数学公式…"], ), ("Insert Pages…", ["Insert Pages…", "Insérer des pages…", "Insert Pages…", "Insert Pages…", "插入页面…"]), + ("Insert Pages", ["Seiten einfügen", "Insérer des pages", "Insertar páginas", "ページを挿入", "插入页面"]), ("Insert:", ["Einfügen:", "Insérer :", "Insertar:", "挿入:", "插入："]), ("Parent:", ["Vorlagenseite:", "Gabarit :", "Página principal:", "親ページ:", "主页："]), ("After Page", ["Nach Seite", "Après la page", "Después de la página", "ページの後", "页面后"]), diff --git a/crates/ui-egui/src/i18n/fr.rs b/crates/ui-egui/src/i18n/fr.rs index 78c5d5948..46558bb58 100644 --- a/crates/ui-egui/src/i18n/fr.rs +++ b/crates/ui-egui/src/i18n/fr.rs @@ -84,7 +84,6 @@ pub(super) const TABLE: &[(&str, &str)] = &[ ("Insert Anchored Object", "Insérer un objet ancré"), ("Insert Cross-Reference", "Insérer une référence croisée"), ("Insert Math Expression", "Insérer une expression mathématique"), - ("Insert Pages", "Insérer des pages"), ("Internet Standard RGB (sRGB)", "RVB standard Internet (sRGB)"), ("Join the ArtCraft Discord", "Rejoindre le Discord d'ArtCraft"), ("Join the community", "Rejoindre la communauté"), diff --git a/crates/ui-egui/src/i18n/ka.rs b/crates/ui-egui/src/i18n/ka.rs index fb802b878..35561798a 100644 --- a/crates/ui-egui/src/i18n/ka.rs +++ b/crates/ui-egui/src/i18n/ka.rs @@ -33,7 +33,7 @@ pub(super) const TABLE: &[(&str, &str)] = &[ ("Paste", "ჩასმა"), ("At End of Document", "დოკუმენტის ბოლოს"), ("At Start of Document", "დოკუმენტის დასაწყისში"), - ("Before Page", "გვერდამდე"), + ("Before Page", "გვერდის წინ"), ("After Page", "გვერდის შემდეგ"), ("Parent:", "საბაზო გვერდი:"), ("Insert:", "ჩასმა:"),