From 8dc6717ff28e7991a9ce501d85237aa2261bd42b Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sat, 24 Oct 2026 08:32:10 +0000 Subject: [PATCH 01/13] chore: replace glob imports in test modules with explicit names (Sonar S2208) Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/file/eps_place_tests.rs | 2 +- crates/engine/src/tests_ime.rs | 2 +- crates/engine/src/tests_table.rs | 2 +- crates/fonts/src/tests_docfonts.rs | 2 +- crates/fonts/src/tests_fallback.rs | 2 +- crates/fonts/src/tests_vertical.rs | 2 +- crates/pdf/src/rtl_tests.rs | 2 +- crates/ui-egui/src/guide_selection_tests.rs | 2 +- crates/ui-egui/src/panel_docking_tests.rs | 2 +- crates/ui-egui/src/parent_canvas_tests.rs | 2 +- 10 files changed, 10 insertions(+), 10 deletions(-) diff --git a/crates/engine/src/cmd/file/eps_place_tests.rs b/crates/engine/src/cmd/file/eps_place_tests.rs index 9b2c89da9..740e2b56a 100644 --- a/crates/engine/src/cmd/file/eps_place_tests.rs +++ b/crates/engine/src/cmd/file/eps_place_tests.rs @@ -3,7 +3,7 @@ //! These helpers write real files, so they live in their own `_tests.rs` file: the //! file-system guard in `tests_file_guard.rs` checks `src` files but exempts test ones. -use super::*; +use super::{Arc, AssetId, ItemId, Session, base64_encode, json}; pub(crate) fn tiff_eps() -> Vec { let mut tiff = b"II".to_vec(); diff --git a/crates/engine/src/tests_ime.rs b/crates/engine/src/tests_ime.rs index 266470914..def3b6cc0 100644 --- a/crates/engine/src/tests_ime.rs +++ b/crates/engine/src/tests_ime.rs @@ -4,7 +4,7 @@ use designcraft_tools::{Mods, PointerEvent, PointerKind, ToolKey}; use serde_json::json; -use super::*; +use super::{Arc, ViewInfo}; fn session() -> Session { let mut s = Session::new(); diff --git a/crates/engine/src/tests_table.rs b/crates/engine/src/tests_table.rs index 2fc4fd338..28387f19c 100644 --- a/crates/engine/src/tests_table.rs +++ b/crates/engine/src/tests_table.rs @@ -1,6 +1,6 @@ use serde_json::json; -use super::*; +use super::doc; fn session_with_frame() -> (Session, u64, u64) { let mut s = Session::new(); diff --git a/crates/fonts/src/tests_docfonts.rs b/crates/fonts/src/tests_docfonts.rs index 67b1d8748..7d22ece3d 100644 --- a/crates/fonts/src/tests_docfonts.rs +++ b/crates/fonts/src/tests_docfonts.rs @@ -4,7 +4,7 @@ use std::path::{Path, PathBuf}; -use super::*; +use super::{Arc, DOCUMENT_FONTS_FOLDER, FALLBACK_FAMILY, FontDb, FontSource, MAX_DOCUMENT_FONT_BYTES, MAX_DOCUMENT_FONT_FILES}; use crate::testing::{font_with, font_with_names, font_with_wws, with_fs_type}; /// An empty temporary folder unique to `name` and this process. diff --git a/crates/fonts/src/tests_fallback.rs b/crates/fonts/src/tests_fallback.rs index d9e27ac62..9ed897d3d 100644 --- a/crates/fonts/src/tests_fallback.rs +++ b/crates/fonts/src/tests_fallback.rs @@ -1,6 +1,6 @@ //! CJK fallback follows the character's language (and the script where it is unambiguous). -use super::*; +use super::FontDb; use crate::testing::font_with; const HAN: char = '直'; diff --git a/crates/fonts/src/tests_vertical.rs b/crates/fonts/src/tests_vertical.rs index 3fdf91b3b..bf48ac426 100644 --- a/crates/fonts/src/tests_vertical.rs +++ b/crates/fonts/src/tests_vertical.rs @@ -5,7 +5,7 @@ use skrifa::raw::TableProvider; -use super::*; +use super::{Arc, FontBytes, FontDb, FontFace, FontSource, GlyphId, LocationRef, Size, make_face}; use crate::testing::{font_mapping, font_with, with_base, with_code_pages, with_table_len, with_table_u16, with_vmtx, with_vorg}; use crate::{ShapeContext, feature, shape_vertical, shape_with_context}; diff --git a/crates/pdf/src/rtl_tests.rs b/crates/pdf/src/rtl_tests.rs index 53db2ac25..82974029e 100644 --- a/crates/pdf/src/rtl_tests.rs +++ b/crates/pdf/src/rtl_tests.rs @@ -8,7 +8,7 @@ use designcraft_doc::{Align, Document, ParaAttrs, ParaFormat, SpreadRef, TextDir use designcraft_geom::Rect; use crate::tests::extract_text; -use crate::*; +use crate::{PdfOptions, export_pdf}; const HEBREW: &str = "DC Test PDF Hebrew"; const ARABIC: &str = "DC Test PDF Arabic"; diff --git a/crates/ui-egui/src/guide_selection_tests.rs b/crates/ui-egui/src/guide_selection_tests.rs index d69026712..9d8dc1add 100644 --- a/crates/ui-egui/src/guide_selection_tests.rs +++ b/crates/ui-egui/src/guide_selection_tests.rs @@ -4,7 +4,7 @@ use egui::{Event, Modifiers, PointerButton}; use egui_kittest::Harness; -use super::*; +use super::{CanvasLayout, Mods, Overlay, Point, Pos2, RULER, SpreadRef, ToolKey, Xf, json, pos2, vec2}; fn harness(app: DesignApp) -> Harness<'static, DesignApp> { let mut h = Harness::builder().with_size(vec2(1440.0, 900.0)).build_ui_state( diff --git a/crates/ui-egui/src/panel_docking_tests.rs b/crates/ui-egui/src/panel_docking_tests.rs index f436cfb5e..9be8afd56 100644 --- a/crates/ui-egui/src/panel_docking_tests.rs +++ b/crates/ui-egui/src/panel_docking_tests.rs @@ -1,4 +1,4 @@ -use super::*; +use super::{Layout, Node, cancel_gesture, json, valid}; use egui::{Event, Modifiers, PointerButton, Pos2, vec2}; use egui_kittest::{Harness, kittest::Queryable}; diff --git a/crates/ui-egui/src/parent_canvas_tests.rs b/crates/ui-egui/src/parent_canvas_tests.rs index b3197a7e0..6a84519af 100644 --- a/crates/ui-egui/src/parent_canvas_tests.rs +++ b/crates/ui-egui/src/parent_canvas_tests.rs @@ -1,4 +1,4 @@ -use super::*; +use super::{CanvasLayout, Color32, Point, Pos2, Rect, View, Xf, json, render_texture, vec2}; fn app_with_document_object() -> DesignApp { let mut app = DesignApp::new(designcraft_engine::Session::new(), crate::Services::default()); From 9770002c5c1dc43d29866af46c42aa161ceec3b5 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sat, 24 Oct 2026 08:32:21 +0000 Subject: [PATCH 02/13] chore: method paths for remaining trivial closures (Sonar S1612) Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/datamerge/parse.rs | 2 +- crates/idml/src/xml.rs | 2 +- crates/textimport/src/xlsx.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/cmd/datamerge/parse.rs b/crates/engine/src/cmd/datamerge/parse.rs index c76c37f1e..741b66e67 100644 --- a/crates/engine/src/cmd/datamerge/parse.rs +++ b/crates/engine/src/cmd/datamerge/parse.rs @@ -128,7 +128,7 @@ pub fn table_from_objects(rows: &[Value]) -> Result { warnings.push(format!("Row {} has an extra field \"{key}\". It was ignored.", i + 1)); } } - if cells.iter().all(|c| c.is_empty()) { + if cells.iter().all(String::is_empty) { continue; } out.push(cells); diff --git a/crates/idml/src/xml.rs b/crates/idml/src/xml.rs index 374e70c7f..62b853368 100644 --- a/crates/idml/src/xml.rs +++ b/crates/idml/src/xml.rs @@ -88,7 +88,7 @@ impl El { if let Some(v) = self.get(k) { return Some(v.to_string()); } - self.prop_el(k).map(|e| e.text_content()) + self.prop_el(k).map(El::text_content) } pub fn prop_el(&self, k: &str) -> Option<&El> { self.find("Properties").and_then(|p| p.find(k)) diff --git a/crates/textimport/src/xlsx.rs b/crates/textimport/src/xlsx.rs index 83bea3c87..314dc9810 100644 --- a/crates/textimport/src/xlsx.rs +++ b/crates/textimport/src/xlsx.rs @@ -173,7 +173,7 @@ pub fn records(bytes: &[u8], sheet: Option<&str>) -> Result>, Im extra.push(v.clone()); } } - if *r != header_row && row.iter().all(|c| c.is_empty()) && extra.is_empty() { + if *r != header_row && row.iter().all(String::is_empty) && extra.is_empty() { continue; } row.extend(extra); From 681493f458d77cfbcf6053b78811f25e8d4a9861 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sat, 24 Oct 2026 08:34:17 +0000 Subject: [PATCH 03/13] refactor(engine): split data merge fill and record selection (Sonar S3776, S107) Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/datamerge/fill.rs | 130 ++++++++++++++--------- crates/engine/src/cmd/datamerge/mod.rs | 90 ++++++---------- crates/engine/src/cmd/datamerge/parse.rs | 65 ++++++------ 3 files changed, 145 insertions(+), 140 deletions(-) diff --git a/crates/engine/src/cmd/datamerge/fill.rs b/crates/engine/src/cmd/datamerge/fill.rs index d4b2235e9..1c964bae5 100644 --- a/crates/engine/src/cmd/datamerge/fill.rs +++ b/crates/engine/src/cmd/datamerge/fill.rs @@ -128,21 +128,33 @@ struct Rep { text: String, } +/// Where `fill_row` looks for linked images: the merge options and the two search folders. +#[derive(Clone, Copy)] +pub struct Sources<'a> { + pub options: &'a MergeOptions, + pub data_dir: Option<&'a Path>, + pub doc_dir: Option<&'a Path>, +} + +/// Replacements and hyperlinks collected while walking the placeholders of one record. +#[derive(Default)] +struct Pending { + reps: Vec, + links: Vec<(PlaceholderAnchor, HyperlinkDest, String)>, + parent: bool, + report: FillReport, +} + /// Fill `row` (0-based) into the items `maps` points at. `record` is the 1-based source row. pub fn fill_row( doc: &mut designcraft_doc::Document, fields: &[DataField], row: &[String], record: u32, - options: &MergeOptions, maps: &Maps, - data_dir: Option<&Path>, - doc_dir: Option<&Path>, + sources: &Sources, ) -> FillReport { - let mut report = FillReport::default(); - let mut parent = false; - let mut reps: Vec = Vec::new(); - let mut links: Vec<(PlaceholderAnchor, HyperlinkDest, String)> = Vec::new(); + let mut pending = Pending::default(); let placeholders = doc.data_merge.placeholders.clone(); for ph in &placeholders { if !fields.iter().any(|f| f.name == ph.field) { @@ -150,43 +162,11 @@ pub fn fill_row( } let cell = cell_of(fields, row, &ph.field).to_string(); match &ph.anchor { - PlaceholderAnchor::Text { story, start, end } => { - let Some(&mapped) = maps.stories.get(story) else { - if anchor_is_parent(doc, &ph.anchor) { - parent = true; - } - continue; - }; - match ph.role { - PlaceholderRole::Text => reps.push(Rep { story: mapped, start: *start, end: *end, text: cell }), - PlaceholderRole::Hyperlink => { - if let Some(dest) = classify_link(&cell) { - links.push((PlaceholderAnchor::Text { story: mapped, start: *start, end: *end }, dest, ph.field.clone())); - } - } - PlaceholderRole::Image | PlaceholderRole::Qr => {} - } - } - PlaceholderAnchor::Item { id } => { - let Some(&mapped) = maps.items.get(id) else { - if anchor_is_parent(doc, &ph.anchor) { - parent = true; - } - continue; - }; - match ph.role { - PlaceholderRole::Image => place_image(doc, mapped, &ph.field, &cell, record, options, data_dir, doc_dir, &mut report), - PlaceholderRole::Qr => place_qr(doc, mapped, &ph.field, &cell, record, &mut report), - PlaceholderRole::Hyperlink => { - if let Some(dest) = classify_link(&cell) { - links.push((PlaceholderAnchor::Item { id: mapped }, dest, ph.field.clone())); - } - } - PlaceholderRole::Text => {} - } - } + PlaceholderAnchor::Text { story, start, end } => plan_text(doc, maps, ph, (*story, *start, *end), cell, &mut pending), + PlaceholderAnchor::Item { .. } => plan_item(doc, maps, ph, &cell, record, sources, &mut pending), } } + let Pending { mut reps, links, parent, mut report } = pending; typed_reps(doc, fields, row, maps, &placeholders, &mut reps); apply_reps(doc, &reps); for (anchor, dest, field) in links { @@ -207,6 +187,59 @@ pub fn fill_row( report } +fn plan_text( + doc: &designcraft_doc::Document, + maps: &Maps, + ph: &designcraft_doc::Placeholder, + (story, start, end): (StoryId, usize, usize), + cell: String, + pending: &mut Pending, +) { + let Some(&mapped) = maps.stories.get(&story) else { + if anchor_is_parent(doc, &ph.anchor) { + pending.parent = true; + } + return; + }; + match ph.role { + PlaceholderRole::Text => pending.reps.push(Rep { story: mapped, start, end, text: cell }), + PlaceholderRole::Hyperlink => { + if let Some(dest) = classify_link(&cell) { + pending.links.push((PlaceholderAnchor::Text { story: mapped, start, end }, dest, ph.field.clone())); + } + } + PlaceholderRole::Image | PlaceholderRole::Qr => {} + } +} + +fn plan_item( + doc: &mut designcraft_doc::Document, + maps: &Maps, + ph: &designcraft_doc::Placeholder, + cell: &str, + record: u32, + sources: &Sources, + pending: &mut Pending, +) { + let PlaceholderAnchor::Item { id } = &ph.anchor else { return }; + let Some(&mapped) = maps.items.get(id) else { + if anchor_is_parent(doc, &ph.anchor) { + pending.parent = true; + } + return; + }; + match ph.role { + PlaceholderRole::Image => place_image(doc, mapped, &ph.field, cell, record, sources, &mut pending.report), + PlaceholderRole::Qr => place_qr(doc, mapped, &ph.field, cell, record, &mut pending.report), + PlaceholderRole::Hyperlink => { + if let Some(dest) = classify_link(cell) { + pending.links.push((PlaceholderAnchor::Item { id: mapped }, dest, ph.field.clone())); + } + } + PlaceholderRole::Text => {} + } +} + fn typed_reps( doc: &designcraft_doc::Document, fields: &[DataField], @@ -326,17 +359,8 @@ fn create_link(doc: &mut designcraft_doc::Document, anchor: PlaceholderAnchor, d true } -fn place_image( - doc: &mut designcraft_doc::Document, - item: ItemId, - field: &str, - cell: &str, - record: u32, - options: &MergeOptions, - data_dir: Option<&Path>, - doc_dir: Option<&Path>, - report: &mut FillReport, -) { +fn place_image(doc: &mut designcraft_doc::Document, item: ItemId, field: &str, cell: &str, record: u32, sources: &Sources, report: &mut FillReport) { + let Sources { options, data_dir, doc_dir } = *sources; let cands = image_candidates(cell, data_dir, doc_dir); let found = cands.iter().find_map(|p| { // Refuse huge files before reading them; a too-large image counts as missing. diff --git a/crates/engine/src/cmd/datamerge/mod.rs b/crates/engine/src/cmd/datamerge/mod.rs index c07e2fff4..e96feb9fb 100644 --- a/crates/engine/src/cmd/datamerge/mod.rs +++ b/crates/engine/src/cmd/datamerge/mod.rs @@ -18,7 +18,7 @@ use serde_json::{Value, json}; use super::{CommandSpec, bad, cmd, has_doc, id_param, ok, str_param}; use crate::{DocState, Result, Session}; -use fill::{FillReport, Maps, MissingImage, copy_item, fill_row, identity_map}; +use fill::{FillReport, Maps, MissingImage, Sources, copy_item, fill_row, identity_map}; use parse::{ Table, TileInput, extension, fingerprint_of, relative_between, select_records, status_of, table_from_bytes, table_from_grid, table_from_objects, tile, @@ -210,7 +210,8 @@ fn preview_cmd(s: &mut Session, p: &Value) -> Result { let row = source.rows.get((record as usize).saturating_sub(1)).cloned().unwrap_or_default(); let data_dir = source_file(&source, st.path.as_deref().map(Path::new)).as_deref().and_then(Path::parent).map(Path::to_path_buf); let doc_dir = st.path.as_deref().and_then(parent_dir); - fill_row(&mut doc, &source.fields, &row, record as u32, &options, &maps, data_dir.as_deref(), doc_dir.as_deref()); + let sources = Sources { options: &options, data_dir: data_dir.as_deref(), doc_dir: doc_dir.as_deref() }; + fill_row(&mut doc, &source.fields, &row, record as u32, &maps, &sources); st.doc = Arc::new(doc); st.preview_record = Some(record as u32); st.bump_revision(); @@ -245,10 +246,12 @@ fn merge_cmd(s: &mut Session, p: &Value) -> Result { warnings.extend(table.warnings.iter().cloned()); let mut merged = src.clone(); let mut missing = Vec::new(); + let sources = Sources { options: &options, data_dir: data_dir.as_deref(), doc_dir: doc_dir.as_deref() }; + let mut sink = Sink { warnings: &mut warnings, missing: &mut missing, parent: false }; if options.per_page == "multiple" { - tile_records(&mut merged, &src, &table, &selected, &options, data_dir.as_deref(), doc_dir.as_deref(), &mut warnings, &mut missing)?; + tile_records(&mut merged, &src, &table, &selected, &sources, &mut sink)?; } else { - copy_records(&mut merged, &src, &table, &selected, &options, data_dir.as_deref(), doc_dir.as_deref(), &mut warnings, &mut missing)?; + copy_records(&mut merged, &src, &table, &selected, &sources, &mut sink)?; } // Status is written only after every error return. The new document then becomes active, // so this template edit does not push an undo step. The fingerprint stays the last read. @@ -272,25 +275,14 @@ fn merge_cmd(s: &mut Session, p: &Value) -> Result { })) } -fn copy_records( - dst: &mut Document, - src: &Document, - table: &Table, - selected: &[usize], - options: &MergeOptions, - data_dir: Option<&Path>, - doc_dir: Option<&Path>, - warnings: &mut Vec, - missing: &mut Vec, -) -> Result<()> { - let mut parent = false; +fn copy_records(dst: &mut Document, src: &Document, table: &Table, selected: &[usize], sources: &Sources, sink: &mut Sink) -> Result<()> { if let Some(&idx) = selected.first() { let maps = identity_map(dst); - fill_at(dst, table, idx, options, &maps, data_dir, doc_dir, warnings, missing, &mut parent); + fill_at(dst, table, idx, sources, &maps, sink); } for &idx in selected.iter().skip(1) { let maps = append_copy(dst, src)?; - fill_at(dst, table, idx, options, &maps, data_dir, doc_dir, warnings, missing, &mut parent); + fill_at(dst, table, idx, sources, &maps, sink); } Ok(()) } @@ -320,17 +312,8 @@ fn append_copy(dst: &mut Document, src: &Document) -> Result { Ok(maps) } -fn tile_records( - dst: &mut Document, - src: &Document, - table: &Table, - selected: &[usize], - options: &MergeOptions, - data_dir: Option<&Path>, - doc_dir: Option<&Path>, - warnings: &mut Vec, - missing: &mut Vec, -) -> Result<()> { +fn tile_records(dst: &mut Document, src: &Document, table: &Table, selected: &[usize], sources: &Sources, sink: &mut Sink) -> Result<()> { + let options = sources.options; let (block, ids) = block_of(src); let (width, height) = src.page(0).map(|p| (p.width, p.height)).unwrap_or((612.0, 792.0)); let plan = tile(&TileInput { @@ -344,7 +327,7 @@ fn tile_records( count: selected.len(), }); if plan.one_per_page { - warnings.push("Only one record fits on the page, so each record has its own page.".into()); + sink.warnings.push("Only one record fits on the page, so each record has its own page.".into()); } if plan.pages > 1 { let parent = src.page(0).and_then(|p| p.parent); @@ -356,12 +339,11 @@ fn tile_records( } } } - let mut parent = false; for hit in plan.hits { let Some(&idx) = selected.get(hit.index) else { continue }; if hit.index == 0 && hit.page == 0 && hit.col == 0 && hit.row == 0 { let maps = identity_map(dst); - fill_at(dst, table, idx, options, &maps, data_dir, doc_dir, warnings, missing, &mut parent); + fill_at(dst, table, idx, sources, &maps, sink); continue; } let Some((dsi, _)) = dst.page_loc(hit.page) else { continue }; @@ -370,39 +352,37 @@ fn tile_records( let Some(it) = src.item(*id) else { continue }; copy_item(dst, src, it, SpreadRef::Doc(dsi), Vec2::new(hit.dx, hit.dy), &mut maps)?; } - fill_at(dst, table, idx, options, &maps, data_dir, doc_dir, warnings, missing, &mut parent); + fill_at(dst, table, idx, sources, &maps, sink); } Ok(()) } -fn fill_at( - doc: &mut Document, - table: &Table, - idx: usize, - options: &MergeOptions, - maps: &Maps, - data_dir: Option<&Path>, - doc_dir: Option<&Path>, - warnings: &mut Vec, - missing: &mut Vec, - parent: &mut bool, -) { +fn fill_at(doc: &mut Document, table: &Table, idx: usize, sources: &Sources, maps: &Maps, sink: &mut Sink) { let Some(row) = table.rows.get(idx) else { return }; - let report = fill_row(doc, &table.fields, row, (idx as u32).saturating_add(1), options, maps, data_dir, doc_dir); - absorb(report, warnings, missing, parent); + let report = fill_row(doc, &table.fields, row, (idx as u32).saturating_add(1), maps, sources); + sink.absorb(report); } -fn absorb(report: FillReport, warnings: &mut Vec, missing: &mut Vec, parent: &mut bool) { - for w in report.warnings { - if w == PARENT_WARNING { - if *parent { - continue; +/// Warnings and missing images collected across every record, with the parent-page warning kept to one. +struct Sink<'a> { + warnings: &'a mut Vec, + missing: &'a mut Vec, + parent: bool, +} + +impl Sink<'_> { + fn absorb(&mut self, report: FillReport) { + for w in report.warnings { + if w == PARENT_WARNING { + if self.parent { + continue; + } + self.parent = true; } - *parent = true; + self.warnings.push(w); } - warnings.push(w); + self.missing.extend(report.missing); } - missing.extend(report.missing); } fn block_of(doc: &Document) -> (Rect, Vec) { diff --git a/crates/engine/src/cmd/datamerge/parse.rs b/crates/engine/src/cmd/datamerge/parse.rs index c76c37f1e..36774733e 100644 --- a/crates/engine/src/cmd/datamerge/parse.rs +++ b/crates/engine/src/cmd/datamerge/parse.rs @@ -159,38 +159,7 @@ pub fn select_records(count: usize, records: &str, one: u32, range: &str, limit: let i = record_index(one, count)?; idx.push(i); } - "range" => { - if range.trim().is_empty() { - return Err("no records".into()); - } - for token in range.split(',') { - let token = token.trim(); - if token.is_empty() { - continue; - } - if let Some((a, b)) = token.split_once('-') { - let a = parse_record_no(a.trim())?; - let b = parse_record_no(b.trim())?; - if a > b { - return Err(format!("range {a}-{b} is reversed")); - } - let mut n = a; - while n <= b { - let i = record_index(n, count)?; - if seen.insert(i) { - idx.push(i); - } - n = n.saturating_add(1); - } - } else { - let n = parse_record_no(token)?; - let i = record_index(n, count)?; - if seen.insert(i) { - idx.push(i); - } - } - } - } + "range" => select_range(range, count, &mut idx, &mut seen)?, other => return Err(format!("unknown records value \"{other}\"")), } if let Some(limit) = limit { @@ -205,6 +174,38 @@ pub fn select_records(count: usize, records: &str, one: u32, range: &str, limit: Ok(idx) } +/// Expand a comma-separated list of record numbers and `a-b` ranges into `idx`, skipping repeats. +fn select_range(range: &str, count: usize, idx: &mut Vec, seen: &mut std::collections::HashSet) -> Result<(), String> { + if range.trim().is_empty() { + return Err("no records".into()); + } + for token in range.split(',') { + let token = token.trim(); + if token.is_empty() { + continue; + } + let (a, b) = match token.split_once('-') { + Some((a, b)) => (parse_record_no(a.trim())?, parse_record_no(b.trim())?), + None => { + let n = parse_record_no(token)?; + (n, n) + } + }; + if a > b { + return Err(format!("range {a}-{b} is reversed")); + } + let mut n = a; + while n <= b { + let i = record_index(n, count)?; + if seen.insert(i) { + idx.push(i); + } + n = n.saturating_add(1); + } + } + Ok(()) +} + fn parse_record_no(s: &str) -> Result { s.parse::().map_err(|_| format!("bad record \"{s}\"")) } From 6e28627ab503f69b9e6c42c166d3898081329c34 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sat, 24 Oct 2026 08:39:14 +0000 Subject: [PATCH 04/13] fix: import MetadataProvider in tests_vertical Co-Authored-By: Claude Sonnet 5.5 --- crates/fonts/src/tests_vertical.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/fonts/src/tests_vertical.rs b/crates/fonts/src/tests_vertical.rs index bf48ac426..a22dea0d7 100644 --- a/crates/fonts/src/tests_vertical.rs +++ b/crates/fonts/src/tests_vertical.rs @@ -3,6 +3,7 @@ //! The synthetic font (Source Sans 3 with `vhea`/`vmtx`, and `VORG` where asked) runs everywhere; //! the checks against Shippori Mincho's own tables run when built with craft-fonts. +use skrifa::MetadataProvider; use skrifa::raw::TableProvider; use super::{Arc, FontBytes, FontDb, FontFace, FontSource, GlyphId, LocationRef, Size, make_face}; From 9897e6e22ec9f1d6a19ea45f54066292f05bba58 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sat, 24 Oct 2026 08:48:09 +0000 Subject: [PATCH 05/13] fix: complete explicit imports in test modules Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/file/eps_place_tests.rs | 2 +- crates/engine/src/tests_ime.rs | 2 +- crates/engine/src/tests_table.rs | 2 +- crates/ui-egui/src/guide_selection_tests.rs | 5 ++++- crates/ui-egui/src/panel_docking_tests.rs | 2 +- crates/ui-egui/src/parent_canvas_tests.rs | 2 +- 6 files changed, 9 insertions(+), 6 deletions(-) diff --git a/crates/engine/src/cmd/file/eps_place_tests.rs b/crates/engine/src/cmd/file/eps_place_tests.rs index 740e2b56a..42d3dd774 100644 --- a/crates/engine/src/cmd/file/eps_place_tests.rs +++ b/crates/engine/src/cmd/file/eps_place_tests.rs @@ -3,7 +3,7 @@ //! These helpers write real files, so they live in their own `_tests.rs` file: the //! file-system guard in `tests_file_guard.rs` checks `src` files but exempts test ones. -use super::{Arc, AssetId, ItemId, Session, base64_encode, json}; +use super::{Arc, AssetId, Content, ItemId, Session, base64_encode, json}; pub(crate) fn tiff_eps() -> Vec { let mut tiff = b"II".to_vec(); diff --git a/crates/engine/src/tests_ime.rs b/crates/engine/src/tests_ime.rs index def3b6cc0..4e4e4ed14 100644 --- a/crates/engine/src/tests_ime.rs +++ b/crates/engine/src/tests_ime.rs @@ -4,7 +4,7 @@ use designcraft_tools::{Mods, PointerEvent, PointerKind, ToolKey}; use serde_json::json; -use super::{Arc, ViewInfo}; +use super::{Arc, Session, ViewInfo}; fn session() -> Session { let mut s = Session::new(); diff --git a/crates/engine/src/tests_table.rs b/crates/engine/src/tests_table.rs index 28387f19c..acb0d2352 100644 --- a/crates/engine/src/tests_table.rs +++ b/crates/engine/src/tests_table.rs @@ -1,6 +1,6 @@ use serde_json::json; -use super::doc; +use super::{Session, doc}; fn session_with_frame() -> (Session, u64, u64) { let mut s = Session::new(); diff --git a/crates/ui-egui/src/guide_selection_tests.rs b/crates/ui-egui/src/guide_selection_tests.rs index 9d8dc1add..626a7b826 100644 --- a/crates/ui-egui/src/guide_selection_tests.rs +++ b/crates/ui-egui/src/guide_selection_tests.rs @@ -4,7 +4,10 @@ use egui::{Event, Modifiers, PointerButton}; use egui_kittest::Harness; -use super::{CanvasLayout, Mods, Overlay, Point, Pos2, RULER, SpreadRef, ToolKey, Xf, json, pos2, vec2}; +use super::{ + CanvasLayout, DesignApp, GuideDrag, GuidePress, Mods, Overlay, Point, Pos2, RULER, SpreadRef, ToolKey, Xf, guide_drag_delta, guide_spot, json, + moving_guides, pos2, vec2, +}; fn harness(app: DesignApp) -> Harness<'static, DesignApp> { let mut h = Harness::builder().with_size(vec2(1440.0, 900.0)).build_ui_state( diff --git a/crates/ui-egui/src/panel_docking_tests.rs b/crates/ui-egui/src/panel_docking_tests.rs index 9be8afd56..fefc42617 100644 --- a/crates/ui-egui/src/panel_docking_tests.rs +++ b/crates/ui-egui/src/panel_docking_tests.rs @@ -1,4 +1,4 @@ -use super::{Layout, Node, cancel_gesture, json, valid}; +use super::{DesignApp, Layout, Node, cancel_gesture, json, valid}; use egui::{Event, Modifiers, PointerButton, Pos2, vec2}; use egui_kittest::{Harness, kittest::Queryable}; diff --git a/crates/ui-egui/src/parent_canvas_tests.rs b/crates/ui-egui/src/parent_canvas_tests.rs index 6a84519af..b2e44b8ea 100644 --- a/crates/ui-egui/src/parent_canvas_tests.rs +++ b/crates/ui-egui/src/parent_canvas_tests.rs @@ -1,4 +1,4 @@ -use super::{CanvasLayout, Color32, Point, Pos2, Rect, View, Xf, json, render_texture, vec2}; +use super::{CanvasLayout, Color32, DesignApp, Point, Pos2, Rect, View, Xf, json, render_texture, vec2}; fn app_with_document_object() -> DesignApp { let mut app = DesignApp::new(designcraft_engine::Session::new(), crate::Services::default()); From 30fbbb8c042d6c4add174ecf038476b0cb29375d Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sat, 24 Oct 2026 08:50:03 +0000 Subject: [PATCH 06/13] fix(engine): stop range selection after the last record number Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/datamerge/parse.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/datamerge/parse.rs b/crates/engine/src/cmd/datamerge/parse.rs index 36774733e..3e7d92eb6 100644 --- a/crates/engine/src/cmd/datamerge/parse.rs +++ b/crates/engine/src/cmd/datamerge/parse.rs @@ -200,7 +200,10 @@ fn select_range(range: &str, count: usize, idx: &mut Vec, seen: &mut std: if seen.insert(i) { idx.push(i); } - n = n.saturating_add(1); + if n == b { + break; + } + n += 1; } } Ok(()) From 88dd71cc51df181ceeef87596c48bb3dd0fe75fe Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sat, 24 Oct 2026 08:50:41 +0000 Subject: [PATCH 07/13] refactor(engine): split find change and used fonts (Sonar S3776) Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/find.rs | 44 ++++++++++++++++---------- crates/engine/src/cmd/fonts.rs | 57 ++++++++++++++++++---------------- 2 files changed, 58 insertions(+), 43 deletions(-) diff --git a/crates/engine/src/cmd/find.rs b/crates/engine/src/cmd/find.rs index a40e8da51..a0afcfbb6 100644 --- a/crates/engine/src/cmd/find.rs +++ b/crates/engine/src/cmd/find.rs @@ -323,18 +323,38 @@ fn replacement(p: &Value) -> String { if bool_or(p, "grep", false) { translate_replacement(c) } else { metachars(c) } } +/// Repair byte validity of a text selection once against the final story, without remapping valid offsets. +fn repair_selection(t: &mut designcraft_doc::TextSel, st: &designcraft_doc::Story) { + let range = t.range(); + let start = floor_char_boundary(&st.text, range.start); + let mut end = range.end.min(st.len()); + if t.is_caret() { + end = start; + } else if !st.text.is_char_boundary(end) { + let floor = floor_char_boundary(&st.text, end); + end = st.text.get(floor..).and_then(|rest| rest.chars().next()).map_or(floor, |ch| floor + ch.len_utf8()); + } + (t.anchor, t.focus) = if t.anchor <= t.focus { (start, end) } else { (end, start) }; +} + +/// The character overrides a Change applies to every replacement, from its `attrs` object. +fn char_overrides(attrs: Option<&Value>) -> Result { + let mut over = designcraft_doc::CharAttrs::default(); + if let Some(o) = attrs.and_then(Value::as_object) { + for (k, v) in o { + over.set_json(k, v).map_err(|e| bad("find.change", e))?; + } + } + Ok(over) +} + fn change(s: &mut Session, p: &Value) -> Result { let re = build(p)?; let grep = bool_or(p, "grep", false); let rep = replacement(p); let first_only = bool_or(p, "first", false); let attrs = p.get("attrs").cloned(); - let mut over = designcraft_doc::CharAttrs::default(); - if let Some(o) = attrs.as_ref().and_then(Value::as_object) { - for (k, v) in o { - over.set_json(k, v).map_err(|e| bad("find.change", e))?; - } - } + let over = char_overrides(attrs.as_ref())?; super::text::validate_char_fill(&s.doc()?.doc, &over, "find.change")?; let targets = scope(s, p)?.targets; s.edit(|d, sel| { @@ -361,17 +381,7 @@ fn change(s: &mut Session, p: &Value) -> Result { count += 1; } if let Some(t) = sel.text.as_mut().filter(|t| t.story == sid && t.cell == cell) { - // Repair byte validity once against the final story, without remapping valid offsets. - let range = t.range(); - let start = floor_char_boundary(&st.text, range.start); - let mut end = range.end.min(st.len()); - if t.is_caret() { - end = start; - } else if !st.text.is_char_boundary(end) { - let floor = floor_char_boundary(&st.text, end); - end = st.text.get(floor..).and_then(|rest| rest.chars().next()).map_or(floor, |ch| floor + ch.len_utf8()); - } - (t.anchor, t.focus) = if t.anchor <= t.focus { (start, end) } else { (end, start) }; + repair_selection(t, st); } // `first` is the first match of the whole search, not of each story. if first_only && count > 0 { diff --git a/crates/engine/src/cmd/fonts.rs b/crates/engine/src/cmd/fonts.rs index b1577400c..ee80eb914 100644 --- a/crates/engine/src/cmd/fonts.rs +++ b/crates/engine/src/cmd/fonts.rs @@ -81,36 +81,41 @@ fn list(d: &Document) -> Vec { used_fonts(d).into_iter().map(|font| json!(font)).collect() } -pub(super) fn used_fonts(d: &Document) -> Vec { - let db = designcraft_fonts::FontDb::global().scoped(d.font_scope); - let mut used: BTreeMap<(String, String), (usize, std::collections::BTreeSet)> = BTreeMap::new(); - for_each_story(d, &mut |st| { - let ranges = st.para_ranges(); - for (pi, r) in ranges.iter().enumerate() { - let (_, base) = d.styles.resolve_para(&st.paras[pi]); - for (rr, fmt) in st.runs() { - let n = st.text[rr.start.max(r.start)..rr.end.min(r.end).max(rr.start.max(r.start))].chars().count(); - if n == 0 && !(r.is_empty() && rr.contains(&r.start)) { - continue; - } - let p = d.styles.resolve_char(&base, fmt); - if let Some(f) = d.styles.composite_font(&p.font_family) { - let text = st.text.get(rr.start.max(r.start)..rr.end.min(r.end).max(rr.start.max(r.start))).unwrap_or(""); - for c in text.chars() { - if let Some(e) = f.entry(c) { - let entry = used.entry((e.family.clone(), e.style.clone())).or_default(); - entry.0 += 1; - entry.1.insert(c); - } +type FontTally = BTreeMap<(String, String), (usize, std::collections::BTreeSet)>; + +/// Count the characters of every run of one story against the font each resolves to. +fn tally_story(d: &Document, st: &Story, used: &mut FontTally) { + let ranges = st.para_ranges(); + for (pi, r) in ranges.iter().enumerate() { + let (_, base) = d.styles.resolve_para(&st.paras[pi]); + for (rr, fmt) in st.runs() { + let n = st.text[rr.start.max(r.start)..rr.end.min(r.end).max(rr.start.max(r.start))].chars().count(); + if n == 0 && !(r.is_empty() && rr.contains(&r.start)) { + continue; + } + let p = d.styles.resolve_char(&base, fmt); + if let Some(f) = d.styles.composite_font(&p.font_family) { + let text = st.text.get(rr.start.max(r.start)..rr.end.min(r.end).max(rr.start.max(r.start))).unwrap_or(""); + for c in text.chars() { + if let Some(e) = f.entry(c) { + let entry = used.entry((e.family.clone(), e.style.clone())).or_default(); + entry.0 += 1; + entry.1.insert(c); } - } else { - let entry = used.entry((p.font_family, p.font_style)).or_default(); - entry.0 += n; - entry.1.extend(st.text.get(rr.start.max(r.start)..rr.end.min(r.end).max(rr.start.max(r.start))).unwrap_or("").chars()); } + } else { + let entry = used.entry((p.font_family, p.font_style)).or_default(); + entry.0 += n; + entry.1.extend(st.text.get(rr.start.max(r.start)..rr.end.min(r.end).max(rr.start.max(r.start))).unwrap_or("").chars()); } } - }); + } +} + +pub(super) fn used_fonts(d: &Document) -> Vec { + let db = designcraft_fonts::FontDb::global().scoped(d.font_scope); + let mut used = FontTally::new(); + for_each_story(d, &mut |st| tally_story(d, st, &mut used)); let mut out: Vec = used .into_iter() .map(|((family, style), (n, characters))| { From 71d938613852f943c8321ebd846c4fde0d695f18 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sat, 24 Oct 2026 08:36:06 +0000 Subject: [PATCH 08/13] refactor(idml): split graphic part and spread import helpers (Sonar S107, S3776) Co-Authored-By: Claude Sonnet 5.5 --- crates/idml/src/export.rs | 113 +++++++++++++++++++++----------------- crates/idml/src/import.rs | 64 ++++++++++++--------- 2 files changed, 100 insertions(+), 77 deletions(-) diff --git a/crates/idml/src/export.rs b/crates/idml/src/export.rs index c174c1889..b95d61a8d 100644 --- a/crates/idml/src/export.rs +++ b/crates/idml/src/export.rs @@ -431,6 +431,45 @@ impl<'a> Ex<'a> { ) } + /// A `Gradient` element; stop colours missing from the swatches become anonymous colours. + fn gradient_el( + &mut self, + s: &designcraft_color::Swatch, + gradient: &designcraft_color::Gradient, + colors: &mut Vec, + by_value: &mut Vec<(Color, String)>, + ) -> El { + let id = self.sw(&s.name); + let mut g = El::new("Gradient") + .attr("Self", &id) + .attr("Type", if gradient.kind == GradientKind::Radial { "Radial" } else { "Linear" }) + .attr("Name", &s.name) + .attr("ColorEditable", "true") + .attr("ColorRemovable", "true") + .attr("Visible", "true") + .attr("SwatchCreatorID", "7937"); + for (i, stop) in gradient.stops.iter().enumerate() { + let cid = match by_value.iter().find(|(c, _)| *c == stop.color) { + Some((_, id)) => id.clone(), + None => { + let cid = format!("Color/{}", self.fresh()); + colors.push(color_el(&cid, "$ID/", "Process", &stop.color, "Normal", (true, true, false))); + by_value.push((stop.color, cid.clone())); + cid + } + }; + let mut e = El::new("GradientStop") + .attr("Self", format!("{}GradientStop{i}", id.replace(['/', ' '], "_"))) + .attr("StopColor", cid) + .attr("Location", pct(stop.offset as f64)); + if i > 0 { + e.set("Midpoint", pct(gradient.stops[i - 1].midpoint as f64)); + } + g.push(e); + } + g + } + // ---------- Graphic.xml ---------- fn graphic_part(&mut self) -> Vec { @@ -440,39 +479,21 @@ impl<'a> Ex<'a> { let mut tints: Vec = Vec::new(); let mut gradients: Vec = Vec::new(); let mut inks: Vec = Vec::new(); - let color_el = |self_id: &str, name: &str, model: &str, c: &Color, over: &str, editable: bool, removable: bool, visible: bool| { - let (space, val) = color_value(c); - El::new("Color") - .attr("Self", self_id) - .attr("Model", model) - .attr("Space", space) - .attr("ColorValue", val) - .attr("ColorOverride", over) - .attr("AlternateSpace", "NoAlternateColor") - .attr("AlternateColorValue", "") - .attr("Name", name) - .attr("ColorEditable", bool_s(editable)) - .attr("ColorRemovable", bool_s(removable)) - .attr("Visible", bool_s(visible)) - .attr("SwatchCreatorID", "7937") - }; // Specials (always present). - colors.push(color_el("Color/Black", "Black", "Process", &Color::cmyk(0.0, 0.0, 0.0, 1.0), "Specialblack", false, false, true)); + colors.push(color_el("Color/Black", "Black", "Process", &Color::cmyk(0.0, 0.0, 0.0, 1.0), "Specialblack", (false, false, true))); let paper = match d.swatch(swatch::PAPER).map(|s| &s.value) { Some(SwatchValue::Paper { color }) => *color, _ => Color::cmyk(0.0, 0.0, 0.0, 0.0), }; let paper = if paper == Color::WHITE { Color::cmyk(0.0, 0.0, 0.0, 0.0) } else { paper }; - colors.push(color_el("Color/Paper", "Paper", "Process", &paper, "Specialpaper", true, false, true)); + colors.push(color_el("Color/Paper", "Paper", "Process", &paper, "Specialpaper", (true, false, true))); colors.push(color_el( "Color/Registration", "Registration", "Registration", &Color::cmyk(1.0, 1.0, 1.0, 1.0), "Specialregistration", - false, - false, - true, + (false, false, true), )); for (i, ink) in [("Process Cyan", 75, 0.61), ("Process Magenta", 15, 0.76), ("Process Yellow", 0, 0.16), ("Process Black", 45, 1.7)].iter().enumerate() @@ -500,7 +521,7 @@ impl<'a> Ex<'a> { match &s.value { SwatchValue::Color { color, color_type } => { let model = if *color_type == ColorType::Spot { "Spot" } else { "Process" }; - colors.push(color_el(&id, &s.name, model, color, "Normal", true, true, true)); + colors.push(color_el(&id, &s.name, model, color, "Normal", (true, true, true))); by_value.push((*color, id.clone())); if *color_type == ColorType::Spot { let mut ink = El::new("Ink"); @@ -542,35 +563,7 @@ impl<'a> Ex<'a> { } for s in &d.swatches { let SwatchValue::Gradient { gradient } = &s.value else { continue }; - let id = self.sw(&s.name); - let mut g = El::new("Gradient") - .attr("Self", &id) - .attr("Type", if gradient.kind == GradientKind::Radial { "Radial" } else { "Linear" }) - .attr("Name", &s.name) - .attr("ColorEditable", "true") - .attr("ColorRemovable", "true") - .attr("Visible", "true") - .attr("SwatchCreatorID", "7937"); - for (i, stop) in gradient.stops.iter().enumerate() { - let cid = match by_value.iter().find(|(c, _)| *c == stop.color) { - Some((_, id)) => id.clone(), - None => { - let cid = format!("Color/{}", self.fresh()); - colors.push(color_el(&cid, "$ID/", "Process", &stop.color, "Normal", true, true, false)); - by_value.push((stop.color, cid.clone())); - cid - } - }; - let mut e = El::new("GradientStop") - .attr("Self", format!("{}GradientStop{i}", id.replace(['/', ' '], "_"))) - .attr("StopColor", cid) - .attr("Location", pct(stop.offset as f64)); - if i > 0 { - e.set("Midpoint", pct(gradient.stops[i - 1].midpoint as f64)); - } - g.push(e); - } - gradients.push(g); + gradients.push(self.gradient_el(s, gradient, &mut colors, &mut by_value)); } for c in colors { root.push(c); @@ -3213,6 +3206,24 @@ fn layer_el(l: &designcraft_doc::Layer) -> El { with_props(el, vec![color]) } +/// A `Color` element. `flags` is (editable, removable, visible). +fn color_el(self_id: &str, name: &str, model: &str, c: &Color, over: &str, (editable, removable, visible): (bool, bool, bool)) -> El { + let (space, val) = color_value(c); + El::new("Color") + .attr("Self", self_id) + .attr("Model", model) + .attr("Space", space) + .attr("ColorValue", val) + .attr("ColorOverride", over) + .attr("AlternateSpace", "NoAlternateColor") + .attr("AlternateColorValue", "") + .attr("Name", name) + .attr("ColorEditable", bool_s(editable)) + .attr("ColorRemovable", bool_s(removable)) + .attr("Visible", bool_s(visible)) + .attr("SwatchCreatorID", "7937") +} + /// A text variable definition with its type's settings element. fn text_variable_el(v: &designcraft_doc::vars::TextVariable) -> El { use designcraft_doc::vars::{Use, VarKind}; diff --git a/crates/idml/src/import.rs b/crates/idml/src/import.rs index fed9f49b9..112622c89 100644 --- a/crates/idml/src/import.rs +++ b/crates/idml/src/import.rs @@ -2494,10 +2494,9 @@ impl<'r> Importer<'r> { .collect() } - fn spread(&mut self, e: &El, id: SpreadId, master: bool) -> Spread { - let facing = self.settings.facing_pages; - // Page rectangles in IDML spread coordinates. - let mut pages: Vec<(Rect, &El)> = Vec::new(); + /// The page rectangles of a spread element, in IDML spread coordinates. + fn page_rects<'a>(&self, e: &'a El) -> Vec<(Rect, &'a El)> { + let mut pages: Vec<(Rect, &'a El)> = Vec::new(); for p in e.find_all("Page") { let gb = nums(p.get("GeometricBounds").unwrap_or("")); let (t, l, bt, r) = @@ -2507,6 +2506,40 @@ impl<'r> Importer<'r> { let c = xf * Point::new(r, bt); pages.push((Rect::from_points(a, c), p)); } + pages + } + + /// A page's margins and columns, and whether they are local (differ from its parent page's). + fn page_margins(&self, p: &El, r: &Rect, master: bool, parent: Option, side: PageSide) -> (Margins, Columns, bool) { + // A document page's margins are local when they differ from its parent page's; a + // page without its own takes the parent page's. + let from_parent = if master { + None + } else { + parent + .and_then(|id| self.parents.iter().find(|sp| sp.id == id)) + .and_then(|sp| sp.pages.get(sp.parent_page_for_side(side)?)) + .map(|q| (q.margins, q.columns.clone())) + }; + let (margins, columns) = match (p.find("MarginPreference"), &from_parent) { + (Some(m), _) => { + let (margins, columns) = margins_of(m); + (margins, fit_columns(columns, &margins, r.width())) + } + (None, Some(own)) => own.clone(), + (None, None) => { + let (margins, columns) = self.default_margins.clone().unwrap_or((Margins::uniform(36.0), Columns::default())); + (margins, fit_columns(columns, &margins, r.width())) + } + }; + let local_margins = from_parent.is_some_and(|(m, c)| m != margins || c != columns); + (margins, columns, local_margins) + } + + fn spread(&mut self, e: &El, id: SpreadId, master: bool) -> Spread { + let facing = self.settings.facing_pages; + // Page rectangles in IDML spread coordinates. + let pages = self.page_rects(e); let min_x = pages.iter().map(|(r, _)| r.x0).fold(f64::INFINITY, f64::min); let min_y = pages.iter().map(|(r, _)| r.y0).fold(f64::INFINITY, f64::min); let (min_x, min_y) = if min_x.is_finite() { (min_x, min_y) } else { (0.0, 0.0) }; @@ -2533,28 +2566,7 @@ impl<'r> Importer<'r> { PageSide::Right }; let parent = p.get("AppliedMaster").and_then(|m| self.parent_ids.get(m)).copied(); - // A document page's margins are local when they differ from its parent page's; a - // page without its own takes the parent page's. - let from_parent = if master { - None - } else { - parent - .and_then(|id| self.parents.iter().find(|sp| sp.id == id)) - .and_then(|sp| sp.pages.get(sp.parent_page_for_side(side)?)) - .map(|q| (q.margins, q.columns.clone())) - }; - let (margins, columns) = match (p.find("MarginPreference"), &from_parent) { - (Some(m), _) => { - let (margins, columns) = margins_of(m); - (margins, fit_columns(columns, &margins, r.width())) - } - (None, Some(own)) => own.clone(), - (None, None) => { - let (margins, columns) = self.default_margins.clone().unwrap_or((Margins::uniform(36.0), Columns::default())); - (margins, fit_columns(columns, &margins, r.width())) - } - }; - let local_margins = from_parent.is_some_and(|(m, c)| m != margins || c != columns); + let (margins, columns, local_margins) = self.page_margins(p, r, master, parent, side); out_pages.push(Page { id: pid, width: r.width(), From 140681429f15331045673162df0f09c048bb294d Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sat, 24 Oct 2026 08:52:37 +0000 Subject: [PATCH 09/13] refactor: split complex functions in epub table and pdf frame text (Sonar S3776) Co-Authored-By: Claude Sonnet 5.5 --- crates/epub/src/lib.rs | 25 ++++++++++++-------- crates/pdf/src/text.rs | 53 +++++++++++++++++++++++------------------- 2 files changed, 44 insertions(+), 34 deletions(-) diff --git a/crates/epub/src/lib.rs b/crates/epub/src/lib.rs index 1df759608..0ecc4e1a8 100644 --- a/crates/epub/src/lib.rs +++ b/crates/epub/src/lib.rs @@ -967,6 +967,20 @@ impl<'a, 'f> StoryWriter<'a, 'f> { out } + /// One table cell with its row and column spans, as `th` or `td`. + fn table_cell(&mut self, text: &Story, cell_tag: &str, (rs, cs): (usize, usize), depth: usize, out: &mut String) { + let _ = write!(out, "<{cell_tag}"); + if rs > 1 { + let _ = write!(out, " rowspan=\"{rs}\""); + } + if cs > 1 { + let _ = write!(out, " colspan=\"{cs}\""); + } + out.push('>'); + self.paragraphs(text, depth, "", out); + let _ = writeln!(out, ""); + } + /// A table: header rows as `thead`, footer rows as `tfoot`, merged cells as row and column /// spans (kept within their row group). fn table(&mut self, t: &Table, depth: usize, out: &mut String) { @@ -1000,16 +1014,7 @@ impl<'a, 'f> StoryWriter<'a, 'f> { } } } - let _ = write!(out, "<{cell_tag}"); - if rs > 1 { - let _ = write!(out, " rowspan=\"{rs}\""); - } - if cs > 1 { - let _ = write!(out, " colspan=\"{cs}\""); - } - out.push('>'); - self.paragraphs(&cell.text, depth, "", out); - let _ = writeln!(out, ""); + self.table_cell(&cell.text, cell_tag, (rs, cs), depth, out); } out.push_str("\n"); } diff --git a/crates/pdf/src/text.rs b/crates/pdf/src/text.rs index 1aaf435d9..b68a70571 100644 --- a/crates/pdf/src/text.rs +++ b/crates/pdf/src/text.rs @@ -233,6 +233,34 @@ impl Exporter<'_> { } } + /// One line's glyphs as tagged runs; a glyph that is turned or slanted gets a transform of its own. + fn line_glyphs(&mut self, s: &mut Surface, cs: &ComposedStory, ft: &FrameText, l: &designcraft_compose::Line, story: &str) { + let gs = &l.glyphs; + let mut i = 0; + while i < gs.len() { + let g = &gs[i]; + if !g.visible || g.sx <= 0.0 || g.sy <= 0.0 { + i += 1; + continue; + } + // Vertical type: an upright glyph turns back a quarter about its em box centre. + // Shatai slants each glyph about its own em box centre first. + let turn = if ft.vertical { g.vertical_xf(l.baseline) } else { None }; + let shatai = cs.styles.get(g.style as usize).and_then(|st| st.shatai_xf(g, l.baseline)); + if turn.is_some() || shatai.is_some() { + let m = turn.unwrap_or(Affine::IDENTITY) * shatai.unwrap_or(Affine::IDENTITY); + s.push_transform(&crate::export::tf(m)); + self.tagged_run(s, cs, &gs[i..i + 1], l.baseline, story); + s.pop(); + i += 1; + continue; + } + let j = run_end(ft, gs, i); + self.tagged_run(s, cs, &gs[i..j], l.baseline, story); + i = j; + } + } + pub(crate) fn frame_text(&mut self, s: &mut Surface, cs: &ComposedStory, ft: &FrameText, story: &str) { // Paragraph shading and rules under the text. self.as_artifact(s, |me, s| { @@ -260,30 +288,7 @@ impl Exporter<'_> { }); } for l in &ft.lines { - let gs = &l.glyphs; - let mut i = 0; - while i < gs.len() { - let g = &gs[i]; - if !g.visible || g.sx <= 0.0 || g.sy <= 0.0 { - i += 1; - continue; - } - // Vertical type: an upright glyph turns back a quarter about its em box centre. - // Shatai slants each glyph about its own em box centre first. - let turn = if ft.vertical { g.vertical_xf(l.baseline) } else { None }; - let shatai = cs.styles.get(g.style as usize).and_then(|st| st.shatai_xf(g, l.baseline)); - if turn.is_some() || shatai.is_some() { - let m = turn.unwrap_or(Affine::IDENTITY) * shatai.unwrap_or(Affine::IDENTITY); - s.push_transform(&crate::export::tf(m)); - self.tagged_run(s, cs, &gs[i..i + 1], l.baseline, story); - s.pop(); - i += 1; - continue; - } - let j = run_end(ft, gs, i); - self.tagged_run(s, cs, &gs[i..j], l.baseline, story); - i = j; - } + self.line_glyphs(s, cs, ft, l, story); } self.as_artifact(s, |me, s| { for (rule, r) in text_rules(cs, ft, false) { From 04f0c55afb64076d693d29d3efd855469cd6e35b Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sat, 24 Oct 2026 08:52:41 +0000 Subject: [PATCH 10/13] refactor(idml): extract graphics and table cell import helpers (Sonar S3776) Co-Authored-By: Claude Sonnet 5.5 --- crates/idml/src/import.rs | 153 +++++++++++++++++++++----------------- 1 file changed, 83 insertions(+), 70 deletions(-) diff --git a/crates/idml/src/import.rs b/crates/idml/src/import.rs index fed9f49b9..76e62c0be 100644 --- a/crates/idml/src/import.rs +++ b/crates/idml/src/import.rs @@ -916,6 +916,45 @@ impl<'r> Importer<'r> { Some((c, ty)) } + /// Custom stroke styles. + fn read_stroke_styles(&mut self, top: &[El]) { + for e in top.iter().filter(|e| matches!(e.local(), "StripedStrokeStyle" | "DashedStrokeStyle" | "DottedStrokeStyle")) { + let (Some(id), Some(name)) = (e.get("Self"), e.get("Name")) else { continue }; + let kind = match e.local() { + "StripedStrokeStyle" => designcraft_doc::StrokeType::Stripes { + bands: nums(e.get("StripeArray").unwrap_or("")) + .as_chunks::<2>() + .0 + .iter() + .map(|c| (c[0] / 100.0, (c[1] - c[0]) / 100.0)) + .collect(), + }, + "DashedStrokeStyle" => designcraft_doc::StrokeType::Dashed { pattern: nums(e.get("DashArray").unwrap_or("")) }, + _ => designcraft_doc::StrokeType::Dotted, + }; + let _ = id; + self.stroke_styles.push(designcraft_doc::StrokeStyleDef { name: name.to_string(), kind }); + } + } + + /// Ink Manager settings: inks converted to process and ink aliases. + fn read_inks(&mut self, top: &[El]) { + for e in top.iter().filter(|e| e.local() == "Ink") { + let Some(name) = e + .get("Name") + .filter(|n| !n.starts_with("$ID/") && !["Process Cyan", "Process Magenta", "Process Yellow", "Process Black"].contains(n)) + else { + continue; + }; + if e.get("ConvertToProcess") == Some("true") { + self.inks.to_process.push(name.to_string()); + } + if let Some(a) = e.get("AliasInkName").filter(|a| !a.is_empty() && *a != name && !a.starts_with("$ID/")) { + self.inks.aliases.push((name.to_string(), a.to_string())); + } + } + } + fn graphics(&mut self, top: &[El]) { let mut order: Vec<(String, Swatch)> = Vec::new(); // Colours. @@ -959,39 +998,8 @@ impl<'r> Importer<'r> { Swatch { name, value: SwatchValue::Tint { base, tint: (t / 100.0) as f32 }, locked: false, named: true, hidden: false }, )); } - // Custom stroke styles. - for e in top.iter().filter(|e| matches!(e.local(), "StripedStrokeStyle" | "DashedStrokeStyle" | "DottedStrokeStyle")) { - let (Some(id), Some(name)) = (e.get("Self"), e.get("Name")) else { continue }; - let kind = match e.local() { - "StripedStrokeStyle" => designcraft_doc::StrokeType::Stripes { - bands: nums(e.get("StripeArray").unwrap_or("")) - .as_chunks::<2>() - .0 - .iter() - .map(|c| (c[0] / 100.0, (c[1] - c[0]) / 100.0)) - .collect(), - }, - "DashedStrokeStyle" => designcraft_doc::StrokeType::Dashed { pattern: nums(e.get("DashArray").unwrap_or("")) }, - _ => designcraft_doc::StrokeType::Dotted, - }; - let _ = id; - self.stroke_styles.push(designcraft_doc::StrokeStyleDef { name: name.to_string(), kind }); - } - // Ink Manager. - for e in top.iter().filter(|e| e.local() == "Ink") { - let Some(name) = e - .get("Name") - .filter(|n| !n.starts_with("$ID/") && !["Process Cyan", "Process Magenta", "Process Yellow", "Process Black"].contains(n)) - else { - continue; - }; - if e.get("ConvertToProcess") == Some("true") { - self.inks.to_process.push(name.to_string()); - } - if let Some(a) = e.get("AliasInkName").filter(|a| !a.is_empty() && *a != name && !a.starts_with("$ID/")) { - self.inks.aliases.push((name.to_string(), a.to_string())); - } - } + self.read_stroke_styles(top); + self.read_inks(top); for e in top.iter().filter(|e| e.local() == "Gradient") { let Some(id) = e.get("Self") else { continue }; let mut stops: Vec = Vec::new(); @@ -2213,43 +2221,7 @@ impl<'r> Importer<'r> { cell.style = if cell_style == designcraft_doc::NO_CELL_STYLE { String::new() } else { cell_style }; let default_para = cell.text.paras.first().cloned().unwrap_or_default(); cell.text = self.story_with_para(StoryId(0), ce, default_para); - if let Some(fc) = ce.get("FillColor") { - cell.fill = self.swatch_ref(fc); - } - if let Some(tint) = tint(ce.num("FillTint")) { - cell.fill_tint = tint; - } - for (i, k) in ["TopInset", "LeftInset", "BottomInset", "RightInset"].iter().enumerate() { - if let Some(v) = ce.num(k) { - cell.insets[i] = v.max(0.0); - } - } - if let Some(vj) = ce.get("VerticalJustification") { - cell.vj = names::vj_in(vj); - } - if let Some(fb) = ce.get("FirstBaselineOffset") { - cell.first_baseline = names::first_baseline_in(fb); - } - if let Some(min) = ce.num("MinimumFirstBaselineOffset").filter(|v| v.is_finite()) { - cell.first_baseline_min = min.clamp(0.0, 8640.0); - } - cell.rotation = ce.num("RotationAngle").unwrap_or(cell.rotation); - if cell.rotation != 0.0 { - self.rotated_cells += 1; - } - for (i, side) in ["TopEdge", "LeftEdge", "BottomEdge", "RightEdge"].iter().enumerate() { - let attrs = self.cell_stroke_attrs(ce, side); - attrs.apply_to(&mut cell.strokes[i]); - cell.stroke_defined[i] |= !attrs.is_empty(); - let priority = ce.prop(&format!("{side}StrokePriority")).and_then(|v| v.trim().parse::().ok()).unwrap_or(0); - cell.stroke_priorities[i] = priority; - // Preserve style-defined edges and #102's positive-priority local overrides. - cell.border_overrides[i] |= priority > 0; - // No table or table style border is given anywhere, so the built-in 1 pt default must not - // draw over an outer edge the cell explicitly hides (weight 0 or the None swatch), as InDesign draws nothing. - let hidden = attrs.weight == Some(0.0) || attrs.color.as_deref() == Some(swatch::NONE); - cell.border_overrides[i] |= border_unset && hidden; - } + self.read_cell_attrs(&mut cell, ce, border_unset); if let Some(slot) = t.cell_mut(r, c) { *slot = cell; } @@ -2263,6 +2235,47 @@ impl<'r> Importer<'r> { t } + /// A cell's local fill, insets, alignment, rotation and edge strokes. + fn read_cell_attrs(&mut self, cell: &mut designcraft_doc::Cell, ce: &El, border_unset: bool) { + if let Some(fc) = ce.get("FillColor") { + cell.fill = self.swatch_ref(fc); + } + if let Some(tint) = tint(ce.num("FillTint")) { + cell.fill_tint = tint; + } + for (i, k) in ["TopInset", "LeftInset", "BottomInset", "RightInset"].iter().enumerate() { + if let Some(v) = ce.num(k) { + cell.insets[i] = v.max(0.0); + } + } + if let Some(vj) = ce.get("VerticalJustification") { + cell.vj = names::vj_in(vj); + } + if let Some(fb) = ce.get("FirstBaselineOffset") { + cell.first_baseline = names::first_baseline_in(fb); + } + if let Some(min) = ce.num("MinimumFirstBaselineOffset").filter(|v| v.is_finite()) { + cell.first_baseline_min = min.clamp(0.0, 8640.0); + } + cell.rotation = ce.num("RotationAngle").unwrap_or(cell.rotation); + if cell.rotation != 0.0 { + self.rotated_cells += 1; + } + for (i, side) in ["TopEdge", "LeftEdge", "BottomEdge", "RightEdge"].iter().enumerate() { + let attrs = self.cell_stroke_attrs(ce, side); + attrs.apply_to(&mut cell.strokes[i]); + cell.stroke_defined[i] |= !attrs.is_empty(); + let priority = ce.prop(&format!("{side}StrokePriority")).and_then(|v| v.trim().parse::().ok()).unwrap_or(0); + cell.stroke_priorities[i] = priority; + // Preserve style-defined edges and #102's positive-priority local overrides. + cell.border_overrides[i] |= priority > 0; + // No table or table style border is given anywhere, so the built-in 1 pt default must not + // draw over an outer edge the cell explicitly hides (weight 0 or the None swatch), as InDesign draws nothing. + let hidden = attrs.weight == Some(0.0) || attrs.color.as_deref() == Some(swatch::NONE); + cell.border_overrides[i] |= border_unset && hidden; + } + } + /// The format of the text in a `CharacterStyleRange`: its style, and its local attributes over /// the enclosing paragraph range's, inside a tracked change if `outer` (the enclosing format) is. fn range_format(&mut self, c: &El, pchars: &CharAttrs, outer: &CharFormat) -> CharFormat { From 98ada498d5572648a3974dfa0ad7641f2179dbb6 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sun, 25 Oct 2026 16:49:17 -0400 Subject: [PATCH 11/13] Fix the imports the explicit-import refactor missed The Sonar S2208 sweep replaced use super::* with explicit lists, but tests_ime.rs and panel_docking_tests.rs use helpers added after it was written, so main stopped compiling. --- crates/engine/src/tests_ime.rs | 2 +- crates/ui-egui/src/panel_docking_tests.rs | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/tests_ime.rs b/crates/engine/src/tests_ime.rs index a6a1ff5cd..222142c80 100644 --- a/crates/engine/src/tests_ime.rs +++ b/crates/engine/src/tests_ime.rs @@ -4,7 +4,7 @@ use designcraft_tools::{Mods, PointerEvent, PointerKind, ToolKey}; use serde_json::json; -use super::{Arc, Session, ViewInfo}; +use super::{Arc, Session, ViewInfo, ime_delete_ranges}; fn session() -> Session { let mut s = Session::new(); diff --git a/crates/ui-egui/src/panel_docking_tests.rs b/crates/ui-egui/src/panel_docking_tests.rs index 148512613..9de7a41f7 100644 --- a/crates/ui-egui/src/panel_docking_tests.rs +++ b/crates/ui-egui/src/panel_docking_tests.rs @@ -1,4 +1,5 @@ use super::{DesignApp, Layout, Node, cancel_gesture, json, valid}; +use crate::panel_docking::{area_id, close_visible, default_layout, dock_columns, hit_id, icon_column, in_iconic_column, layout_of, modal_open, open_window_drops, panel_menu, set_iconic, showing, tab_group, tab_id, window_groups}; use egui::{Event, Modifiers, PointerButton, Pos2, vec2}; use egui_kittest::{Harness, kittest::Queryable}; From 165cbbbff1b27e28e734bfc7b3134ecad78fe5e6 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sun, 25 Oct 2026 16:54:34 -0400 Subject: [PATCH 12/13] rustfmt the added import list --- crates/ui-egui/src/panel_docking_tests.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/src/panel_docking_tests.rs b/crates/ui-egui/src/panel_docking_tests.rs index 9de7a41f7..62a8b814c 100644 --- a/crates/ui-egui/src/panel_docking_tests.rs +++ b/crates/ui-egui/src/panel_docking_tests.rs @@ -1,5 +1,8 @@ use super::{DesignApp, Layout, Node, cancel_gesture, json, valid}; -use crate::panel_docking::{area_id, close_visible, default_layout, dock_columns, hit_id, icon_column, in_iconic_column, layout_of, modal_open, open_window_drops, panel_menu, set_iconic, showing, tab_group, tab_id, window_groups}; +use crate::panel_docking::{ + area_id, close_visible, default_layout, dock_columns, hit_id, icon_column, in_iconic_column, layout_of, modal_open, open_window_drops, + panel_menu, set_iconic, showing, tab_group, tab_id, window_groups, +}; use egui::{Event, Modifiers, PointerButton, Pos2, vec2}; use egui_kittest::{Harness, kittest::Queryable}; From 5478eb655ee34d666c62af15b8cc1f2aa67749f2 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sun, 25 Oct 2026 17:00:59 -0400 Subject: [PATCH 13/13] Import the panel-docking helpers the test module uses from its parent panel_docking_tests is a child module of panel_docking, so every helper it calls lives in super; list them instead of the wildcard the sweep removed. --- crates/ui-egui/src/panel_docking_tests.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/ui-egui/src/panel_docking_tests.rs b/crates/ui-egui/src/panel_docking_tests.rs index 62a8b814c..73f98f612 100644 --- a/crates/ui-egui/src/panel_docking_tests.rs +++ b/crates/ui-egui/src/panel_docking_tests.rs @@ -1,7 +1,7 @@ -use super::{DesignApp, Layout, Node, cancel_gesture, json, valid}; -use crate::panel_docking::{ - area_id, close_visible, default_layout, dock_columns, hit_id, icon_column, in_iconic_column, layout_of, modal_open, open_window_drops, - panel_menu, set_iconic, showing, tab_group, tab_id, window_groups, +use super::{ + CANVAS, Column, DOCK_LABELS, DesignApp, Floating, ICON_GROUPS, Layout, Node, PanelContent, Placed, Side, Value, area_id, cancel_gesture, + close_visible, default_layout, dock_columns, hit_id, icon_column, in_iconic_column, json, layout_of, modal_open, open_window_drops, panel_menu, + set_iconic, showing, tab_group, tab_id, valid, window_groups, }; use egui::{Event, Modifiers, PointerButton, Pos2, vec2}; use egui_kittest::{Harness, kittest::Queryable};