From 8f48869e589f1eb1adddd11d72bb374312437edb Mon Sep 17 00:00:00 2001 From: sanyamsingh001 <314051937+sanyamsingh001@users.noreply.github.com> Date: Tue, 13 Oct 2026 10:56:09 +1000 Subject: [PATCH] PARITY-IDML-INDEX: retain index reference source sort keys Co-Authored-By: Codex --- crates/idml/src/export.rs | 25 ++- crates/idml/src/import.rs | 55 ++++--- crates/idml/src/tests.rs | 1 + crates/idml/src/tests/index_sort.rs | 240 ++++++++++++++++++++++++++++ 4 files changed, 295 insertions(+), 26 deletions(-) create mode 100644 crates/idml/src/tests/index_sort.rs diff --git a/crates/idml/src/export.rs b/crates/idml/src/export.rs index 9d73562b9..6c5b00a4f 100644 --- a/crates/idml/src/export.rs +++ b/crates/idml/src/export.rs @@ -33,10 +33,17 @@ impl Default for ExportOptions { } /// Export a document as an IDML package (ZIP bytes). +/// +/// IDML has one sort key per index topic path. If native references disagree, the first +/// reference serialized for that path supplies its key, including an empty/default key. +/// Later references cannot overwrite it; no synthetic topic names or sort keys are created. +/// Consistent references retain their keys. Conflicting per-reference keys cannot all survive +/// this topic-level representation. This policy also applies to [`export_idml_with`]. pub fn export_idml(doc: &Document) -> Vec { export_idml_with(doc, &ExportOptions::default()) } +/// Export with options, using the index sort-key conflict policy of [`export_idml`]. pub fn export_idml_with(doc: &Document, opts: &ExportOptions) -> Vec { let mut ex = Ex::new(doc, opts); let parts = ex.build(); @@ -133,8 +140,8 @@ struct Ex<'a> { threads: HashMap, Option)>, /// Cross-reference sources written so far: (Self, target anchor). xref_sources: Vec<(String, u64)>, - /// Index topics (paths) and their See / See also cross-references. - index_topics: BTreeSet>, + /// Index topic path → first serialized sort key; See / See also cross-references. + index_topics: BTreeMap, String>, index_xrefs: Vec<(Vec, &'static str, String)>, } @@ -148,7 +155,7 @@ impl<'a> Ex<'a> { fonts: BTreeMap::new(), threads: HashMap::new(), xref_sources: Vec::new(), - index_topics: BTreeSet::new(), + index_topics: BTreeMap::new(), index_xrefs: Vec::new(), }; ex.assign_swatch_ids(); @@ -1809,12 +1816,12 @@ impl<'a> Ex<'a> { fn index_el(&mut self) -> El { let mut topics = self.index_topics.clone(); for (_, _, target) in &self.index_xrefs { - topics.insert(vec![target.clone()]); + topics.entry(vec![target.clone()]).or_default(); } - fn build(ex: &mut Ex, topics: &BTreeSet>, prefix: &[String]) -> Vec { + fn build(ex: &mut Ex, topics: &BTreeMap, String>, prefix: &[String]) -> Vec { let mut out = Vec::new(); - for t in topics.iter().filter(|t| t.len() == prefix.len() + 1 && t.starts_with(prefix)) { - let mut el = El::new("Topic").attr("Self", topic_self(t)).attr("SortOrder", "").attr("Name", t.last().map_or("", String::as_str)); + for (t, sort) in topics.iter().filter(|(t, _)| t.len() == prefix.len() + 1 && t.starts_with(prefix)) { + let mut el = El::new("Topic").attr("Self", topic_self(t)).attr("SortOrder", sort).attr("Name", t.last().map_or("", String::as_str)); for (from, kind, target) in ex.index_xrefs.clone() { if from == *t { el.push( @@ -2069,7 +2076,9 @@ impl<'a> Ex<'a> { if let Some(r) = s.index_refs.get(k).filter(|r| !r.topics.is_empty()) { use designcraft_doc::index::IndexRange as R; for n in 1..=r.topics.len() { - self.index_topics.insert(r.topics[..n].to_vec()); + // A shared topic has one key: keep the first reference's + // value (even empty), rather than inventing a merged key. + self.index_topics.entry(r.topics[..n].to_vec()).or_insert_with(|| r.sort.get(n - 1).cloned().unwrap_or_default()); } let (kind, limit) = match &r.range { R::CurrentPage => (Some("CurrentPage"), None), diff --git a/crates/idml/src/import.rs b/crates/idml/src/import.rs index 46c3253b6..1b1cb7d8e 100644 --- a/crates/idml/src/import.rs +++ b/crates/idml/src/import.rs @@ -206,11 +206,32 @@ struct Importer<'r> { /// Cross-reference format Self → name. xref_format_names: HashMap, xref_formats: Vec, - /// Index topic Self → topic path; topic See / See also cross-references. - index_topics: HashMap>, + /// Index topic Self → topic path and per-level sort keys; topic cross-references. + index_topics: HashMap, index_xrefs: Vec, } +/// IDML stores a sort key on each topic, rather than on its page references. +#[derive(Clone, Default)] +struct IndexTopic { + path: Vec, + sort: Vec, +} + +impl IndexTopic { + fn child(&self, topic: &El) -> Self { + let mut path = self.path.clone(); + path.push(topic.get("Name").unwrap_or("").to_string()); + let mut sort = self.sort.clone(); + if let Some(key) = topic.get("SortOrder").filter(|key| !key.is_empty()) { + // Keep empty positions before a child's key, but omit trailing defaults. + sort.resize(self.path.len(), String::new()); + sort.push(key.to_string()); + } + Self { path, sort } + } +} + fn lab_to_color(l: f32, a: f32, b: f32) -> Color { let rgb = lab::xyz_to_srgb(lab::lab_to_xyz(lab::Lab::new(l, a, b))); Color::rgb(rgb[0], rgb[1], rgb[2]) @@ -453,7 +474,7 @@ impl<'r> Importer<'r> { self.xref_format(e); } if let Some(ix) = top.iter().find(|e| e.local() == "Index") { - self.index_topics_of(ix, &[]); + self.index_topics_of(ix, &IndexTopic::default()); } // Stories (ids first so frames can reference them). for e in top.iter().filter(|e| e.local() == "Story") { @@ -1010,33 +1031,31 @@ impl<'r> Importer<'r> { } /// Index topics (recursively); at the root, their See / See also cross-references too. - fn index_topics_of(&mut self, e: &El, prefix: &[String]) { + fn index_topics_of(&mut self, e: &El, prefix: &IndexTopic) { for t in e.find_all("Topic") { - let mut path = prefix.to_vec(); - path.push(t.get("Name").unwrap_or("").to_string()); + let topic = prefix.child(t); if let Some(me) = t.get("Self") { - self.index_topics.insert(me.to_string(), path.clone()); + self.index_topics.insert(me.to_string(), topic.clone()); } - self.index_topics_of(t, &path); + self.index_topics_of(t, &topic); } - if !prefix.is_empty() { + if !prefix.path.is_empty() { return; } - let mut stack = vec![(e, Vec::::new())]; - while let Some((el, path)) = stack.pop() { + let mut stack = vec![(e, IndexTopic::default())]; + while let Some((el, prefix)) = stack.pop() { for t in el.find_all("Topic") { - let mut p = path.clone(); - p.push(t.get("Name").unwrap_or("").to_string()); + let topic = prefix.child(t); for x in t.find_all("CrossReference") { - let target = x.get("ReferencedTopic").and_then(|r| self.index_topics.get(r)).map(|p| p.join(": ")).unwrap_or_default(); + let target = x.get("ReferencedTopic").and_then(|r| self.index_topics.get(r)).map(|t| t.path.join(": ")).unwrap_or_default(); use designcraft_doc::index::IndexRange as R; let range = match x.get("CrossReferenceType") { Some("SeeAlso") | Some("SeeAlsoHerein") => R::SeeAlso(target), _ => R::See(target), }; - self.index_xrefs.push(designcraft_doc::IndexRef { topics: p.clone(), sort: vec![], range }); + self.index_xrefs.push(designcraft_doc::IndexRef { topics: topic.path.clone(), sort: topic.sort.clone(), range }); } - stack.push((t, p)); + stack.push((t, topic)); } } } @@ -1796,14 +1815,14 @@ impl<'r> Importer<'r> { } "PageReference" => { use designcraft_doc::index::IndexRange as R; - let topics = c.get("ReferencedTopic").and_then(|r| self.index_topics.get(r)).cloned().unwrap_or_default(); + let topic = c.get("ReferencedTopic").and_then(|r| self.index_topics.get(r)).cloned().unwrap_or_default(); let range = match c.get("PageReferenceType") { Some("ToEndOfStory") | Some("ToEndOfDocument") | Some("ToEndOfSection") => R::ToEndOfStory, Some("ForNextNParagraphs") => R::NextParagraphs(c.num("PageReferenceLimit").unwrap_or(1.0).max(1.0) as u32), Some("SuppressPageNumbers") => R::SuppressPageRange, _ => R::CurrentPage, }; - b.index_refs.push(designcraft_doc::IndexRef { topics, sort: vec![], range }); + b.index_refs.push(designcraft_doc::IndexRef { topics: topic.path, sort: topic.sort, range }); b.push(&designcraft_doc::INDEX_MARK.to_string(), cf); } "CrossReferenceSource" => { diff --git a/crates/idml/src/tests.rs b/crates/idml/src/tests.rs index 23ec99bdf..087a2b5d7 100644 --- a/crates/idml/src/tests.rs +++ b/crates/idml/src/tests.rs @@ -9,6 +9,7 @@ use super::*; mod assets; mod decorations; +mod index_sort; fn zip_files(files: &[(&str, &str)]) -> Vec { use zip::write::SimpleFileOptions; diff --git a/crates/idml/src/tests/index_sort.rs b/crates/idml/src/tests/index_sort.rs new file mode 100644 index 000000000..e35e7fed9 --- /dev/null +++ b/crates/idml/src/tests/index_sort.rs @@ -0,0 +1,240 @@ +//! Original synthetic fixtures: no Adobe-generated files or implementation details. +//! Behavioral reference: https://developer.adobe.com/indesign/uxp/dom/api/t/topic/ + +use std::io::Read; + +use designcraft_doc::IndexRef; +use designcraft_doc::index::{self, IndexRange, IndexSpec, Resolved}; + +use super::*; +use crate::xml::El; + +fn strings(values: &[&str]) -> Vec { + values.iter().map(|v| (*v).to_string()).collect() +} + +fn reference(topics: &[&str], sort: &[&str], range: IndexRange) -> IndexRef { + IndexRef { topics: strings(topics), sort: strings(sort), range } +} + +fn references(doc: &Document) -> Vec { + doc.stories.values().flat_map(|s| s.index_refs.iter().map(|r| r.as_ref().clone())).collect() +} + +fn native(refs: Vec) -> Document { + let mut doc = Document::new(&NewDocument::default()); + let layer = doc.default_layer(); + let (_, sid) = doc.add_text_frame(SpreadRef::Doc(0), Rect::new(36.0, 36.0, 400.0, 400.0), layer, "Source", ParaFormat::default()).unwrap(); + let story = doc.story_mut(sid).unwrap(); + for r in refs { + story.insert_index_ref(story.len(), r); + } + doc.check().unwrap(); + doc +} + +fn import_xml(index: &str, refs: &str) -> Document { + let map = format!( + r#" + {index} + + + + "# + ); + let story = format!( + r#" + + Source{refs} + + "# + ); + let spread = r#" + + + + + + + + + "#; + import_idml(&zip_files(&[("designmap.xml", &map), ("Stories/s.xml", &story), ("Spreads/s.xml", spread)])).unwrap() +} + +/// Inspect the exported XML directly, without calling the IDML importer. +fn exported_index(doc: &Document) -> El { + let bytes = export_idml(doc); + let mut zip = zip::ZipArchive::new(std::io::Cursor::new(bytes)).unwrap(); + let mut map = String::new(); + zip.by_name("designmap.xml").unwrap().read_to_string(&mut map).unwrap(); + crate::xml::parse(map.as_bytes()).unwrap().find("Index").unwrap().clone() +} + +fn topic<'a>(index: &'a El, path: &[&str]) -> &'a El { + let mut el = index; + for name in path { + el = el.find_all("Topic").find(|t| t.get("Name") == Some(*name)).unwrap(); + } + el +} + +#[test] +fn imports_synthetic_sort_keys_for_nested_topics_and_cross_references() { + let doc = import_xml( + r#" + + + + + + + + + + + + + "#, + r#" + + + + + "#, + ); + doc.check().unwrap(); + let refs = references(&doc); + assert_eq!(refs.len(), 8); + for r in [ + reference(&["Zulu"], &["Alpha"], IndexRange::CurrentPage), + reference(&["Zulu"], &["Alpha"], IndexRange::NextParagraphs(4)), + reference(&["Beta"], &[], IndexRange::CurrentPage), + reference(&["Empty"], &[], IndexRange::SuppressPageRange), + reference(&["Parent", "É & "], &["", " A&<é> "], IndexRange::CurrentPage), + reference(&["Parent", "É & "], &["", " A&<é> "], IndexRange::See("Beta".into())), + reference(&["Parent", "É & "], &["", " A&<é> "], IndexRange::SeeAlso("Zulu".into())), + reference(&["Zoo", "Leaf"], &["Aardvark"], IndexRange::CurrentPage), + ] { + assert!(refs.contains(&r), "missing {r:?} in {refs:?}"); + } + let story = doc.stories.values().next().unwrap(); + assert_eq!(story.text.replace(designcraft_doc::INDEX_MARK, ""), "Source"); +} + +#[test] +fn exports_sort_keys_as_topic_attributes_without_changing_names() { + let doc = native(vec![ + reference(&["Zulu"], &["Alpha"], IndexRange::CurrentPage), + reference(&["Zulu"], &["Alpha"], IndexRange::NextParagraphs(2)), + reference(&["Beta"], &[], IndexRange::CurrentPage), + reference(&["Empty"], &[""], IndexRange::CurrentPage), + reference(&["Parent", "É & "], &["", " A&<é> "], IndexRange::CurrentPage), + reference(&["Alias"], &["Able"], IndexRange::See("Zulu".into())), + reference(&["Related"], &["Azure"], IndexRange::SeeAlso("Zulu".into())), + reference(&["Parent", "Alias"], &["", "Able"], IndexRange::See("Zulu".into())), + reference(&["Parent", "Related"], &["", "Azure"], IndexRange::SeeAlso("Zulu".into())), + ]); + let index = exported_index(&doc); + assert_eq!(index.find_all("Topic").count(), 6, "repeated references and cross-reference targets share their topic"); + assert_eq!(topic(&index, &["Zulu"]).get("SortOrder"), Some("Alpha")); + assert_eq!(topic(&index, &["Beta"]).get("SortOrder"), Some("")); + assert_eq!(topic(&index, &["Empty"]).get("SortOrder"), Some("")); + assert_eq!(topic(&index, &["Parent"]).get("SortOrder"), Some("")); + assert_eq!(topic(&index, &["Parent", "É & "]).get("SortOrder"), Some(" A&<é> ")); + for (name, key, kind) in [("Alias", "Able", "See"), ("Related", "Azure", "SeeAlso")] { + for path in [&[name][..], &["Parent", name][..]] { + let t = topic(&index, path); + assert_eq!(t.get("SortOrder"), Some(key)); + let cross_ref = t.find("CrossReference").unwrap(); + assert_eq!(cross_ref.get("CrossReferenceType"), Some(kind)); + assert_eq!(cross_ref.get("ReferencedTopic"), topic(&index, &["Zulu"]).get("Self")); + } + } +} + +/// These references all belong to the same page. Fix the page resolution to isolate index +/// ordering from fonts/composition, and use the real index generator for its text and headings. +fn generated_index(doc: &Document) -> Vec<(String, String)> { + let refs: Vec<_> = references(doc).into_iter().map(|r| Resolved { r, pages: Some((0, 0)) }).collect(); + index::build(&IndexSpec { title: String::new(), ..Default::default() }, &refs, &|p| (p + 1).to_string()) +} + +#[test] +fn round_trip_preserves_generated_index_order_and_headings() { + let doc = native(vec![ + reference(&["Zulu"], &["Alpha"], IndexRange::CurrentPage), + reference(&["Zulu"], &["Alpha"], IndexRange::CurrentPage), + reference(&["Beta"], &[], IndexRange::CurrentPage), + reference(&["Parent", "Zulu"], &["", "Alpha"], IndexRange::CurrentPage), + reference(&["Parent", "Beta"], &[], IndexRange::CurrentPage), + reference(&["Alias"], &["Able"], IndexRange::See("Zulu".into())), + reference(&["Related"], &["Delta"], IndexRange::SeeAlso("Beta".into())), + reference(&["Parent", "Alias"], &["", "Able"], IndexRange::See("Zulu".into())), + reference(&["Parent", "Related"], &["", "Azure"], IndexRange::SeeAlso("Beta".into())), + ]); + let before = generated_index(&doc); + let expected: Vec<_> = [ + ("Index Section Head", "A"), + ("Index Level 1", "Alias See Zulu"), + ("Index Level 1", "Zulu 1"), + ("Index Section Head", "B"), + ("Index Level 1", "Beta 1"), + ("Index Section Head", "D"), + ("Index Level 1", "Related 1. See also Beta"), + ("Index Section Head", "P"), + ("Index Level 1", "Parent"), + ("Index Level 2", "Alias See Zulu"), + ("Index Level 2", "Zulu 1"), + ("Index Level 2", "Related 1. See also Beta"), + ("Index Level 2", "Beta 1"), + ] + .into_iter() + .map(|(style, text)| (style.to_string(), text.to_string())) + .collect(); + assert_eq!(before, expected); + let back = import_idml(&export_idml(&doc)).unwrap(); + back.check().unwrap(); + assert_eq!(generated_index(&back), before); + assert_eq!(back.stories.values().next().unwrap().text.replace(designcraft_doc::INDEX_MARK, ""), "Source"); +} + +#[test] +fn conflicting_sort_keys_keep_the_first_serialized_value_including_defaults() { + fn check(first: &[&str], second: &[&str], expected: &str) { + let doc = native(vec![reference(&["Shared"], first, IndexRange::CurrentPage), reference(&["Shared"], second, IndexRange::CurrentPage)]); + let index = exported_index(&doc); + assert_eq!(index.find_all("Topic").count(), 1, "conflicts do not manufacture topics"); + assert_eq!(topic(&index, &["Shared"]).get("SortOrder"), Some(expected)); + let back = import_idml(&export_idml(&doc)).unwrap(); + let refs = references(&back); + assert_eq!(refs.len(), 2); + for r in refs { + assert_eq!(r.sort.first().map_or("", String::as_str), expected); + } + assert_eq!(references(&doc)[0].sort, strings(first), "export does not edit native references"); + assert_eq!(references(&doc)[1].sort, strings(second)); + } + check(&["Alpha"], &["Omega"], "Alpha"); + check(&["Omega"], &["Alpha"], "Omega"); + check(&[], &["Alpha"], ""); + check(&[""], &["Alpha"], ""); + check(&["Alpha"], &[], "Alpha"); + check(&["Alpha"], &[""], "Alpha"); +} + +#[test] +fn shared_ancestors_and_identical_leaf_names_keep_keys_by_full_path() { + let doc = native(vec![ + reference(&["One", "Zulu"], &["Alpha", "First"], IndexRange::CurrentPage), + reference(&["One", "Zulu"], &["Beta", "Second"], IndexRange::CurrentPage), + reference(&["One", "Beta"], &["Gamma", "Third"], IndexRange::CurrentPage), + reference(&["Two", "Zulu"], &["Delta", "Fourth"], IndexRange::CurrentPage), + ]); + let index = exported_index(&doc); + assert_eq!(topic(&index, &["One"]).get("SortOrder"), Some("Alpha")); + assert_eq!(topic(&index, &["One", "Zulu"]).get("SortOrder"), Some("First")); + assert_eq!(topic(&index, &["One", "Beta"]).get("SortOrder"), Some("Third")); + assert_eq!(topic(&index, &["Two"]).get("SortOrder"), Some("Delta")); + assert_eq!(topic(&index, &["Two", "Zulu"]).get("SortOrder"), Some("Fourth")); +}