diff --git a/crates/officecli/tests/cli_smoke.rs b/crates/officecli/tests/cli_smoke.rs
index 02804b3..32d1a4e 100644
--- a/crates/officecli/tests/cli_smoke.rs
+++ b/crates/officecli/tests/cli_smoke.rs
@@ -617,6 +617,92 @@ fn test_xlsx_view_outline() {
.stdout(predicate::str::contains("/Sheet1"));
}
+#[test]
+fn test_xlsx_sheet_order_mutations_preserve_defined_name_scopes() {
+ let tmp = temp_dir();
+ let path = tmp.path().join("test_xlsx_sheet_order.xlsx");
+ let p = path.to_string_lossy().to_string();
+
+ officecli().args(["create", &p]).assert().success();
+ officecli()
+ .args([
+ "add",
+ &p,
+ "--parent",
+ "/",
+ "--type-name",
+ "sheet",
+ "--properties",
+ "name=Second",
+ ])
+ .assert()
+ .success();
+ officecli()
+ .args([
+ "add",
+ &p,
+ "--parent",
+ "/",
+ "--type-name",
+ "sheet",
+ "--position",
+ "1",
+ "--properties",
+ "name=Inserted",
+ ])
+ .assert()
+ .success();
+
+ // Inject three sheet-scoped names so the following CLI mutations exercise
+ // localSheetId remapping rather than only visible sheet order.
+ {
+ let mut package = oxml::OxmlPackage::open(&p, true).unwrap();
+ let workbook = package.read_part_xml("xl/workbook.xml").unwrap();
+ let defined_names = r#"
+Sheet1!$A$1
+Inserted!$A$1
+Second!$A$1
+"#;
+ let updated = workbook.replace("", &format!("{}", defined_names));
+ package.write_part_xml("xl/workbook.xml", &updated).unwrap();
+ package.save().unwrap();
+ }
+
+ officecli()
+ .args(["move", &p, "/Sheet1", "--position", "after:/Second"])
+ .assert()
+ .success();
+
+ {
+ let package = oxml::OxmlPackage::open(&p, false).unwrap();
+ let workbook = package.read_part_xml("xl/workbook.xml").unwrap();
+ let inserted = workbook.find(r#"name="Inserted""#).unwrap();
+ let second = workbook.find(r#"name="Second""#).unwrap();
+ let sheet1 = workbook.find(r#"name="Sheet1""#).unwrap();
+ assert!(inserted < second && second < sheet1);
+ assert!(workbook.contains(r#"name="scopeSheet1" localSheetId="2""#));
+ assert!(workbook.contains(r#"name="scopeInserted" localSheetId="0""#));
+ assert!(workbook.contains(r#"name="scopeSecond" localSheetId="1""#));
+ }
+
+ officecli()
+ .args(["remove", &p, "/Second"])
+ .assert()
+ .success();
+ officecli()
+ .args(["validate", &p])
+ .assert()
+ .success()
+ .stdout(predicate::str::contains("No validation errors"));
+
+ let package = oxml::OxmlPackage::open(&p, false).unwrap();
+ let workbook = package.read_part_xml("xl/workbook.xml").unwrap();
+ assert!(!workbook.contains(r#"name="Second""#));
+ assert!(!workbook.contains(r#"name="scopeSecond""#));
+ assert!(workbook.contains(r#"name="scopeSheet1" localSheetId="1""#));
+ assert!(workbook.contains(r#"name="scopeInserted" localSheetId="0""#));
+}
+
// ═══════════════════════════════════════════════════════════════════════
// PPTX-specific: add slide + textbox
// ═══════════════════════════════════════════════════════════════════════
diff --git a/crates/xlsx-handler/src/add.rs b/crates/xlsx-handler/src/add.rs
index 3d9906f..dcdd9fd 100644
--- a/crates/xlsx-handler/src/add.rs
+++ b/crates/xlsx-handler/src/add.rs
@@ -171,7 +171,7 @@ fn add_cell(
fn add_sheet(
package: &mut OxmlPackage,
_parent: &str,
- _position: InsertPosition,
+ position: InsertPosition,
properties: &HashMap,
) -> Result {
let name = properties.get("name").ok_or_else(|| {
@@ -188,8 +188,19 @@ fn add_sheet(
)));
}
- let new_sheet_index = model.sheets.len() + 1;
- let part_path = format!("xl/worksheets/sheet{}.xml", new_sheet_index);
+ let workbook_xml = package
+ .read_part_xml("xl/workbook.xml")
+ .map_err(|e| HandlerError::OperationFailed(e.to_string()))?;
+ let workbook_rels_path = "xl/_rels/workbook.xml.rels";
+ let workbook_rels = package.read_part_xml(workbook_rels_path).map_err(|e| {
+ HandlerError::OperationFailed(format!("failed to read workbook rels: {}", e))
+ })?;
+ let entries = crate::mutations::workbook_sheet_entries(&workbook_xml)?;
+ let insert_index = resolve_sheet_insert_index(&entries, &position)?;
+ let part_number = next_worksheet_part_number(package);
+ let sheet_id = next_workbook_sheet_id(&workbook_xml)?;
+ let relationship_id = next_workbook_relationship_id(&workbook_rels)?;
+ let part_path = format!("xl/worksheets/sheet{}.xml", part_number);
// Create minimal worksheet XML
let sheet_xml = "\n\
@@ -203,60 +214,177 @@ fn add_sheet(
.write_part_xml(&part_path, &sheet_xml)
.map_err(|e| HandlerError::SaveError(e.to_string()))?;
- // Update workbook.xml to include the new sheet
- let wb_xml = package
- .read_part_xml("xl/workbook.xml")
- .map_err(|e| HandlerError::OperationFailed(e.to_string()))?;
-
- // Find and insert before it
let new_sheet_entry = format!(
- "",
- name, new_sheet_index, new_sheet_index
+ "",
+ escape_xml_attribute(name),
+ sheet_id,
+ relationship_id
);
+ let shifted_workbook = crate::mutations::rewrite_defined_name_scopes(&workbook_xml, |scope| {
+ if scope as usize >= insert_index {
+ Some(scope + 1)
+ } else {
+ Some(scope)
+ }
+ })?;
+ let modified_workbook = insert_sheet_entry(&shifted_workbook, insert_index, &new_sheet_entry)?;
- let modified_wb = if let Some(sheets_end) = wb_xml.find("") {
- let mut result = wb_xml[..sheets_end].to_string();
- result.push_str(&new_sheet_entry);
- result.push_str(&wb_xml[sheets_end..]);
+ let new_rel = format!(
+ "",
+ relationship_id, part_number
+ );
+ let modified_rels = if let Some(rels_end) = workbook_rels.find("") {
+ let mut result = workbook_rels[..rels_end].to_string();
+ result.push_str(&new_rel);
+ result.push_str(&workbook_rels[rels_end..]);
result
} else {
return Err(HandlerError::OperationFailed(
- "no in workbook.xml".to_string(),
+ "no in workbook rels".to_string(),
));
};
+ let content_types_path = "[Content_Types].xml";
+ let content_types = package
+ .read_part_xml(content_types_path)
+ .map_err(|e| HandlerError::OperationFailed(e.to_string()))?;
+ let modified_content_types = register_worksheet_content_type(&content_types, &part_path)?;
+
+ package
+ .write_part_xml("xl/workbook.xml", &modified_workbook)
+ .map_err(|e| HandlerError::SaveError(e.to_string()))?;
+ package
+ .write_part_xml(workbook_rels_path, &modified_rels)
+ .map_err(|e| HandlerError::SaveError(e.to_string()))?;
package
- .write_part_xml("xl/workbook.xml", &modified_wb)
+ .write_part_xml(content_types_path, &modified_content_types)
.map_err(|e| HandlerError::SaveError(e.to_string()))?;
- // Update workbook relationships
- let rels_xml = package
- .read_part_xml("xl/_rels/workbook.xml.rels")
- .map_err(|e| {
- HandlerError::OperationFailed(format!("failed to read workbook rels: {}", e))
- })?;
+ Ok(format!("/{}", name))
+}
- let new_rel = format!(
- "",
- new_sheet_index, new_sheet_index
- );
+fn resolve_sheet_insert_index(
+ entries: &[crate::mutations::WorkbookSheetEntry],
+ position: &InsertPosition,
+) -> Result {
+ let anchor_name = |path: &str| path.trim().trim_start_matches('/').to_string();
+ match position {
+ InsertPosition::AtIndex(index) => Ok((*index).min(entries.len())),
+ InsertPosition::BeforeElement(anchor) => {
+ let anchor = anchor_name(anchor);
+ entries
+ .iter()
+ .position(|entry| entry.name == anchor)
+ .ok_or_else(|| HandlerError::PathNotFound(format!("sheet '{}'", anchor)))
+ }
+ InsertPosition::AfterElement(anchor) => {
+ let anchor = anchor_name(anchor);
+ entries
+ .iter()
+ .position(|entry| entry.name == anchor)
+ .map(|index| index + 1)
+ .ok_or_else(|| HandlerError::PathNotFound(format!("sheet '{}'", anchor)))
+ }
+ InsertPosition::Append => Ok(entries.len()),
+ }
+}
- let modified_rels = if let Some(rels_end) = rels_xml.find("") {
- let mut result = rels_xml[..rels_end].to_string();
- result.push_str(&new_rel);
- result.push_str(&rels_xml[rels_end..]);
- result
+fn insert_sheet_entry(
+ workbook_xml: &str,
+ insert_index: usize,
+ entry_xml: &str,
+) -> Result {
+ let entries = crate::mutations::workbook_sheet_entries(workbook_xml)?;
+ let insert_at = if insert_index < entries.len() {
+ entries[insert_index].range.start
} else {
- return Err(HandlerError::OperationFailed(
- "no in workbook rels".to_string(),
- ));
+ let doc = roxmltree::Document::parse(workbook_xml)
+ .map_err(|e| HandlerError::OperationFailed(format!("invalid workbook.xml: {}", e)))?;
+ let sheets = doc
+ .descendants()
+ .find(|node| node.is_element() && node.tag_name().name() == "sheets")
+ .ok_or_else(|| {
+ HandlerError::OperationFailed("workbook has no sheets list".to_string())
+ })?;
+ let range = sheets.range();
+ let container = &workbook_xml[range.clone()];
+ range.start
+ + container
+ .rfind(" usize {
package
- .write_part_xml("xl/_rels/workbook.xml.rels", &modified_rels)
- .map_err(|e| HandlerError::SaveError(e.to_string()))?;
+ .list_parts()
+ .into_iter()
+ .filter_map(|path| {
+ path.strip_prefix("xl/worksheets/sheet")
+ .and_then(|value| value.strip_suffix(".xml"))
+ .and_then(|value| value.parse::().ok())
+ })
+ .max()
+ .unwrap_or(0)
+ + 1
+}
- Ok(format!("/{}", name))
+fn next_workbook_sheet_id(workbook_xml: &str) -> Result {
+ let doc = roxmltree::Document::parse(workbook_xml)
+ .map_err(|e| HandlerError::OperationFailed(format!("invalid workbook.xml: {}", e)))?;
+ Ok(doc
+ .descendants()
+ .filter(|node| node.is_element() && node.tag_name().name() == "sheet")
+ .filter_map(|node| node.attribute("sheetId"))
+ .filter_map(|value| value.parse::().ok())
+ .max()
+ .unwrap_or(0)
+ + 1)
+}
+
+fn next_workbook_relationship_id(rels_xml: &str) -> Result {
+ let doc = roxmltree::Document::parse(rels_xml).map_err(|e| {
+ HandlerError::OperationFailed(format!("invalid workbook relationships: {}", e))
+ })?;
+ let next = doc
+ .descendants()
+ .filter(|node| node.is_element() && node.tag_name().name() == "Relationship")
+ .filter_map(|node| node.attribute("Id"))
+ .filter_map(|value| value.strip_prefix("rId"))
+ .filter_map(|value| value.parse::().ok())
+ .max()
+ .unwrap_or(0)
+ + 1;
+ Ok(format!("rId{}", next))
+}
+
+fn register_worksheet_content_type(xml: &str, part_path: &str) -> Result {
+ let part_name = format!("/{}", part_path.trim_start_matches('/'));
+ if xml.contains(&format!("PartName=\"{}\"", part_name)) {
+ return Ok(xml.to_string());
+ }
+ let entry = format!(
+ "",
+ part_name
+ );
+ let close = xml.find("").ok_or_else(|| {
+ HandlerError::OperationFailed("invalid [Content_Types].xml: missing ".to_string())
+ })?;
+ let mut result = xml.to_string();
+ result.insert_str(close, &entry);
+ Ok(result)
+}
+
+fn escape_xml_attribute(value: &str) -> String {
+ value
+ .replace('&', "&")
+ .replace('<', "<")
+ .replace('>', ">")
+ .replace('"', """)
+ .replace('\'', "'")
}
// ─── New Element Types ─────────────────────────────────────────────────
diff --git a/crates/xlsx-handler/src/handler.rs b/crates/xlsx-handler/src/handler.rs
index e881240..8d167c6 100644
--- a/crates/xlsx-handler/src/handler.rs
+++ b/crates/xlsx-handler/src/handler.rs
@@ -215,7 +215,7 @@ impl DocumentHandler for ExcelHandler {
&self,
source: &str,
target_parent: Option<&str>,
- _position: InsertPosition,
+ position: InsertPosition,
) -> Result {
if !self.editable {
return Err(HandlerError::OperationFailed(
@@ -223,7 +223,12 @@ impl DocumentHandler for ExcelHandler {
));
}
let mut pkg = self.package.borrow_mut();
- mutations::move_cell(&mut pkg, source, target_parent)
+ let parsed = navigation::parse_path(source)?;
+ if parsed.sheet_name.is_some() && parsed.cell_ref.is_none() {
+ mutations::move_sheet(&mut pkg, source, target_parent, position)
+ } else {
+ mutations::move_cell(&mut pkg, source, target_parent)
+ }
}
fn copy_from(
diff --git a/crates/xlsx-handler/src/mutations.rs b/crates/xlsx-handler/src/mutations.rs
index c1241e9..ca1e81f 100644
--- a/crates/xlsx-handler/src/mutations.rs
+++ b/crates/xlsx-handler/src/mutations.rs
@@ -4,9 +4,11 @@ use crate::helpers;
use crate::navigation;
use handler_common::{
self, extract_find_replace_props, replace_in_string, FindReplaceOptions, HandlerError,
+ InsertPosition,
};
use oxml::OxmlPackage;
use std::collections::HashMap;
+use std::ops::Range;
/// Remove an element from the workbook.
/// Supported paths:
@@ -81,41 +83,153 @@ fn remove_cell(
fn remove_sheet(package: &mut OxmlPackage, sheet_name: &str) -> Result<(), HandlerError> {
let model = helpers::build_workbook_model(package).map_err(HandlerError::OperationFailed)?;
+ if model.sheets.len() <= 1 {
+ return Err(HandlerError::InvalidArgument(
+ "cannot remove the workbook's only worksheet".to_string(),
+ ));
+ }
+
let ws = model
.sheets
.iter()
.find(|s| s.name == sheet_name)
.ok_or_else(|| HandlerError::PathNotFound(format!("sheet '{}'", sheet_name)))?;
- // Remove the sheet part from the package
- if package.has_part(&ws.part_path) {
- package
- .write_part(&ws.part_path, Vec::::new())
- .map_err(|e| HandlerError::OperationFailed(e.to_string()))?;
- }
-
- // Remove the entry from workbook.xml
let wb_xml = package
.read_part_xml("xl/workbook.xml")
.map_err(|e| HandlerError::OperationFailed(e.to_string()))?;
+ let entries = workbook_sheet_entries(&wb_xml)?;
+ let removed_index = entries
+ .iter()
+ .position(|entry| entry.name == sheet_name)
+ .ok_or_else(|| HandlerError::PathNotFound(format!("sheet '{}'", sheet_name)))?;
+ let removed = &entries[removed_index];
+ let workbook_without_sheet = remove_ranges(&wb_xml, vec![removed.range.clone()]);
+ let updated_workbook = rewrite_defined_name_scopes(&workbook_without_sheet, |scope| {
+ let scope = scope as usize;
+ if scope == removed_index {
+ None
+ } else if scope > removed_index {
+ Some((scope - 1) as u32)
+ } else {
+ Some(scope as u32)
+ }
+ })?;
- // Find the sheet entry by name
- let sheet_entry_pattern = format!("name=\"{}\"", sheet_name);
- if let Some(name_pos) = wb_xml.find(&sheet_entry_pattern) {
- // Find the element containing this name
- let element_start = wb_xml[..name_pos].rfind(",
+ position: InsertPosition,
+) -> Result {
+ let source_path = navigation::parse_path(source)?;
+ let source_name = source_path
+ .sheet_name
+ .ok_or_else(|| HandlerError::InvalidPath("move source requires a sheet".to_string()))?;
+ if source_path.cell_ref.is_some() {
+ return Err(HandlerError::InvalidPath(
+ "worksheet move source must not include a cell".to_string(),
+ ));
+ }
+
+ let workbook_xml = package
+ .read_part_xml("xl/workbook.xml")
+ .map_err(|e| HandlerError::OperationFailed(e.to_string()))?;
+ let entries = workbook_sheet_entries(&workbook_xml)?;
+ let old_order: Vec = entries.iter().map(|entry| entry.name.clone()).collect();
+ let source_index = old_order
+ .iter()
+ .position(|name| name == &source_name)
+ .ok_or_else(|| HandlerError::PathNotFound(format!("sheet '{}'", source_name)))?;
+
+ let mut new_entries = entries.clone();
+ let moved = new_entries.remove(source_index);
+ let anchor_name = |path: &str| path.trim().trim_start_matches('/').to_string();
+ let insert_index = match position {
+ InsertPosition::AtIndex(index) => index.min(new_entries.len()),
+ InsertPosition::BeforeElement(anchor) => {
+ let anchor = anchor_name(&anchor);
+ new_entries
+ .iter()
+ .position(|entry| entry.name == anchor)
+ .ok_or_else(|| HandlerError::PathNotFound(format!("sheet '{}'", anchor)))?
+ }
+ InsertPosition::AfterElement(anchor) => {
+ let anchor = anchor_name(&anchor);
+ new_entries
+ .iter()
+ .position(|entry| entry.name == anchor)
+ .map(|index| index + 1)
+ .ok_or_else(|| HandlerError::PathNotFound(format!("sheet '{}'", anchor)))?
+ }
+ InsertPosition::Append => {
+ if let Some(target) = target_parent.filter(|target| !matches!(*target, "" | "/")) {
+ let target = anchor_name(target);
+ new_entries
+ .iter()
+ .position(|entry| entry.name == target)
+ .ok_or_else(|| HandlerError::PathNotFound(format!("sheet '{}'", target)))?
+ } else {
+ new_entries.len()
+ }
+ }
+ };
+ new_entries.insert(insert_index, moved);
+
+ let new_order: Vec = new_entries.iter().map(|entry| entry.name.clone()).collect();
+ if old_order == new_order {
+ return Ok(format!("/{}", source_name));
+ }
+
+ let reordered = rewrite_sheet_order(&workbook_xml, &new_entries)?;
+ let updated = rewrite_defined_name_scopes(&reordered, |scope| {
+ let old_index = scope as usize;
+ if old_index >= old_order.len() {
+ return Some(scope);
+ }
+ new_order
+ .iter()
+ .position(|name| name == &old_order[old_index])
+ .map(|index| index as u32)
+ })?;
+ package
+ .write_part_xml("xl/workbook.xml", &updated)
+ .map_err(|e| HandlerError::SaveError(e.to_string()))?;
+ Ok(format!("/{}", source_name))
+}
+
/// Move a cell's content from source to target.
/// Source: /SheetName/A1, Target: /SheetName/B1 (or different sheet)
pub fn move_cell(
@@ -300,27 +414,223 @@ pub fn swap_cells(
Ok((path1_str, path2_str))
}
-/// Find the end position of an XML element (handles both self-closing and regular closing tags).
-fn find_element_end(xml: &str, start: usize, tag: &str) -> usize {
- // Check if self-closing: look for /> before >
- let first_gt = xml[start..]
- .find('>')
- .map(|pos| start + pos)
- .unwrap_or(xml.len());
+#[derive(Clone, Debug)]
+pub(crate) struct WorkbookSheetEntry {
+ pub name: String,
+ pub relationship_id: String,
+ pub xml: String,
+ pub range: Range,
+}
+
+pub(crate) fn workbook_sheet_entries(
+ workbook_xml: &str,
+) -> Result, HandlerError> {
+ const RELATIONSHIPS_NS: &str =
+ "http://schemas.openxmlformats.org/officeDocument/2006/relationships";
+ let doc = roxmltree::Document::parse(workbook_xml)
+ .map_err(|e| HandlerError::OperationFailed(format!("invalid workbook.xml: {}", e)))?;
+ let sheets = doc
+ .descendants()
+ .find(|node| node.is_element() && node.tag_name().name() == "sheets")
+ .ok_or_else(|| HandlerError::OperationFailed("workbook has no sheets list".to_string()))?;
+
+ sheets
+ .children()
+ .filter(|node| node.is_element() && node.tag_name().name() == "sheet")
+ .map(|node| {
+ let name = node.attribute("name").ok_or_else(|| {
+ HandlerError::OperationFailed("worksheet entry has no name".to_string())
+ })?;
+ let relationship_id = node
+ .attribute((RELATIONSHIPS_NS, "id"))
+ .or_else(|| node.attribute("r:id"))
+ .ok_or_else(|| {
+ HandlerError::OperationFailed(format!(
+ "worksheet '{}' has no relationship ID",
+ name
+ ))
+ })?;
+ let range = node.range();
+ Ok(WorkbookSheetEntry {
+ name: name.to_string(),
+ relationship_id: relationship_id.to_string(),
+ xml: workbook_xml[range.clone()].to_string(),
+ range,
+ })
+ })
+ .collect()
+}
+
+/// Rewrite localSheetId values. Returning `None` removes that defined name.
+pub(crate) fn rewrite_defined_name_scopes(
+ xml: &str,
+ mapper: impl Fn(u32) -> Option,
+) -> Result {
+ let doc = roxmltree::Document::parse(xml)
+ .map_err(|e| HandlerError::OperationFailed(format!("invalid workbook.xml: {}", e)))?;
+ let mut replacements: Vec<(Range, String)> = Vec::new();
+
+ for node in doc
+ .descendants()
+ .filter(|node| node.is_element() && node.tag_name().name() == "definedName")
+ {
+ let Some(value) = node.attribute("localSheetId") else {
+ continue;
+ };
+ let scope = value.parse::().map_err(|_| {
+ HandlerError::OperationFailed(format!(
+ "definedName has invalid localSheetId '{}'",
+ value
+ ))
+ })?;
+ match mapper(scope) {
+ None => replacements.push((node.range(), String::new())),
+ Some(new_scope) if new_scope != scope => {
+ let value_range = attribute_value_range(xml, node.range(), "localSheetId")
+ .ok_or_else(|| {
+ HandlerError::OperationFailed(
+ "cannot locate localSheetId attribute in workbook XML".to_string(),
+ )
+ })?;
+ replacements.push((value_range, new_scope.to_string()));
+ }
+ Some(_) => {}
+ }
+ }
+ Ok(apply_replacements(xml, replacements))
+}
+
+fn attribute_value_range(
+ xml: &str,
+ node_range: Range,
+ attribute_name: &str,
+) -> Option> {
+ let node_xml = &xml[node_range.clone()];
+ let opening_end = node_xml.find('>')?;
+ let opening = &node_xml[..opening_end];
+ let name_start = opening.find(attribute_name)?;
+ let mut cursor = name_start + attribute_name.len();
+ while opening.as_bytes().get(cursor)?.is_ascii_whitespace() {
+ cursor += 1;
+ }
+ if opening.as_bytes().get(cursor) != Some(&b'=') {
+ return None;
+ }
+ cursor += 1;
+ while opening.as_bytes().get(cursor)?.is_ascii_whitespace() {
+ cursor += 1;
+ }
+ let quote = *opening.as_bytes().get(cursor)?;
+ if quote != b'\'' && quote != b'"' {
+ return None;
+ }
+ let value_start = cursor + 1;
+ let value_end = opening.as_bytes()[value_start..]
+ .iter()
+ .position(|byte| *byte == quote)?
+ + value_start;
+ Some((node_range.start + value_start)..(node_range.start + value_end))
+}
- if first_gt > 0 && xml.as_bytes().get(first_gt - 1) == Some(&b'/') {
- // Self-closing element:
- first_gt + 1
+fn rewrite_sheet_order(
+ workbook_xml: &str,
+ entries: &[WorkbookSheetEntry],
+) -> Result {
+ let doc = roxmltree::Document::parse(workbook_xml)
+ .map_err(|e| HandlerError::OperationFailed(format!("invalid workbook.xml: {}", e)))?;
+ let sheets = doc
+ .descendants()
+ .find(|node| node.is_element() && node.tag_name().name() == "sheets")
+ .ok_or_else(|| HandlerError::OperationFailed("workbook has no sheets list".to_string()))?;
+ let range = sheets.range();
+ let container = &workbook_xml[range.clone()];
+ let opening_end = container.find('>').ok_or_else(|| {
+ HandlerError::OperationFailed("malformed sheets list opening tag".to_string())
+ })? + 1;
+ let closing_start = container.rfind("
- let close_tag = format!("", tag);
- xml[first_gt..]
- .find(&close_tag)
- .map(|pos| first_gt + pos + close_tag.len())
- .unwrap_or(xml.len())
+ format!(
+ "\n {}\n ",
+ entries
+ .iter()
+ .map(|entry| entry.xml.as_str())
+ .collect::>()
+ .join("\n ")
+ )
+ };
+ Ok(apply_replacements(
+ workbook_xml,
+ vec![(content_range, content)],
+ ))
+}
+
+fn remove_relationship_by_id(xml: &str, relationship_id: &str) -> Result {
+ let doc = roxmltree::Document::parse(xml).map_err(|e| {
+ HandlerError::OperationFailed(format!("invalid workbook 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!(
+ "workbook relationship {} not found",
+ relationship_id
+ ))
+ })?;
+ Ok(remove_ranges(xml, vec![relationship.range()]))
+}
+
+fn remove_content_type_override(xml: &str, part_path: &str) -> Result {
+ let doc = roxmltree::Document::parse(xml)
+ .map_err(|e| HandlerError::OperationFailed(format!("invalid content types: {}", e)))?;
+ let part_name = format!("/{}", part_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_ranges(xml, ranges))
+}
+
+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),
}
}
+fn remove_ranges(xml: &str, ranges: Vec>) -> String {
+ apply_replacements(
+ xml,
+ ranges
+ .into_iter()
+ .map(|range| (range, String::new()))
+ .collect(),
+ )
+}
+
+fn apply_replacements(xml: &str, mut replacements: Vec<(Range, String)>) -> String {
+ replacements.sort_by(|left, right| right.0.start.cmp(&left.0.start));
+ let mut result = xml.to_string();
+ for (range, replacement) in replacements {
+ result.replace_range(range, &replacement);
+ }
+ result
+}
+
/// Set properties on a cell identified by path like /Sheet1/A1.
pub fn set_cell_properties(
package: &mut OxmlPackage,
@@ -1460,3 +1770,146 @@ fn replace_in_xml_text_nodes(
// Re-export the find/replace property key list so the handler surface
// matches the C# command registration.
pub use handler_common::find_replace_property_keys;
+
+#[cfg(test)]
+mod sheet_order_tests {
+ use super::*;
+
+ const WORKBOOK_XML: &str = r#"
+
+
+
+
+
+
+
+ A!$A$1
+ B!$A$1
+ C!$A$1
+ A!$B$1
+
+"#;
+
+ const WORKBOOK_RELS: &str = r#"
+
+
+
+
+"#;
+
+ const CONTENT_TYPES: &str = r#"
+
+
+
+
+
+"#;
+
+ fn package_fixture() -> OxmlPackage {
+ let mut package = OxmlPackage::create("unused.xlsx");
+ package.add_part("xl/workbook.xml", WORKBOOK_XML.as_bytes());
+ package.add_part("xl/_rels/workbook.xml.rels", WORKBOOK_RELS.as_bytes());
+ package.add_part("[Content_Types].xml", CONTENT_TYPES.as_bytes());
+ for part in ["sheet1.xml", "sheet2.xml", "sheet5.xml"] {
+ package.add_part(
+ &format!("xl/worksheets/{}", part),
+ br#""#,
+ );
+ }
+ package.add_part("xl/worksheets/_rels/sheet2.xml.rels", b"");
+ package
+ }
+
+ fn sheet_names(xml: &str) -> Vec {
+ workbook_sheet_entries(xml)
+ .unwrap()
+ .into_iter()
+ .map(|entry| entry.name)
+ .collect()
+ }
+
+ fn scope(xml: &str, name: &str) -> Option {
+ let doc = roxmltree::Document::parse(xml).unwrap();
+ doc.descendants()
+ .find(|node| {
+ node.is_element()
+ && node.tag_name().name() == "definedName"
+ && node.attribute("name") == Some(name)
+ })
+ .and_then(|node| node.attribute("localSheetId"))
+ .and_then(|value| value.parse().ok())
+ }
+
+ #[test]
+ fn insert_sheet_shifts_scopes_and_allocates_unique_package_ids() {
+ let mut package = package_fixture();
+ let properties = HashMap::from([("name".to_string(), "Inserted".to_string())]);
+
+ let path = crate::add::add_element(
+ &mut package,
+ "/",
+ "sheet",
+ InsertPosition::AtIndex(1),
+ &properties,
+ )
+ .unwrap();
+
+ assert_eq!(path, "/Inserted");
+ let workbook = package.read_part_xml("xl/workbook.xml").unwrap();
+ assert_eq!(sheet_names(&workbook), ["A", "Inserted", "B", "C"]);
+ assert_eq!(scope(&workbook, "scopeA"), Some(0));
+ assert_eq!(scope(&workbook, "scopeB"), Some(2));
+ assert_eq!(scope(&workbook, "scopeC"), Some(3));
+ assert!(workbook.contains(r#"sheetId="10""#));
+ assert!(workbook.contains(r#"r:id="rId8""#));
+ assert!(package.has_part("xl/worksheets/sheet6.xml"));
+ assert!(package
+ .read_part_xml("[Content_Types].xml")
+ .unwrap()
+ .contains("/xl/worksheets/sheet6.xml"));
+ }
+
+ #[test]
+ fn move_sheet_remaps_scopes_by_sheet_identity() {
+ let mut package = package_fixture();
+
+ move_sheet(
+ &mut package,
+ "/A",
+ None,
+ InsertPosition::AfterElement("/C".to_string()),
+ )
+ .unwrap();
+
+ let workbook = package.read_part_xml("xl/workbook.xml").unwrap();
+ assert_eq!(sheet_names(&workbook), ["B", "C", "A"]);
+ assert_eq!(scope(&workbook, "scopeA"), Some(2));
+ assert_eq!(scope(&workbook, "scopeB"), Some(0));
+ assert_eq!(scope(&workbook, "scopeC"), Some(1));
+ assert_eq!(scope(&workbook, "global"), None);
+ }
+
+ #[test]
+ fn remove_sheet_drops_own_scopes_and_cleans_package_references() {
+ let mut package = package_fixture();
+
+ remove_sheet(&mut package, "B").unwrap();
+
+ let workbook = package.read_part_xml("xl/workbook.xml").unwrap();
+ assert_eq!(sheet_names(&workbook), ["A", "C"]);
+ assert_eq!(scope(&workbook, "scopeA"), Some(0));
+ assert_eq!(scope(&workbook, "scopeB"), None);
+ assert_eq!(scope(&workbook, "scopeC"), Some(1));
+ assert!(!package.has_part("xl/worksheets/sheet2.xml"));
+ assert!(!package.has_part("xl/worksheets/_rels/sheet2.xml.rels"));
+ assert!(!package
+ .read_part_xml("xl/_rels/workbook.xml.rels")
+ .unwrap()
+ .contains("rId4"));
+ assert!(!package
+ .read_part_xml("[Content_Types].xml")
+ .unwrap()
+ .contains("/xl/worksheets/sheet2.xml"));
+ }
+}
diff --git a/crates/xlsx-handler/src/view.rs b/crates/xlsx-handler/src/view.rs
index b48180d..6645037 100644
--- a/crates/xlsx-handler/src/view.rs
+++ b/crates/xlsx-handler/src/view.rs
@@ -360,6 +360,39 @@ pub fn view_as_issues(
}
}
+ // localSheetId is a 0-based index into the workbook's sheet list. Excel
+ // rejects out-of-range scopes, usually left behind by an incomplete sheet
+ // remove/reorder operation.
+ if let Ok(workbook_xml) = package.read_part_xml("xl/workbook.xml") {
+ if let Ok(doc) = roxmltree::Document::parse(&workbook_xml) {
+ for defined_name in doc
+ .descendants()
+ .filter(|node| node.is_element() && node.tag_name().name() == "definedName")
+ {
+ let Some(scope) = defined_name
+ .attribute("localSheetId")
+ .and_then(|value| value.parse::().ok())
+ else {
+ continue;
+ };
+ if scope >= model.sheets.len() {
+ let name = defined_name.attribute("name").unwrap_or("(unnamed)");
+ issues.push(DocumentIssue {
+ severity: IssueSeverity::Error,
+ issue_type: "broken-defined-name-scope".to_string(),
+ description: format!(
+ "Defined name '{}' has out-of-range localSheetId={} but the workbook has {} sheet(s)",
+ name,
+ scope,
+ model.sheets.len()
+ ),
+ path: Some(format!("/namedrange[{}]", name)),
+ });
+ }
+ }
+ }
+ }
+
// Filter by issue type
if let Some(filter_type) = issue_type {
issues.retain(|i| i.issue_type == filter_type);
@@ -431,7 +464,8 @@ pub fn validate(package: &OxmlPackage) -> Result, HandlerEr
#[cfg(test)]
mod tests {
- use super::truncate_cell_value;
+ use super::{truncate_cell_value, view_as_issues};
+ use oxml::OxmlPackage;
#[test]
fn truncate_cell_value_handles_multibyte_text() {
@@ -447,4 +481,28 @@ mod tests {
assert_eq!(truncate_cell_value("中文", 1), "…");
assert_eq!(truncate_cell_value("中文", 0), "");
}
+
+ #[test]
+ fn issues_report_out_of_range_defined_name_scope() {
+ let mut package = OxmlPackage::create("unused.xlsx");
+ package.add_part(
+ "xl/workbook.xml",
+ br#"Only!$A$1"#,
+ );
+ package.add_part(
+ "xl/_rels/workbook.xml.rels",
+ br#""#,
+ );
+ package.add_part(
+ "xl/worksheets/sheet1.xml",
+ br#""#,
+ );
+
+ let issues = view_as_issues(&package, None, None).unwrap();
+
+ assert!(issues.iter().any(|issue| {
+ issue.issue_type == "broken-defined-name-scope"
+ && issue.description.contains("localSheetId=3")
+ }));
+ }
}
diff --git a/docs/csharp-parity/README.zh.md b/docs/csharp-parity/README.zh.md
index dcd6c7c..021f0da 100644
--- a/docs/csharp-parity/README.zh.md
+++ b/docs/csharp-parity/README.zh.md
@@ -149,3 +149,17 @@ relationship part 和 `[Content_Types].xml`，不能用另一格式的实现推
验证覆盖纯包级 custom-show 场景，以及
`create → add → remove middle → edit logical slide → add → validate` 的 CLI 流程。
+
+`XLSX-001` 已实现：
+
+- 在指定位置插入工作表时调整后续 `definedName@localSheetId`，并分别分配未占用的
+ worksheet part、`sheetId` 和 workbook relationship ID；
+- 移动工作表时按工作表身份重映射本地名称作用域，不把名称错误地绑定到原索引上的
+ 另一张工作表；
+- 删除工作表时移除该表作用域的 defined name，递减后续作用域，并清理 worksheet
+ part、worksheet rels、workbook relationship 和 content type override；
+- `view --mode issues` 会报告越界的 `localSheetId`，避免损坏状态静默通过。
+
+验证覆盖非连续物理 part/ID 的包级插入、移动和删除场景，以及
+`create → add at index → move → remove → validate` 的 CLI 流程。下一批从 ledger 中
+尚未实现的高优先级条目继续，且仍按 DOCX、XLSX、PPTX 分支隔离。
diff --git a/docs/csharp-parity/migration-ledger.tsv b/docs/csharp-parity/migration-ledger.tsv
index f0428f4..e0698a1 100644
--- a/docs/csharp-parity/migration-ledger.tsv
+++ b/docs/csharp-parity/migration-ledger.tsv
@@ -12,7 +12,7 @@ SCHEMA-PPTX-001 pptx help schemas partial P1 schemas/help/pptx schemas/help/pptx
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-001 xlsx definedName localSheetId on sheet edits implemented P0 423f17fe; 58b8f970; e89bd6fb crates/xlsx-handler/src/add.rs; handler.rs; mutations.rs; view.rs unit package tests plus add/move/remove CLI smoke preserve scopes by sheet identity and validate out-of-range scopes
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