Skip to content

Commit 440c189

Browse files
alanbldclaude
alanbld
and
claude
committed
refactor(ooxml,pptx): Polish and code quality improvements
OOXML crate: - Fix potential panic in style_contract_validator: replace .min().unwrap() with filter_map pattern for safer iteration - Apply rustfmt formatting improvements throughout - Improve method chaining readability in writer.rs - Clean up style_map.rs formatting PPTX crate: - Remove unused imports and dead code - Apply consistent rustfmt formatting - Simplify template and layout handling 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent d928d86 commit 440c189

File tree

12 files changed

+192
-184
lines changed

12 files changed

+192
-184
lines changed

‎crates/utf8dok-ooxml/src/extract.rs‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -433,12 +433,8 @@ impl AsciiDocExtractor {
433433
let comment_ranges = CommentRanges::parse(doc_xml);
434434

435435
// Build the style contract (ADR-007)
436-
let style_contract = self.build_style_contract(
437-
&document,
438-
&styles,
439-
relationships.as_ref(),
440-
source_file,
441-
);
436+
let style_contract =
437+
self.build_style_contract(&document, &styles, relationships.as_ref(), source_file);
442438

443439
let asciidoc = self.convert_to_asciidoc(
444440
&document,

‎crates/utf8dok-ooxml/src/style_contract_validator.rs‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -464,7 +464,10 @@ impl StyleContractValidator {
464464
}
465465
let canonical_bookmark: HashMap<&str, &str> = semantic_to_bookmarks
466466
.iter()
467-
.map(|(sem_id, bookmarks)| (*sem_id, *bookmarks.iter().min().unwrap()))
467+
.filter_map(|(sem_id, bookmarks)| {
468+
// bookmarks is never empty because we only insert when pushing
469+
bookmarks.iter().min().map(|b| (*sem_id, *b))
470+
})
468471
.collect();
469472

470473
// Property: ∀ anchor: get_word_bookmark(get_semantic_anchor(anchor)) == canonical(anchor)
@@ -613,15 +616,19 @@ mod tests {
613616
#[test]
614617
fn test_valid_semantic_id() {
615618
assert!(StyleContractValidator::is_valid_semantic_id("introduction"));
616-
assert!(StyleContractValidator::is_valid_semantic_id("purpose-and-scope"));
619+
assert!(StyleContractValidator::is_valid_semantic_id(
620+
"purpose-and-scope"
621+
));
617622
assert!(StyleContractValidator::is_valid_semantic_id("section-1"));
618623
assert!(StyleContractValidator::is_valid_semantic_id("a"));
619624
assert!(StyleContractValidator::is_valid_semantic_id("1-overview"));
620625

621626
assert!(!StyleContractValidator::is_valid_semantic_id(""));
622627
assert!(!StyleContractValidator::is_valid_semantic_id("-invalid"));
623628
assert!(!StyleContractValidator::is_valid_semantic_id("UPPERCASE"));
624-
assert!(!StyleContractValidator::is_valid_semantic_id("has_underscore"));
629+
assert!(!StyleContractValidator::is_valid_semantic_id(
630+
"has_underscore"
631+
));
625632
assert!(!StyleContractValidator::is_valid_semantic_id("has space"));
626633
}
627634

@@ -634,7 +641,9 @@ mod tests {
634641

635642
assert!(!StyleContractValidator::is_valid_xml_ncname(""));
636643
assert!(!StyleContractValidator::is_valid_xml_ncname("123start"));
637-
assert!(!StyleContractValidator::is_valid_xml_ncname("-hyphen-start"));
644+
assert!(!StyleContractValidator::is_valid_xml_ncname(
645+
"-hyphen-start"
646+
));
638647
assert!(!StyleContractValidator::is_valid_xml_ncname("has space"));
639648
}
640649

@@ -652,9 +661,7 @@ mod tests {
652661

653662
let result = StyleContractValidator::validate_schema(&contract);
654663
assert!(result.has_errors());
655-
assert!(result.errors()[0]
656-
.message
657-
.contains("out of range"));
664+
assert!(result.errors()[0].message.contains("out of range"));
658665
}
659666

660667
#[test]

‎crates/utf8dok-ooxml/src/style_map.rs‎

Lines changed: 26 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -586,38 +586,23 @@ impl CoverConfig {
586586

587587
if position.ends_with('%') {
588588
// Percentage of page height
589-
let pct: f64 = position
590-
.trim_end_matches('%')
591-
.parse()
592-
.unwrap_or(35.0);
589+
let pct: f64 = position.trim_end_matches('%').parse().unwrap_or(35.0);
593590
(page_height_emu as f64 * pct / 100.0) as i64
594591
} else if position.ends_with("pt") {
595592
// Points (1 pt = 12700 EMU)
596-
let pts: f64 = position
597-
.trim_end_matches("pt")
598-
.parse()
599-
.unwrap_or(0.0);
593+
let pts: f64 = position.trim_end_matches("pt").parse().unwrap_or(0.0);
600594
(pts * 12700.0) as i64
601595
} else if position.ends_with("in") {
602596
// Inches (1 in = 914400 EMU)
603-
let inches: f64 = position
604-
.trim_end_matches("in")
605-
.parse()
606-
.unwrap_or(0.0);
597+
let inches: f64 = position.trim_end_matches("in").parse().unwrap_or(0.0);
607598
(inches * 914400.0) as i64
608599
} else if position.ends_with("cm") {
609600
// Centimeters (1 cm = 360000 EMU)
610-
let cm: f64 = position
611-
.trim_end_matches("cm")
612-
.parse()
613-
.unwrap_or(0.0);
601+
let cm: f64 = position.trim_end_matches("cm").parse().unwrap_or(0.0);
614602
(cm * 360000.0) as i64
615603
} else if position.ends_with("emu") {
616604
// Already EMU
617-
position
618-
.trim_end_matches("emu")
619-
.parse()
620-
.unwrap_or(0)
605+
position.trim_end_matches("emu").parse().unwrap_or(0)
621606
} else {
622607
// Default: treat as percentage
623608
let pct: f64 = position.parse().unwrap_or(35.0);
@@ -628,11 +613,7 @@ impl CoverConfig {
628613
/// Expand a content template with metadata values
629614
///
630615
/// Supports: {title}, {subtitle}, {author}, {email}, {revnumber}, {revdate}, {delimiter}
631-
pub fn expand_template(
632-
template: &str,
633-
metadata: &CoverMetadata,
634-
delimiter: &str,
635-
) -> String {
616+
pub fn expand_template(template: &str, metadata: &CoverMetadata, delimiter: &str) -> String {
636617
template
637618
.replace("{title}", &metadata.title)
638619
.replace("{subtitle}", &metadata.subtitle)
@@ -705,7 +686,9 @@ impl StyleContract {
705686

706687
/// Get the semantic role for a paragraph style
707688
pub fn get_paragraph_role(&self, word_style: &str) -> Option<&str> {
708-
self.paragraph_styles.get(word_style).map(|m| m.role.as_str())
689+
self.paragraph_styles
690+
.get(word_style)
691+
.map(|m| m.role.as_str())
709692
}
710693

711694
/// Get the heading level for a paragraph style
@@ -717,7 +700,9 @@ impl StyleContract {
717700

718701
/// Get the semantic anchor ID for a Word bookmark
719702
pub fn get_semantic_anchor(&self, word_bookmark: &str) -> Option<&str> {
720-
self.anchors.get(word_bookmark).map(|m| m.semantic_id.as_str())
703+
self.anchors
704+
.get(word_bookmark)
705+
.map(|m| m.semantic_id.as_str())
721706
}
722707

723708
/// Get the Word bookmark for a semantic anchor ID
@@ -1082,10 +1067,7 @@ mod tests {
10821067
CoverConfig::parse_position_to_emu("100%", page_height),
10831068
10_000_000
10841069
);
1085-
assert_eq!(
1086-
CoverConfig::parse_position_to_emu("0%", page_height),
1087-
0
1088-
);
1070+
assert_eq!(CoverConfig::parse_position_to_emu("0%", page_height), 0);
10891071
}
10901072

10911073
#[test]
@@ -1162,11 +1144,7 @@ mod tests {
11621144
);
11631145
assert_eq!(result, "Version 1.0.0 | 2025-12-31");
11641146

1165-
let result2 = CoverConfig::expand_template(
1166-
"{author} <{email}>",
1167-
&metadata,
1168-
"",
1169-
);
1147+
let result2 = CoverConfig::expand_template("{author} <{email}>", &metadata, "");
11701148
assert_eq!(result2, "Jane Doe <jane@example.com>");
11711149
}
11721150

@@ -1221,7 +1199,10 @@ content = "v{revnumber} ({revdate})"
12211199
assert_eq!(cover.title.top, "40%");
12221200
assert_eq!(cover.subtitle.color, "00FF00");
12231201
assert!(cover.subtitle.italic);
1224-
assert_eq!(cover.authors.content, Some("{author} ({email})".to_string()));
1202+
assert_eq!(
1203+
cover.authors.content,
1204+
Some("{author} ({email})".to_string())
1205+
);
12251206
assert_eq!(cover.revision.delimiter, " - ");
12261207
}
12271208

@@ -1285,7 +1266,10 @@ content = "Version {revnumber}{delimiter}{revdate}"
12851266

12861267
let contract: StyleContract = toml::from_str(toml_str).unwrap();
12871268

1288-
assert_eq!(contract.meta.template, Some("open_template.dotx".to_string()));
1269+
assert_eq!(
1270+
contract.meta.template,
1271+
Some("open_template.dotx".to_string())
1272+
);
12891273
assert!(contract.cover.is_some());
12901274

12911275
let cover = contract.cover.unwrap();
@@ -1294,6 +1278,9 @@ content = "Version {revnumber}{delimiter}{revdate}"
12941278
assert!(cover.title.bold);
12951279
assert_eq!(cover.subtitle.top, "45%");
12961280
assert_eq!(cover.authors.content, Some("{author}".to_string()));
1297-
assert_eq!(cover.revision.content, "Version {revnumber}{delimiter}{revdate}");
1281+
assert_eq!(
1282+
cover.revision.content,
1283+
"Version {revnumber}{delimiter}{revdate}"
1284+
);
12981285
}
12991286
}

‎crates/utf8dok-ooxml/src/writer.rs‎

Lines changed: 58 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -869,7 +869,8 @@ impl DocxWriter {
869869
let (filename, image_bytes) = cover_data;
870870

871871
// Get cover configuration from StyleContract or use defaults
872-
let cover_config = self.style_contract
872+
let cover_config = self
873+
.style_contract
873874
.as_ref()
874875
.and_then(|sc| sc.cover.clone())
875876
.unwrap_or_else(CoverConfig::for_dark_background);
@@ -931,11 +932,7 @@ impl DocxWriter {
931932

932933
// === TITLE ===
933934
if !metadata.title.is_empty() {
934-
self.generate_cover_element(
935-
&metadata.title,
936-
&cover_config.title,
937-
page_height_emu,
938-
);
935+
self.generate_cover_element(&metadata.title, &cover_config.title, page_height_emu);
939936
}
940937

941938
// === SUBTITLE ===
@@ -954,11 +951,7 @@ impl DocxWriter {
954951
metadata.author.clone()
955952
};
956953
if !authors_text.is_empty() {
957-
self.generate_cover_element(
958-
&authors_text,
959-
&cover_config.authors,
960-
page_height_emu,
961-
);
954+
self.generate_cover_element(&authors_text, &cover_config.authors, page_height_emu);
962955
}
963956

964957
// === REVISION ===
@@ -993,23 +986,49 @@ impl DocxWriter {
993986
let author = if !doc.metadata.authors.is_empty() {
994987
doc.metadata.authors.join(", ")
995988
} else {
996-
doc.metadata.attributes.get("author").cloned().unwrap_or_default()
989+
doc.metadata
990+
.attributes
991+
.get("author")
992+
.cloned()
993+
.unwrap_or_default()
997994
};
998995

999996
// Email
1000-
let email = doc.metadata.attributes.get("email").cloned().unwrap_or_default();
997+
let email = doc
998+
.metadata
999+
.attributes
1000+
.get("email")
1001+
.cloned()
1002+
.unwrap_or_default();
10011003

10021004
// Revision: try direct field first, then attribute
1003-
let revnumber = doc.metadata.revision.clone()
1005+
let revnumber = doc
1006+
.metadata
1007+
.revision
1008+
.clone()
10041009
.or_else(|| doc.metadata.attributes.get("revnumber").cloned())
10051010
.unwrap_or_default();
10061011

1007-
let revdate = doc.metadata.attributes.get("revdate").cloned().unwrap_or_default();
1012+
let revdate = doc
1013+
.metadata
1014+
.attributes
1015+
.get("revdate")
1016+
.cloned()
1017+
.unwrap_or_default();
10081018

1009-
let revremark = doc.metadata.attributes.get("revremark").cloned().unwrap_or_default();
1019+
let revremark = doc
1020+
.metadata
1021+
.attributes
1022+
.get("revremark")
1023+
.cloned()
1024+
.unwrap_or_default();
10101025

10111026
// Subtitle: try multiple sources
1012-
let subtitle = doc.metadata.attributes.get("description").cloned()
1027+
let subtitle = doc
1028+
.metadata
1029+
.attributes
1030+
.get("description")
1031+
.cloned()
10131032
.or_else(|| doc.metadata.attributes.get("subtitle").cloned())
10141033
.or_else(|| doc.metadata.attributes.get("revremark").cloned())
10151034
.unwrap_or_default();
@@ -1044,7 +1063,8 @@ impl DocxWriter {
10441063
TextAlign::Center => "center",
10451064
TextAlign::Right => "right",
10461065
};
1047-
self.output.push_str(&format!("<w:jc w:val=\"{}\"/>\n", align_val));
1066+
self.output
1067+
.push_str(&format!("<w:jc w:val=\"{}\"/>\n", align_val));
10481068

10491069
// Frame positioning for absolute placement
10501070
self.output.push_str(&format!(
@@ -1056,11 +1076,14 @@ impl DocxWriter {
10561076
self.output.push_str("<w:r>\n<w:rPr>\n");
10571077

10581078
// Font size
1059-
self.output.push_str(&format!("<w:sz w:val=\"{}\"/>\n", config.font_size));
1060-
self.output.push_str(&format!("<w:szCs w:val=\"{}\"/>\n", config.font_size));
1079+
self.output
1080+
.push_str(&format!("<w:sz w:val=\"{}\"/>\n", config.font_size));
1081+
self.output
1082+
.push_str(&format!("<w:szCs w:val=\"{}\"/>\n", config.font_size));
10611083

10621084
// Color
1063-
self.output.push_str(&format!("<w:color w:val=\"{}\"/>\n", config.color));
1085+
self.output
1086+
.push_str(&format!("<w:color w:val=\"{}\"/>\n", config.color));
10641087

10651088
// Bold
10661089
if config.bold {
@@ -1076,7 +1099,8 @@ impl DocxWriter {
10761099
if let Some(ref font) = config.font_family {
10771100
self.output.push_str(&format!(
10781101
"<w:rFonts w:ascii=\"{}\" w:hAnsi=\"{}\"/>\n",
1079-
escape_xml(font), escape_xml(font)
1102+
escape_xml(font),
1103+
escape_xml(font)
10801104
));
10811105
}
10821106

@@ -1103,7 +1127,8 @@ impl DocxWriter {
11031127
TextAlign::Center => "center",
11041128
TextAlign::Right => "right",
11051129
};
1106-
self.output.push_str(&format!("<w:jc w:val=\"{}\"/>\n", align_val));
1130+
self.output
1131+
.push_str(&format!("<w:jc w:val=\"{}\"/>\n", align_val));
11071132
self.output.push_str(&format!(
11081133
"<w:framePr w:vAnchor=\"page\" w:y=\"{}\"/>\n",
11091134
position_twips
@@ -1112,9 +1137,12 @@ impl DocxWriter {
11121137
self.output.push_str("</w:pPr>\n");
11131138
self.output.push_str("<w:r>\n<w:rPr>\n");
11141139

1115-
self.output.push_str(&format!("<w:sz w:val=\"{}\"/>\n", config.font_size));
1116-
self.output.push_str(&format!("<w:szCs w:val=\"{}\"/>\n", config.font_size));
1117-
self.output.push_str(&format!("<w:color w:val=\"{}\"/>\n", config.color));
1140+
self.output
1141+
.push_str(&format!("<w:sz w:val=\"{}\"/>\n", config.font_size));
1142+
self.output
1143+
.push_str(&format!("<w:szCs w:val=\"{}\"/>\n", config.font_size));
1144+
self.output
1145+
.push_str(&format!("<w:color w:val=\"{}\"/>\n", config.color));
11181146

11191147
if config.bold {
11201148
self.output.push_str("<w:b/>\n");
@@ -1126,7 +1154,8 @@ impl DocxWriter {
11261154
if let Some(ref font) = config.font_family {
11271155
self.output.push_str(&format!(
11281156
"<w:rFonts w:ascii=\"{}\" w:hAnsi=\"{}\"/>\n",
1129-
escape_xml(font), escape_xml(font)
1157+
escape_xml(font),
1158+
escape_xml(font)
11301159
));
11311160
}
11321161

@@ -1680,10 +1709,8 @@ impl DocxWriter {
16801709
bookmark_id,
16811710
escape_xml(&bookmark_name)
16821711
));
1683-
self.output.push_str(&format!(
1684-
"<w:bookmarkEnd w:id=\"{}\"/>\n",
1685-
bookmark_id
1686-
));
1712+
self.output
1713+
.push_str(&format!("<w:bookmarkEnd w:id=\"{}\"/>\n", bookmark_id));
16871714
}
16881715
}
16891716
}

0 commit comments

Comments
 (0)