diff --git a/PHASE-1.2.md b/PHASE-1.2.md new file mode 100644 index 0000000..de64946 --- /dev/null +++ b/PHASE-1.2.md @@ -0,0 +1,101 @@ +# Phase 1.2 — Bug Fixes & Feature Gaps + +> Status: 10 of 12 items addressed. See ✅ for completed fixes. + +--- + +## ✅ Fixed in This Phase + +### 1. `add image` with `file=` property now works +**File:** `crates/docx-handler/src/mutations.rs:2921`, `crates/pptx-handler/src/add.rs:642`, `crates/docx-handler/src/add.rs:2141` + +The code only checked for `src` or `path` property keys, but the CLI passes `file=`. +Added `.or_else(|| properties.get("file"))` as fallback in all three image add paths. + +### 2. `--properties 'border=all'` now creates visible borders +**File:** `crates/docx-handler/src/mutations.rs:902` + +`"all"` didn't match `"all="` (prefix), `"single"`, or `"thin"`. Added `|| value == "all"` so `border=all` behaves like `border=all=single`. + +### 3. `set cell text=` now accepted +**File:** `crates/xlsx-handler/src/mutations.rs:376` + +Added `| "text"` alongside `"value"` match arm so `text=Hello` works for xlsx cells. + +### 4. Multi-row table `rNcN` text fixed +**File:** `crates/docx-handler/src/add.rs:1871-1874` + +Outer loop had `for _ in 0..rows` (no row counter) and format string used `format!("r{}c{}", col_idx + 1, 1)` (hardcoded column). Changed to `for row in 0..rows` and `format!("r{}c{}", row + 1, col_idx + 1)`. + +### 5. Image dimensions: bare numbers treated as points +**File:** `crates/docx-handler/src/mutations.rs:3470-3473` + +`parse_emu()` treated bare numbers as raw EMU (e.g., `width=200` = 200 EMU ≈ 0.016pt). +Changed else branch to multiply by 12700, treating bare numbers as points. + +### 6. Table caption/title support +**File:** `crates/docx-handler/src/add.rs:1840-1846` + +`--properties 'title=My Table'` now creates `` in table properties. + +### 7. Column widths for tables +**File:** `crates/docx-handler/src/add.rs:1899-1915` + +`--properties 'colWidths=100,200,150'` now sets column widths (in points → twips) on `` elements. + +### 8. Page break support +**File:** `crates/docx-handler/src/mutations.rs:245-279` + +`officecli set docx '/body/p[1]' pageBreak=true` inserts `` at paragraph start. + +### 9. Validate detects empty media files +**File:** `crates/docx-handler/src/handler.rs:771-788` + +`officecli validate` now reports an error if any file under `word/media/` has 0 bytes. + +### 10. Paragraph background shading +Already existed in the codebase via `shading`/`shd` property on paragraph `set`. + +--- + +## Remaining Issues (Phase 2 candidates) + +> Identified by building a real-world laptop shop quotation (.docx) and testing +> all formats (docx, xlsx, pptx, pdf) with the `officecli-v1` binary built from +> Phase 1 changes. + +--- + +## 🟡 Phase 2 Candidates + +### 1. Colspan / rowspan (cell merging) + +`--properties 'span=2'` on cell exists in the schema but has no observable +effect. Need to verify `` and `` work through the DOM +serialization path. + +### 2. Tedious cell-by-cell table creation + +A 5×6 table requires ~25 CLI calls (1 table + 5 rows + 20 cells). Could add +`--cells 'r1c1=val,r1c2=val,...'` shorthand. + +### 3. Column widths in `set` command + +`officecli set docx '/body/tbl[1]/col[1]' width=100` is not supported. Column +widths only work at table creation time via `colWidths`. + +### 4. `--range` and `--grid` screenshots (verify) + +These flags parse correctly but haven't been verified with actual screenshot +capture in this phase. + +--- + +## Summary + +| Priority | Count | Key Items | +|----------|-------|-----------| +| ✅ Fixed | 10 | Image 0-byte, Image dims (pt), Xlsx text, Border all, Multi-row text, Caption, Col widths, Page break, Validate media, Paragraph bg | +| 🟡 Phase 2 | 4 | Colspan/rowspan, Cell shorthand, Col set-widths, Screenshot verify | + +**Total gaps identified: 14 items — 10 fixed, 4 deferred to Phase 2** diff --git a/crates/docx-handler/src/add.rs b/crates/docx-handler/src/add.rs index 0471aa2..709f8d7 100644 --- a/crates/docx-handler/src/add.rs +++ b/crates/docx-handler/src/add.rs @@ -1840,6 +1840,12 @@ fn add_table( .with_attribute("val", style.as_str()), ); } + if let Some(caption) = properties.get("title").or_else(|| properties.get("caption")) { + children.push( + WordNode::new(WordElementType::Unknown("tblCaption".to_string())) + .with_attribute("val", caption.as_str()), + ); + } if let Some(width) = properties.get("width") { children.push( WordNode::new(WordElementType::Unknown("tblW".to_string())) @@ -1848,7 +1854,12 @@ fn add_table( ); } if let Some(border) = properties.get("border") { - children.push(crate::mutations::build_table_borders(border)); + let color = properties.get("borderColor") + .or_else(|| properties.get("tblBorderColor")) + .or_else(|| properties.get("bdrColor")) + .map(|s| s.as_str()) + .unwrap_or("000000"); + children.push(crate::mutations::build_table_borders(border, color)); } if let Some(shading) = properties.get("shading").or_else(|| properties.get("shd")) { children.push(crate::mutations::build_shd_node(shading)); @@ -1868,10 +1879,10 @@ fn add_table( // Build table grid let mut rows_nodes = Vec::new(); - for _ in 0..rows { + for row in 0..rows { let mut cells = Vec::new(); for col_idx in 0..cols { - let text = properties.get(&format!("r{}c{}", col_idx + 1, 1)).cloned(); + let text = properties.get(&format!("r{}c{}", row + 1, col_idx + 1)).cloned(); let mut cell = WordNode::new(WordElementType::TableCell); if let Some(text) = text { let para = WordNode::new(WordElementType::Paragraph) @@ -1893,10 +1904,20 @@ fn add_table( // Add tblGrid if multiple columns if cols > 1 { let mut grid = WordNode::new(WordElementType::Unknown("tblGrid".to_string())); - for _ in 0..cols { - grid.children.push(WordNode::new(WordElementType::Unknown( - "gridCol".to_string(), - ))); + let widths: Vec<&str> = properties + .get("colWidths") + .or_else(|| properties.get("colWidth")) + .map(|s| s.split(',').collect()) + .unwrap_or_default(); + for col_idx in 0..cols { + let mut gc = WordNode::new(WordElementType::Unknown("gridCol".to_string())); + if let Some(w) = widths.get(col_idx) { + if let Ok(pt) = w.trim().parse::() { + let twips = (pt * 20.0) as i64; + gc = gc.with_attribute("w", &twips.to_string()); + } + } + grid.children.push(gc); } table.children.push(grid); } @@ -2140,9 +2161,11 @@ fn add_image( let _src = properties .get("src") .or_else(|| properties.get("path")) + .or_else(|| properties.get("file")) .ok_or_else(|| { HandlerError::InvalidArgument( - "image requires 'src' or 'path' property pointing to image file".to_string(), + "image requires 'src', 'path', or 'file' property pointing to image file" + .to_string(), ) })?; diff --git a/crates/docx-handler/src/handler.rs b/crates/docx-handler/src/handler.rs index 0f33a8f..0bfc1ee 100644 --- a/crates/docx-handler/src/handler.rs +++ b/crates/docx-handler/src/handler.rs @@ -771,6 +771,22 @@ impl DocumentHandler for WordHandler { }); } + // Check for empty media files under word/media/ + for part_path in pkg.list_parts() { + if part_path.starts_with("word/media/") { + if let Ok(bytes) = pkg.read_part_bytes(part_path) { + if bytes.is_empty() { + errors.push(ValidationError { + error_type: "broken-reference".to_string(), + description: format!("media file '{}' is empty (0 bytes)", part_path), + path: None, + part: Some(part_path.clone()), + }); + } + } + } + } + Ok(errors) } diff --git a/crates/docx-handler/src/layout.rs b/crates/docx-handler/src/layout.rs new file mode 100644 index 0000000..936b92e --- /dev/null +++ b/crates/docx-handler/src/layout.rs @@ -0,0 +1,336 @@ +use crate::dom_types::{WordElementType, WordNode}; + +const DEFAULT_PAGE_WIDTH: f32 = 612.0; +const DEFAULT_PAGE_HEIGHT: f32 = 792.0; +const DEFAULT_MARGIN: f32 = 72.0; +const DEFAULT_LINE_HEIGHT: f32 = 12.0; +const TWIP_PER_PT: f32 = 20.0; +const EMU_PER_PT: f32 = 12700.0; + +pub fn twip_to_pt(twips: &str) -> f32 { + twips.parse::().unwrap_or(0.0) / TWIP_PER_PT +} + +pub fn emu_to_pt(emu: &str) -> f32 { + emu.parse::().unwrap_or(0.0) / EMU_PER_PT +} + +pub struct DocxLayout { + pub page_width: f32, + pub page_height: f32, + pub margin_left: f32, + pub margin_right: f32, + pub margin_top: f32, + pub current_y: f32, +} + +impl DocxLayout { + pub fn new() -> Self { + Self { + page_width: DEFAULT_PAGE_WIDTH, + page_height: DEFAULT_PAGE_HEIGHT, + margin_left: DEFAULT_MARGIN, + margin_right: DEFAULT_MARGIN, + margin_top: DEFAULT_MARGIN, + current_y: DEFAULT_MARGIN, + } + } + + pub fn content_width(&self) -> f32 { + self.page_width - self.margin_left - self.margin_right + } + + pub fn read_section_properties(&mut self, node: &WordNode) { + for child in &node.children { + if child.element_type != WordElementType::SectionProperties { + continue; + } + for prop in &child.children { + match prop.element_type { + WordElementType::Unknown(ref n) if n == "pgSz" => { + if let Some(w) = prop.attributes.get("w") { + let v = twip_to_pt(w); + if v > 0.0 { + self.page_width = v; + } + } + if let Some(h) = prop.attributes.get("h") { + let v = twip_to_pt(h); + if v > 0.0 { + self.page_height = v; + } + } + } + WordElementType::Unknown(ref n) if n == "pgMar" => { + if let Some(left) = prop.attributes.get("left") { + self.margin_left = twip_to_pt(left); + } + if let Some(right) = prop.attributes.get("right") { + self.margin_right = twip_to_pt(right); + } + if let Some(top) = prop.attributes.get("top") { + self.margin_top = twip_to_pt(top); + self.current_y = self.margin_top; + } + } + _ => {} + } + } + } + } +} + +#[derive(Debug, Clone)] +pub struct ParaLayoutInfo { + pub x: f32, + pub y: f32, + pub width: f32, + pub height: f32, +} + +#[derive(Debug, Clone)] +pub struct TableLayoutInfo { + pub x: f32, + pub y: f32, + pub width: f32, + pub height: f32, + pub rows: Vec, +} + +#[derive(Debug, Clone)] +pub struct RowLayoutInfo { + pub y: f32, + pub height: f32, + pub cells: Vec, +} + +#[derive(Debug, Clone)] +pub struct CellLayoutInfo { + pub x: f32, + pub y: f32, + pub width: f32, + pub height: f32, +} + +pub fn get_alignment(ppr: &WordNode) -> Option<&str> { + ppr.children + .iter() + .find(|c| matches!(&c.element_type, WordElementType::Unknown(n) if n == "jc")) + .and_then(|c| c.attributes.get("val")) + .map(|s| s.as_str()) +} + +pub fn get_indent_pt(ppr: &WordNode) -> (f32, f32, f32, f32) { + if let Some(ind) = ppr + .children + .iter() + .find(|c| matches!(&c.element_type, WordElementType::Unknown(n) if n == "ind")) + { + let left = ind.attributes.get("left").map(|v| twip_to_pt(v)).unwrap_or(0.0); + let right = ind.attributes.get("right").map(|v| twip_to_pt(v)).unwrap_or(0.0); + let first_line = ind.attributes.get("firstLine").map(|v| twip_to_pt(v)).unwrap_or(0.0); + let hanging = ind.attributes.get("hanging").map(|v| twip_to_pt(v)).unwrap_or(0.0); + (left, right, first_line, hanging) + } else { + (0.0, 0.0, 0.0, 0.0) + } +} + +pub fn get_spacing_pt(ppr: &WordNode) -> (f32, f32, f32) { + if let Some(spacing) = ppr + .children + .iter() + .find(|c| matches!(&c.element_type, WordElementType::Unknown(n) if n == "spacing")) + { + let before = spacing.attributes.get("before").map(|v| twip_to_pt(v)).unwrap_or(0.0); + let after = spacing.attributes.get("after").map(|v| twip_to_pt(v)).unwrap_or(0.0); + let line = spacing + .attributes + .get("line") + .and_then(|v| v.parse::().ok()) + .map(|v| v / 240.0 * DEFAULT_LINE_HEIGHT) + .unwrap_or(DEFAULT_LINE_HEIGHT); + (before, after, line) + } else { + (0.0, 0.0, DEFAULT_LINE_HEIGHT) + } +} + +pub fn estimate_para_height(text: &str, width: f32, line_height: f32) -> f32 { + if width <= 0.0 || text.is_empty() { + return line_height; + } + let avg_char_width = line_height * 0.5; + let chars_per_line = (width / avg_char_width).max(1.0) as usize; + let line_count = { + let from_newlines = text.chars().filter(|&c| c == '\n').count() + 1; + let from_wrapping = (text.chars().count() + chars_per_line - 1) / chars_per_line.max(1); + from_newlines.max(from_wrapping) + }; + (line_count as f32) * line_height +} + +pub fn calc_para_layout(para: &WordNode, layout: &mut DocxLayout) -> ParaLayoutInfo { + let ppr = para.paragraph_properties(); + let (left_indent, right_indent, _first_line, _hanging) = + ppr.map(|p| get_indent_pt(p)).unwrap_or_default(); + let (spacing_before, spacing_after, line_height) = + ppr.map(|p| get_spacing_pt(p)).unwrap_or((0.0, 0.0, DEFAULT_LINE_HEIGHT)); + + let x = layout.margin_left + left_indent; + let para_width = (layout.page_width - layout.margin_left - layout.margin_right - left_indent - right_indent).max(0.0); + let text = para.paragraph_text(); + let para_height = estimate_para_height(&text, para_width, line_height); + + let y = layout.current_y + spacing_before; + let height = para_height.max(line_height); + layout.current_y = y + para_height + spacing_after; + + ParaLayoutInfo { + x, + y, + width: para_width, + height, + } +} + +pub fn calc_table_layout(tbl: &WordNode, layout: &mut DocxLayout) -> TableLayoutInfo { + let mut tbl_width = layout.content_width(); + if let Some(tbl_pr) = tbl + .children + .iter() + .find(|c| c.element_type == WordElementType::TableProperties) + { + if let Some(tbl_w) = tbl_pr + .children + .iter() + .find(|c| matches!(&c.element_type, WordElementType::Unknown(n) if n == "tblW")) + { + if let Some(w) = tbl_w.attributes.get("w") { + let parsed = twip_to_pt(w); + if parsed > 0.0 { + tbl_width = parsed; + } + } + } + } + + let x = layout.margin_left; + let mut y = layout.current_y; + let mut total_height = 0.0; + let mut rows = Vec::new(); + + for row_child in &tbl.children { + if row_child.element_type != WordElementType::TableRow { + continue; + } + let mut row_height = 20.0; + if let Some(tr_pr) = row_child + .children + .iter() + .find(|c| c.element_type == WordElementType::TableRowProperties) + { + if let Some(tr_h) = tr_pr + .children + .iter() + .find(|c| matches!(&c.element_type, WordElementType::Unknown(n) if n == "trHeight")) + { + if let Some(val) = tr_h.attributes.get("val") { + row_height = twip_to_pt(val).max(10.0); + } + } + } + + let cell_count = row_child + .children + .iter() + .filter(|c| c.element_type == WordElementType::TableCell) + .count() as f32; + let default_cell_width = if cell_count > 0.0 { tbl_width / cell_count } else { tbl_width }; + + let mut cells = Vec::new(); + let mut cell_x = x; + for tc_child in &row_child.children { + if tc_child.element_type != WordElementType::TableCell { + continue; + } + let mut cell_width = default_cell_width; + if let Some(tc_pr) = tc_child + .children + .iter() + .find(|c| c.element_type == WordElementType::TableCellProperties) + { + if let Some(tc_w) = tc_pr + .children + .iter() + .find(|c| matches!(&c.element_type, WordElementType::Unknown(n) if n == "tcW")) + { + if let Some(w) = tc_w.attributes.get("w") { + let parsed = twip_to_pt(w); + if parsed > 0.0 { + cell_width = parsed; + } + } + } + } + cells.push(CellLayoutInfo { + x: cell_x, + y, + width: cell_width, + height: row_height, + }); + cell_x += cell_width; + } + + rows.push(RowLayoutInfo { + y, + height: row_height, + cells, + }); + y += row_height; + total_height += row_height; + } + + layout.current_y = y; + TableLayoutInfo { + x, + y: y - total_height, + width: tbl_width, + height: total_height, + rows, + } +} + +pub fn drawing_extent_in_para(para: &WordNode) -> Option<(f32, f32)> { + for child in ¶.children { + if child.element_type == WordElementType::Run { + if let Some(d) = find_extent_in_drawing(child) { + return Some(d); + } + } + } + None +} + +fn find_extent_in_drawing(node: &WordNode) -> Option<(f32, f32)> { + if node.element_type == WordElementType::Drawing { + for anchor_child in &node.children { + for grandchild in &anchor_child.children { + if matches!(&grandchild.element_type, WordElementType::Unknown(n) if n == "extent") { + let cx = grandchild.attributes.get("cx").map(|v| emu_to_pt(v)).unwrap_or(0.0); + let cy = grandchild.attributes.get("cy").map(|v| emu_to_pt(v)).unwrap_or(0.0); + if cx > 0.0 && cy > 0.0 { + return Some((cx, cy)); + } + } + } + } + return None; + } + for child in &node.children { + if let Some(d) = find_extent_in_drawing(child) { + return Some(d); + } + } + None +} diff --git a/crates/docx-handler/src/lib.rs b/crates/docx-handler/src/lib.rs index c3d2405..e7695fe 100644 --- a/crates/docx-handler/src/lib.rs +++ b/crates/docx-handler/src/lib.rs @@ -3,6 +3,7 @@ pub mod dom_types; pub mod handler; pub mod helpers; pub mod html_preview; +pub mod layout; pub mod mutations; pub mod navigation; pub mod para_id; diff --git a/crates/docx-handler/src/mutations.rs b/crates/docx-handler/src/mutations.rs index 88482c9..3f79eda 100644 --- a/crates/docx-handler/src/mutations.rs +++ b/crates/docx-handler/src/mutations.rs @@ -242,9 +242,46 @@ fn set_paragraph_properties( } } - // Recognized = text + all PARA_LEVEL_KEYS + all RUN_LEVEL_KEYS + // Handle pageBreak property: insert + // at the beginning of the paragraph content (after pPr). + if let Some(val) = properties + .get("pageBreak") + .or_else(|| properties.get("page-break")) + { + if val == "true" || val == "1" { + // Remove any existing page break run we may have inserted before + para.children.retain(|c| { + if c.element_type != WordElementType::Run { + return true; + } + let is_page_break = c.children.len() == 1 + && c.children[0].element_type == WordElementType::Break + && c.children[0].attributes.get("type").map(|s| s.as_str()) == Some("page"); + !is_page_break + }); + + let br_node = WordNode::new(WordElementType::Break) + .with_attribute("type", "page"); + let run = WordNode::new(WordElementType::Run) + .with_children(vec![br_node]); + + // Insert after pPr if it exists, otherwise at position 0 + let insert_pos = if para + .children + .first() + .map_or(false, |c| c.element_type == WordElementType::ParagraphProperties) + { + 1 + } else { + 0 + }; + para.children.insert(insert_pos, run); + } + } + + // Recognized = text + pageBreak + page-break + all PARA_LEVEL_KEYS + all RUN_LEVEL_KEYS let recognized: Vec<&str> = { - let mut v = vec!["text"]; + let mut v = vec!["text", "pageBreak", "page-break"]; v.extend_from_slice(PARA_LEVEL_KEYS); v.extend_from_slice(RUN_LEVEL_KEYS); v @@ -817,7 +854,12 @@ fn set_table_properties( children.push(shd); } "border" | "borders" | "tblBorders" => { - let borders = build_table_borders(value); + let color = properties.get("borderColor") + .or_else(|| properties.get("tblBorderColor")) + .or_else(|| properties.get("bdrColor")) + .map(|s| s.as_str()) + .unwrap_or("000000"); + let borders = build_table_borders(value, color); children.push(borders); } _ => {} @@ -851,6 +893,9 @@ fn set_table_properties( "border", "borders", "tblBorders", + "borderColor", + "tblBorderColor", + "bdrColor", ]; let unsupported: Vec = properties .keys() @@ -884,7 +929,7 @@ pub fn build_shd_node(value: &str) -> WordNode { .with_attribute("fill", fill) } -pub fn build_table_borders(value: &str) -> WordNode { +pub fn build_table_borders(value: &str, color: &str) -> WordNode { let mut tbl_bdr = WordNode::new(WordElementType::Unknown("tblBorders".to_string())); let mut children = Vec::new(); // Format: "top=single;bottom=single;left=none;right=none;insideH=single;insideV=single" @@ -896,10 +941,10 @@ pub fn build_table_borders(value: &str) -> WordNode { .with_attribute("val", "none") .with_attribute("sz", "0") .with_attribute("space", "0") - .with_attribute("color", "auto"), + .with_attribute("color", color), ); } - } else if value.starts_with("all=") || value == "single" || value == "thin" { + } else if value.starts_with("all=") || value == "all" || value == "single" || value == "thin" { let style = value.strip_prefix("all=").unwrap_or("single"); for border_name in ["top", "bottom", "left", "right", "insideH", "insideV"] { children.push( @@ -907,7 +952,7 @@ pub fn build_table_borders(value: &str) -> WordNode { .with_attribute("val", style) .with_attribute("sz", "4") .with_attribute("space", "0") - .with_attribute("color", "auto"), + .with_attribute("color", color), ); } } else { @@ -927,7 +972,7 @@ pub fn build_table_borders(value: &str) -> WordNode { .with_attribute("val", style) .with_attribute("sz", sz) .with_attribute("space", "0") - .with_attribute("color", "auto"), + .with_attribute("color", color), ); } } @@ -2882,6 +2927,7 @@ pub fn add_image_part_aware( properties .get("src") .or_else(|| properties.get("path")) + .or_else(|| properties.get("file")) .and_then(|p| Path::new(p).extension()) .and_then(|e| e.to_str()) }) @@ -2917,7 +2963,7 @@ pub fn add_image_part_aware( let media_path = format!("word/media/image{}.{}", image_idx, ext_norm); // Write image binary — priority: src file > payloadBase64 > payloadHex > empty stub. - let bytes_written = if let Some(src) = properties.get("src").or_else(|| properties.get("path")) + let bytes_written = if let Some(src) = properties.get("src").or_else(|| properties.get("path")).or_else(|| properties.get("file")) { std::fs::read(src).ok() } else if let Some(b64) = properties.get("payloadBase64") { @@ -2952,7 +2998,7 @@ pub fn add_image_part_aware( // docPr id — use the image index so it stays unique across the document. let doc_pr_id = image_idx; let drawing_xml = format!( - r#""#, + r#""#, w = width_emu, h = height_emu, id = doc_pr_id, @@ -3051,7 +3097,7 @@ pub fn add_chart_part_aware( let (width_emu, height_emu) = parse_image_dimensions_emu(properties); let doc_pr_id = chart_idx; let drawing_xml = format!( - r#""#, + r#""#, w = width_emu, h = height_emu, id = doc_pr_id, @@ -3193,14 +3239,17 @@ fn update_docx_content_types_for_chart( "", chart_path ); - let new_xml = if let Some(close) = xml.find('>') { + // Find the closing `>` of `` — skip past `` + let types_start = xml.find("') { + let insert_at = types_start + close + 1; let mut out = String::with_capacity(xml.len() + override_xml.len()); - out.push_str(&xml[..close + 1]); + out.push_str(&xml[..insert_at]); out.push_str(&override_xml); - out.push_str(&xml[close + 1..]); + out.push_str(&xml[insert_at..]); out } else { - format!("{}", override_xml) + format!("\n{}", override_xml) }; package .write_part_xml("[Content_Types].xml", &new_xml) @@ -3288,15 +3337,20 @@ fn update_docx_content_types_for_image( "", ext, content_type ); - let new_xml = if let Some(close) = xml.find('>') { - // Insert Default right after . + // Find the closing `>` of `` — skip past `` + let types_start = xml.find("') { + let insert_at = types_start + close + 1; let mut out = String::with_capacity(xml.len() + default_xml.len()); - out.push_str(&xml[..close + 1]); + out.push_str(&xml[..insert_at]); out.push_str(&default_xml); - out.push_str(&xml[close + 1..]); + out.push_str(&xml[insert_at..]); out } else { - xml.replace("", &format!("{}{}", default_xml, "")) + let mut out = String::from("\n"); + out.push_str(&default_xml); + out.push_str(""); + out }; package .write_part_xml("[Content_Types].xml", &new_xml) @@ -3432,7 +3486,9 @@ fn parse_emu(s: &str) -> i64 { .map(|n| (n * 9525.0) as i64) .unwrap_or(3_657_600) } else { - s.parse::().unwrap_or(3_657_600) + s.trim().parse::() + .map(|n| (n * 12700.0) as i64) + .unwrap_or(3_657_600) } } diff --git a/crates/docx-handler/src/text_offset.rs b/crates/docx-handler/src/text_offset.rs index 58cdc97..dbadd8a 100644 --- a/crates/docx-handler/src/text_offset.rs +++ b/crates/docx-handler/src/text_offset.rs @@ -1,17 +1,20 @@ use crate::dom_types::{WordDom, WordElementType}; -use handler_common::{HandlerError, TextOffsetMap}; +use crate::layout::{self, DocxLayout}; +use handler_common::{BBoxSpan, HandlerError, TextOffsetMap}; use std::collections::HashMap; -/// Build a TextOffsetMap from the Word DOM. -/// Each paragraph contributes its text, and each run gets its own span. -/// Paragraph breaks are represented as separate "paragraph-break" spans. pub fn extract_text_with_offsets(dom: &WordDom) -> Result { let mut map = TextOffsetMap::empty("docx"); + let mut layout = DocxLayout::new(); let body = dom .body() .ok_or_else(|| HandlerError::OperationFailed("body element not found".to_string()))?; + layout.read_section_properties(&dom.root); + // Also check body's last child for sectPr (body-level placement) + layout.read_section_properties(body); + let mut para_idx = 0; let mut content_idx = 0; let mut tbl_idx = 0; @@ -23,12 +26,9 @@ pub fn extract_text_with_offsets(dom: &WordDom) -> Result 1 { - // Add paragraph separator (newline) between paragraphs map.push_span_with_id( "\n", &format!("/body/p[{}]/break", para_idx), @@ -40,11 +40,33 @@ pub fn extract_text_with_offsets(dom: &WordDom) -> Result { tbl_idx += 1; @@ -59,6 +81,7 @@ pub fn extract_text_with_offsets(dom: &WordDom) -> Result Result Result Result {} } @@ -124,11 +183,6 @@ pub fn extract_text_with_offsets(dom: &WordDom) -> Result String { let mut result = String::new(); let mut para_count = 0; @@ -258,7 +309,6 @@ fn extract_cell_text(cell: &crate::dom_types::WordNode) -> String { result } -/// Count cells in a table row. fn count_cells_in_row(children: &[crate::dom_types::WordNode]) -> usize { children .iter() @@ -266,7 +316,6 @@ fn count_cells_in_row(children: &[crate::dom_types::WordNode]) -> usize { .count() } -/// Count rows in a table. fn count_rows_in_table(children: &[crate::dom_types::WordNode]) -> usize { children .iter() @@ -343,4 +392,25 @@ mod tests { assert_eq!(map.full_text, "Choice text"); assert!(map.spans.iter().all(|span| !span.text.contains("Fallback"))); } + + #[test] + fn paragraphs_have_bbox() { + let dom = WordDom::new(WordNode::new(WordElementType::Document).with_children(vec![ + WordNode::new(WordElementType::Body).with_children(vec![ + WordNode::new(WordElementType::Paragraph).with_children(vec![text_run("Hello")]), + WordNode::new(WordElementType::Paragraph).with_children(vec![text_run("World")]), + ]), + ])); + + let map = extract_text_with_offsets(&dom).unwrap(); + let spans_with_bbox: Vec<&handler_common::OffsetSpan> = + map.spans.iter().filter(|s| s.bbox.is_some()).collect(); + + assert!(!spans_with_bbox.is_empty(), "expected at least one span with bbox"); + for span in &spans_with_bbox { + let bbox = span.bbox.as_ref().unwrap(); + assert!(bbox.width > 0.0, "bbox width should be positive"); + assert!(bbox.height > 0.0, "bbox height should be positive"); + } + } } diff --git a/crates/handler-common/src/output_format.rs b/crates/handler-common/src/output_format.rs index 0ebc518..5201c74 100644 --- a/crates/handler-common/src/output_format.rs +++ b/crates/handler-common/src/output_format.rs @@ -17,6 +17,12 @@ pub struct ViewOptions { pub cols: Option>, /// Page filter string (e.g. "1", "2-5", "1,3,5"). Parsed by each handler. pub page: Option, + /// Element path or cell range to restrict output (e.g. "/slide[1]/shape[@id=2]" or "Sheet1!A1:C3") + pub range: Option, + /// Grid columns for thumbnail contact sheet (0 = off) + pub grid: u32, + /// Rendering backend: "auto", "native", "html" + pub render: String, } /// Options for raw commands. diff --git a/crates/officecli/src/commands/mod.rs b/crates/officecli/src/commands/mod.rs index 0a4a06f..4639847 100644 --- a/crates/officecli/src/commands/mod.rs +++ b/crates/officecli/src/commands/mod.rs @@ -22,6 +22,7 @@ mod refresh; mod remove; mod save; mod set; +mod skills; mod swap; mod validate; mod view; @@ -69,6 +70,7 @@ pub use refresh::RefreshCommand; pub use remove::RemoveCommand; pub use save::SaveCommand; pub use set::SetCommand; +pub use skills::SkillsCommand; pub use swap::SwapCommand; pub use validate::ValidateCommand; pub use view::ViewCommand; @@ -256,6 +258,8 @@ pub enum Command { Plugins(PluginsCommand), /// Install officecli binary, skills, and MCP configuration Install(InstallCommand), + /// List and install agent skill definitions + Skills(SkillsCommand), /// Open a document in resident mode (keeps handler in memory for fast subsequent commands) Open(OpenCommand), /// Close a document in resident mode (stops the background server) @@ -306,6 +310,7 @@ pub use refresh::handle_refresh; pub use remove::handle_remove; pub use save::handle_save; pub use set::handle_set; +pub use skills::handle_skills; pub use swap::handle_swap; pub use validate::handle_validate; pub use view::handle_view; diff --git a/crates/officecli/src/commands/skills.rs b/crates/officecli/src/commands/skills.rs new file mode 100644 index 0000000..77d7dfc --- /dev/null +++ b/crates/officecli/src/commands/skills.rs @@ -0,0 +1,199 @@ +use clap::{Args, Subcommand}; +use handler_common::{HandlerError, OutputFormat}; + +/// Install agent skill definitions (Claude Code, Cursor, Copilot, etc.) +#[derive(Args)] +pub struct SkillsCommand { + #[command(subcommand)] + pub action: SkillsAction, +} + +#[derive(Subcommand)] +pub enum SkillsAction { + /// List all available skills + List, + /// Install a specific skill + Install { + /// Skill name to install (e.g. "pitch-deck", "academic-paper"), or "all" for all + skill: String, + /// Target agent (optional, default: all agents). Supported: claude, copilot, cursor, windsurf, opencode, all + agent: Option, + }, +} + +pub fn handle_skills(cmd: SkillsCommand, _format: OutputFormat) -> Result { + match cmd.action { + SkillsAction::List => handle_list(), + SkillsAction::Install { skill, agent } => handle_install(&skill, agent.as_deref()), + } +} + +fn handle_list() -> Result { + let skills_dir = find_skills_dir(); + if !skills_dir.exists() { + return Ok("No skills found (skills/ directory not found)".to_string()); + } + + let mut output = String::from("Available skills:\n"); + let mut count = 0; + + if let Ok(entries) = std::fs::read_dir(&skills_dir) { + for entry in entries.flatten() { + let path = entry.path(); + if path.is_dir() { + let skill_name = path + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or(""); + let skill_md = path.join("SKILL.md"); + let description = if skill_md.exists() { + std::fs::read_to_string(&skill_md) + .ok() + .and_then(|content| { + content + .lines() + .next() + .map(|l| l.trim_start_matches("# ").trim().to_string()) + }) + .unwrap_or_default() + } else { + String::new() + }; + output.push_str(&format!(" {:<30} {}\n", skill_name, description)); + count += 1; + } + } + } + + output.push_str(&format!("\n{} skill(s) available\n", count)); + output.push_str("Usage: officecli skills install [agent]\n"); + Ok(output) +} + +fn handle_install(skill: &str, agent: Option<&str>) -> Result { + let skills_dir = find_skills_dir(); + if !skills_dir.exists() { + return Err(HandlerError::OperationFailed( + "skills/ directory not found".into(), + )); + } + + if skill == "all" { + let mut installed = 0; + if let Ok(entries) = std::fs::read_dir(&skills_dir) { + for entry in entries.flatten() { + let path = entry.path(); + if path.is_dir() && path.join("SKILL.md").exists() { + if let Some(name) = path.file_name().and_then(|n| n.to_str()) { + install_single_skill(name, agent)?; + installed += 1; + } + } + } + } + Ok(format!("Installed {} skill(s) to agent(s)", installed)) + } else { + let skill_path = skills_dir.join(skill).join("SKILL.md"); + if !skill_path.exists() { + return Err(HandlerError::OperationFailed(format!( + "Skill '{}' not found. Use 'officecli skills list' to see available skills", + skill + ))); + } + install_single_skill(skill, agent)?; + Ok(format!("Installed skill '{}'", skill)) + } +} + +fn install_single_skill(skill_name: &str, agent: Option<&str>) -> Result<(), HandlerError> { + let skills_dir = find_skills_dir(); + let source_path = skills_dir.join(skill_name).join("SKILL.md"); + let content = std::fs::read_to_string(&source_path).map_err(|e| { + HandlerError::OperationFailed(format!("Failed to read skill: {}", e)) + })?; + + let target_agents: Vec = match agent { + Some("all") | None => vec![ + "claude".to_string(), + "cursor".to_string(), + "windsurf".to_string(), + "opencode".to_string(), + ], + Some(a) => vec![a.to_string()], + }; + + for agent_name in &target_agents { + let dest_dir = get_agent_skill_dir(agent_name); + if let Some(dest_dir) = dest_dir { + let dest_path = dest_dir.join(format!("officecli-{}.md", skill_name)); + if let Some(parent) = dest_path.parent() { + let _ = std::fs::create_dir_all(parent); + } + let _ = std::fs::write(&dest_path, &content); + } + } + + Ok(()) +} + +fn find_skills_dir() -> std::path::PathBuf { + // 1. Look next to the binary (installed location) + if let Ok(exe) = std::env::current_exe() { + if let Some(exe_dir) = exe.parent() { + // Walk up the directory tree to find skills/ (handles target/release/ -> project root) + let mut dir = Some(exe_dir.to_path_buf()); + while let Some(current) = dir { + let candidate = current.join("skills"); + if candidate.is_dir() { + return candidate; + } + // Stop at filesystem root + let parent = current.parent().map(|p| p.to_path_buf()); + if parent.as_ref() == Some(¤t) { + break; + } + dir = parent; + } + } + } + + // 2. Look relative to cwd + if let Ok(cwd) = std::env::current_dir() { + let path = cwd.join("skills"); + if path.exists() { + return path; + } + } + + // 3. Look in user config dir + if let Some(home) = home_dir() { + let path = home.join(".officecli").join("skills"); + if path.exists() { + return path; + } + } + + // 4. Check OFFICECLI_SKILLS_DIR env var + if let Ok(env_dir) = std::env::var("OFFICECLI_SKILLS_DIR") { + let path = std::path::PathBuf::from(env_dir); + if path.exists() { + return path; + } + } + + std::path::PathBuf::from("skills") +} + +fn get_agent_skill_dir(agent: &str) -> Option { + match agent { + "claude" => home_dir().map(|h| h.join(".claude").join("skills")), + "cursor" => home_dir().map(|h| h.join(".cursor").join("skills")), + "windsurf" => home_dir().map(|h| h.join(".windsurf").join("skills")), + "opencode" => home_dir().map(|h| h.join(".config").join("opencode").join("skills")), + _ => None, + } +} + +fn home_dir() -> Option { + std::env::var("HOME").ok().map(std::path::PathBuf::from) +} diff --git a/crates/officecli/src/commands/view.rs b/crates/officecli/src/commands/view.rs index 03b7e3a..7837544 100644 --- a/crates/officecli/src/commands/view.rs +++ b/crates/officecli/src/commands/view.rs @@ -66,6 +66,18 @@ pub struct ViewCommand { /// Limit number of results (for issues mode) #[arg(long)] pub limit: Option, + + /// Restrict output to a region (element path like "/slide[1]/shape[@id=2]" or xlsx cell range "Sheet1!A1:C3") + #[arg(long)] + pub range: Option, + + /// Zoom factor for --range screenshots (e.g. "2x") + #[arg(long)] + pub zoom: Option, + + /// Padding in pixels around cropped element + #[arg(long, default_value_t = 0)] + pub padding: u32, } pub fn handle_view(cmd: ViewCommand, format: OutputFormat) -> Result { @@ -76,6 +88,13 @@ pub fn handle_view(cmd: ViewCommand, format: OutputFormat) -> Result Result 0 { + let stats = handler.view_as_stats_json()?; + let page_count = stats + .get("pages") + .or(stats.get("slides")) + .or(stats.get("sheets")) + .and_then(|v| v.as_u64()) + .unwrap_or(1) as u32; + wrap_in_grid(handler, page_count, cmd.grid)? + } else { + handler.view_as_html(opts)? + }; // Step 2: Write to temp file let temp_dir = std::env::temp_dir(); @@ -332,3 +368,108 @@ fn open_path_in_browser(path: &std::path::Path) { .spawn(); } } + +/// Handle `view -m screenshot --range ` — crop screenshot to a single element's bbox. +fn handle_screenshot_with_range( + handler: &dyn handler_common::DocumentHandler, + cmd: &ViewCommand, + range: &str, +) -> Result { + // 1. Verify element exists + let _node = handler.get(range, 0)?; + + // 2. Get text offset map for bbox + let text_map = handler.extract_text_with_offsets()?; + let spans = text_map.spans_for_path(range); + + // 3. Find first span with bbox + let bbox = spans + .iter() + .find_map(|s| s.bbox.as_ref()) + .ok_or_else(|| HandlerError::OperationFailed("no bbox data for range".into()))?; + + // 4. Calculate zoom + let zoom = parse_zoom(&cmd.zoom).unwrap_or(1.0); + + // 5. Generate HTML for just this element + let html = handler.view_as_html(ViewOptions::default())?; + + let crop_w = (bbox.width * zoom + cmd.padding as f32 * 2.0) as u32; + let crop_h = (bbox.height * zoom + cmd.padding as f32 * 2.0) as u32; + + let cropped_html = format!( + r#"
+
+ {} +
+
"#, + crop_w, crop_h, bbox.x, bbox.y, zoom, html + ); + + // 6. Write to temp and capture screenshot + let timestamp = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap_or_default() + .as_millis(); + let temp_dir = std::env::temp_dir(); + let html_path = temp_dir.join(format!("officecli_range_{}.html", timestamp)); + std::fs::write(&html_path, &cropped_html)?; + + let out_path = cmd.out.clone().unwrap_or_else(|| { + let stem = std::path::Path::new(&cmd.file) + .file_stem() + .and_then(|s| s.to_str()) + .unwrap_or("range"); + temp_dir + .join(format!("officecli_range_{}.png", stem)) + .to_string_lossy() + .to_string() + }); + + let result = crate::screenshot::capture(&html_path.to_string_lossy(), &out_path, crop_w, crop_h) + .map_err(|e| HandlerError::OperationFailed(e))?; + + let _ = std::fs::remove_file(&html_path); + + Ok(format!("Range screenshot saved: {}", result.output_path)) +} + +/// Wrap multiple pages/slides into a grid layout for --grid screenshots. +fn wrap_in_grid( + handler: &dyn handler_common::DocumentHandler, + page_count: u32, + grid_cols: u32, +) -> Result { + let cols = grid_cols.max(1); + let mut table = String::from(""); + for i in 0..page_count { + if i % cols == 0 { + table.push_str(""); + } + let page_opts = ViewOptions { + page: Some((i + 1).to_string()), + ..Default::default() + }; + let page_html = handler.view_as_html(page_opts)?; + table.push_str(&format!( + "", + page_html + )); + if i % cols == cols - 1 || i == page_count - 1 { + table.push_str(""); + } + } + table.push_str("
{}
"); + Ok(table) +} + +/// Parse a zoom string like "2x" or "3.0" into a float multiplier. +fn parse_zoom(zoom: &Option) -> Option { + zoom.as_ref().and_then(|z| { + if let Some(v) = z.strip_suffix('x') { + v.parse::().ok() + } else { + z.parse::().ok() + } + }) +} diff --git a/crates/officecli/src/main.rs b/crates/officecli/src/main.rs index 658d511..69b07d8 100644 --- a/crates/officecli/src/main.rs +++ b/crates/officecli/src/main.rs @@ -151,6 +151,7 @@ fn main() { commands::Command::Import(cmd) => commands::handle_import(cmd, format), commands::Command::Plugins(cmd) => commands::handle_plugins(cmd, format), commands::Command::Install(cmd) => commands::handle_install(cmd, format), + commands::Command::Skills(cmd) => commands::handle_skills(cmd, format), commands::Command::Open(cmd) => handle_open(cmd), commands::Command::Close(cmd) => handle_close(cmd), commands::Command::Watch(cmd) => handle_watch(cmd), diff --git a/crates/officecli/src/mcp.rs b/crates/officecli/src/mcp.rs index 203b105..587ca0d 100644 --- a/crates/officecli/src/mcp.rs +++ b/crates/officecli/src/mcp.rs @@ -360,6 +360,19 @@ fn execute_tool(name: &str, params: &HashMap) -> Result) -> ViewOpt .get("page") .and_then(|v| v.as_str()) .map(|s| s.to_string()), + range: params + .get("range") + .and_then(|v| v.as_str()) + .map(|s| s.to_string()), + grid: params + .get("grid") + .and_then(|v| v.as_u64()) + .unwrap_or(0) as u32, + render: params + .get("render") + .and_then(|v| v.as_str()) + .unwrap_or("auto") + .to_string(), } } diff --git a/crates/officecli/src/watch.rs b/crates/officecli/src/watch.rs index 483d1f0..6f2e22a 100644 --- a/crates/officecli/src/watch.rs +++ b/crates/officecli/src/watch.rs @@ -1033,6 +1033,15 @@ async fn handle_view( .get("cols") .map(|c| c.split(',').map(|s| s.to_string()).collect()), page: params.get("page").cloned(), + range: params.get("range").cloned(), + grid: params + .get("grid") + .and_then(|v| v.parse::().ok()) + .unwrap_or(0), + render: params + .get("render") + .cloned() + .unwrap_or_else(|| "auto".to_string()), }; let op = match mode.as_str() { diff --git a/crates/pptx-handler/src/add.rs b/crates/pptx-handler/src/add.rs index 79e21b6..20211bf 100644 --- a/crates/pptx-handler/src/add.rs +++ b/crates/pptx-handler/src/add.rs @@ -639,7 +639,7 @@ fn add_picture( ) -> Result { use std::path::Path; - let src = properties.get("src").or_else(|| properties.get("path")); + let src = properties.get("src").or_else(|| properties.get("path")).or_else(|| properties.get("file")); // Resolve image extension — explicit property takes priority, then derive // from `src` filename extension. Default to png. diff --git a/crates/pptx-handler/src/dom_types.rs b/crates/pptx-handler/src/dom_types.rs index c92a708..e74dc0b 100644 --- a/crates/pptx-handler/src/dom_types.rs +++ b/crates/pptx-handler/src/dom_types.rs @@ -27,6 +27,8 @@ pub struct Shape { pub text: String, /// Individual paragraphs in the shape's text body pub paragraphs: Vec, + /// Bounding box from (position/size in EMU, converted to points) + pub bbox: Option, } /// A paragraph within a shape's text body. diff --git a/crates/pptx-handler/src/navigation.rs b/crates/pptx-handler/src/navigation.rs index fec9f54..b8058be 100644 --- a/crates/pptx-handler/src/navigation.rs +++ b/crates/pptx-handler/src/navigation.rs @@ -211,12 +211,48 @@ fn parse_shape_node(sp: &roxmltree::Node) -> Option { } } + // Parse for bounding box + let bbox = sp + .descendants() + .find(|n| n.has_tag_name("xfrm")) + .and_then(|xfrm| { + let off = xfrm.children().find(|n| n.has_tag_name("off"))?; + let ext = xfrm.children().find(|n| n.has_tag_name("ext"))?; + let x = off + .attribute("x") + .and_then(|s| s.parse::().ok()) + .unwrap_or(0.0) + / 12700.0; + let y = off + .attribute("y") + .and_then(|s| s.parse::().ok()) + .unwrap_or(0.0) + / 12700.0; + let cx = ext + .attribute("cx") + .and_then(|s| s.parse::().ok()) + .unwrap_or(0.0) + / 12700.0; + let cy = ext + .attribute("cy") + .and_then(|s| s.parse::().ok()) + .unwrap_or(0.0) + / 12700.0; + Some(handler_common::BBoxSpan { + x: x as f32, + y: y as f32, + width: cx as f32, + height: cy as f32, + }) + }); + Some(Shape { name, id, placeholder_type, text: full_text, paragraphs, + bbox, }) } diff --git a/crates/pptx-handler/src/text_offset.rs b/crates/pptx-handler/src/text_offset.rs index 1126a4c..ab57b5c 100644 --- a/crates/pptx-handler/src/text_offset.rs +++ b/crates/pptx-handler/src/text_offset.rs @@ -25,8 +25,8 @@ pub fn extract_text_with_offsets( let shape_path = format!("/slide[{}]/shape[{}]", slide.index, si + 1); - // Push shape text as a single span - map.push_span(&shape.text, &shape_path, "shape"); + // Push shape text as a single span with bbox metadata + map.push_span_with_metadata(&shape.text, &shape_path, "shape", shape.bbox.clone(), None); // Also push individual paragraph spans for finer granularity for (pi, para) in shape.paragraphs.iter().enumerate() { diff --git a/crates/xlsx-handler/src/dom_types.rs b/crates/xlsx-handler/src/dom_types.rs index 381b3c7..52eda25 100644 --- a/crates/xlsx-handler/src/dom_types.rs +++ b/crates/xlsx-handler/src/dom_types.rs @@ -129,6 +129,10 @@ pub struct Worksheet { pub max_col: usize, /// Maximum row that has data (1-based) pub max_row: usize, + /// Column widths in points, keyed by 1-based column number + pub col_widths: HashMap, + /// Row heights in points, keyed by 1-based row number + pub row_heights: HashMap, } /// Workbook model: sheets + shared strings. diff --git a/crates/xlsx-handler/src/helpers.rs b/crates/xlsx-handler/src/helpers.rs index a090dda..6de1ca2 100644 --- a/crates/xlsx-handler/src/helpers.rs +++ b/crates/xlsx-handler/src/helpers.rs @@ -147,8 +147,13 @@ pub fn parse_sheet( std::collections::HashMap::new(); let mut max_col: usize = 0; let mut max_row: usize = 0; + let mut col_widths: std::collections::HashMap = + std::collections::HashMap::new(); + let mut row_heights: std::collections::HashMap = + std::collections::HashMap::new(); let mut in_cell = false; + let mut in_cols = false; let mut cell_ref_str = String::new(); let mut cell_value_type = CellValueType::Number; let mut cell_style_index: Option = None; @@ -164,6 +169,45 @@ pub fn parse_sheet( loop { match reader.read_event() { Ok(Event::Start(e)) => match e.local_name().as_ref() { + b"cols" => in_cols = true, + b"col" if in_cols => { + let mut min = 0usize; + let mut max = 0usize; + let mut width = 0.0_f64; + for attr in e.attributes().filter_map(|a| a.ok()) { + let key = attr.key.as_ref(); + let val = String::from_utf8_lossy(attr.value.as_ref()); + if key == b"min" { + min = val.parse().unwrap_or(0); + } else if key == b"max" { + max = val.parse().unwrap_or(0); + } else if key == b"width" { + width = val.parse().unwrap_or(0.0); + } + } + if min > 0 && max >= min { + let width_pt = width * 7.0; + for col in min..=max { + col_widths.insert(col, width_pt); + } + } + } + b"row" => { + let mut r = 0usize; + let mut ht = 15.0_f64; + for attr in e.attributes().filter_map(|a| a.ok()) { + let key = attr.key.as_ref(); + let val = String::from_utf8_lossy(attr.value.as_ref()); + if key == b"r" { + r = val.parse().unwrap_or(0); + } else if key == b"ht" { + ht = val.parse().unwrap_or(15.0); + } + } + if r > 0 { + row_heights.insert(r, ht); + } + } b"c" => { in_cell = true; cell_ref_str.clear(); @@ -218,6 +262,7 @@ pub fn parse_sheet( } } Ok(Event::End(e)) => match e.local_name().as_ref() { + b"cols" => in_cols = false, b"v" => in_v = false, b"f" => in_f = false, b"t" if in_is_t => in_is_t = false, @@ -258,6 +303,29 @@ pub fn parse_sheet( _ => {} }, Ok(Event::Empty(e)) => { + // Handle elements inside + if e.local_name().as_ref() == b"col" && in_cols { + let mut min = 0usize; + let mut max = 0usize; + let mut width = 0.0_f64; + for attr in e.attributes().filter_map(|a| a.ok()) { + let key = attr.key.as_ref(); + let val = String::from_utf8_lossy(attr.value.as_ref()); + if key == b"min" { + min = val.parse().unwrap_or(0); + } else if key == b"max" { + max = val.parse().unwrap_or(0); + } else if key == b"width" { + width = val.parse().unwrap_or(0.0); + } + } + if min > 0 && max >= min { + let width_pt = width * 7.0; + for col in min..=max { + col_widths.insert(col, width_pt); + } + } + } // Handle cells without v or f children if e.local_name().as_ref() == b"c" { cell_ref_str.clear(); @@ -321,6 +389,8 @@ pub fn parse_sheet( cells, max_col, max_row, + col_widths, + row_heights, }) } diff --git a/crates/xlsx-handler/src/mutations.rs b/crates/xlsx-handler/src/mutations.rs index c1241e9..97eb1fd 100644 --- a/crates/xlsx-handler/src/mutations.rs +++ b/crates/xlsx-handler/src/mutations.rs @@ -373,7 +373,7 @@ pub fn set_cell_properties( for (key, value) in properties { match key.as_str() { - "value" => { + "value" | "text" => { modified_xml = set_cell_value( &modified_xml, &cell_ref_str, diff --git a/crates/xlsx-handler/src/text_offset.rs b/crates/xlsx-handler/src/text_offset.rs index f10b85e..324b2c5 100644 --- a/crates/xlsx-handler/src/text_offset.rs +++ b/crates/xlsx-handler/src/text_offset.rs @@ -1,7 +1,9 @@ /// Text offset mapping for xlsx documents. /// Maps each cell's display text to a path for AI agent navigation. +use std::collections::HashMap; use crate::dom_types::*; use crate::helpers; +use handler_common::BBoxSpan; use handler_common::HandlerError; use handler_common::TextOffsetMap; use oxml::OxmlPackage; @@ -22,22 +24,57 @@ pub fn build_text_offset_map_internal( let mut sorted_cells = cell_refs; sorted_cells.sort_by(|a, b| (a.row, a.col).cmp(&(b.row, b.col))); + // Pre-compute column X positions from col_widths + let mut col_x_positions: HashMap = HashMap::new(); + let mut x = 0.0_f64; + let max_col = ws.max_col.max( + ws.col_widths.keys().copied().max().unwrap_or(0), + ); + for col in 1..=max_col { + col_x_positions.insert(col, x); + x += ws.col_widths.get(&col).copied().unwrap_or(64.0); + } + // Sheet header let sheet_header = format!("[{}]\n", ws.name); map.push_span(&sheet_header, &format!("/{}", ws.name), "sheet-header"); - for cell in sorted_cells { + let mut current_y = 0.0_f64; + + for i in 0..sorted_cells.len() { + let cell = sorted_cells[i]; let path = format!("/{}{}", ws.name, cell.ref_str); let text = format!("{}: {}\n", cell.ref_str, cell.display_value); - // Cell content span - map.push_span(&text, &path, "cell"); + // Compute BBox for this cell + let x = col_x_positions.get(&cell.col).copied().unwrap_or(0.0); + let y = current_y; + let w = ws.col_widths.get(&cell.col).copied().unwrap_or(64.0); + let h = ws.row_heights.get(&cell.row).copied().unwrap_or(15.0); + + let bbox = BBoxSpan { + x: x as f32, + y: y as f32, + width: w as f32, + height: h as f32, + }; + + // Cell content span with bbox + map.push_span_with_metadata(&text, &path, "cell", Some(bbox), None); // Formula span (if present) if let Some(formula) = &cell.formula { let formula_text = format!(" ={}\n", formula); map.push_span(&formula_text, &format!("{}:formula", path), "cell-formula"); } + + // Advance Y after each row (look ahead to next cell) + if i + 1 < sorted_cells.len() { + let next = sorted_cells[i + 1]; + if next.row != cell.row { + current_y += ws.row_heights.get(&cell.row).copied().unwrap_or(15.0); + } + } } // Row separator between sheets