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([]); +});