diff --git a/crates/doc/src/tests.rs b/crates/doc/src/tests.rs index 38e243825..65fa12ee5 100644 --- a/crates/doc/src/tests.rs +++ b/crates/doc/src/tests.rs @@ -235,6 +235,23 @@ fn hit_testing_respects_layers() { assert_eq!(d.hit_item(0, designcraft_geom::Point::new(50.0, 50.0), 2.0), None); } +#[test] +fn page_numbers_saturate_at_the_largest_start() { + // A start number from a file or an old command: page 2 overflowed (a panic in debug builds). + let mut d = crate::Document::new(&crate::build::NewDocument { pages: 3, facing_pages: false, ..Default::default() }); + d.sections = vec![crate::Section { + start: 0, + start_number: Some(u32::MAX), + style: Default::default(), + prefix: String::new(), + marker: String::new(), + include_prefix: false, + }]; + assert_eq!((0..3).map(|i| d.page_number(i)).collect::>(), [u32::MAX; 3]); + d.sections.push(crate::Section { start: 2, start_number: None, ..d.sections[0].clone() }); + assert_eq!(d.page_number(2), u32::MAX, "a section continuing it saturates too"); +} + #[test] fn page_names_follow_sections() { let mut d = Document::new(&NewDocument { pages: 6, ..Default::default() }); diff --git a/crates/engine/src/cmd/layout.rs b/crates/engine/src/cmd/layout.rs index 2af1babc7..10933b024 100644 --- a/crates/engine/src/cmd/layout.rs +++ b/crates/engine/src/cmd/layout.rs @@ -436,58 +436,12 @@ pub fn specs() -> Vec { "Numbering & Section Options…", ["Layout"], None, - "{page (1-based; starts a section there), startNumber?: n|null (continue), style?: arabic|upperRoman|lowerRoman|upperLetters|lowerLetters|arabicLeadingZero|arabicThreeDigits|arabicFourDigits|kanji, prefix? (up to 8 characters; no + or comma), includePrefix?, marker?, remove?: bool}", + "{page (1-based; starts a section there), startNumber?: n (1–99999)|null (continue), style?: arabic|upperRoman|lowerRoman|upperLetters|lowerLetters|arabicLeadingZero|arabicThreeDigits|arabicFourDigits|kanji, prefix? (up to 8 characters; no + or comma), includePrefix?, marker?, remove?: bool}", has_doc, |s, p| { - let page = p.get("page").and_then(Value::as_u64).ok_or_else(|| bad("layout.section", "missing page"))? as usize; - let n = s.doc()?.doc.page_count(); - if page == 0 || page > n { - return Err(bad("layout.section", format!("no page {page}"))); - } - let start = page - 1; - if let Some(prefix) = p.get("prefix").and_then(Value::as_str) - && (prefix.chars().count() > 8 || prefix.contains(['+', ','])) - { - return Err(bad("layout.section", "section prefix: up to 8 characters, without + or ,")); - } - let p = p.clone(); + let edit = SectionEdit::parse("layout.section", s, p)?; s.edit(|d, _| { - d.sections.retain(|x| x.start != start || start == 0); - if p.get("remove").and_then(Value::as_bool).unwrap_or(false) && start != 0 { - return ok(); - } - let mut sec = d.sections.iter().find(|x| x.start == start).cloned().unwrap_or(designcraft_doc::Section { - start, - start_number: None, - style: Default::default(), - prefix: String::new(), - marker: String::new(), - include_prefix: false, - }); - match p.get("startNumber") { - Some(Value::Null) => sec.start_number = None, - Some(v) => sec.start_number = Some(start_number("layout.section", "startNumber", v)?), - None => {} - } - if let Some(v) = p.get("style") { - sec.style = match serde_json::from_value(v.clone()) { - Ok(designcraft_doc::NumberStyle::Symbols) => return Err(bad("layout.section", "symbols don't number pages")), - Ok(style) => style, - Err(e) => return Err(bad("layout.section", e.to_string())), - }; - } - if let Some(v) = p.get("prefix").and_then(Value::as_str) { - sec.prefix = v.into(); - } - if let Some(v) = p.get("includePrefix").and_then(Value::as_bool) { - sec.include_prefix = v; - } - if let Some(v) = p.get("marker").and_then(Value::as_str) { - sec.marker = v.into(); - } - d.sections.retain(|x| x.start != start); - d.sections.push(sec); - d.sections.sort_by_key(|x| x.start); + edit.apply(d); Ok(json!({"names": (0..d.page_count()).map(|i| d.page_name(i)).collect::>()})) }) } @@ -503,49 +457,36 @@ pub fn specs() -> Vec { if p.as_object().is_none_or(|o| o.is_empty()) { return chapter_numbering(s); } - let style = match p.get("style") { - Some(v) => match serde_json::from_value::(v.clone()) { - Ok(designcraft_doc::NumberStyle::Symbols) | Err(_) => { - return Err(bad("layout.chapterNumbering", format!("unknown chapter style {v}"))); - } - Ok(style) => Some(style), - }, - None => None, - }; - let mut source = match p.get("source") { - Some(v) => Some( - serde_json::from_value::(v.clone()) - .map_err(|_| bad("layout.chapterNumbering", format!("unknown chapter number source {v}")))?, - ), - None => None, - }; - let start = match p.get("start") { - Some(v) => Some( - v.as_u64() - .filter(|n| (1..=MAX_CHAPTER).contains(n)) - .ok_or_else(|| bad("layout.chapterNumbering", format!("start: 1 to {MAX_CHAPTER}")))? as u32, - ), - None => None, - }; - // A start number alone means Start Chapter Numbering at. - if start.is_some() && source.is_none() { - source = Some(designcraft_doc::ChapterSource::UserDefined); - } + let edit = ChapterEdit::parse("layout.chapterNumbering", p)?; s.edit(|d, _| { - if let Some(style) = style { - d.settings.chapter_style = style; - } - if let Some(source) = source { - d.settings.chapter_source = source; - } - if let Some(n) = start { - d.settings.chapter_number = n; - } + edit.apply(d); ok() })?; chapter_numbering(s) } ), + cmd!( + "layout.numberingOptions", + "Apply Numbering & Section Options", + [], + None, + "{page, …layout.section's settings, chapter?: {…layout.chapterNumbering's settings}} — the section and the document's chapter numbering in one undo step; if either is refused nothing changes", + has_doc, + |s, p| { + let section = SectionEdit::parse("layout.numberingOptions", s, p)?; + let chapter = match p.get("chapter") { + Some(c) => Some(ChapterEdit::parse("layout.numberingOptions", c)?), + None => None, + }; + s.edit(|d, _| { + section.apply(d); + if let Some(chapter) = &chapter { + chapter.apply(d); + } + Ok(json!({"names": (0..d.page_count()).map(|i| d.page_name(i)).collect::>(), "chapter": d.chapter_label()})) + }) + } + ), cmd!("layer.move", "Move Layer", [], None, "{id, to: index (0 = top/frontmost)}", has_doc, |s, p| { let id = LayerId(p.get("id").and_then(Value::as_u64).ok_or_else(|| bad("layer.move", "missing id"))?); let to = p.get("to").and_then(Value::as_u64).ok_or_else(|| bad("layer.move", "missing to"))?; @@ -838,7 +779,141 @@ fn document_setup(d: &designcraft_doc::Document) -> Value { } /// The largest chapter number (as for page numbers). -const MAX_CHAPTER: u64 = 99_999; +const MAX_CHAPTER: u64 = designcraft_doc::MAX_PAGE_NUMBER as u64; + +/// A `layout.section` edit, checked before the document changes. +struct SectionEdit { + /// Absolute index of the section's first page. + start: usize, + remove: bool, + /// Some(None): continue from the previous section. + start_number: Option>, + style: Option, + prefix: Option, + include_prefix: Option, + marker: Option, +} + +impl SectionEdit { + fn parse(cmd: &str, s: &Session, p: &Value) -> Result { + let page = p.get("page").and_then(Value::as_u64).ok_or_else(|| bad(cmd, "missing page"))?; + let n = s.doc()?.doc.page_count() as u64; + if page == 0 || page > n { + return Err(bad(cmd, format!("no page {page}"))); + } + let prefix = p.get("prefix").and_then(Value::as_str).map(str::to_string); + if prefix.as_ref().is_some_and(|x| x.chars().count() > 8 || x.contains(['+', ','])) { + return Err(bad(cmd, "section prefix: up to 8 characters, without + or ,")); + } + let start_number = match p.get("startNumber") { + None => None, + Some(Value::Null) => Some(None), + Some(v) => Some(Some(start_number(cmd, "startNumber", v)?)), + }; + let style = match p.get("style") { + None => None, + Some(v) => match serde_json::from_value(v.clone()) { + Ok(designcraft_doc::NumberStyle::Symbols) => return Err(bad(cmd, "symbols don't number pages")), + Ok(style) => Some(style), + Err(e) => return Err(bad(cmd, e.to_string())), + }, + }; + Ok(SectionEdit { + start: (page - 1) as usize, + remove: p.get("remove").and_then(Value::as_bool).unwrap_or(false), + start_number, + style, + prefix, + include_prefix: p.get("includePrefix").and_then(Value::as_bool), + marker: p.get("marker").and_then(Value::as_str).map(str::to_string), + }) + } + + fn apply(&self, d: &mut designcraft_doc::Document) { + let start = self.start; + // Page 1 always starts a section. + if self.remove && start != 0 { + d.sections.retain(|x| x.start != start); + return; + } + let mut sec = d.sections.iter().find(|x| x.start == start).cloned().unwrap_or(designcraft_doc::Section { + start, + start_number: None, + style: Default::default(), + prefix: String::new(), + marker: String::new(), + include_prefix: false, + }); + if let Some(n) = self.start_number { + sec.start_number = n; + } + if let Some(style) = self.style { + sec.style = style; + } + if let Some(prefix) = &self.prefix { + sec.prefix = prefix.clone(); + } + if let Some(on) = self.include_prefix { + sec.include_prefix = on; + } + if let Some(marker) = &self.marker { + sec.marker = marker.clone(); + } + d.sections.retain(|x| x.start != start); + d.sections.push(sec); + d.sections.sort_by_key(|x| x.start); + } +} + +/// A `layout.chapterNumbering` edit, checked before the document changes. +struct ChapterEdit { + style: Option, + source: Option, + start: Option, +} + +impl ChapterEdit { + fn parse(cmd: &str, p: &Value) -> Result { + let style = match p.get("style") { + Some(v) => match serde_json::from_value::(v.clone()) { + Ok(designcraft_doc::NumberStyle::Symbols) | Err(_) => return Err(bad(cmd, format!("unknown chapter style {v}"))), + Ok(style) => Some(style), + }, + None => None, + }; + let mut source = match p.get("source") { + Some(v) => Some( + serde_json::from_value::(v.clone()) + .map_err(|_| bad(cmd, format!("unknown chapter number source {v}")))?, + ), + None => None, + }; + let start = match p.get("start") { + Some(v) => { + Some(v.as_u64().filter(|n| (1..=MAX_CHAPTER).contains(n)).ok_or_else(|| bad(cmd, format!("chapter start: 1 to {MAX_CHAPTER}")))? + as u32) + } + None => None, + }; + // A start number alone means Start Chapter Numbering at. + if start.is_some() && source.is_none() { + source = Some(designcraft_doc::ChapterSource::UserDefined); + } + Ok(ChapterEdit { style, source, start }) + } + + fn apply(&self, d: &mut designcraft_doc::Document) { + if let Some(style) = self.style { + d.settings.chapter_style = style; + } + if let Some(source) = self.source { + d.settings.chapter_source = source; + } + if let Some(n) = self.start { + d.settings.chapter_number = n; + } + } +} /// The active document's chapter numbering, and the open book that holds it (what /// `layout.chapterNumbering {}` reports, read without running a command). @@ -1212,6 +1287,39 @@ mod page_numbering_view_tests { assert_eq!(r["names"], json!(["99999", "100000"])); } + #[test] + fn numbering_options_apply_in_one_undo_step_or_not_at_all() { + let mut s = crate::Session::new(); + s.execute("file.new", &json!({"pages": 4})).unwrap(); + let names = |s: &crate::Session| (0..4).map(|i| s.doc().unwrap().doc.page_name(i)).collect::>(); + let r = s + .execute( + "layout.numberingOptions", + &json!({"page": 3, "startNumber": 5, "style": "lowerRoman", "prefix": "A-", "includePrefix": true, "chapter": {"start": 4, "style": "upperRoman"}}), + ) + .unwrap(); + assert_eq!((r["names"].clone(), r["chapter"].clone()), (json!(["1", "2", "A-v", "A-vi"]), json!("IV"))); + s.execute("edit.undo", &json!({})).unwrap(); + assert_eq!(names(&s), ["1", "2", "3", "4"], "one undo reverts the section…"); + assert_eq!(s.doc().unwrap().doc.chapter_label(), "1", "…and the chapter numbering"); + // A refused part leaves the other undone too. + for bad in [ + json!({"page": 3, "startNumber": 5, "chapter": {"start": 100_000}}), + json!({"page": 3, "startNumber": 4_294_967_295u64, "chapter": {"start": 2}}), + json!({"page": 3, "prefix": "A+B", "chapter": {"start": 2}}), + json!({"page": 9, "chapter": {"start": 2}}), + ] { + assert!(s.execute("layout.numberingOptions", &bad).is_err(), "{bad}"); + assert_eq!(names(&s), ["1", "2", "3", "4"], "{bad}"); + assert_eq!(s.doc().unwrap().doc.settings.chapter_source, designcraft_doc::ChapterSource::Automatic, "{bad}"); + } + // Removing a section keeps the chapter part. + s.execute("layout.section", &json!({"page": 3, "startNumber": 1})).unwrap(); + s.execute("layout.numberingOptions", &json!({"page": 3, "remove": true, "chapter": {"source": "userDefined", "start": 2}})).unwrap(); + assert_eq!(names(&s), ["1", "2", "3", "4"]); + assert_eq!(s.doc().unwrap().doc.chapter_label(), "2"); + } + #[test] fn section_prefixes_are_short_and_avoid_plus_and_comma() { let mut s = crate::Session::new(); diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 469cf9d50..1a121803b 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -5740,6 +5740,38 @@ mod tests { assert_eq!([t.cell(0, 0).unwrap().strokes[2].weight, t.cell(1, 1).unwrap().strokes[0].weight], [1.0; 2]); } + #[test] + fn enter_and_escape_close_the_style_menu_not_numbering_and_section_options() { + let mut app = DesignApp::new(designcraft_engine::Session::new(), crate::Services::default()); + app.run("file.new", json!({"pages": 2})).unwrap(); + crate::menus::activate(&mut app, "layout.section", &Value::Null); + let ctx = dialog_context("en"); + let screen = egui::Rect::from_min_size(egui::Pos2::ZERO, egui::vec2(1178.0, 814.0)); + for _ in 0..3 { + dialog_frame(&mut app, &ctx, screen, vec![]); + } + let key = |key| egui::Event::Key { key, physical_key: None, pressed: true, repeat: false, modifiers: egui::Modifiers::NONE }; + for pressed in [egui::Key::Enter, egui::Key::Escape] { + let labels = dialog_frame(&mut app, &ctx, screen, vec![]); + // The section's Style menu (the first of the two). + let style = labels.iter().filter(|(t, r, c)| t.trim() == "1, 2, 3, 4…" && c.contains_rect(*r)).map(|(_, r, _)| *r).next().unwrap(); + click_dialog(&mut app, &ctx, screen, style.center()); + dialog_frame(&mut app, &ctx, screen, vec![]); + assert!(egui::Popup::is_any_open(&ctx), "the Style menu opened"); + dialog_frame(&mut app, &ctx, screen, vec![key(pressed)]); + dialog_frame(&mut app, &ctx, screen, vec![]); + assert!(app.ui.dialog.is_some(), "{pressed:?} with the menu open leaves the dialog open"); + assert!(app.session.active().unwrap().doc.sections.iter().all(|s| s.start_number != Some(0))); + if egui::Popup::is_any_open(&ctx) { + egui::Popup::close_all(&ctx); + dialog_frame(&mut app, &ctx, screen, vec![]); + } + } + // With no menu open, Escape closes the dialog. + dialog_frame(&mut app, &ctx, screen, vec![key(egui::Key::Escape)]); + assert!(app.ui.dialog.is_none()); + } + #[test] fn numbering_and_section_options_keep_their_actions_in_view() { for language in ["en", "ar"] { diff --git a/crates/ui-egui/src/section_options.rs b/crates/ui-egui/src/section_options.rs index cf69483bb..899b9fbed 100644 --- a/crates/ui-egui/src/section_options.rs +++ b/crates/ui-egui/src/section_options.rs @@ -60,7 +60,8 @@ fn number(d: &Dialog, key: &str) -> Option { d.n(key).filter(|v| v.is_finite() && *v >= 1.0 && v.fract() == 0.0).map(|v| v as u64) } -/// Apply the dialog: the section, then the chapter numbering. Refused values leave both as they were. +/// Apply the dialog: the section and the chapter numbering together (`layout.numberingOptions`, +/// one undo step). A refused value leaves both as they were. pub(crate) fn confirm(app: &mut DesignApp, d: &Dialog) -> Result { let page = number(d, "page").ok_or("no page")?; let source: ChapterSource = d.fields.get("chapterSource").and_then(|v| serde_json::from_value(v.clone()).ok()).unwrap_or_default(); @@ -68,18 +69,15 @@ pub(crate) fn confirm(app: &mut DesignApp, d: &Dialog) -> Result if source == ChapterSource::UserDefined { chapter["start"] = json!(number(d, "chapterStart").ok_or("Start Chapter Numbering at: a whole number from 1")?); } - if page != 1 && !d.b("startSection") { - app.run("layout.section", json!({"page": page, "remove": true}))?; + let params = if page != 1 && !d.b("startSection") { + json!({"page": page, "remove": true, "chapter": chapter}) } else { let start = if d.b("automatic") { Value::Null } else { json!(number(d, "startNumber").ok_or("Start Page Numbering at: a whole number from 1")?) }; - app.run( - "layout.section", - json!({"page": page, "startNumber": start, "style": d.fields.get("style").cloned().unwrap_or(json!("arabic")), - "prefix": d.s("prefix"), "marker": d.s("marker"), "includePrefix": d.b("includePrefix")}), - )?; - } - app.run("layout.chapterNumbering", chapter) + json!({"page": page, "startNumber": start, "style": d.fields.get("style").cloned().unwrap_or(json!("arabic")), + "prefix": d.s("prefix"), "marker": d.s("marker"), "includePrefix": d.b("includePrefix"), "chapter": chapter}) + }; + app.run("layout.numberingOptions", params) } /// A row that runs right to left in a right-to-left interface. @@ -271,6 +269,14 @@ mod tests { let doc = &app.session.active().unwrap().doc; assert_eq!((0..4).map(|i| doc.page_name(i)).collect::>(), ["1", "2", "A-v", "A-vi"]); assert_eq!(doc.chapter_label(), "IV"); + // One OK is one undo step. + app.run("edit.undo", json!({})).unwrap(); + let doc = &app.session.active().unwrap().doc; + assert_eq!( + ((0..4).map(|i| doc.page_name(i)).collect::>(), doc.chapter_label()), + (vec!["1".to_string(), "2".into(), "3".into(), "4".into()], "1".into()) + ); + app.run("edit.redo", json!({})).unwrap(); // Reopened on that page, it shows the section. crate::menus::activate(&mut app, "layout.section", &Value::Null); set(&mut app, json!({"page": 3, "startSection": false})); @@ -281,7 +287,14 @@ mod tests { #[test] fn refused_values_keep_the_dialog_open_and_change_nothing() { - for (key, value) in [("prefix", json!("A+B")), ("prefix", json!("Appendix1")), ("startNumber", json!("0")), ("chapterStart", json!("x"))] { + for (key, value) in [ + ("prefix", json!("A+B")), + ("prefix", json!("Appendix1")), + ("startNumber", json!("0")), + ("startNumber", json!("4294967295")), + ("chapterStart", json!("x")), + ("chapterStart", json!("100000")), + ] { let mut app = open_app(); set( &mut app, diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index 5edde1c9f..f98ccfb76 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -354,6 +354,7 @@ It is evidence of breadth: each item exists. It is not a claim of InDesign-level - Separations Preview plates now come from each object's own colour (CMYK as authored; RGB and images through the colour settings, images via a cached 17³ table), so pure black shows only on the black plate. - Language (Character panel ▸ Language, Properties ▸ Character, `type.char {language}`): text in other languages isn't hyphenated or spell-checked with the English dictionaries, typographer's quotes follow the language (German „…“, French and Russian «…», Swiss, Spanish, Dutch, Nordic, Japanese and Traditional Chinese 「…」, Simplified Chinese “…”), the font's localized forms (OpenType `locl`: Turkish i, Bulgarian Cyrillic, Japanese / Chinese / Korean glyph variants) follow it too, and so does the fallback for CJK characters the font lacks (with Draw Missing Glyphs from Fallback Fonts on): Japanese, Simplified Chinese, Traditional Chinese and Korean each try their own installed system fonts (Japanese starts with the craft-fonts Japanese faces, Mincho first, when built with them; Hangul, kana and Bopomofo pick their language's fonts whatever the language). Language names from IDML keep their spelling; locale codes as InDesign writes them for some languages (`de_DE_2006`, `nl_NL_2005`; any `ll`, `ll_CC`, `ll-CC`) are their language and region for all of the above, and Chinese is also recognised in its common spellings ("Simplified Chinese", "Chinese (Traditional)", `zh_CN`, `zh-Hant`). Spanish is hyphenated with Javier Bezos's Liang patterns (`hyph-es.tex` 5.0, MIT), Portuguese with hyph-utf8's `hyph-pt.tex` 1.4 (BSD-3-Clause) and Norwegian with hyph-utf8's `hyph-no.tex` (permissive), all through the `hypher` crate; hyphenation for other languages isn't bundled yet. - Books (`book.new/open/add/remove/list/styleSource/paginate/syncStyles/exportPdf`, Book panel): `.dcbook` files, continuous page numbering, chapter numbers resolved in book order (Automatic, Start Chapter Numbering at, Same as Previous Document; `layout.chapterNumbering`, with a style for Chapter Number variables and cross-references), style sync from a style source, one merged PDF. Section prefixes follow InDesign's rule (up to 8 characters, no + or comma). +- Numbering & Section Options window as in InDesign (Start Section, automatic or explicit page numbering up to 99999, prefix, style, marker, Include Prefix; the document's chapter numbering and its book); OK applies both in one undo step (`layout.numberingOptions`). - Track Changes (`changes.track/list/acceptAll/rejectAll`, Type ▸ Track Changes): typing is marked added (highlighted on screen), deletions are kept but hidden from layout and exports until accepted or rejected. Per-change accept/reject and authors aren't there yet. - Track Changes: per-change Accept / Reject (`changes.accept`, `changes.reject`) and a Track Changes panel listing every change. - Move table rows and columns (`table.moveRow`, `table.moveColumn`, Table panel buttons; with whole rows or columns selected, drag them with the Type tool — `table.dropCells`).