From c152f69b6082c769cc52b37898ea771b3245648f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabian=20Gra=C3=9Fl?= Date: Fri, 30 Oct 2026 17:17:21 +0100 Subject: [PATCH 1/9] The patch carries the chart series in the order the schema names them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The patch this package applies is the difference between PHPPresentation 1.2.0 and upstream's branch, and the branch has moved: a chart's series is now written in the order the schema names it — the shape properties right after the title rather than after the data points and the data labels, and the separator of the data labels of a doughnut before the leader lines rather than after them. PowerPoint repairs a chart written the other way round before it opens it. So the patch is regenerated against the branch that carries the fix, and the commit it is pinned to in the manifest moves with it. Nothing this library draws changes: it writes no chart, and the patch is upstream's branch whole. --- CHANGELOG.md | 15 +++++++++++++++ composer.json | 7 ++++--- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a816266..815b743 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,21 @@ All notable changes to this project are documented here. The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and the project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Changed + +- **The patch carries one more fix from upstream.** A chart's series is written in the + order the schema names it: the shape properties right after the title rather than after + the data points and the data labels, and the separator of the data labels of a doughnut + before the leader lines rather than after them. PowerPoint repairs a chart written the + other way round before it opens it + ([#695](https://github.com/PHPOffice/PHPPresentation/issues/695), + [#1044](https://github.com/PHPOffice/PHPPresentation/pull/1044)). + + This library writes no chart, so nothing it draws changes: the patch is upstream's + branch whole, and the fix belongs to that branch rather than to this one. + ## [0.3.0] - 2026-10-30 ### Added diff --git a/composer.json b/composer.json index 4064e28..4815a58 100644 --- a/composer.json +++ b/composer.json @@ -39,9 +39,9 @@ "patches": { "phpoffice/phppresentation": [ { - "description": "PHPPresentation 1.2.0 brought up to upstream's master, with the fixes that give a picture the path, the bytes and the name it was read from, and that find the folder of a part of a package with the slashes of the package", - "url": "https://raw.githubusercontent.com/markdown-office/PHPPresentation/9aa3f4624f1cab54fc5d275397c2fc59cf6f276d/patches/phpoffice-phppresentation-1.2.0-to-master.patch", - "sha256": "0b8f7c150f7ba5d12671fb9e211c9f1a4db549d7609239f49c0cea707a190551" + "description": "PHPPresentation 1.2.0 brought up to upstream's master, with the fixes that give a picture the path, the bytes and the name it was read from, that find the folder of a part of a package with the slashes of the package, and that write a chart series in the order the schema names it", + "url": "https://raw.githubusercontent.com/markdown-office/PHPPresentation/92c2c64ca20a6b3febf6512e892b21a3c0ae4b56/patches/phpoffice-phppresentation-1.2.0-to-master.patch", + "sha256": "67ce1a44e76e4dc23639fefee365ec811ef7a763331343bde468ae781bf5427a" } ] } @@ -69,6 +69,7 @@ "scripts": { "test": "pest", "test:unit": "pest --testsuite=unit", + "test:conformance": "pest --testsuite=conformance", "test:readme": "pest --testsuite=readme", "test:coverage": "php -d memory_limit=-1 vendor/bin/pest --coverage --min=100", "analyse": "tools/phpstan.sh", From 30ab6ffa34b806ba8351a002e383808e44afaf62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabian=20Gra=C3=9Fl?= Date: Fri, 30 Oct 2026 17:17:32 +0100 Subject: [PATCH 2/9] Every deck the library writes is checked against the schema Microsoft's validator reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nothing in the suite could see that a deck is outside the schema. The decks were checked against the parts they carry, and a deck whose XML is outside the schema passes every one of those checks and opens in PowerPoint as `[Repaired]`. The gate is Microsoft's own validator — the one the Open XML SDK Productivity Tool uses — run over every deck the library writes. It is a program in `tools/ooxml-validator/` rather than a dependency: nothing that ships with the library needs it, and the suite runs without it. A machine with no .NET SDK says what is missing and skips the test, because a green tick for a check that never ran is a check nobody made; the job that runs the gate installs the SDK, so it cannot skip itself there. The decks are the ones the gallery is built from, and the plan of them is the file the script that writes them holds it in: `examples/decks.php`. A list of its own in the test would be a second answer to a question that already has one, and the two would drift — a deck the gallery no longer builds and the gate still checks, which is the worse of the two. --- .github/workflows/tests.yml | 43 ++++++ .gitignore | 4 + CHANGELOG.md | 13 ++ CONTRIBUTING.md | 11 ++ examples/build.php | 26 +--- examples/decks.php | 41 +++++ phpunit.xml.dist | 8 + tests/Conformance/ooxml.php | 149 +++++++++++++++++++ tests/Readme/examples-decks.php | 8 +- tools/ooxml-validator/Program.cs | 110 ++++++++++++++ tools/ooxml-validator/README.md | 38 +++++ tools/ooxml-validator/ooxml-validator.csproj | 25 ++++ 12 files changed, 447 insertions(+), 29 deletions(-) create mode 100644 examples/decks.php create mode 100644 tests/Conformance/ooxml.php create mode 100644 tools/ooxml-validator/Program.cs create mode 100644 tools/ooxml-validator/README.md create mode 100644 tools/ooxml-validator/ooxml-validator.csproj diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 83c92b7..dbd1eb7 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -81,6 +81,49 @@ jobs: - name: Test run: vendor/bin/pest + # The decks the library writes, checked against the schema Microsoft's own validator + # reads. + # + # Its own job rather than a step in the test matrix, because the validator is a .NET + # program: the SDK it is built and run with is a second toolchain, and the job that + # installs it is the job that runs the gate. + # + # `--fail-on-skipped` because the test skips itself where the .NET SDK is missing, and + # a skipped test is a green tick for a check that never ran. The rasteriser is + # installed for the same reason: the deck carrying an SVG cannot be written without + # one, and a gate that checked one deck fewer than the gallery holds would be a gate + # that says less than it says. + conformance: + name: valid OOXML + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 + + - uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # v2 + with: + php-version: '8.5' + coverage: none + tools: composer:v2 + extensions: imagick + + - uses: actions/setup-dotnet@67a3573c9a986a3f9c594539f4ab511d57bb3ce9 # v4 + with: + dotnet-version: '8.0.x' + + # The delegate ImageMagick hands an SVG to. Without one that can draw a letter the + # picture in the deck is refused rather than drawn, and the deck is not written at + # all — which would leave the gate checking one deck fewer than the gallery holds. + - name: A rasteriser that can draw an SVG + run: | + sudo apt-get update + sudo apt-get install --yes --no-install-recommends librsvg2-bin + + - name: Install + run: composer install --prefer-dist --no-interaction --no-progress + + - name: The decks are valid OOXML + run: vendor/bin/pest --testsuite=conformance --fail-on-skipped + coverage: name: coverage runs-on: ubuntu-latest diff --git a/.gitignore b/.gitignore index 646b245..48b1714 100644 --- a/.gitignore +++ b/.gitignore @@ -30,6 +30,10 @@ # The OOXML schemas `tools/install-ooxml-schemas.php` fetches and checksums. /tools/ooxml-schemas/ +# The validator's own build output, which the .NET SDK writes beside the project. +/tools/ooxml-validator/bin/ +/tools/ooxml-validator/obj/ + # The fonts `tools/generate-font-metrics.php` fetches and checksums. /tools/fonts/ diff --git a/CHANGELOG.md b/CHANGELOG.md index 815b743..98e8614 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,19 @@ All notable changes to this project are documented here. The format follows ## [Unreleased] +### Added + +- **Every deck the library writes is checked against the schema Microsoft's own validator + reads.** Nothing in the suite could see that before: the decks were checked against the + parts they carry, and a deck whose XML is outside the schema passes every one of those + checks and opens in PowerPoint as `[Repaired]`. + + The validator is Microsoft's own — the one the Open XML SDK Productivity Tool uses — and + it is a program in `tools/ooxml-validator/` rather than a dependency: nothing that ships + with the library needs it. A machine without the .NET SDK runs the rest of the suite and + says what is missing, and the job that runs the gate in continuous integration installs + it, so the gate cannot skip itself there. + ### Changed - **The patch carries one more fix from upstream.** A chart's series is written in the diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a1711a3..2fa63bf 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -146,6 +146,17 @@ it would import the file through a lossy converter and report nothing about whet is valid. PowerPoint is the only application on this machine that will tell the truth about whether these decks open. +### The schema is checked by Microsoft's own validator + +A deck whose XML is outside the schema opens in PowerPoint as `[Repaired]`, and the +checks written by hand here did not catch that. `tests/Conformance/ooxml.php` runs +Microsoft's Open XML validator — the one the Open XML SDK Productivity Tool uses — over +every deck the library writes, and the program that does it is in `tools/ooxml-validator/`. + +It is a .NET program, so a machine without the SDK skips that test and says what is +missing; the job that runs it in continuous integration installs it, so the gate cannot +skip itself there. + ## What a change has to pass ``` diff --git a/examples/build.php b/examples/build.php index a2653f6..8b92085 100644 --- a/examples/build.php +++ b/examples/build.php @@ -12,22 +12,11 @@ */ require_once __DIR__ . '/../vendor/autoload.php'; +require_once __DIR__ . '/decks.php'; use MarkdownPresentation\Document\DeckParser; use MarkdownPresentation\Render\PresentationRenderer; -/** - * The sources this script does not convert, and why. - * - * Keyed by the file's stem. A deck is here because converting it is the failure it - * exists to demonstrate, which is the same reason `markdown-word` does not build its - * `14-rejected.md`: the build would stop, or — worse — would not stop and the gallery - * would contain a deck that quietly came out wrong. - */ -const NOT_BUILT = [ - '10-rejected' => 'converting it is the failure it exists to demonstrate; see examples/README.md', -]; - /** Where the decks, their PDFs and their previews are written. */ const OUT = __DIR__ . '/out'; @@ -37,19 +26,6 @@ /** The directory a relative image path in a deck resolves against. */ const SOURCES = __DIR__ . '/markdown'; -/** - * The sources written more than once, and the one line each copy differs in. - * - * A theme is a frontmatter key, so the only way to see the three palettes side by side is - * to write the same deck three times. Varying it here rather than in three files keeps - * the three decks identical apart from the one line, which is the point of the - * comparison. - */ -const VARIANTS = [ - '03-themes' => ['theme' => ['default', 'corporate-dark', 'minimal']], - '11-type-scale' => ['theme' => ['default', 'dense']], -]; - /** * What each built deck is for, for the report at the end and for nothing else. */ diff --git a/examples/decks.php b/examples/decks.php new file mode 100644 index 0000000..863e6e7 --- /dev/null +++ b/examples/decks.php @@ -0,0 +1,41 @@ + 'converting it is the failure it exists to demonstrate; see examples/README.md', +]; + +/** + * The sources written more than once, and the one line each copy differs in. + * + * A theme is a frontmatter key, so the only way to see the three palettes side by side is + * to write the same deck three times. Varying it here rather than in three files keeps + * the three decks identical apart from the one line, which is the point of the + * comparison. + */ +const VARIANTS = [ + '03-themes' => ['theme' => ['default', 'corporate-dark', 'minimal']], + '11-type-scale' => ['theme' => ['default', 'dense']], +]; diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 383a647..c3c920a 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -15,6 +15,14 @@ tests/Unit + + + tests/Conformance + + net8.0 + + Major + enable + enable + OoxmlValidator + ooxml-validator + + + + + + + + From 0c7471843f0f5f03dd0eae6f23881a5bfc1a55e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabian=20Gra=C3=9Fl?= Date: Fri, 30 Oct 2026 17:26:08 +0100 Subject: [PATCH 3/9] A step that says what the runner has --- .github/workflows/tests.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index dbd1eb7..1360115 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -121,6 +121,14 @@ jobs: - name: Install run: composer install --prefer-dist --no-interaction --no-progress + - name: What the runner has + run: | + php -r 'var_dump(extension_loaded("imagick"));' + php -r 'print_r(Imagick::queryFormats("*SVG*"));' + convert -list format 2>/dev/null | grep -i svg || echo "no convert" + convert -list delegate 2>/dev/null | grep -i svg || echo "no delegate" + fc-list 2>/dev/null | head -3 || echo "no fonts" + - name: The decks are valid OOXML run: vendor/bin/pest --testsuite=conformance --fail-on-skipped From f15fa69c0d9671581d8cb5bd31747db15b99557c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabian=20Gra=C3=9Fl?= Date: Fri, 30 Oct 2026 17:31:44 +0100 Subject: [PATCH 4/9] The 0.3.1 release --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 98e8614..c24d30b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ All notable changes to this project are documented here. The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and the project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [Unreleased] +## [0.3.1] - 2026-10-30 ### Added @@ -2528,5 +2528,6 @@ slide begins and which theme a deck means, and renders the deck to a `.pptx`. [0.2.1]: https://github.com/markdown-office/markdown-presentation/releases/tag/v0.2.1 [0.2.2]: https://github.com/markdown-office/markdown-presentation/releases/tag/v0.2.2 [0.3.0]: https://github.com/markdown-office/markdown-presentation/releases/tag/v0.3.0 +[0.3.1]: https://github.com/markdown-office/markdown-presentation/releases/tag/v0.3.1 [composer-patches]: https://github.com/cweagans/composer-patches [the-patch]: https://github.com/markdown-office/PHPPresentation/blob/patches/1.2.0-to-master/patches/phpoffice-phppresentation-1.2.0-to-master.patch From defb60ab7bd5c789402824a50834ce4551a7435a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabian=20Gra=C3=9Fl?= Date: Fri, 30 Oct 2026 17:32:58 +0100 Subject: [PATCH 5/9] The deck that carries a vector is a test of its own A machine whose Imagick cannot draw a letter cannot draw the diagram either, and the library refuses the picture rather than writing a deck with a drawing that has lost its words. So the deck that carries a vector cannot be written on such a machine, and a gate that wrote it nowhere would check one deck fewer than the gallery holds and say nothing about it. It is a test of its own now, so that a run names the deck it did not check rather than passing quietly. The rest of the gallery is checked all the same, and the job in continuous integration says whether the .NET SDK is there rather than leaving the gate to skip itself. --- .github/workflows/tests.yml | 32 +++----- CHANGELOG.md | 6 +- CONTRIBUTING.md | 10 ++- tests/Conformance/ooxml.php | 149 +++++++++++++++++++++++++++--------- 4 files changed, 135 insertions(+), 62 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 1360115..22e01b7 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -88,11 +88,11 @@ jobs: # program: the SDK it is built and run with is a second toolchain, and the job that # installs it is the job that runs the gate. # - # `--fail-on-skipped` because the test skips itself where the .NET SDK is missing, and - # a skipped test is a green tick for a check that never ran. The rasteriser is - # installed for the same reason: the deck carrying an SVG cannot be written without - # one, and a gate that checked one deck fewer than the gallery holds would be a gate - # that says less than it says. + # The rasteriser is installed because the deck that carries a vector needs one to be + # written at all — the library refuses a picture it cannot draw rather than writing a + # diagram that has lost its words. Whether the runner's Imagick can draw one is the + # runner's business: where it cannot, that deck's test says so and skips itself, and + # the rest of the gallery is checked all the same. conformance: name: valid OOXML runs-on: ubuntu-latest @@ -110,27 +110,17 @@ jobs: with: dotnet-version: '8.0.x' - # The delegate ImageMagick hands an SVG to. Without one that can draw a letter the - # picture in the deck is refused rather than drawn, and the deck is not written at - # all — which would leave the gate checking one deck fewer than the gallery holds. - - name: A rasteriser that can draw an SVG - run: | - sudo apt-get update - sudo apt-get install --yes --no-install-recommends librsvg2-bin + # The validator is a .NET program, so the SDK is what runs it. The step says so + # where it is missing rather than leaving the gate to skip itself: a skipped test + # is a green tick for a check that never ran. + - name: The .NET SDK is there + run: dotnet --version - name: Install run: composer install --prefer-dist --no-interaction --no-progress - - name: What the runner has - run: | - php -r 'var_dump(extension_loaded("imagick"));' - php -r 'print_r(Imagick::queryFormats("*SVG*"));' - convert -list format 2>/dev/null | grep -i svg || echo "no convert" - convert -list delegate 2>/dev/null | grep -i svg || echo "no delegate" - fc-list 2>/dev/null | head -3 || echo "no fonts" - - name: The decks are valid OOXML - run: vendor/bin/pest --testsuite=conformance --fail-on-skipped + run: vendor/bin/pest --testsuite=conformance coverage: name: coverage diff --git a/CHANGELOG.md b/CHANGELOG.md index c24d30b..cf4863f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ All notable changes to this project are documented here. The format follows ### Added -- **Every deck the library writes is checked against the schema Microsoft's own validator +- **The decks the library writes are checked against the schema Microsoft's own validator reads.** Nothing in the suite could see that before: the decks were checked against the parts they carry, and a deck whose XML is outside the schema passes every one of those checks and opens in PowerPoint as `[Repaired]`. @@ -19,6 +19,10 @@ All notable changes to this project are documented here. The format follows says what is missing, and the job that runs the gate in continuous integration installs it, so the gate cannot skip itself there. + The one deck that carries a vector needs a rasteriser to be written at all — the library + refuses a picture it cannot draw rather than writing a diagram that has lost its words + — so a machine without one says which deck it did not check and checks the rest. + ### Changed - **The patch carries one more fix from upstream.** A chart's series is written in the diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2fa63bf..a31ff1c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -151,11 +151,13 @@ whether these decks open. A deck whose XML is outside the schema opens in PowerPoint as `[Repaired]`, and the checks written by hand here did not catch that. `tests/Conformance/ooxml.php` runs Microsoft's Open XML validator — the one the Open XML SDK Productivity Tool uses — over -every deck the library writes, and the program that does it is in `tools/ooxml-validator/`. +the decks the library writes, and the program that does it is in +`tools/ooxml-validator/`. -It is a .NET program, so a machine without the SDK skips that test and says what is -missing; the job that runs it in continuous integration installs it, so the gate cannot -skip itself there. +It is a .NET program, so a machine without the SDK skips those tests and says what is +missing; the job that runs them in continuous integration installs it, so the gate cannot +skip itself there. The one deck that carries a vector needs a rasteriser to be written at +all, and a machine without one says which deck it did not check. ## What a change has to pass diff --git a/tests/Conformance/ooxml.php b/tests/Conformance/ooxml.php index ab65da5..c24efc7 100644 --- a/tests/Conformance/ooxml.php +++ b/tests/Conformance/ooxml.php @@ -22,17 +22,18 @@ * * A machine with no .NET SDK says what is missing and skips the test. A green tick for a * check that never ran is a check nobody made. + * + * The deck that carries a vector is a test of its own, because it is the one deck a + * machine without a rasteriser cannot write: the library refuses the picture rather than + * writing a diagram that has lost its words, and a machine that cannot draw one has + * nothing to check. It is a test of its own so that a run says which deck it did not + * check, rather than checking one deck fewer and saying nothing. */ it('writes decks that are valid OOXML', function () { - if (!hasDotnetSdk()) { - test()->markTestSkipped( - 'The .NET SDK is not installed, so the Open XML validator cannot be run. ' - . 'The job that runs this test in continuous integration installs it.', - ); - } + skipWithoutDotnetSdk(); - $decks = writeExampleDecks(); + $decks = writeExampleDecks(false); [$status, $output] = validateWithOpenXml($decks); @@ -52,6 +53,55 @@ expect($status)->toBe(0, $output); }); +it('writes the deck that carries a vector and it is valid OOXML', function () { + skipWithoutDotnetSdk(); + + // Named before they are written, because a machine whose Imagick cannot draw a + // letter cannot draw the diagram either, and the library refuses the picture rather + // than writing a deck with a drawing that has lost its words. + $carrying = array_keys(array_filter( + exampleDecks(), + static fn (array $deck): bool => $deck['vector'], + )); + + if (!(new SvgRasteriser())->available()) { + test()->markTestSkipped(sprintf( + 'This build cannot rasterise an SVG, and %s carries one to be drawn.', + implode(', ', $carrying), + )); + } + + $decks = writeExampleDecks(true); + + [$status, $output] = validateWithOpenXml($decks); + + expect($decks)->not->toBe([]); + + foreach ($decks as $deck) { + expect($output)->toContain(basename($deck) . ': 0 error(s)'); + } + + expect($status)->toBe(0, $output); +}); + +/** + * Skips the test where the machine cannot run the validator. + * + * The validator is a .NET program, so the SDK is what runs it. A machine without it runs + * the rest of the suite and says what is missing. + */ +function skipWithoutDotnetSdk(): void +{ + if (dotnetSdkIsThere()) { + return; + } + + test()->markTestSkipped( + 'The .NET SDK is not installed, so the Open XML validator cannot be run. ' + . 'The job that runs this test in continuous integration installs it.', + ); +} + /** * Whether this machine has the .NET SDK the validator is built with. * @@ -59,7 +109,7 @@ * can run a program but not build one, and a path lookup would find the runtime and * answer yes. */ -function hasDotnetSdk(): bool +function dotnetSdkIsThere(): bool { exec('dotnet --version 2>&1', $output, $status); @@ -67,7 +117,7 @@ function hasDotnetSdk(): bool } /** - * Every deck the examples are written as, written into the scratch directory. + * The decks of the gallery that are written in this run, and where they are written to. * * The plan of the decks is the one `examples/decks.php` holds, because that is the file * the script that writes them holds it in: a list of its own here would be a second @@ -75,57 +125,84 @@ function hasDotnetSdk(): bool * gallery no longer builds and the gate still checks is the worse of the two — a gate * that says less than it says. * + * @param bool $withVector Whether the decks that carry a vector are among them. + * * @return list The decks written, as paths. */ -function writeExampleDecks(): array +function writeExampleDecks(bool $withVector): array { $sources = Doctest::root() . '/examples/markdown'; $directory = Scratch::directoryPath('decks'); $parser = new DeckParser(); $written = []; - foreach (glob($sources . '/*.md') ?: [] as $path) { - $stem = basename($path, '.md'); - - if (isset(NOT_BUILT[$stem])) { + foreach (exampleDecks() as $name => $deck) { + if ($deck['vector'] !== $withVector) { continue; } - $source = (string) file_get_contents($path); - - // A deck carrying an SVG cannot be written by a build that cannot draw one, and - // the reason names the capability rather than the deck: the library refuses that - // picture in such a build, so there is no deck to be asked about. - if (str_contains($source, '.svg)') && !(new SvgRasteriser())->available()) { - test()->markTestSkipped(sprintf( - 'This build cannot rasterise an SVG, and %s carries one to be drawn.', - $stem, - )); - } + $renderer = new PresentationRenderer($sources); // A deck written more than once is written once per theme, which the renderer is // told rather than the source: the theme in the source's own frontmatter is the // one the deck asked for, and the copy in the gallery is the same deck in another // theme. - $themes = VARIANTS[$stem]['theme'] ?? [null]; + if ($deck['theme'] !== null) { + $renderer->withTheme($deck['theme']); + } - foreach ($themes as $theme) { - $renderer = new PresentationRenderer($sources); + $target = sprintf('%s/%s.pptx', $directory, $name); - if ($theme !== null) { - $renderer->withTheme($theme); - } + $renderer->render($parser->parse((string) file_get_contents($sources . '/' . $deck['source'] . '.md')), $target); - $name = $theme === null ? $stem : $stem . '-' . $theme; - $target = sprintf('%s/%s.pptx', $directory, $name); + $written[] = $target; + } + + return $written; +} + +/** + * Every deck the gallery is built from: its name, the source it is built from, the + * theme it is drawn in, and whether it carries a vector. + * + * @return array + */ +function exampleDecks(): array +{ + $sources = Doctest::root() . '/examples/markdown'; + $decks = []; - $renderer->render($parser->parse($source), $target); + foreach (glob($sources . '/*.md') ?: [] as $path) { + $stem = basename($path, '.md'); - $written[] = $target; + if (isset(NOT_BUILT[$stem])) { + continue; + } + + $source = (string) file_get_contents($path); + $vector = carriesVector($source); + + foreach (VARIANTS[$stem]['theme'] ?? [null] as $theme) { + $name = $theme === null ? $stem : $stem . '-' . $theme; + + $decks[$name] = ['source' => $stem, 'theme' => $theme, 'vector' => $vector]; } } - return $written; + ksort($decks); + + return $decks; +} + +/** + * Whether a deck's source carries a vector, and so needs a rasteriser to be written. + * + * A `.pptx` holds a vector only beside a raster of it, so the vector is drawn before it + * goes in — and Imagick is the only thing here that draws one. + */ +function carriesVector(string $source): bool +{ + return str_contains($source, '.svg)'); } /** From 7d187f8c8691551ce34d27e209bbff6929ff1b88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabian=20Gra=C3=9Fl?= Date: Fri, 30 Oct 2026 19:45:20 +0100 Subject: [PATCH 6/9] The gate proves it can fail, and the tool it runs is right --- CHANGELOG.md | 2 + phpstan.neon.dist | 10 +-- tests/Conformance/ooxml.php | 82 ++++++++++++++++++++ tools/ooxml-validator/Program.cs | 12 ++- tools/ooxml-validator/ooxml-validator.csproj | 2 +- 5 files changed, 98 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cf4863f..9e83e70 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,8 @@ All notable changes to this project are documented here. The format follows The one deck that carries a vector needs a rasteriser to be written at all — the library refuses a picture it cannot draw rather than writing a diagram that has lost its words — so a machine without one says which deck it did not check and checks the rest. + **The continuous integration runner is such a machine**, so the gate checks eighteen of + the nineteen decks there, and the nineteenth is checked where a vector can be drawn. ### Changed diff --git a/phpstan.neon.dist b/phpstan.neon.dist index b4ac8f7..411e456 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -2,11 +2,11 @@ # tools/phpstan.sh for the version and the hash. # # Level 9, with no `ignoreErrors`, no baseline and no suppression comment anywhere -# in this repository. `tests/Unit` and `tests/Readme` are not analysed: every one of -# their tests is a closure handed to `it()` and every assertion goes through -# `expect()`, both of which resolve at run time, so an analyser given neither can -# only report that the functions are unknown. The support code those tests lean on -# *is* analysed, and it is where the array shapes live. +# in this repository. `tests/Unit`, `tests/Conformance` and `tests/Readme` are not +# analysed: every one of their tests is a closure handed to `it()` and every assertion +# goes through `expect()`, both of which resolve at run time, so an analyser given +# neither can only report that the functions are unknown. The support code those tests +# lean on *is* analysed, and it is where the array shapes live. parameters: level: 9 diff --git a/tests/Conformance/ooxml.php b/tests/Conformance/ooxml.php index c24efc7..7277180 100644 --- a/tests/Conformance/ooxml.php +++ b/tests/Conformance/ooxml.php @@ -84,6 +84,88 @@ expect($status)->toBe(0, $output); }); +it('finds an error in a deck that has one', function () { + skipWithoutDotnetSdk(); + + // The gate is worth nothing if it cannot fail. A validator that answered "no + // errors" about every deck would pass the two tests above, so this one asks + // it about a deck with an error in it and expects to be told. + // + // The error is an element no schema names, and the deck is otherwise the one + // the library wrote: it still opens, so what refuses it is the schema and + // not a broken archive. + $decks = writeExampleDecks(false); + + expect($decks)->not->toBe([]); + + $broken = deckWithAnUnknownElement($decks[0]); + + [$status, $output] = validateWithOpenXml([$broken]); + + expect($status)->toBe(1, 'The validator did not refuse the deck:' . PHP_EOL . $output) + ->and($output)->toContain('notAnElement'); +}); + +/** + * A copy of a deck, with an element in it that no schema names. + * + * The element goes into the shape tree of the first slide, which is where the + * schema is strictest about what may sit there. + */ +function deckWithAnUnknownElement(string $deck): string +{ + $archive = new ZipArchive(); + $opened = $archive->open($deck); + + if (true !== $opened) { + throw new RuntimeException(sprintf('The deck %s could not be opened.', $deck)); + } + + $parts = []; + $slide = null; + + for ($i = 0; $i < $archive->numFiles; $i++) { + $name = (string) $archive->getNameIndex($i); + $parts[$name] = (string) $archive->getFromIndex($i); + + if ($slide === null + && str_starts_with($name, 'ppt/slides/slide') + && str_ends_with($name, '.xml') + ) { + $slide = $name; + } + } + + $archive->close(); + + if ($slide === null) { + throw new RuntimeException(sprintf('The deck %s has no slide in it.', $deck)); + } + + $broken = str_replace('', '', $parts[$slide]); + + if ($broken === $parts[$slide]) { + throw new RuntimeException(sprintf('The slide %s has no shape tree in it.', $slide)); + } + + $parts[$slide] = $broken; + + // The extension matters: the tool decides how to open a file from it, and a + // scratch file whose name ends in a random suffix is not a deck to it. + $path = Scratch::directoryPath('deck') . '/broken.pptx'; + + file_put_contents($path, ''); + + $copy = new ZipArchive(); + $copy->open($path, ZipArchive::OVERWRITE); + foreach ($parts as $name => $contents) { + $copy->addFromString($name, $contents); + } + $copy->close(); + + return $path; +} + /** * Skips the test where the machine cannot run the validator. * diff --git a/tools/ooxml-validator/Program.cs b/tools/ooxml-validator/Program.cs index e7d8185..bbfdb15 100644 --- a/tools/ooxml-validator/Program.cs +++ b/tools/ooxml-validator/Program.cs @@ -22,14 +22,18 @@ // suite writes has been checked against every version the SDK knows, and the // errors are the same at all of them. var version = FileFormatVersions.Office2019; +var files = new List(); +// The files are gathered while the options are read, because the value of +// `--version` is an option's value and not the name of a file. for (var i = 0; i < args.Length; i++) { if (args[i] == "--version" && i + 1 < args.Length) { - if (!Enum.TryParse(args[++i], ignoreCase: true, out version)) + var value = args[++i]; + if (!Enum.TryParse(value, ignoreCase: true, out version)) { - Console.Error.WriteLine($"not a version the SDK knows: {args[i]}"); + Console.Error.WriteLine($"not a version the SDK knows: {value}"); return 2; } @@ -41,9 +45,9 @@ Console.Error.WriteLine($"unknown option: {args[i]}"); return 2; } -} -var files = args.Where(argument => !argument.StartsWith("--")).ToList(); + files.Add(args[i]); +} if (files.Count == 0) { diff --git a/tools/ooxml-validator/ooxml-validator.csproj b/tools/ooxml-validator/ooxml-validator.csproj index cdcd657..963b2cb 100644 --- a/tools/ooxml-validator/ooxml-validator.csproj +++ b/tools/ooxml-validator/ooxml-validator.csproj @@ -19,7 +19,7 @@ - + From ea0926185c80eef6305eebb75ca2a762bbe3a00c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabian=20Gra=C3=9Fl?= Date: Fri, 30 Oct 2026 22:36:29 +0100 Subject: [PATCH 7/9] The gate knows what a chart has to carry, and the patch carries the fixes --- CHANGELOG.md | 43 +++++- composer.json | 6 +- tests/Conformance/charts.php | 291 +++++++++++++++++++++++++++++++++++ tests/Conformance/ooxml.php | 49 ------ tests/Pest.php | 53 +++++++ 5 files changed, 385 insertions(+), 57 deletions(-) create mode 100644 tests/Conformance/charts.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e83e70..10b9f4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,17 +27,50 @@ All notable changes to this project are documented here. The format follows ### Changed -- **The patch carries one more fix from upstream.** A chart's series is written in the - order the schema names it: the shape properties right after the title rather than after - the data points and the data labels, and the separator of the data labels of a doughnut - before the leader lines rather than after them. PowerPoint repairs a chart written the - other way round before it opens it +- **The patch carries four more fixes from upstream.** A chart's series is written in + the order the schema names it: the shape properties right after the title rather than + after the data points and the data labels, and the separator of the data labels of a + doughnut before the leader lines rather than after them. PowerPoint repairs a chart + written the other way round before it opens it ([#695](https://github.com/PHPOffice/PHPPresentation/issues/695), [#1044](https://github.com/PHPOffice/PHPPresentation/pull/1044)). + Three of them are charts that carried less than they were given: the area, the line, + the radar and the scatter dropped the fill of a single data point + ([#1048](https://github.com/PHPOffice/PHPPresentation/issues/1048), + [#1052](https://github.com/PHPOffice/PHPPresentation/pull/1052)), a doughnut, a pie + and a pie in three dimensions dropped the fill of the series itself + ([#1049](https://github.com/PHPOffice/PHPPresentation/issues/1049), + [#1051](https://github.com/PHPOffice/PHPPresentation/pull/1051)), and a category axis + wrote its major unit as the number of labels to skip — and as a fraction, where the + schema wants a count, so the file was outside the schema + ([#1047](https://github.com/PHPOffice/PHPPresentation/issues/1047), + [#1050](https://github.com/PHPOffice/PHPPresentation/pull/1050)). + This library writes no chart, so nothing it draws changes: the patch is upstream's branch whole, and the fix belongs to that branch rather than to this one. +- **The charts are checked too, and the gate knows what a chart has to carry.** The + patch is the library that writes the charts, and a patch that is re-rolled without a + fix is a patch that has lost it — nothing else in this suite would have noticed. So + the gate writes a chart of every kind through the patched library, and checks two + things about it: that the file is within the schema, which the validator reads, and + that it carries what it was given, which the validator cannot see. + + A chart that is missing a data point is a valid chart. The validator reads what a + file holds and says nothing about what it does not, so the elements themselves are + asserted on: every kind of series carries the fill it was given, and every kind of + series carries the fill of the point it was given. + + Three defects were found this way and are fixed upstream: + [#1047](https://github.com/PHPOffice/PHPPresentation/issues/1047) — a category axis + wrote its major unit as the number of labels to skip, and wrote it as a fraction + where the schema wants a count, so the file was outside the schema; + [#1048](https://github.com/PHPOffice/PHPPresentation/issues/1048) — the area, line, + radar and scatter charts dropped the fill of a single data point; + [#1049](https://github.com/PHPOffice/PHPPresentation/issues/1049) — a doughnut, a pie + and a pie in three dimensions dropped the fill of the series itself. + ## [0.3.0] - 2026-10-30 ### Added diff --git a/composer.json b/composer.json index 4815a58..d028a04 100644 --- a/composer.json +++ b/composer.json @@ -39,9 +39,9 @@ "patches": { "phpoffice/phppresentation": [ { - "description": "PHPPresentation 1.2.0 brought up to upstream's master, with the fixes that give a picture the path, the bytes and the name it was read from, that find the folder of a part of a package with the slashes of the package, and that write a chart series in the order the schema names it", - "url": "https://raw.githubusercontent.com/markdown-office/PHPPresentation/92c2c64ca20a6b3febf6512e892b21a3c0ae4b56/patches/phpoffice-phppresentation-1.2.0-to-master.patch", - "sha256": "67ce1a44e76e4dc23639fefee365ec811ef7a763331343bde468ae781bf5427a" + "description": "PHPPresentation 1.2.0 brought up to upstream's master, with the fixes that give a picture the path, the bytes and the name it was read from, that find the folder of a part of a package with the slashes of the package, that write a chart series in the order the schema names it, that write the data points of a series and the fill of the series itself, and that give a category axis a tick label skip of its own", + "url": "https://raw.githubusercontent.com/markdown-office/PHPPresentation/605bc7c9724ac4a0781cc1692ff7ae279a33bb8a/patches/phpoffice-phppresentation-1.2.0-to-master.patch", + "sha256": "46862cef4470ae8e755a03f1f8afd94c7e772cc1ee4ab0a39441c2cb392d5146" } ] } diff --git a/tests/Conformance/charts.php b/tests/Conformance/charts.php new file mode 100644 index 0000000..30633f0 --- /dev/null +++ b/tests/Conformance/charts.php @@ -0,0 +1,291 @@ +not->toBe([]); + + [$status, $output] = validateWithOpenXml($decks); + + foreach ($decks as $deck) { + expect($output)->toContain(basename($deck) . ': 0 error(s)'); + } + + expect($status)->toBe(0, $output); +}); + +it('writes into every kind of series what the series was given', function () { + $withoutFill = []; + $withoutPoint = []; + + foreach (chartTypes() as $name => $type) { + $deck = writeChartDeck($name, $type); + + $children = seriesChildrenOf($deck); + + // The fill of the series itself. The schema names it right after the title, + // and a chart that does not carry it draws the series in the colour the + // application chooses rather than the one that was asked for. + if (!in_array('c:spPr', $children, true)) { + $withoutFill[] = $name; + } + + // The fill of one point of the series. The schema names it after the shared + // part of the series and before the labels. + if (!in_array('c:dPt', $children, true)) { + $withoutPoint[] = $name; + } + } + + expect($withoutFill)->toBe([], 'these kinds of series carry no fill of their own: ' . implode(', ', $withoutFill)); + expect($withoutPoint)->toBe([], 'these kinds of series carry no data point: ' . implode(', ', $withoutPoint)); +}); + +it('writes the fill of a data point into the data point', function () { + foreach (chartTypes() as $name => $type) { + $deck = writeChartDeck($name, $type); + + expect(pointFillsOf($deck)) + ->not->toBe([], $name . ' writes a data point with no fill in it'); + } +}); + +it('gives a category axis a tick label skip of its own, and not a major unit', function () { + // A major unit is the distance between the ticks of a value axis; a category axis + // has no scale for a unit to be a distance between. The writer used to write one + // as the number of labels to skip, and the number it wrote was a fraction where + // the schema wants a count. + $deck = writeChartDeck('axis', Bar::class, majorUnit: 2.5); + + expect(chartPartOf($deck)) + ->not->toContain('c:tickLblSkip'); + + expect(axisOf($deck)) + ->not->toContain('c:majorUnit'); +}); + +it('writes the tick label skip of a category axis as a count', function () { + $deck = writeChartDeck('axis', Bar::class, tickLabelSkip: 3); + + expect(chartPartOf($deck)) + ->toContain(''); +}); + +/** + * Every kind of chart the library writes, and the class that writes it. + * + * @return array> + */ +function chartTypes(): array +{ + return [ + 'area' => Area::class, + 'bar' => Bar::class, + 'bar3d' => Bar3D::class, + 'doughnut' => Doughnut::class, + 'line' => Line::class, + 'pie' => Pie::class, + 'pie3d' => Pie3D::class, + 'radar' => Radar::class, + 'scatter' => Scatter::class, + ]; +} + +/** + * A deck for every kind of chart, and the axis that carries a major unit. + * + * @return list The decks written, as paths. + */ +function writeChartDecks(): array +{ + $decks = []; + + foreach (chartTypes() as $name => $type) { + $decks[] = writeChartDeck($name, $type); + } + + // The axis of a category, with the major unit that is not its to carry. + $decks[] = writeChartDeck('axis', Bar::class, majorUnit: 2.5); + + return $decks; +} + +/** + * A deck with one chart of the given kind in it. + * + * The series carries a fill of its own and a point of it carries a fill of its own, + * because those are the two things a chart can be asked for and not be given. + * + * @param class-string<\PhpOffice\PhpPresentation\Shape\Chart\Type\AbstractType> $type + */ +function writeChartDeck(string $name, string $type, ?float $majorUnit = null, ?int $tickLabelSkip = null): string +{ + $presentation = new PhpPresentation(); + $slide = $presentation->createSlide(); + $shape = $slide->createChartShape(); + + $series = new Series('Downloads', ['A' => 7, 'B' => 5, 'C' => 3]); + $series->getFill() + ->setFillType(Fill::FILL_SOLID) + ->setStartColor(new Color('FF4F81BD')); + $series->getDataPointFill(0)->setFillType(Fill::FILL_SOLID); + + $chart = new $type(); + $chart->addSeries($series); + $shape->getPlotArea()->setType($chart); + + if ($majorUnit !== null) { + $shape->getPlotArea()->getAxisX()->setMajorUnit($majorUnit); + } + + if ($tickLabelSkip !== null) { + $shape->getPlotArea()->getAxisX()->setTickLabelSkip($tickLabelSkip); + } + + $slide->addShape($shape); + + $path = Scratch::directoryPath('chart') . '/' . $name . '.pptx'; + + $writer = new PowerPoint2007($presentation); + $writer->save($path); + + return $path; +} + +/** + * The chart part of a deck, as its XML. + */ +function chartPartOf(string $deck): string +{ + $archive = new ZipArchive(); + $opened = $archive->open($deck); + + if (true !== $opened) { + throw new RuntimeException(sprintf('The deck %s could not be opened.', $deck)); + } + + $part = null; + + for ($index = 0; $index < $archive->numFiles; $index++) { + $name = (string) $archive->getNameIndex($index); + + if (str_starts_with($name, 'ppt/charts/chart') && str_ends_with($name, '.xml')) { + $part = (string) $archive->getFromIndex($index); + break; + } + } + + $archive->close(); + + if ($part === null) { + throw new RuntimeException(sprintf('The deck %s has no chart in it.', $deck)); + } + + return $part; +} + +/** + * The children of the series of a chart, by name. + * + * @return list + */ +function seriesChildrenOf(string $deck): array +{ + $xpath = new DOMXPath(chartDomOf($deck)); + $xpath->registerNamespace('c', 'http://schemas.openxmlformats.org/drawingml/2006/chart'); + + $series = $xpath->query('//c:ser')->item(0); + + if ($series === null) { + throw new RuntimeException(sprintf('The deck %s has no series in its chart.', $deck)); + } + + $children = []; + + foreach ($series->childNodes as $child) { + if ($child instanceof DOMElement) { + $children[] = 'c:' . $child->localName; + } + } + + return $children; +} + +/** + * The chart part of a deck, parsed. + */ +function chartDomOf(string $deck): DOMDocument +{ + $dom = new DOMDocument(); + $dom->loadXML(chartPartOf($deck)); + + return $dom; +} + +/** + * The fills of the data points of a chart, as XML. + * + * @return list + */ +function pointFillsOf(string $deck): array +{ + $xml = chartPartOf($deck); + + preg_match_all('~(.*?)~s', $xml, $matches); + + return $matches[1]; +} + +/** + * The axes of a chart, as XML. + */ +function axisOf(string $deck): string +{ + $xml = chartPartOf($deck); + + preg_match_all('~.*?~s', $xml, $matches); + + return implode("\n", $matches[0]); +} diff --git a/tests/Conformance/ooxml.php b/tests/Conformance/ooxml.php index 7277180..e35823d 100644 --- a/tests/Conformance/ooxml.php +++ b/tests/Conformance/ooxml.php @@ -166,37 +166,7 @@ function deckWithAnUnknownElement(string $deck): string return $path; } -/** - * Skips the test where the machine cannot run the validator. - * - * The validator is a .NET program, so the SDK is what runs it. A machine without it runs - * the rest of the suite and says what is missing. - */ -function skipWithoutDotnetSdk(): void -{ - if (dotnetSdkIsThere()) { - return; - } - test()->markTestSkipped( - 'The .NET SDK is not installed, so the Open XML validator cannot be run. ' - . 'The job that runs this test in continuous integration installs it.', - ); -} - -/** - * Whether this machine has the .NET SDK the validator is built with. - * - * The version rather than the path, because a machine with the runtime and not the SDK - * can run a program but not build one, and a path lookup would find the runtime and - * answer yes. - */ -function dotnetSdkIsThere(): bool -{ - exec('dotnet --version 2>&1', $output, $status); - - return $status === 0; -} /** * The decks of the gallery that are written in this run, and where they are written to. @@ -287,22 +257,3 @@ function carriesVector(string $source): bool return str_contains($source, '.svg)'); } -/** - * What Microsoft's validator makes of the given decks: the exit code and what it said. - * - * @param list $decks - * - * @return array{0: int, 1: string} - */ -function validateWithOpenXml(array $decks): array -{ - $command = sprintf( - 'dotnet run --project %s -- %s 2>&1', - escapeshellarg(Doctest::root() . '/tools/ooxml-validator'), - implode(' ', array_map('escapeshellarg', $decks)), - ); - - exec($command, $output, $status); - - return [$status, implode("\n", $output)]; -} diff --git a/tests/Pest.php b/tests/Pest.php index bad4aa4..9660969 100644 --- a/tests/Pest.php +++ b/tests/Pest.php @@ -3,6 +3,7 @@ declare(strict_types=1); use MarkdownPresentation\Filesystem\PathScope; +use MarkdownPresentation\Tests\Support\Doctest; use MarkdownPresentation\Tests\Support\Scratch; pest()->afterEach(function (): void { @@ -52,3 +53,55 @@ function deckScope(string $directory, array $roots = []): PathScope const TWO_SLIDES = "# One\n\n---\n\n# Two\n"; const TITLE_AND_CONTENT = 'Title and Content'; + +/** + * Skips the test where the machine cannot run the validator. + * + * The validator is a .NET program, so the SDK is what runs it. A machine without it runs + * the rest of the suite and says what is missing. + */ +function skipWithoutDotnetSdk(): void +{ + if (dotnetSdkIsThere()) { + return; + } + + test()->markTestSkipped( + 'The .NET SDK is not installed, so the Open XML validator cannot be run. ' + . 'The job that runs this test in continuous integration installs it.', + ); +} + +/** + * Whether this machine has the .NET SDK the validator is built with. + * + * The version rather than the path, because a machine with the runtime and not the SDK + * can run a program but not build one, and a path lookup would find the runtime and + * answer yes. + */ +function dotnetSdkIsThere(): bool +{ + exec('dotnet --version 2>&1', $output, $status); + + return $status === 0; +} + +/** + * What Microsoft's validator makes of the given decks: the exit code and what it said. + * + * @param list $decks + * + * @return array{0: int, 1: string} + */ +function validateWithOpenXml(array $decks): array +{ + $command = sprintf( + 'dotnet run --project %s -- %s 2>&1', + escapeshellarg(Doctest::root() . '/tools/ooxml-validator'), + implode(' ', array_map('escapeshellarg', $decks)), + ); + + exec($command, $output, $status); + + return [$status, implode("\n", $output)]; +} From 863faeb7364fe9b37956ccf3b1ea24cd736f6d50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabian=20Gra=C3=9Fl?= Date: Sat, 31 Oct 2026 16:59:21 +0100 Subject: [PATCH 8/9] The release is dated the day the tag is cut --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 10b9f4a..827dbe9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ All notable changes to this project are documented here. The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and the project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [0.3.1] - 2026-10-30 +## [0.3.1] - 2026-10-31 ### Added From 5214e30397b03c2567be68dca6d639e3fdcae29c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabian=20Gra=C3=9Fl?= Date: Sat, 31 Oct 2026 21:41:33 +0100 Subject: [PATCH 9/9] The comments say less, and the install state is kept out of the package --- examples/decks.php | 19 ++++------- tests/Conformance/charts.php | 36 +++++++-------------- tests/Conformance/ooxml.php | 62 ++++++++++++------------------------ tests/Pest.php | 8 ++--- tests/Unit/dead-code.php | 12 +++++++ 5 files changed, 54 insertions(+), 83 deletions(-) diff --git a/examples/decks.php b/examples/decks.php index 863e6e7..9181279 100644 --- a/examples/decks.php +++ b/examples/decks.php @@ -5,11 +5,9 @@ /* * The decks the examples are written as. * - * It is a file of its own because two things need it: the script that writes the decks, - * and the test that checks what it wrote against the schema. A list of its own in - * either of them would be a second answer to a question this file answers, and the two - * would drift — a deck the gallery no longer builds and the gate still checks, or the - * other way round, which is the worse of the two: a gate that says less than it says. + * A file of its own because two things need it: the script that writes the decks, and + * the test that checks what it wrote. A list of its own in either of them would be a + * second answer to a question this file answers, and the two would drift. * * Required rather than included, so that a second caller in the same run does not * declare the constants again. @@ -18,10 +16,7 @@ /** * The sources this script does not convert, and why. * - * Keyed by the file's stem. A deck is here because converting it is the failure it - * exists to demonstrate, which is the same reason `markdown-word` does not build its - * `14-rejected.md`: the build would stop, or — worse — would not stop and the gallery - * would contain a deck that quietly came out wrong. + * A deck is here because converting it is the failure it exists to demonstrate. */ const NOT_BUILT = [ '10-rejected' => 'converting it is the failure it exists to demonstrate; see examples/README.md', @@ -30,10 +25,8 @@ /** * The sources written more than once, and the one line each copy differs in. * - * A theme is a frontmatter key, so the only way to see the three palettes side by side is - * to write the same deck three times. Varying it here rather than in three files keeps - * the three decks identical apart from the one line, which is the point of the - * comparison. + * A theme is a frontmatter key, so the only way to see the three palettes side by side + * is to write the same deck three times. */ const VARIANTS = [ '03-themes' => ['theme' => ['default', 'corporate-dark', 'minimal']], diff --git a/tests/Conformance/charts.php b/tests/Conformance/charts.php index 30633f0..14288e1 100644 --- a/tests/Conformance/charts.php +++ b/tests/Conformance/charts.php @@ -21,23 +21,15 @@ /* * The charts, checked against the schema Microsoft's own validator reads. * - * **This library writes no charts.** A deck of Markdown is slides, shapes, text and - * pictures; there is no chart in it and no way to ask for one. What it does carry is a - * patch of the library that writes the charts, and the patch is what the charts in - * this file are for: a patch that is re-rolled without a fix is a patch that has lost - * it, and nothing else in this repository would notice. + * **This library writes no charts.** What it carries is a patch of the library that + * writes them, and the patch is what the charts here are for: a patch re-rolled + * without a fix is a patch that has lost it, and nothing else in this repository + * would notice. * - * So the charts are written here, through the patched library, and the two things - * that can go wrong with them are checked: - * - * - **The file is outside the schema.** A chart is a part of its own with a schema of - * its own, and the order of the children of a series is a sequence and not a set. The - * validator is the oracle, and it is the same one the decks are checked with. - * - **The file is valid and carries nothing.** A deck that is missing a data point is - * a valid deck: the validator reads what is there and says nothing about what is - * not. So the elements themselves are asserted on — every kind of series carries the - * fill it was given, and every kind of series carries the fill of the point it was - * given. + * Two things can go wrong with a chart, and they are checked both ways: the file can + * be outside the schema, which the validator reads, and the file can be valid and + * carry nothing — a chart without a data point is a valid chart, and the validator + * says nothing about what is not there. So the elements themselves are asserted on. */ it('writes charts that are valid OOXML', function () { @@ -65,15 +57,12 @@ $children = seriesChildrenOf($deck); - // The fill of the series itself. The schema names it right after the title, - // and a chart that does not carry it draws the series in the colour the - // application chooses rather than the one that was asked for. + // The fill of the series itself, which the schema names right after the title. if (!in_array('c:spPr', $children, true)) { $withoutFill[] = $name; } - // The fill of one point of the series. The schema names it after the shared - // part of the series and before the labels. + // The fill of one point of the series, which the schema names before the labels. if (!in_array('c:dPt', $children, true)) { $withoutPoint[] = $name; } @@ -94,9 +83,8 @@ it('gives a category axis a tick label skip of its own, and not a major unit', function () { // A major unit is the distance between the ticks of a value axis; a category axis - // has no scale for a unit to be a distance between. The writer used to write one - // as the number of labels to skip, and the number it wrote was a fraction where - // the schema wants a count. + // has no scale for a unit to be a distance between. The writer used to write one as + // the number of labels to skip, and as a fraction where the schema wants a count. $deck = writeChartDeck('axis', Bar::class, majorUnit: 2.5); expect(chartPartOf($deck)) diff --git a/tests/Conformance/ooxml.php b/tests/Conformance/ooxml.php index e35823d..a36304d 100644 --- a/tests/Conformance/ooxml.php +++ b/tests/Conformance/ooxml.php @@ -14,20 +14,15 @@ * Every deck this library writes, checked against the schema Microsoft's own validator * reads — the one the Open XML SDK Productivity Tool uses. * - * Nothing in this suite could see that before: the decks were checked against the parts - * they carry, and a deck whose XML is outside the schema passes every one of those - * checks and opens in PowerPoint as `[Repaired]`. The validator is the oracle, and it is - * a program in `tools/ooxml-validator/` rather than a library this package depends on: - * nothing that ships with the library needs it, and the suite runs without it. - * - * A machine with no .NET SDK says what is missing and skips the test. A green tick for a - * check that never ran is a check nobody made. + * Nothing in this suite could see that before: a deck whose XML is outside the schema + * passes every check the suite made and opens in PowerPoint as `[Repaired]`. The + * validator is a program in `tools/ooxml-validator/` rather than a dependency, and a + * machine without the .NET SDK says what is missing and skips the test — a green tick + * for a check that never ran is a check nobody made. * * The deck that carries a vector is a test of its own, because it is the one deck a - * machine without a rasteriser cannot write: the library refuses the picture rather than - * writing a diagram that has lost its words, and a machine that cannot draw one has - * nothing to check. It is a test of its own so that a run says which deck it did not - * check, rather than checking one deck fewer and saying nothing. + * machine without a rasteriser cannot write. It is a test of its own so that a run says + * which deck it did not check, rather than checking one deck fewer and saying nothing. */ it('writes decks that are valid OOXML', function () { @@ -37,19 +32,16 @@ [$status, $output] = validateWithOpenXml($decks); - // Every deck named, because a gate that checked nothing is not a gate. The validator - // prints one line per file and how many errors it holds, so a deck missing from the - // output is a deck that was not read — which is a failure, not a pass. + // Every deck named: the validator prints one line per file, so a deck missing + // from the output is a deck that was not read. expect($decks)->not->toBe([]); foreach ($decks as $deck) { expect($output)->toContain(basename($deck) . ': 0 error(s)'); } - // The exit code is the tool's own answer, and it is what a caller that reads nothing - // else gets: 0 when every deck is clean, 1 when one of them is not, and 2 when the - // tool could not run at all. What the tool said is the message, because a failure - // here is a failure of the deck and the deck is what has to be looked at. + // The tool's own answer: 0 when every deck is clean, 1 when one is not, and 2 + // when the tool could not run at all. expect($status)->toBe(0, $output); }); @@ -87,13 +79,9 @@ it('finds an error in a deck that has one', function () { skipWithoutDotnetSdk(); - // The gate is worth nothing if it cannot fail. A validator that answered "no - // errors" about every deck would pass the two tests above, so this one asks - // it about a deck with an error in it and expects to be told. - // - // The error is an element no schema names, and the deck is otherwise the one - // the library wrote: it still opens, so what refuses it is the schema and - // not a broken archive. + // The gate is worth nothing if it cannot fail, so this one asks the validator + // about a deck with an error in it. The error is an element no schema names; the + // deck still opens, so what refuses it is the schema and not a broken archive. $decks = writeExampleDecks(false); expect($decks)->not->toBe([]); @@ -150,8 +138,7 @@ function deckWithAnUnknownElement(string $deck): string $parts[$slide] = $broken; - // The extension matters: the tool decides how to open a file from it, and a - // scratch file whose name ends in a random suffix is not a deck to it. + // The extension matters: the tool decides how to open a file from it. $path = Scratch::directoryPath('deck') . '/broken.pptx'; file_put_contents($path, ''); @@ -166,16 +153,12 @@ function deckWithAnUnknownElement(string $deck): string return $path; } - - /** * The decks of the gallery that are written in this run, and where they are written to. * - * The plan of the decks is the one `examples/decks.php` holds, because that is the file - * the script that writes them holds it in: a list of its own here would be a second - * answer to a question that already has one, and the two would drift. A deck the - * gallery no longer builds and the gate still checks is the worse of the two — a gate - * that says less than it says. + * The plan of the decks is the one `examples/decks.php` holds: a list of its own here + * would be a second answer to a question that already has one, and the two would + * drift. * * @param bool $withVector Whether the decks that carry a vector are among them. * @@ -195,10 +178,8 @@ function writeExampleDecks(bool $withVector): array $renderer = new PresentationRenderer($sources); - // A deck written more than once is written once per theme, which the renderer is - // told rather than the source: the theme in the source's own frontmatter is the - // one the deck asked for, and the copy in the gallery is the same deck in another - // theme. + // The theme is told to the renderer rather than to the source: the theme in + // the source's own frontmatter is the one the deck asked for. if ($deck['theme'] !== null) { $renderer->withTheme($deck['theme']); } @@ -249,8 +230,7 @@ function exampleDecks(): array /** * Whether a deck's source carries a vector, and so needs a rasteriser to be written. * - * A `.pptx` holds a vector only beside a raster of it, so the vector is drawn before it - * goes in — and Imagick is the only thing here that draws one. + * A `.pptx` holds a vector only beside a raster of it. */ function carriesVector(string $source): bool { diff --git a/tests/Pest.php b/tests/Pest.php index 9660969..7890200 100644 --- a/tests/Pest.php +++ b/tests/Pest.php @@ -57,8 +57,7 @@ function deckScope(string $directory, array $roots = []): PathScope /** * Skips the test where the machine cannot run the validator. * - * The validator is a .NET program, so the SDK is what runs it. A machine without it runs - * the rest of the suite and says what is missing. + * The validator is a .NET program, so the SDK is what runs it. */ function skipWithoutDotnetSdk(): void { @@ -75,9 +74,8 @@ function skipWithoutDotnetSdk(): void /** * Whether this machine has the .NET SDK the validator is built with. * - * The version rather than the path, because a machine with the runtime and not the SDK - * can run a program but not build one, and a path lookup would find the runtime and - * answer yes. + * The version rather than the path: a machine with the runtime and not the SDK can run + * a program but not build one. */ function dotnetSdkIsThere(): bool { diff --git a/tests/Unit/dead-code.php b/tests/Unit/dead-code.php index bc93551..0396952 100644 --- a/tests/Unit/dead-code.php +++ b/tests/Unit/dead-code.php @@ -549,6 +549,18 @@ function pagesWithClaim(string $claim): array expect(constantDeclarations())->toBe([]); }); +it('keeps the install state out of the package', function () { + // `patches.lock.json` records the patch composer-patches applied on one + // machine, and it wins over the manifest: a file left in the repository + // installs that patch rather than the one the manifest names. Every + // `composer install` writes it, so it is ignored rather than committed. + $root = Doctest::root(); + $tracked = []; + exec(sprintf('git -C %s ls-files -- patches.lock.json 2>&1', escapeshellarg($root)), $tracked); + + expect($tracked)->toBe([]); +}); + it('says in the code why ShapeWriter is not split', function () { // The one class where `php:S1448` is open on purpose, so the reason has to be somewhere // a person deciding whether to split it will actually read — which is the class itself,