From 8997352d3bce2511e14fd38b40b98fe01575c6db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=80=99=E5=B8=85?= Date: Mon, 20 Jul 2026 23:42:38 +0800 Subject: [PATCH] fix(pptx): align slide removal with package relationships --- crates/officecli/tests/cli_smoke.rs | 63 +++++ crates/oxml/src/package.rs | 18 ++ crates/pptx-handler/src/add.rs | 129 +++++++--- crates/pptx-handler/src/mutations.rs | 319 +++++++++++++++++++++++- crates/pptx-handler/src/navigation.rs | 33 +++ crates/pptx-handler/src/raw.rs | 4 +- crates/pptx-handler/src/view.rs | 2 +- docs/csharp-parity/README.zh.md | 151 +++++++++++ docs/csharp-parity/migration-ledger.tsv | 24 ++ 9 files changed, 688 insertions(+), 55 deletions(-) create mode 100644 docs/csharp-parity/README.zh.md create mode 100644 docs/csharp-parity/migration-ledger.tsv diff --git a/crates/officecli/tests/cli_smoke.rs b/crates/officecli/tests/cli_smoke.rs index d98f559..02804b3 100644 --- a/crates/officecli/tests/cli_smoke.rs +++ b/crates/officecli/tests/cli_smoke.rs @@ -648,6 +648,69 @@ fn test_pptx_add_slide() { .stdout(predicate::str::contains("Slides")); } +#[test] +fn test_pptx_remove_middle_slide_then_edit_and_add() { + let tmp = temp_dir(); + let path = tmp.path().join("test_pptx_remove_middle.pptx"); + let p = path.to_string_lossy().to_string(); + + officecli().args(["create", &p]).assert().success(); + for _ in 0..2 { + officecli() + .args([ + "add", + &p, + "--parent", + "/presentation", + "--type-name", + "slide", + ]) + .assert() + .success(); + } + + officecli() + .args(["remove", &p, "/slide[2]"]) + .assert() + .success(); + officecli() + .args([ + "add", + &p, + "--parent", + "/slide[2]", + "--type-name", + "rectangle", + "--properties", + "text=logical-slide-two", + ]) + .assert() + .success(); + officecli() + .args([ + "add", + &p, + "--parent", + "/presentation", + "--type-name", + "slide", + ]) + .assert() + .success(); + + officecli() + .args(["validate", &p]) + .assert() + .success() + .stdout(predicate::str::contains("No validation errors")); + officecli() + .args(["view", &p, "-m", "stats"]) + .assert() + .success() + .stdout(predicate::str::contains("Slides: 3")) + .stdout(predicate::str::contains("Shapes: 1")); +} + // ═══════════════════════════════════════════════════════════════════════ // Convert — docx → docx (re-save via oxide engine) // ═══════════════════════════════════════════════════════════════════════ diff --git a/crates/oxml/src/package.rs b/crates/oxml/src/package.rs index 7c3c0a4..a60cbb7 100644 --- a/crates/oxml/src/package.rs +++ b/crates/oxml/src/package.rs @@ -134,6 +134,24 @@ impl OxmlPackage { self.parts.contains_key(part_path) } + /// Remove a part from the package. + /// + /// Callers are responsible for removing relationships and content-type + /// overrides that reference the part. + pub fn remove_part(&mut self, part_path: &str) -> Result { + if !self.editable { + return Err(PackageError::WritePartError( + "package opened in read-only mode".to_string(), + )); + } + + let removed = self.parts.remove(part_path).is_some(); + if removed && !self.dirty_parts.iter().any(|path| path == part_path) { + self.dirty_parts.push(part_path.to_string()); + } + Ok(removed) + } + /// Get the content types. pub fn content_types(&self) -> &ContentTypes { &self.content_types diff --git a/crates/pptx-handler/src/add.rs b/crates/pptx-handler/src/add.rs index 79e21b6..88b0495 100644 --- a/crates/pptx-handler/src/add.rs +++ b/crates/pptx-handler/src/add.rs @@ -44,10 +44,12 @@ pub fn add_element( } fn add_slide(package: &mut OxmlPackage, _parent: &str) -> Result { - // Count existing slides to determine next slide number + // Logical slide index and physical part number are separate: part names can + // have gaps after a deletion. let pres = crate::navigation::build_presentation(package)?; - let slide_num = pres.slides.len() + 1; - let slide_path = format!("ppt/slides/slide{}.xml", slide_num); + let slide_index = pres.slides.len() + 1; + let slide_part_number = next_slide_part_number(package); + let slide_path = format!("ppt/slides/slide{}.xml", slide_part_number); let slide_xml = r#" Result Result { // Parse parent path to find slide let slide_num = parse_slide_num(parent)?; - let slide_path = format!("ppt/slides/slide{}.xml", slide_num); + let slide_path = crate::navigation::resolve_slide_part_path(package, slide_num)?; let text = properties.get("text").cloned().unwrap_or_default(); let name = properties @@ -130,17 +133,21 @@ fn add_text_to_shape( pub fn update_presentation_slides( package: &mut OxmlPackage, - slide_num: usize, + slide_part_number: usize, ) -> Result<(), HandlerError> { let pres_xml = package .read_part_xml("ppt/presentation.xml") .map_err(|e| HandlerError::OperationFailed(e.to_string()))?; - // Add slide ID entry: - // We need to find the next available rId and sldId - let sld_id = 256 + slide_num; - let r_id = format!("rId{}", slide_num + 2); // rId1 is usually the slide master + let sld_id = next_slide_id(&pres_xml)?; + // Relationship IDs share a namespace with masters, themes and other + // presentation parts, so derive the next free ID from the rels part. + let rels_path = "ppt/_rels/presentation.xml.rels"; + let rels_xml = package + .read_part_xml(rels_path) + .map_err(|e| HandlerError::OperationFailed(e.to_string()))?; + let r_id = format!("rId{}", find_max_rel_id(&rels_xml) + 1); let new_entry = format!("", sld_id, r_id); // Insert into @@ -156,15 +163,9 @@ pub fn update_presentation_slides( .write_part_xml("ppt/presentation.xml", &modified) .map_err(|e| HandlerError::OperationFailed(e.to_string()))?; - // Update presentation relationships - let rels_path = "ppt/_rels/presentation.xml.rels"; - let rels_xml = package - .read_part_xml(rels_path) - .map_err(|e| HandlerError::OperationFailed(e.to_string()))?; - let new_rel = format!( "", - r_id, slide_num + r_id, slide_part_number ); let modified_rels = if let Some(pos) = rels_xml.find("") { @@ -182,6 +183,59 @@ pub fn update_presentation_slides( Ok(()) } +pub(crate) fn next_slide_part_number(package: &OxmlPackage) -> usize { + package + .list_parts() + .into_iter() + .filter_map(|path| { + path.strip_prefix("ppt/slides/slide") + .and_then(|value| value.strip_suffix(".xml")) + .and_then(|value| value.parse::().ok()) + }) + .max() + .unwrap_or(0) + + 1 +} + +fn next_slide_id(presentation_xml: &str) -> Result { + let doc = roxmltree::Document::parse(presentation_xml) + .map_err(|e| HandlerError::OperationFailed(format!("invalid presentation.xml: {}", e)))?; + Ok(doc + .descendants() + .filter(|node| node.is_element() && node.tag_name().name() == "sldId") + .filter_map(|node| node.attribute("id")) + .filter_map(|value| value.parse::().ok()) + .max() + .unwrap_or(255) + + 1) +} + +pub(crate) fn register_slide_content_type( + package: &mut OxmlPackage, + slide_path: &str, +) -> Result<(), HandlerError> { + let content_types_path = "[Content_Types].xml"; + let content_types_xml = package + .read_part_xml(content_types_path) + .map_err(|e| HandlerError::OperationFailed(e.to_string()))?; + let part_name = format!("/{}", slide_path.trim_start_matches('/')); + if content_types_xml.contains(&format!("PartName=\"{}\"", part_name)) { + return Ok(()); + } + let override_xml = format!( + "", + part_name + ); + let close = content_types_xml.find("").ok_or_else(|| { + HandlerError::OperationFailed("invalid [Content_Types].xml: missing ".to_string()) + })?; + let mut updated = content_types_xml; + updated.insert_str(close, &override_xml); + package + .write_part_xml(content_types_path, &updated) + .map_err(|e| HandlerError::OperationFailed(e.to_string())) +} + fn create_text_shape_xml(id: usize, name: &str, text: &str) -> String { create_shape_xml_with_geometry( id, name, text, "rect", "457200", "274638", "8382000", "304800", true, None, @@ -400,7 +454,7 @@ fn add_rectangle( properties: &HashMap, ) -> Result { let slide_num = parse_slide_num(parent)?; - let slide_path = format!("ppt/slides/slide{}.xml", slide_num); + let slide_path = crate::navigation::resolve_slide_part_path(package, slide_num)?; let slide_xml = package .read_part_xml(&slide_path) .map_err(|e| HandlerError::OperationFailed(e.to_string()))?; @@ -454,7 +508,7 @@ fn add_ellipse( properties: &HashMap, ) -> Result { let slide_num = parse_slide_num(parent)?; - let slide_path = format!("ppt/slides/slide{}.xml", slide_num); + let slide_path = crate::navigation::resolve_slide_part_path(package, slide_num)?; let slide_xml = package .read_part_xml(&slide_path) .map_err(|e| HandlerError::OperationFailed(e.to_string()))?; @@ -508,7 +562,7 @@ fn add_line_shape( properties: &HashMap, ) -> Result { let slide_num = parse_slide_num(parent)?; - let slide_path = format!("ppt/slides/slide{}.xml", slide_num); + let slide_path = crate::navigation::resolve_slide_part_path(package, slide_num)?; let slide_xml = package .read_part_xml(&slide_path) .map_err(|e| HandlerError::OperationFailed(e.to_string()))?; @@ -580,7 +634,7 @@ fn add_group( properties: &HashMap, ) -> Result { let slide_num = parse_slide_num(parent)?; - let slide_path = format!("ppt/slides/slide{}.xml", slide_num); + let slide_path = crate::navigation::resolve_slide_part_path(package, slide_num)?; let slide_xml = package .read_part_xml(&slide_path) .map_err(|e| HandlerError::OperationFailed(e.to_string()))?; @@ -667,7 +721,7 @@ fn add_picture( }; let slide_num = parse_slide_num(parent)?; - let slide_path = format!("ppt/slides/slide{}.xml", slide_num); + let slide_path = crate::navigation::resolve_slide_part_path(package, slide_num)?; let slide_xml = package .read_part_xml(&slide_path) .map_err(|e| HandlerError::OperationFailed(e.to_string()))?; @@ -704,7 +758,7 @@ fn add_picture( } // Generate a relationship ID for the image - let rels_path = format!("ppt/slides/_rels/slide{}.xml.rels", slide_num); + let rels_path = crate::navigation::relationships_part_path(&slide_path); let rels_xml = package .read_part_xml(&rels_path) .unwrap_or_else(|_| "".to_string()); @@ -809,7 +863,7 @@ fn add_video( properties: &HashMap, ) -> Result { let slide_num = parse_slide_num(parent)?; - let slide_path = format!("ppt/slides/slide{}.xml", slide_num); + let slide_path = crate::navigation::resolve_slide_part_path(package, slide_num)?; let video_ext = properties .get("format") @@ -834,7 +888,7 @@ fn add_video( } // Wire slide→video relationship (Type: video, not image). - let slide_rels_path = format!("ppt/slides/_rels/slide{}.xml.rels", slide_num); + let slide_rels_path = crate::navigation::relationships_part_path(&slide_path); let rels_xml = package .read_part_xml(&slide_rels_path) .unwrap_or_else(|_| "".to_string()); @@ -963,7 +1017,7 @@ fn add_table( properties: &HashMap, ) -> Result { let slide_num = parse_slide_num(parent)?; - let slide_path = format!("ppt/slides/slide{}.xml", slide_num); + let slide_path = crate::navigation::resolve_slide_part_path(package, slide_num)?; let slide_xml = package .read_part_xml(&slide_path) .map_err(|e| HandlerError::OperationFailed(e.to_string()))?; @@ -1078,7 +1132,7 @@ fn add_chart_real( properties: &HashMap, ) -> Result { let slide_num = parse_slide_num(parent)?; - let slide_path = format!("ppt/slides/slide{}.xml", slide_num); + let slide_path = crate::navigation::resolve_slide_part_path(package, slide_num)?; // Chart index — probe parts for the next free number. let chart_idx = next_ppt_chart_index(package); @@ -1127,7 +1181,7 @@ fn add_chart_real( .map_err(|e| HandlerError::SaveError(e.to_string()))?; // Wire slide→chart rels. - let slide_rels_path = format!("ppt/slides/_rels/slide{}.xml.rels", slide_num); + let slide_rels_path = crate::navigation::relationships_part_path(&slide_path); let chart_rel_id = next_ppt_rel_id(package, &slide_rels_path); let chart_target = format!("../charts/chart{}.xml", chart_idx); let rel_xml = format!( @@ -1352,7 +1406,7 @@ fn add_model3d_real( properties: &HashMap, ) -> Result { let slide_num = parse_slide_num(parent)?; - let slide_path = format!("ppt/slides/slide{}.xml", slide_num); + let slide_path = crate::navigation::resolve_slide_part_path(package, slide_num)?; let model_ext = properties .get("format") @@ -1386,7 +1440,7 @@ fn add_model3d_real( } // Wire slide→model rel. - let slide_rels_path = format!("ppt/slides/_rels/slide{}.xml.rels", slide_num); + let slide_rels_path = crate::navigation::relationships_part_path(&slide_path); let model_rel_id = next_ppt_rel_id(package, &slide_rels_path); let model_target = format!("../media/model{}.{}", model_idx, model_ext_lower); let model_rel_xml = format!( @@ -1721,13 +1775,13 @@ fn add_hyperlink( } let slide_num = parse_slide_num(parent)?; - let slide_path = format!("ppt/slides/slide{}.xml", slide_num); + let slide_path = crate::navigation::resolve_slide_part_path(package, slide_num)?; let slide_xml = package .read_part_xml(&slide_path) .map_err(|e| HandlerError::OperationFailed(e.to_string()))?; // Add relationship for the URL - let rels_path = format!("ppt/slides/_rels/slide{}.xml.rels", slide_num); + let rels_path = crate::navigation::relationships_part_path(&slide_path); let rels_xml = package .read_part_xml(&rels_path) .unwrap_or_else(|_| "".to_string()); @@ -2064,14 +2118,7 @@ fn add_transition( /// Falls back to the first slide when the parent isn't a slide path. fn resolve_slide_path(package: &OxmlPackage, parent: &str) -> Result { if let Some(n) = extract_slide_number(parent) { - let path = format!("ppt/slides/slide{}.xml", n); - if package - .list_parts() - .iter() - .any(|p| p.as_str() == path.as_str()) - { - return Ok(path); - } + return crate::navigation::resolve_slide_part_path(package, n); } if parent.starts_with("ppt/slides/") { return Ok(parent.to_string()); diff --git a/crates/pptx-handler/src/mutations.rs b/crates/pptx-handler/src/mutations.rs index 4a322ae..79e867e 100644 --- a/crates/pptx-handler/src/mutations.rs +++ b/crates/pptx-handler/src/mutations.rs @@ -1,6 +1,7 @@ use handler_common::HandlerError; use handler_common::InsertPosition; use oxml::OxmlPackage; +use std::ops::Range; /// Remove an element from the PPTX presentation. pub fn remove_element( @@ -88,8 +89,9 @@ pub fn copy_slide( .map_err(|e| HandlerError::OperationFailed(e.to_string()))?; // Create a new slide at the end - let new_slide_num = pres.slides.len() + 1; - let new_slide_path = format!("ppt/slides/slide{}.xml", new_slide_num); + let new_slide_index = pres.slides.len() + 1; + let new_slide_part_number = crate::add::next_slide_part_number(package); + let new_slide_path = format!("ppt/slides/slide{}.xml", new_slide_part_number); // Write the copied slide content package @@ -97,9 +99,10 @@ pub fn copy_slide( .map_err(|e| HandlerError::OperationFailed(e.to_string()))?; // Update presentation.xml to add the new slide reference - crate::add::update_presentation_slides(package, new_slide_num)?; + crate::add::update_presentation_slides(package, new_slide_part_number)?; + crate::add::register_slide_content_type(package, &new_slide_path)?; - Ok(format!("/slide[{}]", new_slide_num)) + Ok(format!("/slide[{}]", new_slide_index)) } /// Reorder the sldIdLst in presentation.xml by moving an entry from source to target position. @@ -178,24 +181,199 @@ fn reorder_sld_id_list(xml: &str, source: usize, target: usize) -> Result Result<(), HandlerError> { - let slide_path = format!("ppt/slides/slide{}.xml", slide_num); + let presentation_path = "ppt/presentation.xml"; + let presentation_rels_path = "ppt/_rels/presentation.xml.rels"; + let content_types_path = "[Content_Types].xml"; - // Remove the slide part - if package.has_part(&slide_path) { - package - .write_part(&slide_path, Vec::::new()) - .map_err(|e| HandlerError::OperationFailed(e.to_string()))?; + let presentation_xml = package + .read_part_xml(presentation_path) + .map_err(|e| HandlerError::OperationFailed(e.to_string()))?; + let (updated_presentation, relationship_id) = + remove_slide_references(&presentation_xml, slide_num)?; + + let presentation_rels = package + .part_rels(presentation_path) + .map_err(|e| HandlerError::OperationFailed(e.to_string()))?; + let relationship = presentation_rels.get(&relationship_id).ok_or_else(|| { + HandlerError::OperationFailed(format!( + "slide {} references missing relationship {}", + slide_num, relationship_id + )) + })?; + let slide_path = package.resolve_rel_target(presentation_path, &relationship.target); + if !package.has_part(&slide_path) { + return Err(HandlerError::OperationFailed(format!( + "slide {} part not found: {}", + slide_num, slide_path + ))); } + let presentation_rels_xml = package + .read_part_xml(presentation_rels_path) + .map_err(|e| HandlerError::OperationFailed(e.to_string()))?; + let updated_presentation_rels = remove_relationship(&presentation_rels_xml, &relationship_id)?; + + let content_types_xml = package + .read_part_xml(content_types_path) + .map_err(|e| HandlerError::OperationFailed(e.to_string()))?; + let updated_content_types = remove_content_type_override(&content_types_xml, &slide_path)?; + + package + .write_part_xml(presentation_path, &updated_presentation) + .map_err(|e| HandlerError::OperationFailed(e.to_string()))?; + package + .write_part_xml(presentation_rels_path, &updated_presentation_rels) + .map_err(|e| HandlerError::OperationFailed(e.to_string()))?; + package + .write_part_xml(content_types_path, &updated_content_types) + .map_err(|e| HandlerError::OperationFailed(e.to_string()))?; + + package + .remove_part(&slide_path) + .map_err(|e| HandlerError::OperationFailed(e.to_string()))?; + package + .remove_part(&crate::navigation::relationships_part_path(&slide_path)) + .map_err(|e| HandlerError::OperationFailed(e.to_string()))?; + Ok(()) } +/// Remove the selected slide ID and any custom-show entries that reference it. +/// +/// A custom show with no remaining slides is removed, and an empty custom-show +/// list is removed as well. Leaving these references dangling causes PowerPoint +/// to reject the presentation after the slide relationship is deleted. +fn remove_slide_references(xml: &str, slide_num: usize) -> Result<(String, String), HandlerError> { + if slide_num == 0 { + return Err(HandlerError::InvalidPath( + "slide indices are 1-based".to_string(), + )); + } + + let doc = roxmltree::Document::parse(xml) + .map_err(|e| HandlerError::OperationFailed(format!("invalid presentation.xml: {}", e)))?; + let slide_id_list = doc + .descendants() + .find(|node| node.is_element() && node.tag_name().name() == "sldIdLst") + .ok_or_else(|| { + HandlerError::OperationFailed("presentation has no slide ID list".to_string()) + })?; + let slide_id = slide_id_list + .children() + .filter(|node| node.is_element() && node.tag_name().name() == "sldId") + .nth(slide_num - 1) + .ok_or_else(|| HandlerError::PathNotFound(format!("slide {}", slide_num)))?; + let relationship_id = relationship_id_of(&slide_id).ok_or_else(|| { + HandlerError::OperationFailed(format!("slide {} has no relationship ID", slide_num)) + })?; + + let mut ranges = vec![slide_id.range()]; + if let Some(custom_show_list) = doc + .descendants() + .find(|node| node.is_element() && node.tag_name().name() == "custShowLst") + { + let shows: Vec<_> = custom_show_list + .children() + .filter(|node| node.is_element() && node.tag_name().name() == "custShow") + .collect(); + let mut removed_shows = 0; + let mut show_ranges = Vec::new(); + let mut entry_ranges = Vec::new(); + + for show in &shows { + let entries: Vec<_> = show + .descendants() + .filter(|node| node.is_element() && node.tag_name().name() == "sld") + .collect(); + let matching: Vec<_> = entries + .iter() + .filter(|node| { + relationship_id_of(node).as_deref() == Some(relationship_id.as_str()) + }) + .collect(); + + if entries.len() == matching.len() { + show_ranges.push(show.range()); + removed_shows += 1; + } else { + entry_ranges.extend(matching.into_iter().map(|node| node.range())); + } + } + + if removed_shows == shows.len() { + ranges.push(custom_show_list.range()); + } else { + ranges.extend(show_ranges); + ranges.extend(entry_ranges); + } + } + + Ok((remove_xml_ranges(xml, ranges), relationship_id)) +} + +fn remove_relationship(xml: &str, relationship_id: &str) -> Result { + let doc = roxmltree::Document::parse(xml).map_err(|e| { + HandlerError::OperationFailed(format!("invalid presentation relationships: {}", e)) + })?; + let relationship = doc + .descendants() + .find(|node| { + node.is_element() + && node.tag_name().name() == "Relationship" + && node.attribute("Id") == Some(relationship_id) + }) + .ok_or_else(|| { + HandlerError::OperationFailed(format!( + "presentation relationship {} not found", + relationship_id + )) + })?; + Ok(remove_xml_ranges(xml, vec![relationship.range()])) +} + +fn remove_content_type_override(xml: &str, slide_path: &str) -> Result { + let doc = roxmltree::Document::parse(xml) + .map_err(|e| HandlerError::OperationFailed(format!("invalid content types: {}", e)))?; + let part_name = format!("/{}", slide_path.trim_start_matches('/')); + let ranges = doc + .descendants() + .filter(|node| { + node.is_element() + && node.tag_name().name() == "Override" + && node.attribute("PartName") == Some(part_name.as_str()) + }) + .map(|node| node.range()) + .collect(); + Ok(remove_xml_ranges(xml, ranges)) +} + +fn relationship_id_of(node: &roxmltree::Node<'_, '_>) -> Option { + const RELATIONSHIPS_NS: &str = + "http://schemas.openxmlformats.org/officeDocument/2006/relationships"; + node.attribute((RELATIONSHIPS_NS, "id")) + .or_else(|| node.attribute("r:id")) + .map(str::to_string) +} + +fn remove_xml_ranges(xml: &str, mut ranges: Vec>) -> String { + ranges.sort_by(|left, right| right.start.cmp(&left.start)); + let mut result = xml.to_string(); + let mut last_start = xml.len(); + for range in ranges { + if range.end <= last_start { + result.replace_range(range.clone(), ""); + last_start = range.start; + } + } + result +} + fn remove_shape( package: &mut OxmlPackage, slide_num: usize, shape_idx: usize, ) -> Result<(), HandlerError> { - let slide_path = format!("ppt/slides/slide{}.xml", slide_num); + let slide_path = crate::navigation::resolve_slide_part_path(package, slide_num)?; let slide_xml = package .read_part_xml(&slide_path) @@ -349,3 +527,122 @@ fn parse_shape_idx(path: &str) -> Result { .and_then(|s| s.parse::().ok()) .ok_or_else(|| HandlerError::InvalidPath(path.to_string())) } + +#[cfg(test)] +mod tests { + use super::*; + + const PRESENTATION_XML: &str = r#" + + + + + + + + + +"#; + + const PRESENTATION_RELS_XML: &str = r#" + + + +"#; + + const CONTENT_TYPES_XML: &str = r#" + + + +"#; + + #[test] + fn remove_slide_references_prunes_custom_shows() { + let (updated, relationship_id) = remove_slide_references(PRESENTATION_XML, 2).unwrap(); + + assert_eq!(relationship_id, "rId5"); + assert!(!updated.contains(r#"id="257""#)); + assert!(!updated.contains(r#"r:id="rId5""#)); + assert!(!updated.contains(r#"name="drop""#)); + assert!(updated.contains(r#"name="keep""#)); + assert!(updated.contains(r#"r:id="rId2""#)); + roxmltree::Document::parse(&updated).unwrap(); + } + + #[test] + fn remove_slide_uses_relationship_target_and_removes_package_metadata() { + let mut package = OxmlPackage::create("unused.pptx"); + package.add_part("ppt/presentation.xml", PRESENTATION_XML.as_bytes()); + package.add_part( + "ppt/_rels/presentation.xml.rels", + PRESENTATION_RELS_XML.as_bytes(), + ); + package.add_part("[Content_Types].xml", CONTENT_TYPES_XML.as_bytes()); + package.add_part("ppt/slides/slide1.xml", b""); + package.add_part("ppt/slides/slide7.xml", b""); + package.add_part("ppt/slides/_rels/slide7.xml.rels", b""); + + remove_slide(&mut package, 2).unwrap(); + + assert!(package.has_part("ppt/slides/slide1.xml")); + assert!(!package.has_part("ppt/slides/slide7.xml")); + assert!(!package.has_part("ppt/slides/_rels/slide7.xml.rels")); + assert!(!package + .read_part_xml("ppt/presentation.xml") + .unwrap() + .contains("rId5")); + assert!(!package + .read_part_xml("ppt/_rels/presentation.xml.rels") + .unwrap() + .contains("rId5")); + assert!(!package + .read_part_xml("[Content_Types].xml") + .unwrap() + .contains("slide7.xml")); + } + + #[test] + fn add_after_removal_uses_new_part_and_unique_ids() { + let mut package = OxmlPackage::create("unused.pptx"); + package.add_part("ppt/presentation.xml", PRESENTATION_XML.as_bytes()); + package.add_part( + "ppt/_rels/presentation.xml.rels", + PRESENTATION_RELS_XML.as_bytes(), + ); + package.add_part("[Content_Types].xml", CONTENT_TYPES_XML.as_bytes()); + package.add_part("ppt/slides/slide1.xml", b""); + package.add_part("ppt/slides/slide7.xml", b""); + + remove_slide(&mut package, 1).unwrap(); + assert_eq!( + crate::navigation::resolve_slide_part_path(&package, 1).unwrap(), + "ppt/slides/slide7.xml" + ); + + let created = crate::add::add_element( + &mut package, + "/presentation", + "slide", + InsertPosition::Append, + &std::collections::HashMap::new(), + ) + .unwrap(); + + assert_eq!(created, "/slide[2]"); + assert!(package.has_part("ppt/slides/slide7.xml")); + assert!(package.has_part("ppt/slides/slide8.xml")); + let presentation = package.read_part_xml("ppt/presentation.xml").unwrap(); + assert!(presentation.contains(r#"id="258""#)); + assert!(presentation.contains(r#"r:id="rId6""#)); + let relationships = package + .read_part_xml("ppt/_rels/presentation.xml.rels") + .unwrap(); + assert!(relationships.contains(r#"Id="rId6""#)); + assert!(relationships.contains(r#"Target="slides/slide8.xml""#)); + assert!(package + .read_part_xml("[Content_Types].xml") + .unwrap() + .contains(r#"PartName="/ppt/slides/slide8.xml""#)); + } +} diff --git a/crates/pptx-handler/src/navigation.rs b/crates/pptx-handler/src/navigation.rs index fec9f54..2f013a7 100644 --- a/crates/pptx-handler/src/navigation.rs +++ b/crates/pptx-handler/src/navigation.rs @@ -86,6 +86,39 @@ pub fn build_presentation( Ok(Presentation { slides }) } +/// Resolve a 1-based logical slide index to its actual OOXML part path. +/// +/// Physical part names are not required to be contiguous. After deleting or +/// reordering slides, logical `/slide[2]` may point to `slide3.xml`, so callers +/// must resolve through presentation relationships instead of constructing a +/// filename from the logical index. +pub fn resolve_slide_part_path( + package: &oxml::OxmlPackage, + slide_index: usize, +) -> Result { + if slide_index == 0 { + return Err(handler_common::HandlerError::InvalidPath( + "slide indices are 1-based".to_string(), + )); + } + build_presentation(package)? + .slides + .into_iter() + .find(|slide| slide.index == slide_index) + .map(|slide| slide.part_path) + .ok_or_else(|| handler_common::HandlerError::PathNotFound(format!("slide {}", slide_index))) +} + +/// Return the package relationship-part path for an OOXML part. +pub fn relationships_part_path(part_path: &str) -> String { + match part_path.rsplit_once('/') { + Some((directory, file_name)) => { + format!("{}/_rels/{}.rels", directory, file_name) + } + None => format!("_rels/{}.rels", part_path), + } +} + /// Parse from presentation.xml. /// Uses roxmltree for namespace-aware attribute parsing (r:id requires namespace resolution). fn parse_slide_id_list( diff --git a/crates/pptx-handler/src/raw.rs b/crates/pptx-handler/src/raw.rs index a702442..4e3646c 100644 --- a/crates/pptx-handler/src/raw.rs +++ b/crates/pptx-handler/src/raw.rs @@ -98,8 +98,8 @@ pub fn add_part( .find(']') .and_then(|pos| stripped[..pos].parse::().ok()) .ok_or_else(|| HandlerError::InvalidPath(parent.to_string()))?; - let _slide_path = format!("ppt/slides/slide{}.xml", slide_num); - let rels_path = format!("ppt/slides/_rels/slide{}.xml.rels", slide_num); + let slide_path = crate::navigation::resolve_slide_part_path(package, slide_num)?; + let rels_path = crate::navigation::relationships_part_path(&slide_path); let rel_type = "http://schemas.openxmlformats.org/officeDocument/2006/relationships/image"; diff --git a/crates/pptx-handler/src/view.rs b/crates/pptx-handler/src/view.rs index b544b4e..4648f3a 100644 --- a/crates/pptx-handler/src/view.rs +++ b/crates/pptx-handler/src/view.rs @@ -1320,7 +1320,7 @@ pub fn apply_pptx_range_highlights( } for (slide_num, segs) in slide_segs { - let slide_path = format!("ppt/slides/slide{}.xml", slide_num); + let slide_path = crate::navigation::resolve_slide_part_path(package, slide_num)?; let slide_xml = package .read_part_xml(&slide_path) .map_err(|e| HandlerError::OperationFailed(e.to_string()))?; diff --git a/docs/csharp-parity/README.zh.md b/docs/csharp-parity/README.zh.md new file mode 100644 index 0000000..dcd6c7c --- /dev/null +++ b/docs/csharp-parity/README.zh.md @@ -0,0 +1,151 @@ +# OfficeCLI C# → Rust 对齐基线 + +本文档是 Rust 迁移的持久入口。后续对齐工作先读取本目录，不再重新扫描整个 +C# 仓库；只有 `source/OfficeCLI` 的上游提交发生变化时，才检查基线之后的增量。 + +## 当前基线 + +| 项目 | 值 | +|---|---| +| 记录日期 | 2026-07-20 | +| C# 源码目录 | `source/OfficeCLI` | +| C# 上游 | `iOfficeAI/OfficeCLI` 的 `origin/main` | +| C# 基线提交 | `0b3557bbec29f073f5df6b92b4b8dcefa7e3c160` | +| C# 版本 | `1.0.139` | +| Rust 基线提交 | `06f0d89cd8d033b04e3fa6ca9ce3497bbbde55d6` | +| Rust 版本 | `0.1.17` | +| 同步前 C# 本地快照 | 分支 `local/pre-upstream-sync-20260720`，提交 `9f69b9b1` | + +`source/OfficeCLI/main` 已快进到 `origin/main`，工作区干净。同步前发现的 327 个 +暂存文件没有丢失，已经保存在上述本地快照分支。 + +## 后续增量流程 + +1. 获取上游，但不要覆盖本地改动： + + ```bash + git -C source/OfficeCLI fetch --prune origin main + git -C source/OfficeCLI status --short --branch + ``` + +2. 仅检查本基线之后的变化： + + ```bash + git -C source/OfficeCLI log --reverse --oneline \ + 0b3557bbec29f073f5df6b92b4b8dcefa7e3c160..origin/main + git -C source/OfficeCLI diff --stat \ + 0b3557bbec29f073f5df6b92b4b8dcefa7e3c160..origin/main + ``` + +3. 按提交和文件格式更新 `migration-ledger.tsv`。一个迁移批次只处理一个可独立 + 验证的行为，避免 DOCX、XLSX、PPTX 逻辑混在同一修改中。 + +4. 完成并验证增量后，更新本页的 C# 基线提交和版本。若基线未变化，直接从 + ledger 中首个 `missing` 或 `partial` 条目继续。 + +## 已完成的静态盘点 + +本次已经检查： + +- C# 与 Rust 根命令面； +- `schemas/help` 的逐文件差异； +- DOCX、XLSX、PPTX handler 模块与最新 C# 修复提交； +- Rust 中显式的 `TODO`、`unimplemented!` 和 “not implemented”； +- skills 目录及安装/加载命令面。 + +这是一份功能面和高风险行为盘点，不代表所有属性组合已完成行为级测试。 + +### 根命令 + +Rust 已有主要文档命令： + +`open`、`close`、`watch`、`unwatch`、`view`、`get`、`query`、`set`、`add`、 +`remove`、`move`、`swap`、`refresh`、`raw`、`raw-set`、`add-part`、 +`validate`、`save`、`batch`、`dump`、`import`、`create`、`merge`、 +`plugins`、`help`、`install`、`skills` 和 `mcp`。 + +仍需对齐的命令行为： + +- `--output-schema-crc` 缺失； +- `load_skill [name] [--path relpath]` 缺失； +- `config [value]` 缺失； +- C# 的 `mcp list`、`mcp `、`mcp uninstall ` 生命周期管理缺失； +- `skill`/`skills` 别名、skills 自动探测目标和引用文件安装语义不完整； +- C# 支持 `watch mark|unmark|marks|goto`，Rust 目前主要保留隐藏的顶层 + 兼容命令； +- `--help` 统一转发到 schema 驱动的 `help` 尚未完全对齐。 + +Rust 额外提供 `extract-text`、`convert`、`info` 和原生 PDF handler。这些是 Rust +扩展，不应为了 C# 对齐而删除。 + +### Help schema + +C# 有 150 个 schema JSON，Rust 有 139 个。Rust 缺少： + +- DOCX：`abstractNum`、`diagram`、`level`、`num`、`permStart`、`revision`、 + `shape`、`tab`、`textbox`； +- PPTX：`diagram`、`linebreak`； +- XLSX：`detectedtable`。 + +Rust 独有 `docx/trackedchange.json`；需要先确认它能否完整替代 C# 的 +`revision.json`，不能只按文件名直接覆盖。 + +### Handler 风险排序 + +P0 表示可能导致文件损坏、引用悬空或作用域错误： + +- PPTX 删除幻灯片的包级引用清理； +- PPTX 逻辑幻灯片索引与物理 part 名称解耦，避免删除中间页后覆盖或编辑错页； +- XLSX 插入、移动、删除工作表时 `definedName@localSheetId` 作用域调整； +- OOXML 修改的崩溃原子性和 resident 请求期间的生命周期。 + +P1 表示主要功能缺口： + +- XLSX 动态数组、现代公式边界语义和 in-cell image/richValue； +- DOCX 编号定义、修订/权限范围、diagram/shape/textbox 的结构化增删改； +- PPTX diagram、line break、动画/过渡、现代评论及演示文稿级设置的行为覆盖； +- 缺失 schema 与 handler 支持同步迁移； +- `load_skill`、schema CRC 和 MCP 安装生命周期。 + +P2 表示兼容性和体验： + +- help 路由、命令别名、skills 多客户端安装布局； +- C# 的更新检查、配置和遥测类外围能力； +- HTML/SVG 预览与 Office 实际渲染的细节差异。 + +详细状态、源码位置和下一步见 +[`migration-ledger.tsv`](migration-ledger.tsv)。 + +## 每批验证门槛 + +每个条目至少需要： + +```bash +cargo fmt -- --check +cargo clippy --all-targets -- -D warnings +cargo test --workspace +``` + +文档修改还必须提供一个命令级 before/after 流程，并在适用时执行： + +```bash +cargo run -- validate +``` + +对 DOCX、XLSX、PPTX 的包级修改，测试必须分别检查该格式的 XML part、 +relationship part 和 `[Content_Types].xml`，不能用另一格式的实现推断已对齐。 + +## 当前迁移批次 + +`PPTX-001` 已实现： + +- 删除逻辑幻灯片时通过 presentation relationship 找到真实 part； +- 删除 `p:sldId`、presentation relationship、slide part、slide rels 和 content + type override； +- 清除 custom show 中的悬空引用，并删除空 show/list； +- 后续编辑通过关系解析逻辑索引，不再拼接 `slideN.xml`； +- 删除中间页后再新增时使用未占用的 part、slide ID 和 relationship ID，并注册 + content type，避免覆盖现存幻灯片。 + +验证覆盖纯包级 custom-show 场景，以及 +`create → add → remove middle → edit logical slide → add → validate` 的 CLI 流程。 diff --git a/docs/csharp-parity/migration-ledger.tsv b/docs/csharp-parity/migration-ledger.tsv new file mode 100644 index 0000000..f0428f4 --- /dev/null +++ b/docs/csharp-parity/migration-ledger.tsv @@ -0,0 +1,24 @@ +id format area status priority csharp_reference rust_reference verification next_action +BASE-001 all upstream baseline aligned P0 0b3557bb (v1.0.139) source/OfficeCLI main clean; backup branch retained compare only baseline..origin/main +CLI-001 all core document verbs partial P0 src/officecli/CommandBuilder.cs crates/officecli/src/commands/mod.rs command-level parity tests audit options and JSON output per verb +CLI-002 all schema fingerprint missing P1 src/officecli/Program.cs; Help/SchemaCrc.cs - officecli --output-schema-crc returns stable CRC32 implement deterministic schemas/help CRC +CLI-003 all load_skill missing P1 src/officecli/Program.cs; Core/SkillInstaller.cs crates/officecli/src/commands/skills.rs catalog, SKILL.md and --path tests add read-only command without install side effects +CLI-004 all MCP lifecycle partial P1 src/officecli/Program.cs crates/officecli/src/mcp.rs mcp server/list/install/uninstall tests add target registration management +CLI-005 all watch nested commands partial P2 src/officecli/CommandBuilder.Watch.cs crates/officecli/src/commands/mod.rs watch mark/unmark/marks/goto tests keep hidden top-level aliases for compatibility +CLI-006 all config/update behavior missing P2 src/officecli/Core/UpdateChecker.cs - config read/write tests decide which non-document behavior belongs in Rust +SCHEMA-DOCX-001 docx help schemas partial P1 schemas/help/docx schemas/help/docx exact schema set comparison migrate 9 missing schemas with handler support +SCHEMA-XLSX-001 xlsx help schemas partial P1 schemas/help/xlsx schemas/help/xlsx exact schema set comparison add detectedtable +SCHEMA-PPTX-001 pptx help schemas partial P1 schemas/help/pptx schemas/help/pptx exact schema set comparison add diagram and linebreak +DOCX-001 docx revision schema naming partial P1 schemas/help/docx/revision.json schemas/help/docx/trackedchange.json property and verb comparison decide alias versus separate element +DOCX-002 docx numbering and permission structures missing P1 Handlers/Word; abstractNum/num/level/permStart schemas crates/docx-handler/src structural add/get/set/remove tests migrate as separate DOCX batches +DOCX-003 docx diagram/shape/textbox structured operations partial P1 Handlers/Word; matching schemas crates/docx-handler/src/add.rs; text_offset.rs round-trip XML tests separate read, add and mutation support +XLSX-001 xlsx definedName localSheetId on sheet edits missing P0 423f17fe; 58b8f970; e89bd6fb crates/xlsx-handler/src/mutations.rs insert/move/remove sheet scope tests port upstream scope-shift behavior +XLSX-002 xlsx modern formulas and dynamic arrays partial P1 Handlers/Excel/Formula* crates/xlsx-handler/src/formula array shape/error and function corpus tests compare evaluator by function families +XLSX-003 xlsx in-cell image richValue missing P1 205fd5fd and follow-up fixes crates/xlsx-handler/src Excel open/round-trip package tests migrate as isolated feature +XLSX-004 xlsx detected table missing P1 schemas/help/xlsx/detectedtable.json - read/query tests add schema and detection behavior +PPTX-001 pptx slide removal and identity cleanup implemented P0 4278f151; PowerPointHandler.Mutations.cs crates/pptx-handler/src/mutations.rs; navigation.rs; add.rs package tests plus remove/edit/add CLI smoke remove refs/custom shows; resolve logical index; allocate unique part/id/rel +PPTX-002 pptx group resize axis/aspect semantics missing P0 45e432b3 crates/pptx-handler/src/add.rs; view.rs independent x/y and keepAspect tests port after PPTX-001 +PPTX-003 pptx diagram and line break partial P1 schemas/help/pptx/diagram.json; linebreak.json crates/pptx-handler/src schema plus round-trip tests implement separately +PPTX-004 pptx presentation advanced features partial P1 Handlers/Pptx/*.cs crates/pptx-handler/src feature-specific Office round trips split animations, comments, settings and theme into atomic batches +PKG-001 ooxml remove part primitive implemented P0 OpenXmlPackage.DeletePart crates/oxml/src/package.rs PPTX-001 package test retain caller responsibility for rels/content types +RUST-EXT-001 pdf native PDF handler extension P2 - crates/pdf-handler workspace tests maintain independently from C# OOXML parity