feat: Add formula extraction support - #10
Conversation
Add ability to extract formulas from Excel worksheets.
Formulas are stored in <f> elements within <c> cell elements
in the worksheet XML.
Features:
- Extract formulas from file path or Book object
- Extract formulas from specific sheet or all sheets
- Graceful error handling (returns empty hash on failure)
- Full documentation with examples
Usage:
formulas = XlsxParser::Formula.extract_file("sheet.xlsx", 0)
# => {"A1" => "SUM(B1:B10)"}
Based on implementation from sheety spreadsheet compiler.
There was a problem hiding this comment.
Pull request overview
Adds a new API to extract Excel formulas from worksheet XML inside .xlsx files, integrating it into the library’s public entrypoint and documentation.
Changes:
- Introduces
XlsxParser::Formulawith methods to extract formulas from a file, aBook, or all sheets. - Extends
XlsxParser::Bookto retain the original filename (when initialized from a path). - Wires the new feature into
src/xlsx-parser.crand documents usage inREADME.md.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
src/xlsx-parser/formula.cr |
New formula extraction implementation from worksheet XML. |
src/xlsx-parser/book.cr |
Adds filename to support formula extraction via Book. |
src/xlsx-parser.cr |
Requires the new formula module in the public entrypoint. |
README.md |
Documents formula extraction usage and examples. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| begin | ||
| doc = XML.parse(xml_content) | ||
|
|
||
| # Find all <c> (cell) elements using local-name() to handle namespaces | ||
| # Then check for <f> (formula) children | ||
| doc.xpath_nodes("//*[local-name()='c']").each do |cell_node| | ||
| # Get cell reference from r attribute (e.g., "A1") |
There was a problem hiding this comment.
parse_formulas_from_xml parses the entire worksheet XML into memory (XML.parse) after reading it fully (gets_to_end). For large sheets this can be a significant memory/CPU hit; consider using XML::Reader (like Sheet#rows) to stream through <c> / <f> elements instead of building a full DOM.
| def self.extract_file(filename : String, sheet_index : Int32) : Hash(String, String) | ||
| formulas = {} of String => String | ||
|
|
||
| begin | ||
| zip = Compress::Zip::File.new(filename) | ||
|
|
||
| # Worksheet files are named sheet1.xml, sheet2.xml, etc. | ||
| sheet_path = "xl/worksheets/sheet#{sheet_index + 1}.xml" | ||
|
|
||
| if zip[sheet_path]? | ||
| xml_content = zip[sheet_path].open(&.gets_to_end) | ||
| formulas = parse_formulas_from_xml(xml_content) | ||
| end | ||
|
|
||
| zip.close | ||
| rescue ex : Exception | ||
| # If we can't extract formulas, return empty hash | ||
| # This allows the importer to still work with values | ||
| formulas = {} of String => String | ||
| end | ||
|
|
||
| formulas | ||
| end |
There was a problem hiding this comment.
Formula extraction is newly introduced but there are no specs covering it (unlike Book, Sheet, and Styles). Adding specs (and a fixture workbook containing formulas, including shared formulas) would help prevent regressions and validate edge cases.
| all_formulas = XlsxParser::Formula.extract_all_sheets("./my_super_spreadsheet.xlsx", book.sheets.size) | ||
| # => [{"A1" => "SUM(B1:B10)"}, {"C3" => "MAX(A1:A10)"}] | ||
|
|
There was a problem hiding this comment.
The README example for extract_all_sheets has a syntax error: the second hash is missing its closing }. This will fail if someone copies the snippet; update it to show a valid Crystal literal.
| begin | ||
| zip = Compress::Zip::File.new(filename) | ||
|
|
||
| # Worksheet files are named sheet1.xml, sheet2.xml, etc. | ||
| sheet_path = "xl/worksheets/sheet#{sheet_index + 1}.xml" | ||
|
|
||
| if zip[sheet_path]? | ||
| xml_content = zip[sheet_path].open(&.gets_to_end) | ||
| formulas = parse_formulas_from_xml(xml_content) | ||
| end | ||
|
|
||
| zip.close | ||
| rescue ex : Exception | ||
| # If we can't extract formulas, return empty hash | ||
| # This allows the importer to still work with values | ||
| formulas = {} of String => String | ||
| end |
There was a problem hiding this comment.
extract_file doesn’t guarantee the ZIP file gets closed if an exception is raised before zip.close (e.g., invalid XML, read error). This can leak file handles; use an ensure block (or Compress::Zip::File.open with a block) so the zip is always closed.
| # Worksheet files are named sheet1.xml, sheet2.xml, etc. | ||
| sheet_path = "xl/worksheets/sheet#{sheet_index + 1}.xml" | ||
|
|
||
| if zip[sheet_path]? | ||
| xml_content = zip[sheet_path].open(&.gets_to_end) | ||
| formulas = parse_formulas_from_xml(xml_content) | ||
| end |
There was a problem hiding this comment.
The worksheet path is assumed to be xl/worksheets/sheet#{sheet_index + 1}.xml, but in XLSX the actual worksheet file is defined via xl/workbook.xml + xl/_rels/workbook.xml.rels (and may not match the 1-based index). This can return an empty hash for valid workbooks; consider resolving the sheet XML target the same way Book#sheets does.
| def self.extract(book : Book, sheet_index : Int32) : Hash(String, String) | ||
| return {} of String => String unless book.filename | ||
|
|
||
| extract_file(book.filename, sheet_index) | ||
| end |
There was a problem hiding this comment.
extract(book, sheet_index) silently returns {} for books opened from an IO because Book#filename is nil in that case. Since the API advertises extraction from a Book, consider reading from book.zip directly (without reopening the file) so it works for both String and IO sources.
| formula_nodes = cell_node.xpath_nodes("./*[local-name()='f']") | ||
|
|
||
| if formula_nodes.size > 0 | ||
| formula = formula_nodes.first.content.strip | ||
|
|
||
| # Only store non-empty formulas | ||
| unless formula.empty? | ||
| formulas[cell_ref] = formula | ||
| end | ||
| end |
There was a problem hiding this comment.
Excel commonly uses shared formulas where non-master cells have <f t="shared" si="…"/> with empty content. This implementation drops empty <f> nodes, so many formulas will be missing in real-world sheets; handle t="shared" by mapping si to the master formula and applying it (or at least returning the master formula for shared cells).
Improve formula extraction implementation based on PR review: - Use Compress::Zip::File.open with block to ensure proper cleanup - Resolve actual worksheet paths from workbook relationships instead of assuming sheet1.xml naming - Support Book objects opened from both String and IO by using book.zip directly - Add placeholder for shared formulas (SHARED_FORMULA) instead of dropping them - Remove filename getter from Book as it's no longer needed - Better error handling with ensure blocks These changes make the implementation more robust and aligned with how xlsx-parser handles worksheet files elsewhere.
Add ability to extract formulas from Excel worksheets. Formulas are stored in elements within cell elements in the worksheet XML.
Features:
Usage:
formulas = XlsxParser::Formula.extract_file("sheet.xlsx", 0) # => {"A1" => "SUM(B1:B10)"}
Based on implementation from sheety.ralsina.me spreadsheet compiler.