Skip to content

Add minimal XLSX export support - #22

Open
donatj wants to merge 4 commits into
masterfrom
feature/minimal-xlsx-export
Open

donatj wants to merge 4 commits into
masterfrom
feature/minimal-xlsx-export

Conversation

@donatj

@donatj donatj commented Sep 1, 2026

Copy link
Copy Markdown
Member

Summary

  • add a minimal XlsxEngine that mirrors SpreadsheetML cell support
  • package worksheets as streamed Office Open XML using the existing ZipStream dependency
  • add integration coverage and CI ZIP support

Validation

  • vendor/bin/phpunit
  • vendor/bin/phpcs
  • vendor/bin/php-cs-fixer fix --dry-run --diff --sequential
  • LibreOffice headless XLSX-to-CSV smoke test

Copilot AI lite review requested due to automatic review settings September 1, 2026 04:18

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds first-class XLSX export capability to the exporter by introducing a new XlsxEngine that writes a minimal Office Open XML workbook as a streamed ZIP archive, plus integration coverage and CI/dev dependencies to support ZIP-based validation.

Changes:

  • Introduce XlsxEngine to generate a minimal .xlsx (OOXML) archive via ZipStream.
  • Add an integration test that validates the produced ZIP entries and key XML values (workbook, sheets, core properties).
  • Update docs and CI/dev requirements to include XLSX support and ZIP tooling.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
test/Integration/XlsxTest.php New integration test that validates XLSX ZIP contents and XML structure/values.
src/Engines/XlsxEngine.php New streaming OOXML XLSX writer using ZipStream, with minimal worksheet/style/core-props support.
README.md Document XLSX as a supported format and add XlsxEngine API docs.
composer.json Add ext-zip to dev requirements (used by integration test validation).
.mddoc.xml.dist Update supported formats list to include XLSX.
.github/workflows/ci.yml Ensure CI installs the zip extension to run XLSX-related tests.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/Engines/XlsxEngine.php
Comment thread src/Engines/XlsxEngine.php Outdated
@donatj
donatj force-pushed the feature/minimal-xlsx-export branch from 6275802 to c6c7c59 Compare October 6, 2026 03:48
@donatj
donatj requested a balanced review from Copilot October 6, 2026 03:54

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

XLSX relationship metadata, sheet-name validation, and repeated-export state require correction.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
Resolved since last review (2)

Comment thread src/Engines/XlsxEngine.php

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The relationship metadata URI is incorrect, and accepted control characters can produce malformed workbook XML.

Review effort: Balanced
Findings: 1 High severity

Open (1)
Resolved since last review (1)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Use OPC namespace for core-properties relationship

src/​Engines/​XlsxEngine.php:292

The core-properties relationship type uses the office-document namespace, but this relationship is defined by OPC under http://schemas.openxmlformats.org/package/2006/relationships/metadata/core-properties. Package-aware consumers may therefore fail to discover docProps/core.xml; use the package relationships namespace here.

$inlineString = $this->appendElement($rowDocument, $cell, self::SPREADSHEET_NAMESPACE, 'is');
$text = $this->appendElement($rowDocument, $inlineString, self::SPREADSHEET_NAMESPACE, 't');
$text->setAttributeNS(self::XML_NAMESPACE, 'xml:space', 'preserve');
$text->appendChild($rowDocument->createTextNode((string)$value));
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants