From 0cbd7c7f0fe0c1c0ce24d09d0e9b64aa64a38548 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabian=20Gra=C3=9Fl?= Date: Fri, 30 Oct 2026 11:28:51 +0100 Subject: [PATCH] The install says which plugin, and what happens without it The README's requirement said "a plugin" and named it two sentences later, so a consumer who stopped at the requirement did not know what to allow, and "the library refuses to work" said nothing about what they would meet. The requirement names cweagans/composer-patches where it is stated, and the page says what follows: the install stops with Composer's own message, and a conversion against an unpatched PHPPresentation raises MarkdownPresentation\Exception\UnpatchedDependency, whose message names the plugin as well. The page also names the five methods the patch adds and which direction needs each, because the check asks for all five in either direction and refuses a package that carries only some: what is missing fails somewhere else, naming a method rather than the reason, or is quietly left out of the deck that comes back. The guard reads the required methods out of Fixes::REQUIRED by reflection, so a patch that grows a method makes the page a list of the methods there are. --- CHANGELOG.md | 17 +++++ README.md | 35 ++++++--- tests/Readme/installation.php | 129 ++++++++++++++++++++++++++++++++++ 3 files changed, 172 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b586211..7cda0fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -130,6 +130,23 @@ All notable changes to this project are documented here. The format follows ### Changed +- **The install says which plugin, and what happens without it.** The requirement named + "a plugin" and the plugin two sentences later, so a consumer who stopped at the + requirement did not know what to allow. The README names + [`cweagans/composer-patches`][composer-patches] where the requirement is stated now, + and says what follows: the install stops with Composer's own message naming the + plugin, and a conversion against an unpatched PHPPresentation raises + `MarkdownPresentation\Exception\UnpatchedDependency`, whose message names the plugin + as well. + + The page names the five methods [the patch][the-patch] adds and says which direction + needs each: the writer — the direction that writes a deck — needs + `Table::setFirstRow`, `AutoShape::setRoundRectCorner` and `RichText::setDescription`, + and the reader — the direction that takes a deck apart, through the patched + `PowerPoint2007` reader — needs `PowerPoint2007::zipPath` and `Gd::setContents` as well + as the description. The check asks for all five whichever way the library is used, and a + package that carries only some of them is refused rather than half-worked. + - **`PresentationRenderer` is split, and `php:S1448` is closed on it.** Twenty-three methods was one finding too many, and the finding says what to do: split it. diff --git a/README.md b/README.md index 4e07ba6..93a6ec5 100644 --- a/README.md +++ b/README.md @@ -33,12 +33,12 @@ PHP 8.2 or newer, `symfony/yaml` and `phpoffice/phppresentation` as hard depende frontmatter of a deck and every file of a theme is YAML, and a partial parser is worse for the machines that write them than for the people that do. -**The project that installs this one has to allow a plugin.** This library writes a few -things a released PHPPresentation cannot — a table that says which row is its header, a -shape that says how round its corners are, a picture that says where it was read from — and -gets them from a patch applied to the installed package. The patch is applied by -[`cweagans/composer-patches`][composer-patches], which Composer only runs when the project -allows it: +**The project that installs this one has to allow [`cweagans/composer-patches`][composer-patches].** +This library writes a few things a released PHPPresentation cannot — a table that says which +row is its header, a shape that says how round its corners are, a run of text that says what +it is, and a picture that says where it was read from — and gets them from a patch applied to +the installed package. The plugin applies that patch, and Composer runs it only when the +project allows it: ```json { @@ -50,9 +50,26 @@ allows it: } ``` -Without that line the install stops with Composer's own message naming the plugin. The -library refuses to work rather than write a deck with the parts missing, and says which -plugin is the reason. +Without that line the install stops with Composer's own message naming +`cweagans/composer-patches`. A deck converted against an unpatched PHPPresentation raises +`MarkdownPresentation\Exception\UnpatchedDependency`, whose message names the plugin as +well. + +The patch adds five methods to PHPPresentation: + +| Method | Needed by | What it is for | +| --- | --- | --- | +| `Table::setFirstRow` | the writer | whether the first row of a table is its header | +| `AutoShape::setRoundRectCorner` | the writer | how round the corners of a rounded rectangle are | +| `RichText::setDescription` | the writer and the reader | the description a shape carries: the writer sets it, the reader reads it back | +| `PowerPoint2007::zipPath` | the reader | the path a picture was read from, inside the package | +| `Gd::setContents` | the reader | the bytes a picture was read from, as the package holds them | + +The writer is the direction that writes a `.pptx`; the reader is the direction that takes one +apart, and it reads a deck through the patched `PowerPoint2007` reader. The check asks for +all five of them, in either direction, and a package that carries only some is refused +rather than half-worked. What is missing either fails somewhere else — naming a method +rather than the reason — or is quietly left out of the deck that comes back. `ext-imagick` is optional and enables [SVG](#what-an-svg-costs-you). PowerPoint holds a vector only beside a raster of it, so something has to draw the raster; without the diff --git a/tests/Readme/installation.php b/tests/Readme/installation.php index 367b8a6..ee37116 100644 --- a/tests/Readme/installation.php +++ b/tests/Readme/installation.php @@ -20,6 +20,12 @@ * rather than the other way round: when the patch goes, and the pin becomes a release * that carries the fixes on its own, the line stops being asked for and the guard says * so. + * + * The rest of the section says what the plugin is for, and a consumer who has to + * find that out from the failure is a consumer who has been told too late: the + * requirement names the plugin, the consequence names it again, and the methods the + * patch adds are read out of the class that checks for them — so a patch that grows a + * method makes the page a list of the methods there are. */ /** @@ -44,6 +50,84 @@ function installationManifest(): array ); } +/** + * The installation section, as it is on the page. + */ +function installationSection(): string +{ + $readme = installationReadme(); + + return preg_match('~^## Installation$(.*?)^## ~ms', $readme, $section) === 1 ? $section[1] : ''; +} + +/** + * The paragraphs of the installation section, as they are on the page. + * + * @return list + */ +function installationParagraphs(): array +{ + $paragraphs = []; + + foreach (preg_split('#\n\s*\n#', installationSection()) ?: [] as $paragraph) { + $paragraph = trim($paragraph); + + if ($paragraph !== '') { + $paragraphs[] = $paragraph; + } + } + + return $paragraphs; +} + +/** + * The sentence that states the requirement. + * + * The sentence rather than the paragraph, because the paragraph says more than the + * requirement does: a plugin named two sentences later is not named where the + * requirement is stated. + */ +function installationRequirement(): string +{ + $section = installationSection(); + $at = strpos($section, 'has to allow'); + + if ($at === false) { + return ''; + } + + $end = strpos($section, '.', $at); + + return $end === false ? substr($section, $at) : substr($section, $at, $end - $at); +} + +/** + * The methods the library requires from the installed PHPPresentation. + * + * Read out of the class that checks for them rather than written down here, so that + * a patch which grows a method makes the page a list of the methods there are. + * + * @return array The class each method belongs to, and the method. + */ +function requiredFixes(): array +{ + $required = (new ReflectionClass(\MarkdownPresentation\Fixes::class))->getConstant('REQUIRED'); + + if (! is_array($required)) { + return []; + } + + $fixes = []; + + foreach ($required as $class => $method) { + if (is_string($class) && is_string($method) && class_exists($class)) { + $fixes[$class] = $method; + } + } + + return $fixes; +} + it('asks a consumer for the plugin that applies the patch', function () { $manifest = installationManifest(); $patches = $manifest['extra']['patches'] ?? []; @@ -67,3 +151,48 @@ function installationManifest(): array expect(installationReadme())->not->toContain('minimum-stability'); expect($needsIt)->toBeFalse(); }); + +it('names the plugin where the requirement is stated', function () { + // The sentence that says the project has to allow the plugin is the sentence a + // consumer stops at, so the plugin is named in it rather than a sentence later. + $requirement = installationRequirement(); + + expect($requirement)->not->toBe(''); + expect($requirement)->toContain('cweagans/composer-patches'); +}); + +it('states what a consumer gets without the plugin', function () { + $readme = installationReadme(); + + // Two failures, and a consumer meets the first at the install and the second at + // the render: the install stops with Composer's own message, and a conversion + // against an unpatched package raises the library's own. + expect($readme)->toContain('the install stops'); + + $consequences = array_values(array_filter( + installationParagraphs(), + static fn (string $paragraph): bool => str_contains($paragraph, 'UnpatchedDependency'), + )); + + expect($consequences)->toHaveCount(1); + expect($consequences[0])->toContain('cweagans/composer-patches'); +}); + +it('names every method the library requires', function () { + $fixes = requiredFixes(); + + expect($fixes)->not->toBe([]); + + $readme = installationReadme(); + $missing = []; + + foreach ($fixes as $class => $method) { + $named = (new ReflectionClass($class))->getShortName() . '::' . $method; + + if (! str_contains($readme, $named)) { + $missing[] = $named; + } + } + + expect($missing)->toBe([]); +});