From cdd640173accb4ac8ef6116cda16f61bd0c4ddbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabian=20Gra=C3=9Fl?= Date: Thu, 29 Oct 2026 16:42:44 +0100 Subject: [PATCH 1/8] The deck names a templates file, not a chrome file "Chrome" was doing two jobs: it named the shapes a template draws, and it named the file the definitions of those templates live in. A deck wrote chrome: ./templates/chrome.md template: wordmark and the two keys read as two names for one thing, when one is a path and the other is a name. "Chrome" keeps the first job. The file is `templates-file:` now, `Template\ChromeFile` is `Template\TemplatesFile`, `ChromeNotFound` and `ChromeTooLarge` are `TemplatesFileNotFound` and `TemplatesFileTooLarge`, and a templates file that holds a slide is a `MalformedTemplatesFile` rather than a case of `MalformedChrome`. The feature is unreleased, so the rename costs nobody a migration. --- CHANGELOG.md | 38 +++++----- README.md | 12 +-- examples/README.md | 10 +-- examples/markdown/04-frontmatter.md | 8 +- examples/markdown/16-templates.md | 8 +- examples/markdown/17-corporate-template.md | 2 +- examples/markdown/corporate/README.md | 8 +- .../corporate/{chrome.md => templates.md} | 0 .../templates/{chrome.md => templates.md} | 0 src/Exception/MalformedChrome.php | 18 ++--- src/Exception/MalformedTemplatesFile.php | 23 ++++++ ...NotFound.php => TemplatesFileNotFound.php} | 18 ++--- ...TooLarge.php => TemplatesFileTooLarge.php} | 10 +-- src/Render/Furniture.php | 44 +++++------ src/Render/PresentationRenderer.php | 16 ++-- src/Template/Templates.php | 4 +- .../{ChromeFile.php => TemplatesFile.php} | 30 ++++---- tests/Readme/examples-decks.php | 6 +- tests/Unit/exceptions.php | 10 ++- tests/Unit/rendering.php | 2 +- tests/Unit/templates.php | 73 +++++++++---------- 21 files changed, 181 insertions(+), 159 deletions(-) rename examples/markdown/corporate/{chrome.md => templates.md} (100%) rename examples/markdown/templates/{chrome.md => templates.md} (100%) create mode 100644 src/Exception/MalformedTemplatesFile.php rename src/Exception/{ChromeNotFound.php => TemplatesFileNotFound.php} (59%) rename src/Exception/{ChromeTooLarge.php => TemplatesFileTooLarge.php} (53%) rename src/Template/{ChromeFile.php => TemplatesFile.php} (73%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 03f30ca..654aac8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,14 +26,14 @@ All notable changes to this project are documented here. The format follows not over it. The templates may also live in a file beside the deck, named by the deck's frontmatter - and resolved against the deck's own directory, so that a deck and the furniture it - shares travel together. The deck's own templates override the imported ones of the same - name, the way a deck's own theme overrides the shipped one. + with `templates-file:` and resolved against the deck's own directory, so that a deck and + the furniture it shares travel together. The deck's own templates override the imported + ones of the same name, the way a deck's own theme overrides the shipped one. `examples/markdown/16-templates.md` is the deck that shows it: a template defined in the deck, slides that name one, two and three of them, a template imported from - `templates/chrome.md`, and a template of the deck's own of the same name as one the file - holds — the one that is drawn. The company is invented. + `templates/templates.md`, and a template of the deck's own of the same name as one the + file holds — the one that is drawn. The company is invented. - **A whole corporate template, as the files a company has.** A theme, a brand mark and the furniture of a deck are three separate things, and a company that has all three has @@ -42,13 +42,13 @@ All notable changes to this project are documented here. The format follows `examples/markdown/17-corporate-template.md` is a deck that has them: a theme of its own in `examples/markdown/corporate/theme.yml`, a mark in two colours — one for a light page and one for a dark one, with the crosshair knocked out of it rather than painted on it, - so the page shows through it either way — and a chrome file holding the wordmark, the + so the page shows through it either way — and a templates file holding the wordmark, the rule and the footer. **A dark slide and a light one are the same deck, and the furniture on them cannot be the same furniture**: the name is ink on a light page and white on a dark one, so the dark wordmark is the same named style with the colour laid over it. That is the one - thing a chrome file cannot decide for itself, and it is why there are two of them. + thing a templates file cannot decide for itself, and it is why there are two of them. The company is invented, and so is every figure in the deck. @@ -228,20 +228,22 @@ All notable changes to this project are documented here. The format follows struck-through word rather than refused. The arms of the match are the vocabulary now, and an attribute they do not know is refused by name. -- **A chrome file is read with a ceiling.** The chrome file is the one file this library - reads whole, and it is the one a deck names in its frontmatter: a picture is bounded by - the pixels of its raster and a part of a template by the bytes it inflates to, and the - chrome file was bounded by nothing at all. It is read with a ceiling of a megabyte now, and - a file over it is refused with its size and the ceiling in the message. The ceiling is an - argument of `ChromeFile`, so a caller can hold a deck to a smaller one. +- **A templates file is read with a ceiling.** The templates file is the one file this + library reads whole, and it is the one a deck names in its frontmatter: a picture is + bounded by the pixels of its raster and a part of a template by the bytes it inflates to, + and the templates file was bounded by nothing at all. It is read with a ceiling of a + megabyte now, and a file over it is refused with its size and the ceiling in the message. + The ceiling is an argument of `TemplatesFile`, so a caller can hold a deck to a smaller + one. - **A slide carries at most a hundred shapes.** The chrome of a template is read once and drawn on every slide that names it, so the shapes of a template are multiplied by the - slides that draw them: two hundred shapes in a chrome file named by sixty slides is twelve - thousand shapes and ten megabytes of slide XML, and neither factor was bounded. The slide - count is bounded by `PresentationRenderer::MAX_SLIDES`, and `Furniture::MAX_SHAPES` is the - other half of that bound — a slide that carries more than a hundred shapes, the chrome of - a template included, is refused with the number and the ceiling in the message. + slides that draw them: two hundred shapes in a templates file named by sixty slides is + twelve thousand shapes and ten megabytes of slide XML, and neither factor was bounded. + The slide count is bounded by `PresentationRenderer::MAX_SLIDES`, and + `Furniture::MAX_SHAPES` is the other half of that bound — a slide that carries more than + a hundred shapes, the chrome of a template included, is refused with the number and the + ceiling in the message. - **A picture in a template is paid for once.** The chrome of a template is drawn on every slide that names it, and each of those slides had a picture of its own in the package: a diff --git a/README.md b/README.md index 5ce89b2..52e73fb 100644 --- a/README.md +++ b/README.md @@ -1549,19 +1549,19 @@ The page number is furniture, so it is written in a template and drawn on every names it. A slide may write one itself as well, and the number it carries is the number of the slide it is on. -#### The chrome file +#### The templates file A deck may keep its templates in a file beside it and name it in the deck's frontmatter: ```markdown --- theme: ./theme.yml -chrome: ./chrome.md +templates-file: ./templates.md --- ``` -`chrome.md` is a Markdown file of template definitions in the same syntax as a deck, and it is -resolved against the deck's own directory — so a deck and the furniture it shares travel +`templates.md` is a Markdown file of template definitions in the same syntax as a deck, and it +is resolved against the deck's own directory — so a deck and the furniture it shares travel together. A file that is not there is a failure, and so is a file outside the directories the render may read: a path a deck wrote is held to the same scope as a picture in it. @@ -1720,7 +1720,7 @@ slide that names it: | Ceiling | Value | Bounds | | --- | --- | --- | | `Render\Furniture::MAX_SHAPES` | 100 | the shapes one slide may carry, the chrome of a template included | -| `Template\ChromeFile::MAX_BYTES` | one megabyte | the chrome file a deck may name in its frontmatter | +| `Template\TemplatesFile::MAX_BYTES` | one megabyte | the templates file a deck may name in its frontmatter | Both are refused with the number and the ceiling in the message, and the shape count is the other half of the slide count: a hundred shapes on a slide is a hundred shapes on each of the @@ -1997,7 +1997,7 @@ try { ``` ```text -The frontmatter of slide 1 names "transition", which is not a directive. The directives are: title, name, level, layout, theme, author, description, template, chrome. +The frontmatter of slide 1 names "transition", which is not a directive. The directives are: title, name, level, layout, theme, author, description, template, templates-file. ``` Slidev accepts dozens of directives this library does not read, so a deck written for it will diff --git a/examples/README.md b/examples/README.md index f3425f0..b559f50 100644 --- a/examples/README.md +++ b/examples/README.md @@ -20,7 +20,7 @@ quietly. The profile and the scratch script go to the temporary directory, so | `01-kitchen-sink.md` | `01-kitchen-sink.pptx` | Every construct at once: emphasis, `code`, strikethrough, links, nested lists, an ordered list starting at 7, an aligned table with markup in its cells, a two-line quote, code, a rule, and a `::note` becoming a speaker note | | `02-layout-roles.md` | `02-layout-roles.pptx` | Every layout role, one per slide | | `03-themes.md` | three files | The same deck in `default`, `corporate-dark` and `minimal`, changed by nothing but the `theme:` line — so the three can be put side by side and compared. They differ in type scale, margins, rule weight and heading treatment, not only in palette | -| `04-frontmatter.md` | `04-frontmatter.pptx` | Every frontmatter key and what each one does, and a slide drawn with two templates — one the deck defines and one `templates/chrome.md` holds | +| `04-frontmatter.md` | `04-frontmatter.pptx` | Every frontmatter key and what each one does, and a slide drawn with two templates — one the deck defines and one `templates/templates.md` holds | | `05-steps.md` | `05-steps.pptx` | `::click` steps as click animations on one slide, a `:::click` region that is a step of its own, a build-up across two columns, a `::column` written inside a region, a picture held back for its own click, and a `::note` | | `06-tables.md` | `06-tables.pptx` | Table alignment, a bold filled header, widths from content | | `07-images.md` | `07-images.pptx` | A picture fitted to the room left in its column, a WebP converted to PNG, an SVG drawn to PNG, and the three layouts built around one: `image-text`, `image-title` and `image-slide` | @@ -32,7 +32,7 @@ quietly. The profile and the scratch script go to the temporary directory, so | `13-placed-deck.md` | `13-placed-deck.pptx` | A whole business pitch built out of free shapes and nothing else — a dark cover and a dark sign-off, an agenda of numbered circles, a card grid, a hand-built pricing table with zebra rows and a right-aligned column, a four-pillar comparison, a quarterly roadmap with arrows between phases, and translucent decoration over both dark slides. The company is invented | | `14-shadows.md` | `14-shadows.pptx` | Five shadow types on five different kinds of object — a soft shadow straight down, a hard-edged one with no blur at all, a glow with no offset, one cast down and to the right, and one thrown upward at 40% — plus a shadow on a `:::content` block and on a `::image` | | `15-inline-styles.md` | `15-inline-styles.pptx` | A word in a style of the theme inside a box with a colour of its own, the raw form that needs no style of the theme's, an attribute laid over a named style for one property only, the seven attributes this half draws, and the three it refuses — a name the theme does not have, a label holding Markdown, and the `highlight` it cannot write. The company is invented | -| `16-templates.md` | `16-templates.pptx` | The furniture of a deck, written once: a template defined in the deck, a slide that names one, a slide that names two, a slide that names three, a template imported from `templates/chrome.md`, and a template of the deck's own of the same name as one the file holds — the one that is drawn. The company is invented | +| `16-templates.md` | `16-templates.pptx` | The furniture of a deck, written once: a template defined in the deck, a slide that names one, a slide that names two, a slide that names three, a template imported from `templates/templates.md`, and a template of the deck's own of the same name as one the file holds — the one that is drawn. The company is invented | | `17-corporate-template.md` | `17-corporate-template.pptx` | A whole corporate template: a theme of the company's own, a brand mark in two colours — one for a light page and one for a dark one — and the furniture of a deck written once and named by every slide that wants it. Eight slides of the kinds a company deck is made of: a cover, an agenda, a headline, two columns with a chart, a quote, a table, a divider and a closing. The company is invented | ## The deck that is not anybody's business @@ -51,7 +51,7 @@ Rebuilding that deck from another tool is what turned up both defects in [PR #11 the failure it exists to demonstrate, so `build.php` leaves it alone and says which file and why. -`templates/chrome.md` is not a deck either: it is a file of templates, and +`templates/templates.md` is not a deck either: it is a file of templates, and `04-frontmatter.md` and `16-templates.md` are the decks that name it. It is in a directory of its own because every `.md` file beside the decks is one. @@ -152,7 +152,7 @@ A few of these are here because the render is the only honest evidence: of five slides, and two definitions stand between the cover and this slide in the file: a definition is not drawn, it has no number, and it does not count. - **`16`, the third slide.** The band across the head came out of - `templates/chrome.md`, which the deck's frontmatter names, and the deck says + `templates/templates.md`, which the deck's frontmatter names, and the deck says nothing about where it is or how thick it is. Three templates are named on this slide and the chrome of each of them is drawn on it. - **`16`, the fourth slide.** The number is `accent1` at twelve points and reads @@ -163,7 +163,7 @@ A few of these are here because the render is the only honest evidence: - **`17`, the second slide.** One box, three words and three colours is `15`; what is new here is the furniture around them. The wordmark is the company's own mark beside the company's own name, the rule is a hairline under the header, and the footer is the - confidential mark and the page number. All three are templates of `corporate/chrome.md` + confidential mark and the page number. All three are templates of `corporate/templates.md` and every slide that names them is drawn with them. - **`17`, the seventh slide.** The divider is a dark panel with a gold number on it, and the wordmark on it is the light one: the same named style with the colour laid over it, diff --git a/examples/markdown/04-frontmatter.md b/examples/markdown/04-frontmatter.md index 1e6fea4..7bb132b 100644 --- a/examples/markdown/04-frontmatter.md +++ b/examples/markdown/04-frontmatter.md @@ -3,14 +3,14 @@ title: Every frontmatter key description: One slide per key, and what each one reaches. theme: minimal author: markdown-office -chrome: ./templates/chrome.md +templates-file: ./templates/templates.md --- # Every frontmatter key Nine keys: `title`, `name`, `level`, `layout`, `theme`, `author`, `description`, -`template` and `chrome`. A block naming a tenth is a failure that names these -nine, and this slide carries the first four. +`template` and `templates-file`. A block naming a tenth is a failure that names +these nine, and this slide carries the first four. --- name: A title that is not a heading @@ -61,7 +61,7 @@ template: [wordmark, page] --- This slide names two templates. `wordmark` is the one this deck defines below, -and `page` is the one `templates/chrome.md` — the file this deck's frontmatter +and `page` is the one `templates/templates.md` — the file this deck's frontmatter names — holds. A template's frontmatter names the template and nothing else, which is what diff --git a/examples/markdown/16-templates.md b/examples/markdown/16-templates.md index d91ac6b..31de444 100644 --- a/examples/markdown/16-templates.md +++ b/examples/markdown/16-templates.md @@ -1,7 +1,7 @@ --- title: Slide templates theme: minimal -chrome: ./templates/chrome.md +templates-file: ./templates/templates.md description: The furniture of a deck, written once, and the slides that draw it. template: wordmark --- @@ -18,7 +18,7 @@ A deck gets its templates three ways: - a **definition** in the deck, a block whose frontmatter names a template - a **slide** that names a template, or several of them -- a **chrome file** beside the deck, named in the deck's frontmatter +- a **templates file** beside the deck, named in the deck's frontmatter --- template: wordmark @@ -60,7 +60,7 @@ template: [wordmark, band, page] # A template out of a file -The deck's frontmatter names `templates/chrome.md`. +The deck's frontmatter names `templates/templates.md`. That file holds a template called `band`. The band across the head of this slide came out of that file, and this deck says nothing about where it is or how thick it is. @@ -77,7 +77,7 @@ template: [wordmark, page] # The deck's own template wins :::content{left=6.7% top=20% width=86.7% height=70% font-size=17} -`templates/chrome.md` holds a template called `page` too, and it puts a bare number at +`templates/templates.md` holds a template called `page` too, and it puts a bare number at the foot of the slide. This deck defines its own `page` above, and the deck's own one is the one that is drawn: the number is in the theme's `accent1` rather than its `subtle`, at twelve points rather diff --git a/examples/markdown/17-corporate-template.md b/examples/markdown/17-corporate-template.md index 04c3016..5dd2b26 100644 --- a/examples/markdown/17-corporate-template.md +++ b/examples/markdown/17-corporate-template.md @@ -2,7 +2,7 @@ title: Third-quarter review description: A corporate deck built out of the house template. theme: ./corporate/theme.yml -chrome: ./corporate/chrome.md +templates-file: ./corporate/templates.md layout: title template: wordmark --- diff --git a/examples/markdown/corporate/README.md b/examples/markdown/corporate/README.md index 5a7a7ef..c41e0b4 100644 --- a/examples/markdown/corporate/README.md +++ b/examples/markdown/corporate/README.md @@ -12,7 +12,7 @@ decks have, and every part of it is a file you edit. | `theme.yml` | the design — the twelve colour slots, the faces, the type scale and the rhythm | | `layouts.yml` | which layout each of the eight roles gets | | `text-styles.yml` | the named text styles a deck writes words in | -| `chrome.md` | the furniture of a deck: the wordmark, the rule, the footer | +| `templates.md` | the furniture of a deck: the wordmark, the rule, the footer | | `logo.png`, `logo-light.png` | the mark, for a light page and for a dark one | | `make-logo.php` | draws the two marks from the theme's colours | @@ -22,7 +22,7 @@ The deck that uses all of it is `../17-corporate-template.md`. Three things, and each is one file. -**The name.** `chrome.md` holds the wordmark, and the name is in it twice — once in the +**The name.** `templates.md` holds the wordmark, and the name is in it twice — once in the light template and once in the dark one. The dark one is the same named style with the colour laid over it: @@ -45,7 +45,7 @@ the table uses it for the one row that went down. ## The furniture -The chrome file holds four templates, and a slide names the ones it wants: +The templates file holds four templates, and a slide names the ones it wants: ```markdown --- @@ -63,7 +63,7 @@ template: [wordmark, rule, footer] **A dark slide and a light one are the same deck, and the furniture on them cannot be the same furniture** — the name is ink on a light page and white on a dark one. That is what -the second wordmark is for, and it is the one thing a single chrome file cannot decide for +the second wordmark is for, and it is the one thing a single templates file cannot decide for you. The cover and the two dark slides name `wordmark-light` and nothing else; the light slides diff --git a/examples/markdown/corporate/chrome.md b/examples/markdown/corporate/templates.md similarity index 100% rename from examples/markdown/corporate/chrome.md rename to examples/markdown/corporate/templates.md diff --git a/examples/markdown/templates/chrome.md b/examples/markdown/templates/templates.md similarity index 100% rename from examples/markdown/templates/chrome.md rename to examples/markdown/templates/templates.md diff --git a/src/Exception/MalformedChrome.php b/src/Exception/MalformedChrome.php index 188238e..d02fe18 100644 --- a/src/Exception/MalformedChrome.php +++ b/src/Exception/MalformedChrome.php @@ -5,7 +5,11 @@ namespace MarkdownPresentation\Exception; /** - * A template that is not chrome, or a chrome file that is not templates. + * A template that holds the wrong thing. + * + * A template holds chrome — the free shapes it draws — and nothing else, so a body that + * holds ordinary Markdown, a column marker, or nothing at all is a mistake in the template + * rather than in the deck that draws it. * * Kept apart from {@see UnknownTemplate} because the two failures have opposite fixes: a * name nobody has needs a different name, and a body that holds the wrong thing needs a @@ -58,18 +62,6 @@ public static function empty(string $name): self )); } - /** - * A chrome file that holds something other than template definitions. - */ - public static function slide(string $file): self - { - return new self(sprintf( - 'The chrome file "%s" holds a slide that is not a template definition. A chrome file holds definitions: ' - . 'slides whose frontmatter names a template and nothing else.', - $file, - )); - } - /** * A mistake in a template, told with the message the mistake came with. * diff --git a/src/Exception/MalformedTemplatesFile.php b/src/Exception/MalformedTemplatesFile.php new file mode 100644 index 0000000..cddf381 --- /dev/null +++ b/src/Exception/MalformedTemplatesFile.php @@ -0,0 +1,23 @@ + $roots The directories this render may read a chrome file in, + * @param list $roots The directories this render may read a templates file in, * which the message names: the fix is to move the file into * one of them or to have the program rendering the deck allow * where it is. @@ -31,8 +31,8 @@ public static function at(string $path): self public static function outside(string $written, string $found, array $roots): self { return new self(sprintf( - 'The chrome file "%s" is at "%s" rather than one this render may read (%s). Copy it under one of those ' - . 'directories, or give the renderer that directory as one of its additional roots.', + 'The templates file "%s" is at "%s" rather than one this render may read (%s). Copy it under one of ' + . 'those directories, or give the renderer that directory as one of its additional roots.', $written, $found, implode(', ', $roots), @@ -42,8 +42,8 @@ public static function outside(string $written, string $found, array $roots): se public static function aUrl(string $written): self { return new self(sprintf( - 'The chrome file "%s" is a URL rather than a path. A chrome file is a Markdown file beside the deck, and ' - . 'PHP would have handed "phar://" or "http://" to the stream wrapper it names.', + 'The templates file "%s" is a URL rather than a path. A templates file is a Markdown file beside the ' + . 'deck, and PHP would have handed "phar://" or "http://" to the stream wrapper it names.', $written, )); } diff --git a/src/Exception/ChromeTooLarge.php b/src/Exception/TemplatesFileTooLarge.php similarity index 53% rename from src/Exception/ChromeTooLarge.php rename to src/Exception/TemplatesFileTooLarge.php index 1efded7..bbab891 100644 --- a/src/Exception/ChromeTooLarge.php +++ b/src/Exception/TemplatesFileTooLarge.php @@ -5,19 +5,19 @@ namespace MarkdownPresentation\Exception; /** - * A chrome file bigger than the library will read. + * A templates file bigger than the library will read. * - * The ceiling is {@see \MarkdownPresentation\Template\ChromeFile::MAX_BYTES}, and the size + * The ceiling is {@see \MarkdownPresentation\Template\TemplatesFile::MAX_BYTES}, and the size * is in the message because the fix is a number: only the author of the file can say how * much of the furniture belongs in it and how much belongs on the slides. */ -final class ChromeTooLarge extends InvalidInput +final class TemplatesFileTooLarge extends InvalidInput { public static function of(string $file, int $bytes, int $maximum): self { return new self(sprintf( - 'The chrome file "%s" is %d bytes, and a chrome file may be at most %d. Write less of the furniture in ' - . 'it, or write the furniture on the slides themselves.', + 'The templates file "%s" is %d bytes, and a templates file may be at most %d. Write less of the ' + . 'furniture in it, or write the furniture on the slides themselves.', $file, $bytes, $maximum, diff --git a/src/Render/Furniture.php b/src/Render/Furniture.php index ffaaf5b..ccf3421 100644 --- a/src/Render/Furniture.php +++ b/src/Render/Furniture.php @@ -7,16 +7,17 @@ use MarkdownPresentation\Document\Deck; use MarkdownPresentation\Document\DeckParser; use MarkdownPresentation\Document\Slide as SourceSlide; -use MarkdownPresentation\Exception\ChromeNotFound; use MarkdownPresentation\Exception\MalformedChrome; use MarkdownPresentation\Exception\MalformedFrontmatter; +use MarkdownPresentation\Exception\MalformedTemplatesFile; +use MarkdownPresentation\Exception\TemplatesFileNotFound; use MarkdownPresentation\Exception\TooManyShapes; use MarkdownPresentation\Exception\UnknownTemplate; use MarkdownPresentation\Render\Free\FreeShape; use MarkdownPresentation\Render\Free\FreeSlideNumber; -use MarkdownPresentation\Template\ChromeFile; use MarkdownPresentation\Template\Definition; use MarkdownPresentation\Template\Templates; +use MarkdownPresentation\Template\TemplatesFile; use MarkdownPresentation\Theme\Theme; /** @@ -50,27 +51,28 @@ final class Furniture public const MAX_SHAPES = 100; /** - * @param Deck $deck The deck being rendered, which holds the templates it - * defines and the chrome file it names. - * @param ChromeFile $files The reader of the chrome file the deck names. - * @param Chrome $chrome The reader of a template's body. + * @param Deck $deck The deck being rendered, which holds the templates it + * defines and the templates file it names. + * @param TemplatesFile $files The reader of the templates file the deck names. + * @param Chrome $chrome The reader of a template's body. */ public function __construct( private readonly Deck $deck, - private readonly ChromeFile $files, + private readonly TemplatesFile $files, private readonly Chrome $chrome, ) { } /** - * The templates the deck has: its own over the ones a chrome file holds. + * The templates the deck has: its own over the ones a templates file holds. * - * Read before anything is built, so a chrome file that cannot be read fails while there - * is still no file on disk to clean up. + * Read before anything is built, so a templates file that cannot be read fails while + * there is still no file on disk to clean up. * - * @throws ChromeNotFound * @throws MalformedChrome * @throws MalformedFrontmatter + * @throws MalformedTemplatesFile + * @throws TemplatesFileNotFound */ public function templates(): Templates { @@ -203,39 +205,39 @@ private function names(Templates $templates, SourceSlide $slide): array } /** - * The templates a chrome file holds. + * The templates a templates file holds. * * A file beside the deck, named by the deck's frontmatter and resolved against the * deck's own directory, so that a deck and the furniture it shares travel together. * * @return list * - * @throws ChromeNotFound - * @throws MalformedChrome * @throws MalformedFrontmatter + * @throws MalformedTemplatesFile + * @throws TemplatesFileNotFound */ private function imported(): array { - $written = $this->deck->frontmatterValue('chrome'); + $written = $this->deck->frontmatterValue('templates-file'); if ($written === null) { return []; } if (!is_string($written) || trim($written) === '') { - throw MalformedFrontmatter::wrongType(1, 'chrome', 'a path to a file of chrome'); + throw MalformedFrontmatter::wrongType(1, 'templates-file', 'a path to a file of templates'); } $markdown = $this->files->read(trim($written)); - $chrome = (new DeckParser())->parse($markdown); + $templates = (new DeckParser())->parse($markdown); - // A chrome file holds definitions and nothing else: a slide in it is a slide + // A templates file holds definitions and nothing else: a slide in it is a slide // nobody asked for, and one that would be drawn on a deck it was not written for. - if ($chrome->slides() !== []) { - throw MalformedChrome::slide(trim($written)); + if ($templates->slides() !== []) { + throw MalformedTemplatesFile::slide(trim($written)); } - return $chrome->definitions(); + return $templates->definitions(); } /** diff --git a/src/Render/PresentationRenderer.php b/src/Render/PresentationRenderer.php index 6f4bdc4..f56796e 100644 --- a/src/Render/PresentationRenderer.php +++ b/src/Render/PresentationRenderer.php @@ -33,8 +33,8 @@ use MarkdownPresentation\Render\Free\FreeShape; use MarkdownPresentation\Render\Free\FreeSlideNumber; use MarkdownPresentation\Render\Free\FreeText; -use MarkdownPresentation\Template\ChromeFile; use MarkdownPresentation\Template\Templates; +use MarkdownPresentation\Template\TemplatesFile; use MarkdownPresentation\Theme\LayoutRole; use MarkdownPresentation\Theme\LoadedTheme; use MarkdownPresentation\Theme\Theme; @@ -92,10 +92,12 @@ final class PresentationRenderer * the whole of what makes it affordable is that {@see UnknownDirective} names this list * in the failure itself. */ - public const DIRECTIVES = ['title', 'name', 'level', 'layout', 'theme', 'author', 'description', 'template', 'chrome']; + public const DIRECTIVES = [ + 'title', 'name', 'level', 'layout', 'theme', 'author', 'description', 'template', 'templates-file', + ]; /** The keys that describe the deck rather than a slide, and so belong to slide 1's block. */ - public const DECK_DIRECTIVES = ['theme', 'author', 'description', 'chrome']; + public const DECK_DIRECTIVES = ['theme', 'author', 'description', 'templates-file']; /** The theme a deck that names none gets. */ public const DEFAULT_THEME = 'default'; @@ -153,9 +155,9 @@ final class PresentationRenderer private readonly ThemeResolver $themes; /** - * The reader of the chrome file a deck names. + * The reader of the templates file a deck names. */ - private readonly ChromeFile $chromeFiles; + private readonly TemplatesFile $templatesFiles; /** * The reader of a template's body. @@ -229,7 +231,7 @@ public function __construct( $this->files = new PathScope([$this->directory, ...$imageRoots]); $this->overflows = new Overflows($strict); $this->fonts = new FontReport($strict); - $this->chromeFiles = new ChromeFile($this->directory, $this->files); + $this->templatesFiles = new TemplatesFile($this->directory, $this->files); $this->chrome = new Chrome($this->blocks, $this->directory, $this->files); } @@ -332,7 +334,7 @@ public function render(Deck $deck, string $path): string // The templates before the slides are drawn, and the chrome before either: a slide // that names a template that is not there, and a template that cannot be drawn, are // both failures that happen with no file written behind them. - $this->furniture = new Furniture($deck, $this->chromeFiles, $this->chrome); + $this->furniture = new Furniture($deck, $this->templatesFiles, $this->chrome); $this->templates = $this->furniture->templates(); foreach ($deck->slides() as $slide) { diff --git a/src/Template/Templates.php b/src/Template/Templates.php index c559f41..7dbc3fb 100644 --- a/src/Template/Templates.php +++ b/src/Template/Templates.php @@ -11,7 +11,7 @@ * told apart by where they come from: the deck's own templates override the imported ones * of the same name. It is the rule the theme resolver applies by search order — the first * one found under a name is the one that is used — and it is here rather than in the - * resolver because a chrome file is not a theme and is not found by looking through + * resolver because a templates file is not a theme and is not found by looking through * directories: a deck names the one file it wants. */ final class Templates @@ -30,7 +30,7 @@ private function __construct(private readonly array $definitions) * that is left under a name both of them use. * * @param list $local The templates the deck defines. - * @param list $imported The templates a chrome file defines. + * @param list $imported The templates a templates file defines. */ public static function of(array $local, array $imported): self { diff --git a/src/Template/ChromeFile.php b/src/Template/TemplatesFile.php similarity index 73% rename from src/Template/ChromeFile.php rename to src/Template/TemplatesFile.php index b2d6efa..e5f8347 100644 --- a/src/Template/ChromeFile.php +++ b/src/Template/TemplatesFile.php @@ -4,24 +4,24 @@ namespace MarkdownPresentation\Template; -use MarkdownPresentation\Exception\ChromeNotFound; -use MarkdownPresentation\Exception\ChromeTooLarge; +use MarkdownPresentation\Exception\TemplatesFileNotFound; +use MarkdownPresentation\Exception\TemplatesFileTooLarge; use MarkdownPresentation\Filesystem\PathScope; /** - * The chrome file a deck names, read from beside the deck. + * The templates file a deck names, read from beside the deck. * * A path a deck writes, so it is resolved and held to the directories this render may * read — the same rule an image path has to pass, because both are paths a deck wrote and * neither is worth a way out of the directory the deck was given. */ -final class ChromeFile +final class TemplatesFile { /** - * The largest a chrome file may be. + * The largest a templates file may be. * - * A chrome file is the furniture of a deck — a handful of shapes for each of its - * templates — and a megabyte of it is far past any file of chrome. It is the one file + * A templates file is the furniture of a deck — a handful of shapes for each of its + * templates — and a megabyte of it is far past any file of furniture. It is the one file * this library reads whole, so this is what bounds the memory a deck's frontmatter can * ask for. */ @@ -39,21 +39,21 @@ public function __construct( } /** - * The Markdown of the chrome file a deck names. + * The Markdown of the templates file a deck names. * - * @throws ChromeNotFound - * @throws ChromeTooLarge + * @throws TemplatesFileNotFound + * @throws TemplatesFileTooLarge */ public function read(string $written): string { if (PathScope::isUrl($written)) { - throw ChromeNotFound::aUrl($written); + throw TemplatesFileNotFound::aUrl($written); } $joined = str_starts_with($written, '/') ? $written : rtrim($this->directory, '/') . '/' . $written; if (!is_file($joined)) { - throw ChromeNotFound::at($joined); + throw TemplatesFileNotFound::at($joined); } // `realpath()` rather than the path as written, because `..` and a symlink both walk @@ -61,7 +61,7 @@ public function read(string $written): string $resolved = PathScope::canonical($joined); if ($resolved === null || !$this->files->holds($resolved)) { - throw ChromeNotFound::outside($written, $resolved ?? $joined, $this->files->roots()); + throw TemplatesFileNotFound::outside($written, $resolved ?? $joined, $this->files->roots()); } // The read only happens where the file can be read: `file_get_contents()` warns @@ -76,11 +76,11 @@ public function read(string $written): string : false; if ($markdown === false) { - throw ChromeNotFound::at($resolved); + throw TemplatesFileNotFound::at($resolved); } if (strlen($markdown) > $this->maxBytes) { - throw ChromeTooLarge::of($written, $this->size($resolved, strlen($markdown)), $this->maxBytes); + throw TemplatesFileTooLarge::of($written, $this->size($resolved, strlen($markdown)), $this->maxBytes); } return $markdown; diff --git a/tests/Readme/examples-decks.php b/tests/Readme/examples-decks.php index 26f2eac..6aa9a7c 100644 --- a/tests/Readme/examples-decks.php +++ b/tests/Readme/examples-decks.php @@ -686,7 +686,7 @@ function shadowedNames(Pptx $pptx, int $slide = 1): array it('draws the imported template on the slide that names it, and on no other', function () { [$pptx] = builtDeck('16-templates.md'); - // The band is the one piece of furniture that came out of `templates/chrome.md` and + // The band is the one piece of furniture that came out of `templates/templates.md` and // was not overridden, so it is on the slide that names it and nowhere else — and it is // across the head of the slide, which is where the file puts it. foreach (range(1, $pptx->slideCount()) as $slide) { @@ -710,7 +710,7 @@ function shadowedNames(Pptx $pptx, int $slide = 1): array it('draws the furniture of the corporate template on every slide that names it', function () { [$pptx] = builtDeck('17-corporate-template.md'); - // The wordmark, the rule and the footer are three templates of the deck's own chrome + // The wordmark, the rule and the footer are three templates of the deck's own templates // file, and a slide names the ones it wants. The five light slides name all three; the // cover and the two dark slides name the light wordmark and nothing else — a cover has // no page number, and a dark slide has no footer on it. @@ -735,7 +735,7 @@ function shadowedNames(Pptx $pptx, int $slide = 1): array // The name is ink on a light page and white on a dark one, and the two are the same // named style with the colour laid over it — a deck cannot carry one colour on both - // kinds of slide, and a chrome file cannot decide for itself. + // kinds of slide, and a templates file cannot decide for itself. $light = runOfText($pptx, 'Halden Instruments', 2); $dark = runOfText($pptx, 'Halden Instruments', 7); diff --git a/tests/Unit/exceptions.php b/tests/Unit/exceptions.php index 7c13272..2c8dd6c 100644 --- a/tests/Unit/exceptions.php +++ b/tests/Unit/exceptions.php @@ -3,8 +3,6 @@ declare(strict_types=1); use MarkdownPresentation\Document\DeckParser; -use MarkdownPresentation\Exception\ChromeNotFound; -use MarkdownPresentation\Exception\ChromeTooLarge; use MarkdownPresentation\Exception\ContentOverflow; use MarkdownPresentation\Exception\Exception; use MarkdownPresentation\Exception\FencedFrontmatter; @@ -15,9 +13,12 @@ use MarkdownPresentation\Exception\MalformedFrontmatter; use MarkdownPresentation\Exception\MalformedShape; use MarkdownPresentation\Exception\MalformedTemplate; +use MarkdownPresentation\Exception\MalformedTemplatesFile; use MarkdownPresentation\Exception\MalformedTextStyle; use MarkdownPresentation\Exception\MalformedTheme; use MarkdownPresentation\Exception\NothingToConvert; +use MarkdownPresentation\Exception\TemplatesFileNotFound; +use MarkdownPresentation\Exception\TemplatesFileTooLarge; use MarkdownPresentation\Exception\ThemeNotFound; use MarkdownPresentation\Exception\TooManyShapes; use MarkdownPresentation\Exception\TooManySlides; @@ -96,8 +97,6 @@ function reportedExceptions(): array // The base is for everything; the one below it is for the failures a deck author // can fix by naming something else. $authorFixable = [ - ChromeNotFound::class, - ChromeTooLarge::class, ContentOverflow::class, FencedFrontmatter::class, ImageNotFound::class, @@ -105,8 +104,11 @@ function reportedExceptions(): array MalformedFrontmatter::class, MalformedShape::class, MalformedTemplate::class, + MalformedTemplatesFile::class, MalformedTextStyle::class, MalformedTheme::class, + TemplatesFileNotFound::class, + TemplatesFileTooLarge::class, ThemeNotFound::class, TooManyShapes::class, TooManySlides::class, diff --git a/tests/Unit/rendering.php b/tests/Unit/rendering.php index 47369bf..7c81579 100644 --- a/tests/Unit/rendering.php +++ b/tests/Unit/rendering.php @@ -1893,7 +1893,7 @@ function ico(): string ->toThrow( UnknownDirective::class, 'The frontmatter of slide 1 names "transition", which is not a directive. ' - . 'The directives are: title, name, level, layout, theme, author, description, template, chrome.', + . 'The directives are: title, name, level, layout, theme, author, description, template, templates-file.', ); }); diff --git a/tests/Unit/templates.php b/tests/Unit/templates.php index ff7356c..4120afc 100644 --- a/tests/Unit/templates.php +++ b/tests/Unit/templates.php @@ -5,7 +5,6 @@ use MarkdownPresentation\Document\DeckParser; use MarkdownPresentation\Document\Slide; use MarkdownPresentation\Exception\Exception as MarkdownException; -use MarkdownPresentation\Exception\MalformedChrome; use MarkdownPresentation\Exception\MalformedFrontmatter; use MarkdownPresentation\Exception\UnknownFont; use MarkdownPresentation\Exception\UnknownTemplate; @@ -13,7 +12,7 @@ use MarkdownPresentation\Render\PresentationRenderer; use MarkdownPresentation\Tests\Support\Pptx; use MarkdownPresentation\Tests\Support\Scratch; -use MarkdownPresentation\Template\ChromeFile; +use MarkdownPresentation\Template\TemplatesFile; /** * Slide templates: a piece of furniture written once and drawn on every slide that names it. @@ -387,10 +386,10 @@ function templateRenderer(string $markdown, bool $strict = false): PresentationR ->and($file->positionOf('image', 1))->not->toBe($file->positionOf('image', 2)); }); -it('imports the templates of a chrome file beside the deck', function () { - $chrome = Scratch::file('chrome.md', "---\ntemplate: brand\n---\n::line{left=5% top=72.889% width=30% height=0.185%}\n"); +it('imports the templates of a templates file beside the deck', function () { + $templates = Scratch::file('templates.md', "---\ntemplate: brand\n---\n::line{left=5% top=72.889% width=30% height=0.185%}\n"); - $deck = sprintf("---\nchrome: %s\nlayout: blank\n---\n\n---\nlayout: blank\ntemplate: brand\n---\n", $chrome); + $deck = sprintf("---\ntemplates-file: %s\nlayout: blank\n---\n\n---\nlayout: blank\ntemplate: brand\n---\n", $templates); $file = templateDeck($deck); @@ -399,11 +398,11 @@ function templateRenderer(string $markdown, bool $strict = false): PresentationR it('lets the deck\'s own template override an imported one of the same name', function () { // The same rule the theme resolver applies by search order: the deck's own wins. - $chrome = Scratch::file('chrome.md', "---\ntemplate: brand\n---\n::line{left=5% top=72.889% width=30% height=0.185%}\n"); + $templates = Scratch::file('templates.md', "---\ntemplate: brand\n---\n::line{left=5% top=72.889% width=30% height=0.185%}\n"); $deck = sprintf( - "---\nchrome: %s\nlayout: blank\n---\n\n---\ntemplate: brand\n---\n:::content{left=5%% top=5%% width=30%% height=5%%}\nDeck\n:::\n\n---\nlayout: blank\ntemplate: brand\n---\n", - $chrome, + "---\ntemplates-file: %s\nlayout: blank\n---\n\n---\ntemplate: brand\n---\n:::content{left=5%% top=5%% width=30%% height=5%%}\nDeck\n:::\n\n---\nlayout: blank\ntemplate: brand\n---\n", + $templates, ); $file = templateDeck($deck); @@ -411,76 +410,76 @@ function templateRenderer(string $markdown, bool $strict = false): PresentationR expect($file->shapeNames(2))->toBe(['content']); }); -it('refuses a chrome file that is not there', function () { - $deck = "---\nchrome: ./nowhere.md\nlayout: blank\n---\n"; +it('refuses a templates file that is not there', function () { + $deck = "---\ntemplates-file: ./nowhere.md\nlayout: blank\n---\n"; - expect(templateFailure($deck))->toContain('There is no chrome file at'); + expect(templateFailure($deck))->toContain('There is no templates file at'); }); -it('refuses a chrome file that is a URL', function () { - $deck = "---\nchrome: phar://chrome.md\nlayout: blank\n---\n"; +it('refuses a templates file that is a URL', function () { + $deck = "---\ntemplates-file: phar://templates.md\nlayout: blank\n---\n"; expect(templateFailure($deck))->toContain('is a URL rather than a path'); }); -it('refuses a chrome file that is larger than the library will read', function () { - $chrome = Scratch::file('chrome.md', str_repeat('a', ChromeFile::MAX_BYTES + 1)); +it('refuses a templates file that is larger than the library will read', function () { + $templates = Scratch::file('templates.md', str_repeat('a', TemplatesFile::MAX_BYTES + 1)); - $deck = sprintf("---\nchrome: %s\nlayout: blank\n---\n", $chrome); + $deck = sprintf("---\ntemplates-file: %s\nlayout: blank\n---\n", $templates); expect(templateFailure($deck)) ->toContain(sprintf( - 'is %d bytes, and a chrome file may be at most %d', - ChromeFile::MAX_BYTES + 1, - ChromeFile::MAX_BYTES, + 'is %d bytes, and a templates file may be at most %d', + TemplatesFile::MAX_BYTES + 1, + TemplatesFile::MAX_BYTES, )); }); -it('refuses a chrome file this render may not read', function () { - $chrome = Scratch::file('chrome.md', "---\ntemplate: brand\n---\n::line{}\n"); - $outside = sys_get_temp_dir() . '/chrome-' . bin2hex(random_bytes(6)) . '.md'; +it('refuses a templates file this render may not read', function () { + $templates = Scratch::file('templates.md', "---\ntemplate: brand\n---\n::line{}\n"); + $outside = sys_get_temp_dir() . '/templates-' . bin2hex(random_bytes(6)) . '.md'; file_put_contents($outside, "---\ntemplate: brand\n---\n::line{}\n"); - $deck = sprintf("---\nchrome: %s\nlayout: blank\n---\n", $outside); + $deck = sprintf("---\ntemplates-file: %s\nlayout: blank\n---\n", $outside); expect(templateFailure($deck))->toContain('rather than one this render may read'); unlink($outside); }); -it('refuses a chrome file that holds a slide', function () { - $chrome = Scratch::file('chrome.md', "# A slide\n"); +it('refuses a templates file that holds a slide', function () { + $templates = Scratch::file('templates.md', "# A slide\n"); - $deck = sprintf("---\nchrome: %s\nlayout: blank\n---\n", $chrome); + $deck = sprintf("---\ntemplates-file: %s\nlayout: blank\n---\n", $templates); expect(templateFailure($deck))->toContain('holds a slide that is not a template definition'); }); -it('refuses a chrome file it cannot read', function () { +it('refuses a templates file it cannot read', function () { skipWithoutPermissions('The unreadable file'); - $chrome = Scratch::file('chrome.md', "---\ntemplate: brand\n---\n::line{}\n"); + $templates = Scratch::file('templates.md', "---\ntemplate: brand\n---\n::line{}\n"); - chmod($chrome, 0o000); + chmod($templates, 0o000); - $deck = sprintf("---\nchrome: %s\nlayout: blank\n---\n", $chrome); + $deck = sprintf("---\ntemplates-file: %s\nlayout: blank\n---\n", $templates); - expect(templateFailure($deck))->toContain('There is no chrome file at'); + expect(templateFailure($deck))->toContain('There is no templates file at'); }); -it('refuses a chrome file named on a later slide', function () { - $deck = "---\nlayout: blank\n---\n\n---\nchrome: ./chrome.md\n---\n"; +it('refuses a templates file named on a later slide', function () { + $deck = "---\nlayout: blank\n---\n\n---\ntemplates-file: ./templates.md\n---\n"; expect(templateFailure($deck)) - ->toContain('The frontmatter of slide 2 names "chrome", which is a deck directive'); + ->toContain('The frontmatter of slide 2 names "templates-file", which is a deck directive'); }); -it('refuses a chrome file that is not a path', function () { - $deck = "---\nchrome: 42\nlayout: blank\n---\n"; +it('refuses a templates file that is not a path', function () { + $deck = "---\ntemplates-file: 42\nlayout: blank\n---\n"; expect(templateFailure($deck)) - ->toContain('The frontmatter of slide 1 has a "chrome" that is not a path to a file of chrome'); + ->toContain('The frontmatter of slide 1 has a "templates-file" that is not a path to a file of templates'); }); it('refuses a template that is named by something other than a name', function () { From a9ab19ce86b97eae85504cb3f89de15e055c470a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabian=20Gra=C3=9Fl?= Date: Thu, 29 Oct 2026 22:09:40 +0100 Subject: [PATCH 2/8] The finder: where a deck's files come from MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The library reads every file it needs itself, and the only way in is a path — so a source that keeps its themes in a database cannot serve a deck at all. This adds the interface the library will ask by name, and the default that answers out of directories on this machine. The finder answers and never refuses: a file that is not there is null. The scope check stays with the library, where the messages are, because each half of this suite has its own exceptions — a finder that threw would throw one half's exception and the other half would not catch it, and the two interfaces would stop being the same. That is what lets one adapter serve both. The bases are given per role and by the caller, so the finder is built for the deck at hand rather than for a fixed set of directories. A theme is resolved by its directory first, so a theme directory that holds no manifest still shadows a later one: the behaviour of a theme resolved from a directory, kept. --- src/Files/DiskFinder.php | 240 ++++++++++++++++++++++++++++++++++++ src/Files/FileFinder.php | 57 +++++++++ tests/Unit/file-finder.php | 246 +++++++++++++++++++++++++++++++++++++ 3 files changed, 543 insertions(+) create mode 100644 src/Files/DiskFinder.php create mode 100644 src/Files/FileFinder.php create mode 100644 tests/Unit/file-finder.php diff --git a/src/Files/DiskFinder.php b/src/Files/DiskFinder.php new file mode 100644 index 0000000..2e02635 --- /dev/null +++ b/src/Files/DiskFinder.php @@ -0,0 +1,240 @@ + $theme The directories a theme's files are looked for in, in + * the order they are tried. + * @param list $picture The directories a picture is looked for in, in the + * order they are tried. + * @param list $document The directories a deck's own file is looked for in, + * in the order they are tried. + */ + public function __construct( + private readonly array $theme = [], + private readonly array $picture = [], + private readonly array $document = [], + ) { + } + + /** + * The contents of a file a name identifies, or null when there is none. + */ + public function read(string $name, ?string $role = null): ?string + { + $path = $this->resolve($name, $role); + + if ($path === null) { + return null; + } + + // `@file_get_contents()` rather than a check first: a file that cannot be read is + // a file the finder has no contents for, and the warning it would print is not + // a failure anybody can act on. + $contents = @file_get_contents($path); + + return $contents === false ? null : $contents; + } + + /** + * The path of a file a name identifies, when this finder holds it as a file. + * + * The path as it was resolved rather than as the machine spells it: the library + * resolves what it is given and holds it to its own roots, and a path that arrives + * already resolved is a path it cannot check. + */ + public function path(string $name, ?string $role = null): ?string + { + return $this->resolve($name, $role); + } + + /** + * The themes that resolve, sorted. + * + * A directory that holds a `theme.yml` is listed and one without it is not. A base + * that is not there holds nothing: a directory the caller pointed at may not exist + * yet, and a base with nothing in it is not a failure here. + * + * @return list + */ + public function themes(): array + { + $names = []; + + foreach ($this->theme as $base) { + if (!is_dir($base)) { + continue; + } + + foreach (scandir($base) ?: [] as $entry) { + if (str_starts_with($entry, '.')) { + continue; + } + + if (is_file(rtrim($base, '/') . '/' . $entry . '/' . ThemeLoader::THEME_FILE)) { + $names[$entry] = true; + } + } + } + + $found = array_keys($names); + + sort($found); + + return $found; + } + + /** + * The names of the files of a theme, sorted. + * + * The files of the directory the theme's name resolves to, so the listing and the + * file the library asks for are about the same theme: a source that cannot serve a + * master is told apart from a theme that has none. + * + * A name that begins with a dot is not one of the theme's files: it is a thing the + * file system put there rather than something the library reads. + * + * @return list + */ + public function files(string $theme): array + { + $directory = $this->themeDirectory($theme); + + if ($directory === null) { + return []; + } + + $names = []; + + foreach (scandir($directory) ?: [] as $entry) { + if (str_starts_with($entry, '.')) { + continue; + } + + if (is_file($directory . '/' . $entry)) { + $names[] = $entry; + } + } + + sort($names); + + return $names; + } + + /** + * The file a name identifies, or null when there is none. + * + * The first base that has the file decides. + */ + private function resolve(string $name, ?string $role): ?string + { + if (str_starts_with($name, '/')) { + // A name that is already a path is taken as it stands: resolving it against + // a base would name a file nobody meant. + return is_file($name) ? $name : null; + } + + if ($role === 'theme') { + return $this->resolveTheme($name); + } + + foreach ($this->bases($role) as $base) { + $path = rtrim($base, '/') . '/' . $name; + + if (is_file($path)) { + return $path; + } + } + + return null; + } + + /** + * The file of a theme a name identifies, or null when there is none. + * + * A theme's file is named inside its theme, so the name is `/` and + * the theme's directory is found first. The file is then looked for inside that + * directory and nowhere else: a directory that exists but holds no manifest + * shadows a later one, which is what a theme resolved from a directory has always + * done. + */ + private function resolveTheme(string $name): ?string + { + $slash = strpos($name, '/'); + + if ($slash === false) { + // A name without a theme in it names no file of a theme. + return null; + } + + $directory = $this->themeDirectory(substr($name, 0, $slash)); + + if ($directory === null) { + return null; + } + + $path = $directory . '/' . substr($name, $slash + 1); + + return is_file($path) ? $path : null; + } + + /** + * The directory a theme is in, or null when there is no such theme. + * + * The first base that has a directory of that name, whether or not it holds a + * manifest: the manifest decides whether the directory is a theme, and the + * directory decides which theme a name means. + */ + private function themeDirectory(string $theme): ?string + { + foreach ($this->theme as $base) { + $directory = rtrim($base, '/') . '/' . $theme; + + if (is_dir($directory)) { + return $directory; + } + } + + return null; + } + + /** + * The directories a name is looked for in, for a role. + * + * A role the finder was given no bases for looks in none of them: the role says + * which world the name is resolved in, and a world it does not know about is not + * one it can answer for. A name with no role is looked for in every world, in the + * order the roles are declared, and is resolved as a plain name — the role is + * what says how a name is resolved. + * + * @return list + */ + private function bases(?string $role): array + { + return match ($role) { + null => [...$this->theme, ...$this->picture, ...$this->document], + 'picture' => $this->picture, + 'document' => $this->document, + default => [], + }; + } +} diff --git a/src/Files/FileFinder.php b/src/Files/FileFinder.php new file mode 100644 index 0000000..8918610 --- /dev/null +++ b/src/Files/FileFinder.php @@ -0,0 +1,57 @@ + + */ + public function themes(): array; + + /** + * The names of the files of a theme, sorted. + * + * @return list + */ + public function files(string $theme): array; +} diff --git a/tests/Unit/file-finder.php b/tests/Unit/file-finder.php new file mode 100644 index 0000000..c40005e --- /dev/null +++ b/tests/Unit/file-finder.php @@ -0,0 +1,246 @@ +read('logo.png', 'picture'))->toBe('the bytes'); +}); + +it('reads a theme file through the finder', function () { + $base = Scratch::directoryPath('themefiles'); + mkdir($base . '/house'); + file_put_contents($base . '/house/theme.yml', "name: House\n"); + + $finder = new DiskFinder(theme: [$base]); + + expect($finder->read('house/theme.yml', 'theme'))->toBe("name: House\n") + // A theme's file is named inside its theme, so a name without a theme in it + // names no file of a theme. + ->and($finder->read('theme.yml', 'theme'))->toBeNull(); +}); + +it('gives the path of a file it holds, as it was written', function () { + // The library resolves what it is given and holds it to its own roots, so a path + // that arrives already resolved is a path it cannot check. The base is a link + // here, because a path that resolves to itself cannot tell the two apart. + $real = Scratch::directoryPath('real'); + file_put_contents($real . '/logo.png', 'the bytes'); + + $holder = Scratch::directoryPath('holder'); + + if (!@symlink($real, $holder . '/themes')) { + test()->markTestSkipped('symlinks are not available here'); + } + + $finder = new DiskFinder(picture: [$holder . '/themes']); + + expect($finder->path('logo.png', 'picture'))->toBe($holder . '/themes/logo.png'); +}); + +it('has no file for a name that is not there', function () { + $base = finderBase('logo.png'); + + $finder = new DiskFinder(picture: [$base]); + + expect($finder->read('logo.png', 'picture'))->toBe('the bytes') + ->and($finder->read('nowhere.png', 'picture'))->toBeNull() + ->and($finder->path('nowhere.png', 'picture'))->toBeNull(); +}); + +it('takes a name that is already a path as it stands', function () { + // A path is a path, whatever the bases are: resolving it against one would name a + // file nobody meant. + $base = finderBase('report.md'); + + $finder = new DiskFinder(document: [$base]); + + expect($finder->read($base . '/report.md', 'document'))->toBe('the bytes') + ->and($finder->path($base . '/report.md', 'document'))->toBe($base . '/report.md') + ->and($finder->read($base . '/nowhere.md', 'document'))->toBeNull(); +}); + +it('lists the themes that hold a manifest, sorted', function () { + $base = Scratch::directoryPath('themes'); + + mkdir($base . '/house'); + file_put_contents($base . '/house/theme.yml', "name: House\n"); + + mkdir($base . '/zebra'); + file_put_contents($base . '/zebra/theme.yml', "name: Zebra\n"); + + // A directory without a manifest is not a theme, and neither is a file that + // happens to be called like one. + mkdir($base . '/bare'); + file_put_contents($base . '/theme.yml', "name: Not a theme\n"); + + // Nor is a directory the file system put there. + mkdir($base . '/.hidden'); + file_put_contents($base . '/.hidden/theme.yml', "name: Hidden\n"); + + $finder = new DiskFinder(theme: [$base]); + + expect($finder->themes())->toBe(['house', 'zebra']); +}); + +it('looks past a base that is not there', function () { + // A directory the caller pointed at may not exist yet, and a base with nothing in + // it is not a failure here. + $base = Scratch::directoryPath('present'); + mkdir($base . '/house'); + file_put_contents($base . '/house/theme.yml', "name: House\n"); + + $finder = new DiskFinder(theme: [$base . '/not-there', $base]); + + expect($finder->themes())->toBe(['house']) + ->and($finder->read('house/theme.yml', 'theme'))->toBe("name: House\n"); +}); + +it('lists the files of a theme, sorted, and nothing for a theme that is not there', function () { + $base = Scratch::directoryPath('files'); + mkdir($base . '/house'); + file_put_contents($base . '/house/theme.yml', "name: House\n"); + file_put_contents($base . '/house/styles.yml', "heading.1:\n"); + file_put_contents($base . '/house/cover.png', 'the bytes'); + + // Neither a hidden file nor a directory is one of the theme's files. + file_put_contents($base . '/house/.hidden', ''); + mkdir($base . '/house/assets'); + + $finder = new DiskFinder(theme: [$base]); + + expect($finder->files('house'))->toBe(['cover.png', 'styles.yml', 'theme.yml']) + ->and($finder->files('missing'))->toBe([]); +}); + +it('looks for a name in the bases in the order they are given', function () { + $first = Scratch::directoryPath('first'); + $second = Scratch::directoryPath('second'); + + file_put_contents($first . '/logo.png', 'the first'); + file_put_contents($second . '/logo.png', 'the second'); + file_put_contents($second . '/only.png', 'the second only'); + + $finder = new DiskFinder(picture: [$first, $second]); + + expect($finder->read('logo.png', 'picture'))->toBe('the first') + // And the second base is looked at when the first holds nothing. + ->and($finder->read('only.png', 'picture'))->toBe('the second only'); +}); + +it('takes the theme of the first base that has one', function () { + $first = Scratch::directoryPath('first'); + $second = Scratch::directoryPath('second'); + + mkdir($first . '/house'); + file_put_contents($first . '/house/theme.yml', "name: First\n"); + + mkdir($second . '/house'); + file_put_contents($second . '/house/theme.yml', "name: Second\n"); + + $finder = new DiskFinder(theme: [$first, $second]); + + expect($finder->read('house/theme.yml', 'theme'))->toBe("name: First\n") + ->and($finder->files('house'))->toBe(['theme.yml']); +}); + +it('lets a theme directory without a manifest shadow a later one', function () { + // The theme's directory is found first and the file is looked for inside it, so a + // directory that exists but holds no manifest is the theme: the manifest decides + // whether it is one, and the library says so rather than falling through to a + // theme of another name. + $first = Scratch::directoryPath('first'); + $second = Scratch::directoryPath('second'); + + mkdir($first . '/house'); + + mkdir($second . '/house'); + file_put_contents($second . '/house/theme.yml', "name: Second\n"); + + $finder = new DiskFinder(theme: [$first, $second]); + + expect($finder->read('house/theme.yml', 'theme'))->toBeNull() + ->and($finder->path('house/theme.yml', 'theme'))->toBeNull() + // The listing is of the directory the name resolves to, so it is empty rather + // than the files of the theme that was shadowed. + ->and($finder->files('house'))->toBe([]); +}); + +it('finds nothing for a role it has no bases for', function () { + // The role says which world the name is resolved in, and a world the finder was + // not given is not one it can answer for. + $base = Scratch::directoryPath('somewhere'); + mkdir($base . '/house'); + file_put_contents($base . '/house/theme.yml', "name: House\n"); + + $finder = new DiskFinder(theme: [$base]); + + expect($finder->read('logo.png', 'picture'))->toBeNull() + // The theme is there for the role the finder was given bases for. + ->and($finder->read('house/theme.yml', 'theme'))->toBe("name: House\n"); +}); + +it('looks in every base for a name with no role', function () { + $themes = Scratch::directoryPath('worlds'); + $pictures = Scratch::directoryPath('picturesofthings'); + + file_put_contents($pictures . '/logo.png', 'the bytes'); + + $finder = new DiskFinder(theme: [$themes], picture: [$pictures]); + + expect($finder->read('logo.png'))->toBe('the bytes'); +}); + +it('answers with the path of a file outside every base, rather than refusing it', function () { + // The scope belongs to the render and not to the storage: the library holds every + // path a finder answers to its own roots before it reads it. A finder that refused + // here would throw this half's exception, which the other half would not catch. + $outside = Scratch::directoryPath('outside'); + file_put_contents($outside . '/secret.txt', 'not yours'); + + $finder = new DiskFinder(document: [Scratch::directoryPath('inside')]); + + expect($finder->path($outside . '/secret.txt', 'document'))->toBe($outside . '/secret.txt') + ->and($finder->read($outside . '/secret.txt', 'document'))->toBe('not yours'); +}); + +it('does not throw for a name it cannot answer', function () { + // A name that walks out of the base, a name that is a URL, a name holding a null + // byte: none of them is an error. They are names that resolve to nothing, and the + // finder answers null rather than throwing. + $base = finderBase('logo.png'); + + $finder = new DiskFinder(theme: [$base], picture: [$base], document: [$base]); + + expect($finder->read('../elsewhere/secret.txt', 'document'))->toBeNull() + ->and($finder->read('phar://payload.phar/theme.yml', 'theme'))->toBeNull() + ->and($finder->read("logo.png\0.txt", 'picture'))->toBeNull() + ->and($finder->path('../elsewhere/secret.txt', 'document'))->toBeNull(); +}); From 035a7fd94d17bca5e6c27250a6dd84cd403ae68f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabian=20Gra=C3=9Fl?= Date: Thu, 29 Oct 2026 23:06:21 +0100 Subject: [PATCH 3/8] The theme is read through the finder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The theme loader and the resolver read every file of a theme themselves, and the only way in was a path: a source that keeps its themes in a database cannot serve a deck at all. They read through the finder now. The library composes the names — `house/theme.yml`, `house/styles.yml`, `house/layouts.yml`, `house/text-styles.yml` and `house/template.pptx` — and asks, and the finder answers. The scope check stays with the library, where the messages are: each of a theme's own files is resolved and held to the scope before it is read, so a `styles.yml` that is a symlink out of the scope is still refused in this library's own words. The finder answers and never refuses; the library is what knows what a render may read, and a theme that resolves out of the roots is refused by the resolver in the words it already had. `ThemeLoader::load()` and `loadManifest()` take a name and a finder where they took a directory and a scope, and the loader holds the theme's own files to the scope it was given. `ThemeResolver` takes the source of themes as a trailing argument, asks `FileFinder::themes()` for the names that resolve, and keeps the six steps: the directories of themes are the bases the finder is given, so a theme directory without a manifest still shadows a later one. The master is asked for by name like the rest of the theme's files and is still only a path for the renderer to open. The refusal for a master the source cannot serve, the picture reader and the templates file are the next stage. --- CHANGELOG.md | 21 +++ README.md | 16 +- src/Theme/ThemeLoader.php | 267 +++++++++++++++++++------------ src/Theme/ThemeResolver.php | 235 +++++++++++++++++++-------- tests/Support/MemoryFinder.php | 96 +++++++++++ tests/Support/ThemeDirectory.php | 51 ++++++ tests/Unit/theme-finder.php | 181 +++++++++++++++++++++ tests/Unit/theme-loader.php | 115 +++++++------ tests/Unit/theme-text-styles.php | 61 +++---- tests/Unit/theme.php | 11 +- 10 files changed, 784 insertions(+), 270 deletions(-) create mode 100644 tests/Support/MemoryFinder.php create mode 100644 tests/Unit/theme-finder.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 654aac8..f3d7b6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -107,6 +107,27 @@ All notable changes to this project are documented here. The format follows ### Changed +- **`ThemeLoader::load()` and `loadManifest()` take a name and a finder.** The third + parameter was the scope a theme's own files were held to and is now the finder they + are read through; the first was a directory and is now the name the theme is known + by. The library composes the names — `house/theme.yml`, `house/styles.yml`, + `house/layouts.yml`, `house/text-styles.yml` and `house/template.pptx` — and asks + the finder for them, so a theme that never had a directory is read the same way as + one that did. + + The scope check stays where it was, with the messages it had: each of a theme's own + files is resolved and held to the scope before it is read, so a `styles.yml` that is + a symlink out of the scope is still refused in this library's own words. The scope + is the loader's own now — `new ThemeLoader($scope)` — and `ThemeResolver` builds a + loader with its own scope unless it is given one. + +- **`ThemeResolver` takes the source of themes.** A `FileFinder` given to it is where + the themes come from: a theme the source holds resolves the same way as one on disk, + and it is refused in the same words when it resolves out of the roots. The + directories on disk remain the fallback, and the themes that resolve are listed from + the finder and from them together — so a failure still names every theme that would + have worked. + - **The inline translator is told which slide and which theme it reads for.** `runs()`, `runsIn()` and `plain()` each take an `$slide` and a `$theme` now: a word in a text style is drawn in the face and the colour the theme names, and a mistake in one is a mistake on diff --git a/README.md b/README.md index 52e73fb..d8061a3 100644 --- a/README.md +++ b/README.md @@ -2461,9 +2461,16 @@ else can. Here a theme whose template maps only two roles: ```php use MarkdownPresentation\Exception\UnknownLayoutRole; +use MarkdownPresentation\Files\DiskFinder; use MarkdownPresentation\Theme\ThemeLoader; -$theme = (new ThemeLoader())->load((string) getenv('MARKDOWN_PRESENTATION_ROOT') . '/tests/fixtures/themes/sparse'); +$root = (string) getenv('MARKDOWN_PRESENTATION_ROOT'); + +$theme = (new ThemeLoader())->load( + 'sparse', + 'sparse', + new DiskFinder(theme: [$root . '/tests/fixtures/themes']), +); foreach (['three-column', 'section'] as $role) { try { @@ -2554,12 +2561,17 @@ of the styling the theme asked for: ```php use MarkdownPresentation\Exception\UnknownStyleSlot; +use MarkdownPresentation\Files\DiskFinder; use MarkdownPresentation\Theme\ThemeLoader; $root = (string) getenv('MARKDOWN_PRESENTATION_ROOT'); try { - (new ThemeLoader())->load($root . '/tests/fixtures/themes/broken'); + (new ThemeLoader())->load( + 'broken', + 'broken', + new DiskFinder(theme: [$root . '/tests/fixtures/themes']), + ); } catch (UnknownStyleSlot $failure) { echo $failure->getMessage(), "\n"; } diff --git a/src/Theme/ThemeLoader.php b/src/Theme/ThemeLoader.php index 66b4868..e6b7ed6 100644 --- a/src/Theme/ThemeLoader.php +++ b/src/Theme/ThemeLoader.php @@ -9,24 +9,29 @@ use MarkdownPresentation\Exception\ThemeNotFound; use MarkdownPresentation\Exception\UnknownLayoutRole; use MarkdownPresentation\Exception\UnknownStyleSlot; +use MarkdownPresentation\Files\FileFinder; use MarkdownPresentation\Filesystem\PathScope; use Symfony\Component\Yaml\Exception\ParseException; use Symfony\Component\Yaml\Yaml; /** - * Reads a theme out of a directory, or out of the manifest that describes one. + * Reads a theme out of a finder: the `theme.yml` and the files beside it. * * Only `theme.yml` is required. `styles.yml`, `layouts.yml` and `text-styles.yml` are * read when they are there, and `template.pptx` is only a path: nothing here opens a * `.pptx`, because a theme's template belongs to whoever renders the deck rather than to * the loader that found it. * - * Every file of a theme is resolved and held to the caller's {@see PathScope} - * before it is read, and the template is held before it is named. Holding the - * directory is not enough: a `theme.yml` that is a symlink pointing out of the scope - * is read as readily as one inside it, and the `author:` it carries is copied into - * `docProps/core.xml` of the deck — a way to get the contents of a file out through - * a download. + * The finder is asked by name — `house/theme.yml`, `house/styles.yml` — because the + * library composes names and a finder answers them. A theme that never had a directory + * is read the same way as one that did. + * + * Each of the theme's own files is resolved and held to the scope the loader was given + * before it is read, and the scope is asked of the file rather than of the directory it + * was found in: a `styles.yml` that is a symlink pointing out of the scope is read just + * as readily as one inside it, and the `author:` the manifest carries is copied into + * `docProps/core.xml` of the deck — a way to get the contents of a file out through a + * download. */ final class ThemeLoader { @@ -43,111 +48,175 @@ final class ThemeLoader private const MINIMUM_POINTS = 0.5; /** - * @param string $directory The directory the theme is read from. - * @param string|null $name What the deck wrote, which is what a failure names. - * @param PathScope|null $files The directories a theme's own files may resolve into. - * Null takes the files inside a resolved directory on trust, which is what a - * theme this library ships is. + * @param PathScope|null $scope The directories a theme's own files may resolve into. + * Null takes the files the finder answers on trust, which is what a theme + * this library ships is. + */ + public function __construct(private readonly ?PathScope $scope = null) + { + } + + /** + * A theme read by the name it is known by. * - * @throws ThemeNotFound When the directory has no `theme.yml`, so it is not a theme. - * @throws MalformedTheme When a file in it does not describe a theme. - * @throws Exception When a file in it cannot be read or parsed, or resolves out of - * the scope. + * @param string $name The theme's name, which the finder resolves the theme's files by. + * @param string|null $written What the deck wrote, which is what a failure names. + * @param FileFinder|null $files Where the theme's files come from. + * + * @throws ThemeNotFound when the theme has no `theme.yml`, so it is not a theme + * @throws MalformedTheme when a file in it does not describe a theme + * @throws UnknownStyleSlot when `styles.yml` names a slot that does not exist + * @throws UnknownLayoutRole when `layouts.yml` names a role that does not exist + * @throws Exception when a file in it cannot be read or parsed, or resolves out + * of the scope */ - public function load(string $directory, ?string $name = null, ?PathScope $files = null): LoadedTheme + public function load(string $name, ?string $written = null, ?FileFinder $files = null): LoadedTheme { - $written = $name ?? $directory; - $manifest = $this->contained($files, $directory . '/' . self::THEME_FILE, $written); + $written ??= $name; + + $manifest = $this->file($files, $name, self::THEME_FILE, $written); - if (!is_file($manifest)) { - throw ThemeNotFound::noManifest($directory); + if ($manifest === null) { + throw ThemeNotFound::noManifest($written); } - return $this->read($manifest, $directory, $files, $written); + return $this->read($manifest, $name, $written, $files); } /** - * Reads a theme out of the manifest that describes one, the rest of the theme read - * from the directory holding it. + * A theme whose manifest is a file of its own, with the rest of it read from beside + * the manifest. * - * The name the deck wrote is what makes the file the manifest, not the name the file - * carries once it has been resolved: a `theme.yml` that is a link to a file of another - * name is the manifest the deck asked for, and the file it reaches is read. The scope - * still applies to that file, so a manifest reaching out of the roots is refused - * whatever the deck called it. + * The name the deck wrote is what makes the file the manifest rather than one of the + * theme's other files: a `styles.yml` named as the theme is a file the theme's own + * directory holds, not the theme. * * The Word half's loader asks the same question the same way, and the two have to * stay in step: a deck's theme resolves the same way whichever half renders it. * - * @param string $manifest The manifest itself. - * @param string|null $name What the deck wrote, which is what a failure names. - * @param PathScope|null $files The directories a theme's own files may resolve into. + * @param string $name The theme's name, which the finder resolves the manifest by. + * @param string|null $written What the deck wrote, which is what a failure names. + * @param FileFinder|null $files Where the theme's files come from. * - * @throws ThemeNotFound When the file is not a theme manifest. - * @throws MalformedTheme When the file does not describe a theme. - * @throws Exception When a file of the theme cannot be read or parsed, or resolves - * out of the scope. + * @throws ThemeNotFound when the file is not there, or is not a `theme.yml` and so + * is not a manifest + * @throws MalformedTheme when the file does not describe a theme + * @throws UnknownStyleSlot when `styles.yml` names a slot that does not exist + * @throws UnknownLayoutRole when `layouts.yml` names a role that does not exist + * @throws Exception when a file in it cannot be read or parsed, or resolves out + * of the scope */ - public function loadManifest(string $manifest, ?string $name = null, ?PathScope $files = null): LoadedTheme + public function loadManifest(string $name, ?string $written = null, ?FileFinder $files = null): LoadedTheme { - $written = $name ?? $manifest; + $written ??= $name; if (basename($written) !== self::THEME_FILE) { throw ThemeNotFound::noManifest($written); } - $resolved = $this->contained($files, $manifest, $written); + $manifest = $this->file($files, $name, self::THEME_FILE, $written); - if (!is_file($resolved)) { + if ($manifest === null) { throw ThemeNotFound::noManifest($written); } - // The directory the manifest is in, rather than the one the deck wrote: a - // manifest reached through a symlink sits where its target is, and that is - // where the `styles.yml` that belongs to it is. - return $this->read($resolved, dirname($resolved), $files, $written); + return $this->read($manifest, $name, $written, $files); } /** * The theme a manifest describes, with the rest of it read from beside the manifest. * + * @param array{contents: string, path: string} $manifest + * * @throws MalformedTheme - * @throws Exception * @throws ThemeNotFound + * @throws UnknownLayoutRole + * @throws UnknownStyleSlot + * @throws Exception */ - private function read(string $manifest, string $directory, ?PathScope $files, string $name): LoadedTheme + private function read(array $manifest, string $name, string $written, ?FileFinder $files): LoadedTheme { return new LoadedTheme( - $directory, + dirname($manifest['path']), Theme::fromArray( $this->mapping($manifest, 'a theme'), - $manifest, - $this->textStyles($directory, $files, $name), + $manifest['path'], + $this->textStyles($name, $written, $files), ), - $this->styles($directory, $files, $name), - $this->layouts($directory, $files, $name), - $this->template($directory, $files, $name), + $this->styles($name, $written, $files), + $this->layouts($name, $written, $files), + $this->template($name, $written, $files), ); } /** - * A file of the theme, refused when it resolves outside the scope. + * One of the theme's own files, resolved and held to the scope. + * + * A finder with the bytes and no path is asked for them: the caller vouches for its + * own files, and there is no path to hold to the scope. A loader with no finder has + * no source at all, and the file is not there. + * + * @return array{contents: string, path: string}|null + * + * @throws Exception when the file resolves outside the directories this render may read + */ + private function file(?FileFinder $files, string $theme, string $file, string $written): ?array + { + $name = $theme . '/' . $file; + $path = $files?->path($name, 'theme'); + + if ($path === null) { + $contents = $files?->read($name, 'theme'); + + return $contents === null ? null : ['contents' => $contents, 'path' => $name]; + } + + $real = $this->contained($path, $written); + + if ($real === null) { + return null; + } + + // `is_readable()` first, so a file that cannot be read is reported as that rather + // than as a warning the test runner turns into a failure of its own. + $contents = is_readable($real) ? @file_get_contents($real) : false; + + if ($contents === false) { + throw new Exception(sprintf('Unable to read "%s".', $real)); + } + + return ['contents' => $contents, 'path' => $real]; + } + + /** + * A path held to the scope, and the file it names. + * + * The path is resolved before it is held, because a prefix is not containment: `..` + * and a symlink both defeat one, and a symlink pointing outside the roots is the + * ordinary way a path that looks harmless turns into a read of the wrong file. * - * The path is returned as it stands when there is nothing to check, so a theme with - * no `styles.yml` costs no more than the one `file_exists()` call. + * Without a scope there is nothing to hold the path to, and it is taken as the + * finder answered it: a theme this library ships is read on trust, and so is one a + * caller named a directory of its own. * - * @throws ThemeNotFound When the file is there but is somewhere this render may not read. + * @return string|null The path of the file, or null when there is none. + * + * @throws ThemeNotFound when the file is somewhere this render may not read */ - private function contained(?PathScope $files, string $path, string $name): string + private function contained(string $path, string $written): ?string { - if ($files === null || !file_exists($path)) { - return $path; + if ($this->scope === null) { + return file_exists($path) ? $path : null; } $real = PathScope::resolve($path); - if ($real === null || !$files->holds($real)) { - throw ThemeNotFound::outside($name, $real ?? $path, $files->roots()); + if ($real === null) { + return null; + } + + if (!$this->scope->holds($real)) { + throw ThemeNotFound::outside($written, $real, $this->scope->roots()); } return $real; @@ -156,37 +225,35 @@ private function contained(?PathScope $files, string $path, string $name): strin /** * The theme's template, held to the scope before it is named. * - * The path is what the renderer opens, and it opens whatever it is given: a - * template that is a symlink out of the scope is a file outside it, read under a - * name that is inside. + * The path is what the renderer opens, and it opens whatever it is given: a template + * that is a symlink out of the scope is a file outside it, read under a name that is + * inside. * * @throws ThemeNotFound */ - private function template(string $directory, ?PathScope $files, string $name): ?string + private function template(string $name, string $written, ?FileFinder $files): ?string { - $file = $this->contained($files, $directory . '/' . self::TEMPLATE_FILE, $name); + $path = $files?->path($name . '/' . self::TEMPLATE_FILE, 'theme'); + + if ($path === null) { + return null; + } - return is_file($file) ? $file : null; + return $this->contained($path, $written); } /** + * @param array{contents: string, path: string} $file + * * @return array */ - private function mapping(string $file, string $wanted): array + private function mapping(array $file, string $wanted): array { - if (!is_readable($file)) { - throw new Exception(sprintf('Unable to read "%s".', $file)); - } - try { - // `parseFile()` rather than reading the file and handing the bytes over: - // it checks that the file is there and readable itself, so a directory - // that lost a file between two calls fails as a read rather than as YAML - // nobody can parse. - $parsed = Yaml::parseFile($file); + $parsed = Yaml::parse($file['contents']); } catch (ParseException $exception) { throw new Exception( - sprintf('"%s" is not valid YAML: %s', $file, $exception->getMessage()), + sprintf('"%s" is not valid YAML: %s', $file['path'], $exception->getMessage()), previous: $exception, ); } @@ -196,7 +263,7 @@ private function mapping(string $file, string $wanted): array } if (!is_array($parsed) || ($parsed !== [] && array_is_list($parsed))) { - throw MalformedTheme::notAMapping($file, $wanted, self::describe($parsed)); + throw MalformedTheme::notAMapping($file['path'], $wanted, self::describe($parsed)); } /** @var array $parsed */ @@ -218,11 +285,11 @@ private static function describe(mixed $parsed): string * @throws ThemeNotFound * @throws Exception */ - private function styles(string $directory, ?PathScope $files, string $name): array + private function styles(string $name, string $written, ?FileFinder $files): array { - $file = $this->contained($files, $directory . '/' . self::STYLES_FILE, $name); + $file = $this->file($files, $name, self::STYLES_FILE, $written); - if (!is_file($file)) { + if ($file === null) { return []; } @@ -232,7 +299,7 @@ private function styles(string $directory, ?PathScope $files, string $name): arr $slot = (string) $slot; if (!in_array($slot, StyleSlots::names(), true)) { - throw UnknownStyleSlot::named($file, $slot, StyleSlots::names()); + throw UnknownStyleSlot::named($file['path'], $slot, StyleSlots::names()); } $styles[$slot] = $style; @@ -242,20 +309,20 @@ private function styles(string $directory, ?PathScope $files, string $name): arr } /** - * The role mappings, every role checked against the portable roles. The - * mappings may stand on their own or under a `layouts:` key, because the concept - * draft writes the second and a bare mapping is what a person types. + * The role mappings, every role checked against the portable roles. The mappings may + * stand on their own or under a `layouts:` key, because the concept draft writes the + * second and a bare mapping is what a person types. * * @return array * * @throws ThemeNotFound * @throws Exception */ - private function layouts(string $directory, ?PathScope $files, string $name): array + private function layouts(string $name, string $written, ?FileFinder $files): array { - $file = $this->contained($files, $directory . '/' . self::LAYOUTS_FILE, $name); + $file = $this->file($files, $name, self::LAYOUTS_FILE, $written); - if (!is_file($file)) { + if ($file === null) { return []; } @@ -264,7 +331,7 @@ private function layouts(string $directory, ?PathScope $files, string $name): ar if (isset($mapping[self::LAYOUTS_KEY])) { foreach (array_keys($mapping) as $key) { if ((string) $key !== self::LAYOUTS_KEY) { - throw UnknownLayoutRole::unknownKey($file, (string) $key, [self::LAYOUTS_KEY]); + throw UnknownLayoutRole::unknownKey($file['path'], (string) $key, [self::LAYOUTS_KEY]); } } @@ -272,7 +339,7 @@ private function layouts(string $directory, ?PathScope $files, string $name): ar } if (!is_array($mapping)) { - throw UnknownLayoutRole::notTheMappings($file); + throw UnknownLayoutRole::notTheMappings($file['path']); } $layouts = []; @@ -285,7 +352,7 @@ private function layouts(string $directory, ?PathScope $files, string $name): ar } if (!is_string($layout) || trim($layout) === '') { - throw UnknownLayoutRole::notALayoutName($file, $role); + throw UnknownLayoutRole::notALayoutName($file['path'], $role); } $layouts[$role] = trim($layout); @@ -308,11 +375,11 @@ private function layouts(string $directory, ?PathScope $files, string $name): ar * @throws Exception * @throws MalformedTheme */ - private function textStyles(string $directory, ?PathScope $files, string $name): TextStyles + private function textStyles(string $name, string $written, ?FileFinder $files): TextStyles { - $file = $this->contained($files, $directory . '/' . self::TEXT_STYLES_FILE, $name); + $file = $this->file($files, $name, self::TEXT_STYLES_FILE, $written); - if (!is_file($file)) { + if ($file === null) { return new TextStyles(); } @@ -322,21 +389,21 @@ private function textStyles(string $directory, ?PathScope $files, string $name): $style = (string) $style; if (TextStyle::isReserved($style)) { - throw MalformedTheme::reservedStyle($file, $style); + throw MalformedTheme::reservedStyle($file['path'], $style); } if (!is_array($properties) || ($properties !== [] && array_is_list($properties))) { - throw MalformedTheme::wrongType($file, $style, 'a mapping of attributes to values'); + throw MalformedTheme::wrongType($file['path'], $style, 'a mapping of attributes to values'); } if ($properties === []) { - throw MalformedTheme::emptyStyle($file, $style); + throw MalformedTheme::emptyStyle($file['path'], $style); } - $styles[$style] = $this->textStyle($file, $style, $properties); + $styles[$style] = $this->textStyle($file['path'], $style, $properties); } - return new TextStyles($styles, $file); + return new TextStyles($styles, $file['path']); } /** diff --git a/src/Theme/ThemeResolver.php b/src/Theme/ThemeResolver.php index 77a29bc..f1e6ec2 100644 --- a/src/Theme/ThemeResolver.php +++ b/src/Theme/ThemeResolver.php @@ -5,6 +5,8 @@ namespace MarkdownPresentation\Theme; use MarkdownPresentation\Exception\ThemeNotFound; +use MarkdownPresentation\Files\DiskFinder; +use MarkdownPresentation\Files\FileFinder; use MarkdownPresentation\Filesystem\PathScope; /** @@ -48,9 +50,15 @@ * * The files *inside* a theme are held to the same scope, because a directory that is * inside it may hold a `theme.yml` that is a link out of it: the path the deck wrote - * is inside the scope and the file it reaches is not, which is the pair a check on - * the path as written cannot tell. The loader holds each file it reads, and the - * template it names, and refuses the one that resolves elsewhere. + * is inside the scope and the file it reaches is not, which is the pair a check on the + * path as written cannot tell. The loader holds each file it reads, and the template it + * names, and refuses the one that resolves elsewhere. + * + * The themes themselves are read through a finder, which is given the directories of + * themes above: the finder resolves a name against them in the order they are given, + * and a theme directory that holds no manifest still shadows a later one. A finder the + * calling program brought is the source of themes in its own right, and the directories + * on disk remain the fallback rather than the only place a theme is looked for. */ final class ThemeResolver { @@ -64,7 +72,15 @@ final class ThemeResolver */ private readonly array $roots; - private readonly PathScope $files; + private readonly PathScope $scope; + + private readonly ThemeLoader $loader; + + /** The loader the calling program brought, if it brought one. */ + private readonly ?ThemeLoader $given; + + /** The source of themes the calling program brought, if it brought one. */ + private readonly ?FileFinder $files; /** * @param list $roots Further directories a deck may name a theme in, for a @@ -80,19 +96,29 @@ final class ThemeResolver * path is written from. Null for a deck handed in * as a string: there is no file beside it, so a * theme beside it is not there to be found. + * @param ThemeLoader|null $loader The loader the themes are read through. A loader + * given here brings its own scope; the scope of this + * resolver still holds the theme's own directory. + * @param FileFinder|null $files The source of themes, for a program whose themes are + * not in a directory. The directories of themes on disk + * remain the fallback. */ public function __construct( private readonly string $workingDirectory = '.', ?string $builtinDirectory = null, - private readonly ThemeLoader $loader = new ThemeLoader(), + ?ThemeLoader $loader = null, array $roots = [], private readonly ?string $deckDirectory = null, + ?FileFinder $files = null, ) { + $this->roots = $roots; // The themes that ship beside `src/`, so a package installed anywhere // resolves its own themes without being told where they are. $this->builtinDirectory = $builtinDirectory ?? dirname(__DIR__, 2) . self::THEMES; - $this->roots = $roots; - $this->files = new PathScope($this->allowed()); + $this->scope = new PathScope($this->allowed()); + $this->given = $loader; + $this->loader = $loader ?? new ThemeLoader($this->scope); + $this->files = $files; } /** @@ -110,9 +136,10 @@ public function beside(string $directory): self return new self( $this->workingDirectory, $this->builtinDirectory, - $this->loader, + $this->given, $this->roots, $directory, + $this->files, ); } @@ -128,21 +155,63 @@ public function resolve(string $name): LoadedTheme if (str_contains($name, '/')) { // Where a name is written *from* says nothing about whether it is allowed: what is // allowed is where it lands, which is the same question an image path asks. - return $this->path( - $name, - str_starts_with($name, '/') ? $name : $this->base() . '/' . $name, - ); + $path = str_starts_with($name, '/') ? $name : $this->base() . '/' . $name; + + return is_file($path) + ? $this->themeOfManifest($this->held($name, $path), $name) + : $this->themeIn($this->directory($name, $path), $name); + } + + $theme = $this->throughFinder($name); + + if ($theme !== null) { + return $theme; } foreach ($this->themeDirectories() as $themes) { if (is_dir($directory = $themes . '/' . $name)) { - return $this->loader->load($this->held($name, $directory), $name, $this->files); + return $this->themeIn($this->directory($name, $directory), $name); } } throw ThemeNotFound::named($name, $this->available()); } + /** + * A theme in the source the calling program brought, or null when it has none. + * + * The manifest is asked for by name — `house/theme.yml` — because the library + * composes names and a finder answers them. A finder that answers a path has it + * held to the scope, exactly as a theme found on disk has; one that answers the + * bytes has the caller's word for them. + * + * Whether the theme is there at all is answered by the listing of its files, and + * not by asking for the manifest: a theme whose files are there but whose manifest + * is not is a different failure from a theme that is not there, and the listing + * tells the two apart without reading anything. + */ + private function throughFinder(string $name): ?LoadedTheme + { + $finder = $this->finder(); + + $manifest = $finder->path($name . '/' . ThemeLoader::THEME_FILE, 'theme'); + + if ($manifest !== null) { + // The theme's own directory is held to the scope, as it is when the theme was + // found on disk: a `theme:` is a path a deck writes, and what is allowed is + // where it lands. + $this->held($name, dirname($manifest)); + + return $this->loader->load($name, $name, $finder); + } + + if ($finder->files($name) === []) { + return null; + } + + return $this->loader->load($name, $name, $finder); + } + /** * The directory a relative path is written from. * @@ -156,28 +225,71 @@ private function base(): string } /** - * The theme an explicit path names: a directory holding a theme, or the manifest itself. + * The directories a name is looked for in, in the order they are tried. * - * @param string $name What the deck wrote. - * @param string $found The path it resolved to before it was resolved further. + * One list because a name has to resolve the same way whether it is being looked up + * or enumerated: a name the resolver would accept and not list is a name a failure + * message leaves the reader to guess at. * - * @throws ThemeNotFound + * @return list */ - private function path(string $name, string $found): LoadedTheme + private function themeDirectories(): array { - if (is_file($found)) { - return $this->loader->loadManifest($this->held($name, $found), $name, $this->files); - } + return [ + ...($this->deckDirectory === null ? [] : [$this->deckDirectory . self::THEMES]), + $this->workingDirectory . self::THEMES, + ...$this->roots, + $this->builtinDirectory, + ]; + } - if (!is_dir($found)) { - throw ThemeNotFound::noDirectory($found); - } + /** + * The finder the themes are read through: the source the calling program brought, + * or the disk default. + * + * The default holds the directories of themes the six steps name, in the order they + * are tried, so a theme is resolved by its directory and a theme directory that holds + * no manifest still shadows a later one. + */ + private function finder(): FileFinder + { + return $this->files ?? new DiskFinder(theme: $this->themeDirectories()); + } + + /** + * The theme in a directory, read through a finder that resolves it there. + * + * The finder is given the directory that holds the theme, so the name it resolves is + * the theme this resolver found and not another theme that shares its name. + */ + private function themeIn(string $directory, string $written): LoadedTheme + { + return $this->loader->load( + basename($directory), + $written, + new DiskFinder(theme: [dirname($directory)]), + ); + } - return $this->loader->load($this->held($name, $found), $name, $this->files); + /** + * The theme a manifest is, read through a finder that resolves it there. + * + * The manifest's own directory, so the files beside it are the ones the manifest was + * written next to. + */ + private function themeOfManifest(string $file, string $written): LoadedTheme + { + $directory = dirname($file); + + return $this->loader->loadManifest( + basename($directory), + $written, + new DiskFinder(theme: [dirname($directory)]), + ); } /** - * A path resolved and held to the directories this resolver may read. + * The directory a step of the order above found, resolved and held to the roots. * * Two messages rather than one, because "there is nothing there" and "it is there and * this render may not read it" are different problems with different fixes: the first is @@ -188,34 +300,32 @@ private function path(string $name, string $found): LoadedTheme * * @throws ThemeNotFound */ - private function held(string $name, string $found): string + private function directory(string $name, string $found): string { - $real = PathScope::resolve($found); - - if ($real === null || !$this->files->holds($real)) { - throw ThemeNotFound::outside($name, $real ?? $found, $this->files->roots()); + if (!is_dir($found)) { + throw ThemeNotFound::noDirectory($found); } - return $real; + return $this->held($name, $found); } /** - * The directories a name is looked for in, in the order they are tried. + * A path resolved and held to the directories this resolver may read. * - * One list because a name has to resolve the same way whether it is being looked up - * or enumerated: a name the resolver would accept and not list is a name a failure - * message leaves the reader to guess at. + * @param string $name What the deck wrote. + * @param string $found The path it resolved to before it was resolved further. * - * @return list + * @throws ThemeNotFound */ - private function themeDirectories(): array + private function held(string $name, string $found): string { - return [ - ...($this->deckDirectory === null ? [] : [$this->deckDirectory . self::THEMES]), - $this->workingDirectory . self::THEMES, - ...$this->roots, - $this->builtinDirectory, - ]; + $real = PathScope::resolve($found); + + if ($real === null || !$this->scope->holds($real)) { + throw ThemeNotFound::outside($name, $real ?? $found, $this->scope->roots()); + } + + return $real; } /** @@ -240,32 +350,31 @@ private function allowed(): array * reader of the list cannot tell which, so listing one kind ahead of the other would * say something untrue. * + * The source the calling program brought and the directories on disk are listed + * together, because a name that resolves is a name that is offered: a program with + * themes of its own does not lose the ones that ship with this library. + * * @return list */ public function available(): array { - $names = []; - - foreach ($this->themeDirectories() as $themes) { - if (!is_dir($themes)) { - continue; - } + $names = [...$this->finder()->themes(), ...$this->diskThemes()]; - foreach (scandir($themes) ?: [] as $entry) { - if (str_starts_with($entry, '.')) { - continue; - } - - if (is_file($themes . '/' . $entry . '/' . ThemeLoader::THEME_FILE)) { - $names[$entry] = true; - } - } - } + $unique = array_values(array_unique($names)); - $found = array_keys($names); + sort($unique); - sort($found); + return $unique; + } - return $found; + /** + * The themes in the directories on disk, which are the fallback for a name the source + * the calling program brought does not have. + * + * @return list + */ + private function diskThemes(): array + { + return (new DiskFinder(theme: $this->themeDirectories()))->themes(); } } diff --git a/tests/Support/MemoryFinder.php b/tests/Support/MemoryFinder.php new file mode 100644 index 0000000..db017da --- /dev/null +++ b/tests/Support/MemoryFinder.php @@ -0,0 +1,96 @@ + */ + private array $asked = []; + + /** @var array */ + private array $reads = []; + + /** + * @param array $files The files it holds, by name. + */ + public function __construct(private readonly array $files = []) + { + } + + public function read(string $name, ?string $role = null): ?string + { + $this->asked[$name] = true; + $this->reads[$name] = ($this->reads[$name] ?? 0) + 1; + + return $this->files[$name] ?? null; + } + + public function path(string $name, ?string $role = null): ?string + { + $this->asked[$name] = true; + + return null; + } + + public function themes(): array + { + $names = []; + + foreach (array_keys($this->files) as $name) { + $names[explode('/', $name, 2)[0]] = true; + } + + $found = array_keys($names); + + sort($found); + + return $found; + } + + public function files(string $theme): array + { + $names = []; + + foreach (array_keys($this->files) as $name) { + if (str_starts_with($name, $theme . '/')) { + $names[substr($name, strlen($theme) + 1)] = true; + } + } + + $found = array_keys($names); + + sort($found); + + return $found; + } + + /** + * The names it was asked for, in the order they were asked for. + * + * @return list + */ + public function asked(): array + { + return array_keys($this->asked); + } + + /** + * How many times the contents of a name were asked for. + */ + public function reads(string $name): int + { + return $this->reads[$name] ?? 0; + } +} diff --git a/tests/Support/ThemeDirectory.php b/tests/Support/ThemeDirectory.php index 0efdf04..7e91598 100644 --- a/tests/Support/ThemeDirectory.php +++ b/tests/Support/ThemeDirectory.php @@ -4,6 +4,10 @@ namespace MarkdownPresentation\Tests\Support; +use MarkdownPresentation\Files\DiskFinder; +use MarkdownPresentation\Theme\LoadedTheme; +use MarkdownPresentation\Theme\ThemeLoader; + /** * A theme directory built for one test. */ @@ -95,4 +99,51 @@ public static function acme(array $beside = [], string $prefix = 'theme'): strin { return self::create(['theme.yml' => self::ACME] + $beside, $prefix); } + + /** + * A theme read out of a directory, the way the loader reads one. + * + * A theme is read by name: the finder is told where the themes are and the loader + * asks it for `/`. The name is the directory's own, and the finder is + * given the directory that holds it. + * + * @param string $directory The theme's own directory. + * @param ThemeLoader|null $loader The loader, which brings its own scope. Null takes + * a loader that trusts the files it is given. + */ + public static function load(string $directory, ?ThemeLoader $loader = null): LoadedTheme + { + return ($loader ?? new ThemeLoader())->load( + basename($directory), + $directory, + self::finder($directory), + ); + } + + /** + * A theme read out of the manifest that describes it, the way the loader reads one. + * + * @param string $directory The theme's own directory. + * @param string|null $written What the deck wrote, which is what a failure names. + * @param ThemeLoader|null $loader The loader, which brings its own scope. + */ + public static function loadManifest( + string $directory, + ?string $written = null, + ?ThemeLoader $loader = null, + ): LoadedTheme { + return ($loader ?? new ThemeLoader())->loadManifest( + basename($directory), + $written ?? $directory . '/' . ThemeLoader::THEME_FILE, + self::finder($directory), + ); + } + + /** + * The finder that resolves the themes in a directory. + */ + public static function finder(string $directory): DiskFinder + { + return new DiskFinder(theme: [dirname($directory)]); + } } diff --git a/tests/Unit/theme-finder.php b/tests/Unit/theme-finder.php new file mode 100644 index 0000000..a19c501 --- /dev/null +++ b/tests/Unit/theme-finder.php @@ -0,0 +1,181 @@ + "name: House\n", + 'house/styles.yml' => "heading.1: ReportTitle\n", + 'house/layouts.yml' => "layouts:\n content: Title and Content\n", + ]); + + $theme = (new ThemeLoader())->load('house', 'house', $finder); + + expect($theme->theme()->name())->toBe('House') + ->and($theme->styles())->toBe(['heading.1' => 'ReportTitle']) + ->and($theme->layouts())->toBe(['content' => 'Title and Content']) + // A finder with no path has no directory, and the theme's name stands in. + ->and($theme->directory())->toBe('house'); +}); + +it('asks the finder for the theme\'s files by name', function () { + // The names are the library's own: a theme's manifest is `/theme.yml` and + // the files beside it are `/styles.yml` and the rest. + $finder = new MemoryFinder([ + 'house/theme.yml' => "name: House\n", + 'house/styles.yml' => "heading.1: ReportTitle\n", + ]); + + (new ThemeLoader())->load('house', 'house', $finder); + + $asked = $finder->asked(); + + sort($asked); + + expect($asked)->toBe([ + 'house/layouts.yml', + 'house/styles.yml', + 'house/template.pptx', + 'house/text-styles.yml', + 'house/theme.yml', + ]); +}); + +it('resolves a theme the source holds and no directory has', function () { + $finder = new MemoryFinder([ + 'house/theme.yml' => "name: House\n", + 'house/styles.yml' => "heading.1: ReportTitle\n", + ]); + + $theme = finderResolver($finder)->resolve('house'); + + expect($theme->theme()->name())->toBe('House') + ->and($theme->styles())->toBe(['heading.1' => 'ReportTitle']); +}); + +it('refuses a theme file that resolves outside the scope, in the library\'s words', function () { + // The finder answers the path; the library holds it to its own scope and refuses + // it. The message is the library's, because the library is what knows what this + // render may read. + $themes = ThemeDirectory::create(['house/theme.yml' => "name: House\n"], 'themes'); + $outside = ThemeDirectory::create(['styles.yml' => "heading.1: ReportTitle\n"], 'outside'); + + ThemeDirectory::link($themes . '/house/styles.yml', $outside . '/styles.yml'); + + $loader = new ThemeLoader(new PathScope([$themes])); + + expect(fn () => $loader->load('house', 'house', new DiskFinder(theme: [$themes]))) + ->toThrow(ThemeNotFound::class, 'is at "' . realpath($outside . '/styles.yml') . '" rather than one this render may read'); +}); + +it('refuses a theme the source answers for outside the roots, in the library\'s words', function () { + // The finder answers the path; the library holds it to its own scope and refuses + // it. A finder that refused would throw this half's exception, which the other + // half of this suite would not catch. + $outside = ThemeDirectory::create(['secrets/theme.yml' => "name: Secrets\n"], 'outside'); + + expect(fn () => finderResolver(new DiskFinder(theme: [$outside]))->resolve('secrets')) + ->toThrow(ThemeNotFound::class, 'rather than one this render may read'); +}); + +it('reports a theme whose file the finder answers for is not there', function () { + // A finder that answers a path for a file that has since gone: the theme is not + // there, and the library says so rather than reading nothing. + $finder = new class implements FileFinder { + public function read(string $name, ?string $role = null): ?string + { + return null; + } + + public function path(string $name, ?string $role = null): ?string + { + return '/mdpresentation-not-here/theme.yml'; + } + + public function themes(): array + { + return []; + } + + public function files(string $theme): array + { + return []; + } + }; + + $loader = new ThemeLoader(new PathScope([ThemeDirectory::create([], 'gone')])); + + expect(fn () => $loader->load('house', 'house', $finder)) + ->toThrow(ThemeNotFound::class, 'is not a theme: it has no theme.yml'); +}); + +it('lets a theme directory without a manifest shadow a later one', function () { + // The theme's directory is found first and the manifest is looked for inside it, so + // a directory that exists but holds no manifest is the theme: the manifest decides + // whether it is one, and the library says so rather than falling through to a theme + // of another name. + $first = ThemeDirectory::create([], 'first'); + $second = ThemeDirectory::create(['house/theme.yml' => "name: House\n"], 'second'); + + mkdir($first . '/house'); + + expect(fn () => finderResolver(roots: [$first, $second])->resolve('house')) + ->toThrow(ThemeNotFound::class, 'is not a theme: it has no theme.yml'); +}); + +it('lists the themes that resolve, so a failure can name them', function () { + $finder = new MemoryFinder([ + 'house/theme.yml' => "name: House\n", + 'zebra/theme.yml' => "name: Zebra\n", + ]); + + expect(finderResolver($finder)->available())->toBe(['house', 'zebra']); +}); + +it('names the themes that exist when a name is not one of them', function () { + $finder = new MemoryFinder([ + 'house/theme.yml' => "name: House\n", + ]); + + expect(fn () => finderResolver($finder)->resolve('huse')) + ->toThrow(ThemeNotFound::class, 'There is no theme named "huse". Available: house.'); +}); diff --git a/tests/Unit/theme-loader.php b/tests/Unit/theme-loader.php index 5d8d597..796fc9b 100644 --- a/tests/Unit/theme-loader.php +++ b/tests/Unit/theme-loader.php @@ -11,10 +11,8 @@ use MarkdownPresentation\Tests\Support\ThemeDirectory; use MarkdownPresentation\Theme\ThemeLoader; -$loader = new ThemeLoader(); - -it('reads a theme from a directory with nothing but a theme.yml in it', function () use ($loader) { - $loaded = $loader->load(ThemeDirectory::minimal('acme')); +it('reads a theme from a directory with nothing but a theme.yml in it', function () { + $loaded = ThemeDirectory::load(ThemeDirectory::minimal('acme')); expect($loaded->theme()->name())->toBe('acme') ->and($loaded->directory())->toEndWith('/' . basename($loaded->directory())) @@ -23,8 +21,8 @@ ->and($loaded->template())->toBeNull(); }); -it('reads the style slots and the layout roles beside a theme.yml', function () use ($loader) { - $loaded = $loader->load(ThemeDirectory::acme([ +it('reads the style slots and the layout roles beside a theme.yml', function () { + $loaded = ThemeDirectory::load(ThemeDirectory::acme([ 'styles.yml' => "heading.1: ReportTitle\nparagraph: BodyText\ncodeFont:\n name: Fira Code\n size: 10\n", 'layouts.yml' => "layouts:\n title: Title Slide\n two-column: Two Content\n", ])); @@ -37,19 +35,19 @@ ->and($loaded->roles())->toBe(['title', 'two-column']); }); -it('reads a layouts file whose mappings stand on their own', function () use ($loader) { - $loaded = $loader->load(ThemeDirectory::acme([ +it('reads a layouts file whose mappings stand on their own', function () { + $loaded = ThemeDirectory::load(ThemeDirectory::acme([ 'layouts.yml' => "title: Title Slide\nblank: Blank\n", ])); expect($loaded->layout('blank'))->toBe('Blank'); }); -it('says a directory with no theme.yml is not a theme', function () use ($loader) { - $loader->load(ThemeDirectory::create(['layouts.yml' => "title: Title Slide\n"], 'not-a-theme')); +it('says a directory with no theme.yml is not a theme', function () { + ThemeDirectory::load(ThemeDirectory::create(['layouts.yml' => "title: Title Slide\n"], 'not-a-theme')); })->throws(ThemeNotFound::class, 'is not a theme: it has no theme.yml'); -it('reads a theme from the manifest file that describes it', function () use ($loader) { +it('reads a theme from the manifest file that describes it', function () { // The manifest is the theme, and the rest of the theme is read from beside it: a // theme is a directory, and the manifest is the one file that directory cannot do // without. @@ -57,134 +55,136 @@ 'layouts.yml' => "layouts:\n content: Title and Content\n", ]); - $loaded = $loader->loadManifest($directory . '/theme.yml'); + $loaded = ThemeDirectory::loadManifest($directory); expect($loaded->theme()->name())->toBe('Acme') ->and($loaded->directory())->toBe($directory) ->and($loaded->layouts())->toBe(['content' => 'Title and Content']); }); -it('says a file that is not a theme manifest is not one', function () use ($loader) { +it('says a file that is not a theme manifest is not one', function () { $directory = ThemeDirectory::create(['notes.txt' => 'not a theme'], 'not-a-manifest'); - $loader->loadManifest($directory . '/notes.txt'); + ThemeDirectory::loadManifest($directory, 'notes.txt'); })->throws(ThemeNotFound::class, 'is not a theme: it has no theme.yml'); -it('says a manifest file that is not there is not a theme', function () use ($loader) { +it('says a manifest file that is not there is not one', function () { $directory = ThemeDirectory::create([], 'empty'); - $loader->loadManifest($directory . '/theme.yml'); + ThemeDirectory::loadManifest($directory); })->throws(ThemeNotFound::class, 'is not a theme: it has no theme.yml'); -it('hands a style value back as it was written, tokens and all', function () use ($loader) { +it('hands a style value back as it was written, tokens and all', function () { // A `styles.yml` may already reference a theme token. Resolving them is not // written yet, so the value passes through exactly as the file has it rather // than losing the reference. - $loaded = $loader->load(ThemeDirectory::acme([ + $loaded = ThemeDirectory::load(ThemeDirectory::acme([ 'styles.yml' => "linkFont:\n color: \"{accent1}\"\n name: \"{fonts.mono}\"\n", ])); expect($loaded->styles())->toBe(['linkFont' => ['color' => '{accent1}', 'name' => '{fonts.mono}']]); }); -it('refuses a styles.yml slot that is not a style slot, and names the slots that are', function () use ($loader) { +it('refuses a styles.yml slot that is not a style slot, and names the slots that are', function () { // The shipped fixture, so the README can show the same failure from the same // file rather than a directory built for the occasion. - $loader->load(dirname(__DIR__) . '/fixtures/themes/broken'); + $directory = dirname(__DIR__) . '/fixtures/themes/broken'; + + ThemeDirectory::load($directory); })->throws(UnknownStyleSlot::class, '"blockquote" is not a style slot. The slots are: heading.1, heading.2'); -it('refuses a layouts.yml role that is not a role, and names the roles that are', function () use ($loader) { - $loader->load(ThemeDirectory::acme([ +it('refuses a layouts.yml role that is not a role, and names the roles that are', function () { + ThemeDirectory::load(ThemeDirectory::acme([ 'layouts.yml' => "layouts:\n two-columns: Two Content\n", ])); })->throws(UnknownLayoutRole::class, '"two-columns" is not a layout role. The roles are: title, content, two-column, section, blank, image-text, image-title, image-slide'); -it('refuses a key beside the mappings that is not one', function () use ($loader) { - $loader->load(ThemeDirectory::acme([ +it('refuses a key beside the mappings that is not one', function () { + ThemeDirectory::load(ThemeDirectory::acme([ 'layouts.yml' => "layouts:\n title: Title Slide\nlayout: Two Content\n", ])); })->throws(UnknownLayoutRole::class, '"layout" is not a key of a layouts file. Valid keys: layouts'); -it('refuses a layouts key that does not hold the mappings', function () use ($loader) { - $loader->load(ThemeDirectory::acme([ +it('refuses a layouts key that does not hold the mappings', function () { + ThemeDirectory::load(ThemeDirectory::acme([ 'layouts.yml' => "layouts: Two Content\n", ])); })->throws(UnknownLayoutRole::class, '"layouts" is not a mapping of roles to layout names'); -it('refuses a role mapped to nothing', function () use ($loader) { - $loader->load(ThemeDirectory::acme([ +it('refuses a role mapped to nothing', function () { + ThemeDirectory::load(ThemeDirectory::acme([ 'layouts.yml' => "layouts:\n title:\n", ])); })->throws(UnknownLayoutRole::class, 'the "title" role is not a layout name'); -it('refuses a styles file that is not YAML', function () use ($loader) { - $loader->load(ThemeDirectory::acme([ +it('refuses a styles file that is not YAML', function () { + ThemeDirectory::load(ThemeDirectory::acme([ 'styles.yml' => "heading.1: \"unclosed\n", ])); })->throws(Exception::class, 'is not valid YAML'); -it('refuses a styles file that is not a mapping of slots', function () use ($loader) { - $loader->load(ThemeDirectory::acme([ +it('refuses a styles file that is not a mapping of slots', function () { + ThemeDirectory::load(ThemeDirectory::acme([ 'styles.yml' => "- heading.1\n", ])); })->throws(MalformedTheme::class, 'The file does not contain a mapping of style slots to styles'); -it('refuses a layouts file that is not a mapping of roles', function () use ($loader) { - $loader->load(ThemeDirectory::acme([ +it('refuses a layouts file that is not a mapping of roles', function () { + ThemeDirectory::load(ThemeDirectory::acme([ 'layouts.yml' => "- title\n", ])); })->throws(MalformedTheme::class, 'The file does not contain a mapping of layout roles to layout names'); -it('says a file it cannot read is a file it cannot read', function () use ($loader) { +it('says a file it cannot read is a file it cannot read', function () { skipWithoutPermissions('The unreadable file'); $directory = ThemeDirectory::acme(); chmod($directory . '/theme.yml', 0o000); - $loader->load($directory); + ThemeDirectory::load($directory); })->throws(Exception::class, 'Unable to read'); -it('refuses a theme.yml that is not a theme', function () use ($loader) { - $loader->load(ThemeDirectory::create(['theme.yml' => "- one\n- two\n"])); +it('refuses a theme.yml that is not a theme', function () { + ThemeDirectory::load(ThemeDirectory::create(['theme.yml' => "- one\n- two\n"])); })->throws(MalformedTheme::class, 'The file does not contain a theme — it is a list'); -it('refuses a theme.yml that is a bare string rather than a mapping of keys', function () use ($loader) { - $loader->load(ThemeDirectory::create(['theme.yml' => "Corporate Dark\n"])); +it('refuses a theme.yml that is a bare string rather than a mapping of keys', function () { + ThemeDirectory::load(ThemeDirectory::create(['theme.yml' => "Corporate Dark\n"])); })->throws(MalformedTheme::class, 'The file does not contain a theme — it is a string'); -it('reads a file with nothing in it as a file with nothing in it', function () use ($loader) { +it('reads a file with nothing in it as a file with nothing in it', function () { // An empty styles file is a theme that overrides nothing, which is a fact about // the theme rather than a mistake in it. - $loaded = $loader->load(ThemeDirectory::acme([ + $loaded = ThemeDirectory::load(ThemeDirectory::acme([ 'styles.yml' => '', ])); expect($loaded->styles())->toBe([]); }); -it('names a theme that has no layouts.yml when a role cannot be resolved', function () use ($loader) { - $loaded = $loader->load(ThemeDirectory::minimal('acme')); +it('names a theme that has no layouts.yml when a role cannot be resolved', function () { + $loaded = ThemeDirectory::load(ThemeDirectory::minimal('acme')); $loaded->layout('content'); })->throws(UnknownLayoutRole::class, 'The theme "acme" maps no "content" layout. It maps: (none — the theme has no layouts.yml)'); -it('names the roles a theme does have when the one asked for is not one of them', function () use ($loader) { - $loaded = $loader->load(ThemeDirectory::acme([ +it('names the roles a theme does have when the one asked for is not one of them', function () { + $loaded = ThemeDirectory::load(ThemeDirectory::acme([ 'layouts.yml' => "layouts:\n title: Title Slide\n blank: Blank\n", ])); $loaded->layout('section'); })->throws(UnknownLayoutRole::class, 'The theme "Acme" maps no "section" layout. It maps: title, blank'); -it('refuses a layout name that is not a role at all', function () use ($loader) { - $loaded = $loader->load(ThemeDirectory::minimal('acme')); +it('refuses a layout name that is not a role at all', function () { + $loaded = ThemeDirectory::load(ThemeDirectory::minimal('acme')); $loaded->layout('Two Content'); })->throws(UnknownLayoutRole::class, '"Two Content" is not a layout role. The roles are: title, content, two-column, section, blank, image-text, image-title, image-slide'); -it('turns a role into the layout name of the theme\'s own template', function () use ($loader) { - $loaded = $loader->load(ThemeDirectory::acme([ +it('turns a role into the layout name of the theme\'s own template', function () { + $loaded = ThemeDirectory::load(ThemeDirectory::acme([ 'layouts.yml' => "layouts:\n two-column: Two Content\n", 'template.pptx' => 'not really a template', ])); @@ -193,7 +193,7 @@ ->and($loaded->template())->toBe($loaded->directory() . '/template.pptx'); }); -it('refuses a file of a theme that resolves outside the scope', function () use ($loader) { +it('refuses a file of a theme that resolves outside the scope', function () { // The loader's own scope, for a caller that reads a theme without the resolver. // The file is resolved and held, and one that reaches out of the scope is // refused by name. @@ -202,7 +202,7 @@ ThemeDirectory::link($directory . '/styles.yml', $outside . '/styles.yml'); $scope = new PathScope([$directory]); - expect(fn () => $loader->load($directory, 'acme', $scope)) + expect(fn () => ThemeDirectory::load($directory, new ThemeLoader($scope))) ->toThrow(ThemeNotFound::class, 'is at "' . realpath($outside . '/styles.yml') . '" rather than one this render may read'); }); @@ -210,15 +210,14 @@ // The scope is what makes the check; without one the loader reads what it is // given, which is what a theme this library ships is and what a caller naming a // directory of its own is asking for. - $loader = new ThemeLoader(); $outside = ThemeDirectory::create(['styles.yml' => "heading.1: ReportTitle\n"], 'outside'); $directory = ThemeDirectory::acme(); ThemeDirectory::link($directory . '/styles.yml', $outside . '/styles.yml'); - expect($loader->load($directory)->styles())->toBe(['heading.1' => 'ReportTitle']); + expect(ThemeDirectory::load($directory)->styles())->toBe(['heading.1' => 'ReportTitle']); }); -it('reads a manifest the deck named, whatever the file it reached is called', function () use ($loader) { +it('reads a manifest the deck named, whatever the file it reached is called', function () { // The resolver resolves a path before it hands it over, so the loader is given the // file the path reached, whose name is the target's rather than the one the deck // wrote. The name the deck wrote is what makes a file the manifest of a theme, so @@ -226,18 +225,18 @@ $directory = ThemeDirectory::create(['other.yml' => "name: Linked\n"], 'manifest'); ThemeDirectory::link($directory . '/theme.yml', $directory . '/other.yml'); - $loaded = $loader->loadManifest(realpath($directory . '/other.yml'), 'theme.yml', new PathScope([$directory])); + $loaded = ThemeDirectory::loadManifest($directory, 'theme.yml', new ThemeLoader(new PathScope([$directory]))); expect($loaded->theme()->name())->toBe('Linked'); }); -it('refuses a manifest that resolves outside the scope', function () use ($loader) { +it('refuses a manifest that resolves outside the scope', function () { // The scope is about where the file is, not what it is called: a manifest reaching // out of the scope is refused by the scope, whatever the deck called it. $outside = ThemeDirectory::create(['other.yml' => "name: Stolen\n"], 'outside'); $directory = ThemeDirectory::create(['theme.yml' => "name: Acme\n"], 'theme'); ThemeDirectory::link($directory . '/theme.yml', $outside . '/other.yml'); - expect(fn () => $loader->loadManifest($directory . '/theme.yml', 'theme.yml', new PathScope([$directory]))) + expect(fn () => ThemeDirectory::loadManifest($directory, 'theme.yml', new ThemeLoader(new PathScope([$directory])))) ->toThrow(ThemeNotFound::class, 'is at "' . realpath($outside . '/other.yml') . '" rather than one this render may read'); }); diff --git a/tests/Unit/theme-text-styles.php b/tests/Unit/theme-text-styles.php index 4235759..b268cbe 100644 --- a/tests/Unit/theme-text-styles.php +++ b/tests/Unit/theme-text-styles.php @@ -7,7 +7,6 @@ use MarkdownPresentation\Theme\TextStyle; use MarkdownPresentation\Theme\TextStyles; use MarkdownPresentation\Theme\Theme; -use MarkdownPresentation\Theme\ThemeLoader; /** * The theme's named text styles: what a `text-styles.yml` says, and what the loader makes @@ -29,7 +28,7 @@ function themeWithTextStyles(string $styles): string } it('reads the named styles a theme carries', function () { - $loaded = (new ThemeLoader())->load(themeWithTextStyles("brand:\n color: accent1\n size: 20\n")); + $loaded = ThemeDirectory::load(themeWithTextStyles("brand:\n color: accent1\n size: 20\n")); expect($loaded->theme()->textStyles()->names())->toBe(['brand']) ->and($loaded->theme()->textStyles()->get('brand')?->colour)->toBe('accent1') @@ -38,41 +37,31 @@ function themeWithTextStyles(string $styles): string }); it('refuses a text style that sets an attribute that is not one, when the theme is read', function () { - $loader = new ThemeLoader(); - - expect(fn () => $loader->load(themeWithTextStyles("brand:\n colour: accent1\n"))) + expect(fn () => ThemeDirectory::load(themeWithTextStyles("brand:\n colour: accent1\n"))) ->toThrow(MalformedTheme::class, 'which is not an attribute of a text style'); }); it('refuses a text style that styles nothing, when the theme is read', function () { - $loader = new ThemeLoader(); - - expect(fn () => $loader->load(themeWithTextStyles("brand: {}\n"))) + expect(fn () => ThemeDirectory::load(themeWithTextStyles("brand: {}\n"))) ->toThrow(MalformedTheme::class, 'sets nothing'); }); it('refuses a text style named with the word the raw form is written with', function () { - $loader = new ThemeLoader(); - - expect(fn () => $loader->load(themeWithTextStyles("style:\n color: accent1\n"))) + expect(fn () => ThemeDirectory::load(themeWithTextStyles("style:\n color: accent1\n"))) ->toThrow(MalformedTheme::class, 'is named with a reserved word'); }); it('refuses a text style named with the reserved word in another case', function () { // The reserved word is folded with the rest of the names, because `Style` is the same // word as `style`. - $loader = new ThemeLoader(); - - expect(fn () => $loader->load(themeWithTextStyles("Style:\n color: accent1\n"))) + expect(fn () => ThemeDirectory::load(themeWithTextStyles("Style:\n color: accent1\n"))) ->toThrow(MalformedTheme::class, 'is named with a reserved word'); }); it('refuses a theme that names one text style twice, in two spellings', function () { // The names are folded, so `Brand` and `brand` are one name: a theme carrying both has // two styles a deck cannot tell apart, and neither of them is reached. - $loader = new ThemeLoader(); - - expect(fn () => $loader->load(themeWithTextStyles("Brand:\n color: accent1\nbrand:\n color: accent2\n"))) + expect(fn () => ThemeDirectory::load(themeWithTextStyles("Brand:\n color: accent1\nbrand:\n color: accent2\n"))) ->toThrow(MalformedTheme::class, 'is the same style as'); }); @@ -85,7 +74,7 @@ function themeWithTextStyles(string $styles): string }); it('reads a text style the theme spells with a capital letter, under the folded name', function () { - $loaded = (new ThemeLoader())->load(themeWithTextStyles("Brand:\n color: accent1\n")); + $loaded = ThemeDirectory::load(themeWithTextStyles("Brand:\n color: accent1\n")); expect($loaded->theme()->textStyles()->get('brand')?->colour)->toBe('accent1') ->and($loaded->theme()->textStyles()->get('Brand')?->colour)->toBe('accent1') @@ -93,16 +82,12 @@ function themeWithTextStyles(string $styles): string }); it('refuses a text style that sets a highlight, which this half does not draw', function () { - $loader = new ThemeLoader(); - - expect(fn () => $loader->load(themeWithTextStyles("brand:\n highlight: FFFF00\n"))) + expect(fn () => ThemeDirectory::load(themeWithTextStyles("brand:\n highlight: FFFF00\n"))) ->toThrow(MalformedTheme::class, 'which the slides half does not draw'); }); it('refuses a text style whose colour is not a colour', function () { - $loader = new ThemeLoader(); - - expect(fn () => $loader->load(themeWithTextStyles("brand:\n color: chartreuse\n"))) + expect(fn () => ThemeDirectory::load(themeWithTextStyles("brand:\n color: chartreuse\n"))) ->toThrow(MalformedTheme::class, '"brand.color" is "chartreuse", which is not a colour') ->toThrow(MalformedTheme::class, 'A colour is either no colour at all (none or transparent), a role (page, ink, subtle, panel)'); }); @@ -110,7 +95,7 @@ function themeWithTextStyles(string $styles): string it('accepts a colour written as one of the names the concept draft used', function () { // `dark1` is the concept draft's name for `dk1`, and a theme that names a colour the // old way means the same colour: the spelling is not the thing being refused. - $loaded = (new ThemeLoader())->load(themeWithTextStyles("brand:\n color: dark1\n")); + $loaded = ThemeDirectory::load(themeWithTextStyles("brand:\n color: dark1\n")); expect($loaded->theme()->textStyles()->get('brand')?->colour)->toBe('dark1'); }); @@ -119,7 +104,7 @@ function themeWithTextStyles(string $styles): string // A deck's own colour is lowercased before it is resolved, so `DK1` and `dk1` are one // name there; a theme's file is written against the same vocabulary as a deck's // attribute, so it is one name here too. - $loaded = (new ThemeLoader())->load(themeWithTextStyles("brand:\n color: DK1\n")); + $loaded = ThemeDirectory::load(themeWithTextStyles("brand:\n color: DK1\n")); expect($loaded->theme()->textStyles()->get('brand')?->colour)->toBe('DK1'); }); @@ -129,7 +114,7 @@ function themeWithTextStyles(string $styles): string // folded before it is looked up is a name that can be written in any case. The gate that // refuses a colour has to agree with the resolver that draws one, or a theme is refused // for a colour that would have been right. - $loaded = (new ThemeLoader())->load(themeWithTextStyles(sprintf("brand:\n color: %s\n", $written))); + $loaded = ThemeDirectory::load(themeWithTextStyles(sprintf("brand:\n color: %s\n", $written))); expect($loaded->theme()->textStyles()->get('brand')?->colour)->toBe($written) ->and(Theme::hasColour($written))->toBeTrue(); @@ -178,42 +163,32 @@ function themeWithTextStyles(string $styles): string it('refuses a text style whose colour is not a name at all', function () { // A number is not a spelling of a colour, so it is refused as what it is rather than as // a name nobody has heard of: the advice would not help. - $loader = new ThemeLoader(); - - expect(fn () => $loader->load(themeWithTextStyles("brand:\n color: 123\n"))) + expect(fn () => ThemeDirectory::load(themeWithTextStyles("brand:\n color: 123\n"))) ->toThrow(MalformedTheme::class, '"brand.color" is not a colour'); }); it('refuses a text style whose size is not a size', function () { - $loader = new ThemeLoader(); - - expect(fn () => $loader->load(themeWithTextStyles("brand:\n size: big\n"))) + expect(fn () => ThemeDirectory::load(themeWithTextStyles("brand:\n size: big\n"))) ->toThrow(MalformedTheme::class, 'is not a number of points'); }); it('refuses a text style whose weight is not a weight', function () { - $loader = new ThemeLoader(); - - expect(fn () => $loader->load(themeWithTextStyles("brand:\n bold: yes\n"))) + expect(fn () => ThemeDirectory::load(themeWithTextStyles("brand:\n bold: yes\n"))) ->toThrow(MalformedTheme::class, 'is not true or false'); }); it('refuses a text style whose face is not a name', function () { - $loader = new ThemeLoader(); - - expect(fn () => $loader->load(themeWithTextStyles("brand:\n font: 12\n"))) + expect(fn () => ThemeDirectory::load(themeWithTextStyles("brand:\n font: 12\n"))) ->toThrow(MalformedTheme::class, 'is not a font name'); }); it('refuses a text style that is not a mapping of attributes', function () { - $loader = new ThemeLoader(); - - expect(fn () => $loader->load(themeWithTextStyles("brand:\n - color\n"))) + expect(fn () => ThemeDirectory::load(themeWithTextStyles("brand:\n - color\n"))) ->toThrow(MalformedTheme::class, 'is not a mapping of attributes to values'); }); it('reads the face and the weight a style names', function () { - $loaded = (new ThemeLoader())->load(themeWithTextStyles("brand:\n font: Inter\n bold: true\n")); + $loaded = ThemeDirectory::load(themeWithTextStyles("brand:\n font: Inter\n bold: true\n")); expect($loaded->theme()->textStyles()->get('brand')?->font)->toBe('Inter') ->and($loaded->theme()->textStyles()->get('brand')?->bold)->toBeTrue(); diff --git a/tests/Unit/theme.php b/tests/Unit/theme.php index 8f892ba..127942d 100644 --- a/tests/Unit/theme.php +++ b/tests/Unit/theme.php @@ -256,9 +256,10 @@ it('loads every theme that ships with the library', function () { $loader = new MarkdownPresentation\Theme\ThemeLoader(); + $finder = new MarkdownPresentation\Files\DiskFinder(theme: [dirname(__DIR__, 2) . THEMES]); foreach (['default', 'corporate-dark', 'minimal'] as $name) { - $loaded = $loader->load(dirname(__DIR__, 2) . THEMES . $name); + $loaded = $loader->load($name, $name, $finder); expect($loaded->theme()->name())->not->toBeEmpty() ->and($loaded->roles())->toContain('title', 'content', 'two-column'); @@ -267,22 +268,24 @@ it('loads the dark flag off disk, so the shipped dark theme really is one', function () { $loader = new MarkdownPresentation\Theme\ThemeLoader(); + $finder = new MarkdownPresentation\Files\DiskFinder(theme: [dirname(__DIR__, 2) . THEMES]); // The file is read, not assumed: `corporate-dark` has said it is dark since the // flag was added, and a theme with a near-black `dk1` and no flag renders light. - expect($loader->load(dirname(__DIR__, 2) . THEMES . 'corporate-dark')->theme()->isDark())->toBeTrue(); + expect($loader->load('corporate-dark', 'corporate-dark', $finder)->theme()->isDark())->toBeTrue(); foreach (['default', 'minimal'] as $name) { - expect($loader->load(dirname(__DIR__, 2) . THEMES . $name)->theme()->isDark())->toBeFalse(); + expect($loader->load($name, $name, $finder)->theme()->isDark())->toBeFalse(); } }); it('paints every dark theme that ships in type a reader can read', function () { $loader = new MarkdownPresentation\Theme\ThemeLoader(); + $finder = new MarkdownPresentation\Files\DiskFinder(theme: [dirname(__DIR__, 2) . THEMES]); $ratios = []; foreach (['default', 'corporate-dark', 'minimal'] as $name) { - $theme = $loader->load(dirname(__DIR__, 2) . THEMES . $name)->theme(); + $theme = $loader->load($name, $name, $finder)->theme(); if ($theme->isDark() === false) { continue; From 1ec6068434e26d13d0fd814d9640ad6da8f7cc5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabian=20Gra=C3=9Fl?= Date: Thu, 29 Oct 2026 23:26:34 +0100 Subject: [PATCH 4/8] The deck's files come from the finder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The theme is read through the finder now, and the deck's own files were not: a picture a deck names was looked for beside the deck and a templates file was read from there, so a source that keeps its files in a database could serve a theme and nothing else. They come from the finder now. A picture is asked for by name — `logo.png` — and a templates file with it, and the scope check stays where it was, in the same terms and with the same messages: a path the finder answers is held to the directories this render may read, and a source that holds the bytes is taken on the caller's word, because there is no path to hold. The ceiling on a templates file stays where it was, with the read. The master is the third file, and it is the one that cannot be read as bytes. A `template.pptx` is a package, and a package is opened as a file, so a source that holds only the bytes has them staged into one — and the staged file is the render's: it is given back when the render is over, whether the deck was drawn or refused. The renderer keeps the master rather than dropping it, so that the render is what gives the file back and not a destructor that may run much later. A master the theme carries and the source cannot serve is refused by name. A null answer on its own says nothing: the theme may carry no master, or the source may not be able to serve one. The listing of a theme's files tells the two apart, and the second is a failure — a deck drawn without the master's geometry is in the wrong design, with nothing in the file saying so. `Exception\UnservedTemplate` says so, and the deck is not drawn at all. --- CHANGELOG.md | 36 ++++ README.md | 61 ++++++- src/Exception/UnservedTemplate.php | 31 ++++ src/Render/Chrome.php | 8 +- src/Render/CommonMark/BlockTranslator.php | 36 +++- src/Render/CommonMark/PictureReader.php | 145 +++++++++++++-- src/Render/CommonMark/SlideState.php | 8 +- src/Render/PresentationRenderer.php | 80 +++++++- src/Render/TemplateGeometry.php | 24 ++- src/Template/TemplatesFile.php | 87 ++++++++- src/Theme/LoadedTheme.php | 6 +- src/Theme/Master.php | 95 ++++++++++ src/Theme/ThemeLoader.php | 53 ++++-- tests/Support/MemoryFinder.php | 16 ++ tests/Unit/exceptions.php | 6 + tests/Unit/master-finder.php | 211 ++++++++++++++++++++++ tests/Unit/picture-finder.php | 157 ++++++++++++++++ tests/Unit/templates.php | 144 +++++++++++++++ tests/Unit/theme-loader.php | 2 +- 19 files changed, 1147 insertions(+), 59 deletions(-) create mode 100644 src/Exception/UnservedTemplate.php create mode 100644 src/Theme/Master.php create mode 100644 tests/Unit/master-finder.php create mode 100644 tests/Unit/picture-finder.php diff --git a/CHANGELOG.md b/CHANGELOG.md index f3d7b6e..11a2a79 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -128,6 +128,42 @@ All notable changes to this project are documented here. The format follows the finder and from them together — so a failure still names every theme that would have worked. +- **`PresentationRenderer` takes the source of the deck's files.** A `FileFinder` given + to it as `files:` is where the pictures a deck names and the templates file it + imports come from: they are asked for by name — `logo.png`, `templates.md` — and the + disk is where they are found when the source has none. + + The scope check stays with the library, in the same terms and with the same messages: + a path the source answers is held to the directories this render may read, so a + picture or a templates file out of reach is refused in this library's own words. A + source that holds the bytes rather than a file has them taken on the caller's word: + there is no path to hold to the scope. + + The ceiling on a templates file stays where it was, with the read: a file over it is + refused rather than read whole, whichever source it came from. + +- **A theme's master is read through the finder, and it is a file of the library's own + when the source holds only its bytes.** A `template.pptx` is a package, and a package + is opened as a file, so the bytes are staged into one and the deck is drawn in the + master's geometry all the same. The staged file is the render's: it is given back + when the render is over, and nothing of the library's is left on disk after it — not + after a render that worked and not after one that failed. + + A master the theme carries and the source cannot serve is refused by name — + `Exception\UnservedTemplate` — rather than drawn without. The listing of a theme's + files settles what a null answer means: the theme may carry no master, or the source + may not be able to serve one, and a deck drawn without the master's geometry is drawn + in the wrong design with nothing saying so. + +- **`LoadedTheme::template()` returns the master rather than its path.** It is a + `Theme\Master` now: the path the master is opened at, the name a failure calls it, + and the file of the library's own to give back when the render is over. + +- **`TemplateGeometry` takes the name a failure calls the template.** The third + parameter is the name the master is known by, where the file it is in is not the name + it is known by: a master the library staged is a file of its own making under a name + of its own, and a reader told about that cannot tell which template it was. + - **The inline translator is told which slide and which theme it reads for.** `runs()`, `runsIn()` and `plain()` each take an `$slide` and a `$theme` now: a word in a text style is drawn in the face and the colour the theme names, and a mistake in one is a mistake on diff --git a/README.md b/README.md index d8061a3..9d7efdc 100644 --- a/README.md +++ b/README.md @@ -2602,6 +2602,57 @@ linkFont: References to theme tokens — `{accent1}`, `{fonts.mono}` — are **not** resolved yet, so a `styles.yml` written with them carries the braces through untouched. +### Where a deck's files come from + +The theme a deck names is looked for on disk — beside the deck, beside the working +directory, and among the themes that ship with the package — and the pictures a deck +names and the templates file it imports are read from disk as well. **The files a deck +names come from somewhere the caller chooses**, so a program that keeps its own files +somewhere else — in a database, say — says where they are with a **finder** and hands +it over wherever a deck's files are wanted: + +```php +use MarkdownPresentation\Render\PresentationRenderer; +use MarkdownPresentation\Theme\ThemeResolver; + +$finder = new MyFinder(); // a MarkdownPresentation\Files\FileFinder + +$renderer = new PresentationRenderer( + themes: new ThemeResolver(files: $finder), + files: $finder, +); +``` + +A finder is four methods: `read()` and `path()` answer for one file, `themes()` lists +the themes that resolve and `files()` the files of one theme. The names it is asked for +are the library's own — `house/theme.yml` for a theme's manifest, `logo.png` for a +picture — and the role says which world a name is resolved in: `theme`, `picture` or +`document`. + +**A finder answers; it does not refuse.** A file that is not there is `null`, and a +file that is out of reach is answered like any other: the question of what a render +may read belongs to the library, which holds the path a finder answered to its own +scope and refuses what is out of reach in its own words. + +`Files\DiskFinder` is the default and answers out of directories on this machine, one +list of bases per role, tried in the order they are given. The directories of themes +on disk remain the fallback, so a program with a finder of its own does not lose the +themes that ship with the package. + +**A theme's master is a file, and a file the library stages.** A `template.pptx` is a +package, and a package is opened as a file: a source that holds the master as bytes +rather than as a file has them staged into one, and the deck is drawn in the master's +geometry all the same. The staged file is the render's, and it is given back when the +render is over — nothing of the library's is left on the disk afterwards, whether the +render worked or not. + +**A master the source cannot serve is refused by name.** A theme that carries no +master is a theme that works, and nothing is said about it. A theme that carries one +and a source that cannot hand it over is refused with +`MarkdownPresentation\Exception\UnservedTemplate` rather than drawn without: a deck +without the master's geometry is in the wrong design, with nothing in the file saying +so. The listing of a theme's files is what tells the two apart. + ## Loud failures Every mistake above is an exception naming what would have worked instead, because @@ -2659,12 +2710,14 @@ Several sit beside those two rather than under `InvalidInput`: a part of one that cannot be decompressed; `MarkdownPresentation\Exception\UnreadableDeck`, for a deck this library cannot read; `MarkdownPresentation\Exception\UnpatchedDependency`, when the installed PHPPresentation lacks what the library needs of it; -`MarkdownPresentation\Exception\NothingToConvert`, when there is nothing to convert; and +`MarkdownPresentation\Exception\NothingToConvert`, when there is nothing to convert; +`MarkdownPresentation\Exception\UnservedTemplate`, when a theme carries a master the +source of the theme cannot serve; and `MarkdownPresentation\Exception\FileNotWritable`, when the result cannot be written. Nothing a deck names fixes any of them — it is the disk, the download that stopped, the file that -was not a deck, the installer — and a deck author told to go and rename something would go -and rename something. They are all still under the base class, so the same one `catch` -reaches them. +was not a deck, the installer, the source that could not serve what it was asked for — and +a deck author told to go and rename something would go and rename something. They are all +still under the base class, so the same one `catch` reaches them. ```php use MarkdownPresentation\Exception\Exception; diff --git a/src/Exception/UnservedTemplate.php b/src/Exception/UnservedTemplate.php new file mode 100644 index 0000000..e44df3b --- /dev/null +++ b/src/Exception/UnservedTemplate.php @@ -0,0 +1,31 @@ +blocks->parse($definition->body(), $slide, $this->directory, $this->files, $theme); + $blocks = $this->blocks->parse($definition->body(), $slide, $this->directory, $this->scope, $theme, $this->files); } catch (InvalidInput $failure) { // The name of the template, because the slide the message names is the first // one the template is drawn on and the author has to know which template it was. diff --git a/src/Render/CommonMark/BlockTranslator.php b/src/Render/CommonMark/BlockTranslator.php index 5503154..02b64c1 100644 --- a/src/Render/CommonMark/BlockTranslator.php +++ b/src/Render/CommonMark/BlockTranslator.php @@ -27,6 +27,7 @@ use MarkdownPresentation\Exception\UnknownMarker; use MarkdownPresentation\Exception\UnsupportedImageFormat; use MarkdownPresentation\Exception\UnsupportedLink; +use MarkdownPresentation\Files\FileFinder; use MarkdownPresentation\Filesystem\PathScope; use MarkdownPresentation\Render\Block\Block; use MarkdownPresentation\Render\Block\Code; @@ -115,6 +116,10 @@ public function inline(): InlineTranslator /** * The blocks of one slide's content, and the column each was written for. * + * @param FileFinder|null $files Where the deck's files come from: the pictures it + * names. Null for the disk, which is where a picture was + * found before there was a source. + * * @throws ImageNotFound * @throws MalformedShape * @throws UnknownDirective @@ -122,9 +127,22 @@ public function inline(): InlineTranslator * @throws UnsupportedImageFormat * @throws UnsupportedLink */ - public function parse(string $content, int $slide, string $directory, PathScope $files, Theme $theme): SlideBlocks - { - $state = new SlideState($slide, $directory, $files, explode("\n", str_replace("\r\n", "\n", $content)), $theme); + public function parse( + string $content, + int $slide, + string $directory, + PathScope $scope, + Theme $theme, + ?FileFinder $files = null, + ): SlideBlocks { + $state = new SlideState( + $slide, + $directory, + $scope, + explode("\n", str_replace("\r\n", "\n", $content)), + $theme, + $files, + ); $blocks = []; $columns = []; @@ -136,6 +154,18 @@ public function parse(string $content, int $slide, string $directory, PathScope return new SlideBlocks($blocks, $columns, $state->markers, $state->shapes()); } + /** + * Give back the files this translator staged. + * + * A picture the source held as bytes is staged into a file of the library's own, and + * the file is the render's: it goes when the render is over, and the render asks for + * it back here. + */ + public function release(): void + { + $this->pictures->release(); + } + /** * One piece of Markdown read into blocks, with everything a refusal needs to name. * diff --git a/src/Render/CommonMark/PictureReader.php b/src/Render/CommonMark/PictureReader.php index 26ed509..3dec5d5 100644 --- a/src/Render/CommonMark/PictureReader.php +++ b/src/Render/CommonMark/PictureReader.php @@ -9,7 +9,6 @@ use MarkdownPresentation\Exception\UnsupportedImageFormat; use MarkdownPresentation\Filesystem\PathScope; use MarkdownPresentation\Render\Block\Image as ImageBlock; -use MarkdownPresentation\Render\ConvertedImage; use MarkdownPresentation\Render\ImageConverter; /** @@ -24,9 +23,25 @@ * The two entry points differ only in where the path and the description come from, which is why * they are {@see self::node()} and {@see self::path()} rather than one method with two * callers each supplying both. + * + * **A picture comes from the source the deck's files come from.** A source that holds a + * picture as a file gives a path, and the picture is used where it is — held to the scope + * first, exactly as a picture found on disk is. A source that holds the bytes gives no path, + * and the bytes are staged into a file of the library's own, which the deck embeds the + * picture from. */ final class PictureReader { + /** The prefix of a staged picture, so a file in flight is recognisable as one. */ + private const PREFIX = 'mdpres_pic_'; + + /** + * The files this reader staged and has not given back yet. + * + * @var list + */ + private array $staged = []; + public function __construct( private readonly ImageConverter $images = new ImageConverter(), ) { @@ -63,6 +78,64 @@ public function path(string $written, string $alt, SlideState $state): ImageBloc throw ImageNotFound::aUrl($state->slide, $written); } + $resolved = $this->resolve($written, $state); + + return new ImageBlock( + $written, + $resolved, + $this->images->png($state->slide, $written, $resolved), + $alt, + $state->step, + ); + } + + /** + * Give back the files this reader staged. + * + * A staged picture is the library's own file, and it goes when the render is over: the + * deck has been written by then, and nothing has to survive it. + */ + public function release(): void + { + foreach ($this->staged as $path) { + if (is_file($path)) { + @unlink($path); + } + } + + $this->staged = []; + } + + public function __destruct() + { + $this->release(); + } + + /** + * The picture a deck named, and where it is. + * + * The source is asked first: a source that holds the picture as a file gives a path, + * which is held to the scope as any other path is, and one that holds the bytes gives + * the picture itself, which is staged into a file of the library's own. The disk is + * what a picture was found in before there was a source, and it is still where one is + * found when the source has none. + * + * @throws ImageNotFound + */ + private function resolve(string $written, SlideState $state): string + { + $path = $state->files?->path($written, 'picture'); + + if ($path !== null) { + return $this->held($path, $written, $state); + } + + $bytes = $state->files?->read($written, 'picture'); + + if ($bytes !== null) { + return $this->stage($bytes, $written); + } + // Where a path is written *from* says nothing about whether it is allowed: what is // allowed is where it lands. $joined = str_starts_with($written, '/') ? $written : rtrim($state->directory, '/') . '/' . $written; @@ -71,20 +144,66 @@ public function path(string $written, string $alt, SlideState $state): ImageBloc throw ImageNotFound::at($state->slide, $written, $joined); } - // `realpath()` rather than the path as written, because `..` and a symlink both walk - // out of a directory that a prefix test says they are inside. - $resolved = PathScope::canonical($joined); + return $this->held($joined, $written, $state); + } - if ($resolved === null || !$state->files->holds($resolved)) { - throw ImageNotFound::outside($state->slide, $written, $resolved ?? $joined, $state->files->roots()); + /** + * A path, resolved and held to the scope. + * + * `realpath()` rather than the path as written, because `..` and a symlink both walk + * out of a directory that a prefix test says they are inside. + * + * @throws ImageNotFound + */ + private function held(string $path, string $written, SlideState $state): string + { + if (!is_file($path)) { + throw ImageNotFound::at($state->slide, $written, $path); } - return new ImageBlock( - $written, - $resolved, - $this->images->png($state->slide, $written, $resolved), - $alt, - $state->step, - ); + $resolved = PathScope::canonical($path); + + if ($resolved === null || !$state->scope->holds($resolved)) { + throw ImageNotFound::outside($state->slide, $written, $resolved ?? $path, $state->scope->roots()); + } + + return $resolved; + } + + /** + * The picture's bytes, in a file of the library's own. + * + * The file is named after the picture, because the tools that read it — the format of + * a raster, the drawing of a vector, the name of the part in the package — read the + * name as well as the bytes, and a `.svgz` is a gzipped drawing rather than a broken + * one. + * + * It is made with `tempnam()`, so its name is one nobody guessed and its mode is one + * nobody else on the machine can read; renaming it keeps that mode. A file that could + * not be written is left as it is: the reader that opens the picture refuses it and + * says so, which is a better sentence than one about a temporary directory. + */ + private function stage(string $bytes, string $written): string + { + // A file that could not be made leaves a path that is not there, and the reader + // that opens the picture refuses it and says so. + $temporary = @tempnam(sys_get_temp_dir(), self::PREFIX) ?: sys_get_temp_dir() . '/' . self::PREFIX . bin2hex(random_bytes(8)); + + $extension = pathinfo($written, PATHINFO_EXTENSION); + $path = $extension === '' ? $temporary : $temporary . '.' . $extension; + + // Renamed rather than written under a second name, so that the file keeps the mode + // `tempnam()` gave it: a picture in flight through a shared temporary directory is + // not readable by every account on the machine. + @rename($temporary, $path); + + @file_put_contents($path, $bytes); + + // Both names are the library's to remove: the picture is under the second, and the + // first is where the file is if the rename did not happen. + $this->staged[] = $temporary; + $this->staged[] = $path; + + return $path; } } diff --git a/src/Render/CommonMark/SlideState.php b/src/Render/CommonMark/SlideState.php index e4b67c4..12a6d14 100644 --- a/src/Render/CommonMark/SlideState.php +++ b/src/Render/CommonMark/SlideState.php @@ -6,6 +6,7 @@ use League\CommonMark\Node\Block\AbstractBlock; use League\CommonMark\Node\Node; +use MarkdownPresentation\Files\FileFinder; use MarkdownPresentation\Filesystem\PathScope; use MarkdownPresentation\Render\Free\FreeShape; use MarkdownPresentation\Theme\Theme; @@ -24,6 +25,10 @@ * reads has to be refused before anything is built rather than after a shape exists to refuse * it on. * + * The source of the deck's files is here for the same reason again: a picture a deck names + * may come from a source that holds it as bytes rather than as a file, and where it comes + * from is a fact about the deck rather than about the slide it is on. + * * The lines the deck was written as are held too. CommonMark normalises the text it parses — * a code block loses the indentation of its fence, a paragraph loses the trailing space of * each line — and a slide is drawn as its author wrote it, so the lines are what a block @@ -53,9 +58,10 @@ final class SlideState public function __construct( public readonly int $slide, public readonly string $directory, - public readonly PathScope $files, + public readonly PathScope $scope, private readonly array $lines, public readonly Theme $theme, + public readonly ?FileFinder $files = null, ) { } diff --git a/src/Render/PresentationRenderer.php b/src/Render/PresentationRenderer.php index f56796e..bd4af89 100644 --- a/src/Render/PresentationRenderer.php +++ b/src/Render/PresentationRenderer.php @@ -18,6 +18,7 @@ use MarkdownPresentation\Exception\UnknownMarker; use MarkdownPresentation\Exception\UnsupportedColumn; use MarkdownPresentation\Exception\UnsupportedImageFormat; +use MarkdownPresentation\Files\FileFinder; use MarkdownPresentation\Filesystem\PathScope; use MarkdownPresentation\Fixes; use MarkdownPresentation\Render\Block\Code; @@ -37,6 +38,7 @@ use MarkdownPresentation\Template\TemplatesFile; use MarkdownPresentation\Theme\LayoutRole; use MarkdownPresentation\Theme\LoadedTheme; +use MarkdownPresentation\Theme\Master; use MarkdownPresentation\Theme\Theme; use MarkdownPresentation\Theme\ThemeResolver; use PhpOffice\PhpPresentation\PhpPresentation; @@ -150,7 +152,15 @@ final class PresentationRenderer */ private readonly FontReport $fonts; - private readonly PathScope $files; + private readonly PathScope $scope; + + /** + * The source of the deck's files: its pictures and the templates file it names. + * + * Null for the disk, which is where a deck's files were found before there was a + * source. A source that holds them as bytes is asked for them by name. + */ + private readonly ?FileFinder $files; private readonly ThemeResolver $themes; @@ -193,6 +203,15 @@ final class PresentationRenderer */ private Theme $theme; + /** + * The master the deck being rendered was drawn in. + * + * Held rather than dropped, so that the file it is in is given back by the render + * that opened it rather than by a destructor that may run much later: a file of the + * library's own is the render's to give back. + */ + private ?Master $master = null; + /** * @param string $directory The directory a relative image path in the deck * resolves against, which is the directory the deck @@ -217,6 +236,11 @@ final class PresentationRenderer * the deck's: a deck that could widen this would be the * arbitrary path read this constructor argument exists * to stop. + * @param FileFinder|null $files Where the deck's files come from: the pictures it + * names and the templates file it imports. Null for the + * disk, which is where a deck's files were found before + * there was a source. The theme is not among them: the + * resolver the caller brought is where themes come from. */ public function __construct( private readonly string $directory = '.', @@ -224,15 +248,17 @@ public function __construct( private readonly BlockTranslator $blocks = new BlockTranslator(), bool $strict = false, array $imageRoots = [], + ?FileFinder $files = null, ) { // The theme of a deck sits beside it, and a relative image path in the deck // resolves against the same directory: a deck and its theme are one thing. $this->themes = $themes->beside($this->directory); - $this->files = new PathScope([$this->directory, ...$imageRoots]); + $this->scope = new PathScope([$this->directory, ...$imageRoots]); + $this->files = $files; $this->overflows = new Overflows($strict); $this->fonts = new FontReport($strict); - $this->templatesFiles = new TemplatesFile($this->directory, $this->files); - $this->chrome = new Chrome($this->blocks, $this->directory, $this->files); + $this->templatesFiles = new TemplatesFile($this->directory, $this->scope, files: $files); + $this->chrome = new Chrome($this->blocks, $this->directory, $this->scope, $files); } /** @@ -300,6 +326,9 @@ public function unknownFonts(): array * @throws \MarkdownPresentation\Exception\UnpatchedDependency When the installed * PHPPresentation lacks what this library needs of it. * + * @throws \MarkdownPresentation\Exception\UnservedTemplate When the theme carries a + * master the source of the theme cannot serve. + * * @return string The path written. */ public function render(Deck $deck, string $path): string @@ -322,6 +351,37 @@ public function render(Deck $deck, string $path): string } $loaded = $this->loaded($deck); + $this->master = $loaded->template(); + + try { + return $this->draw($deck, $loaded, $path); + } finally { + // The files the library staged for this render: the theme's master, when the + // source of the theme held only its bytes, and the pictures the deck named, + // when the source of the deck's files held only theirs. A render is over when + // it is written, and nothing of the library's is left on disk after it — not + // after a render that worked and not after one that failed. + $this->master?->release(); + $this->blocks->release(); + } + } + + /** + * The deck, drawn and written. + * + * The theme is resolved before this is called, so that the files the library stages + * for the render are given back whether the deck is drawn or refused. + * + * @throws InvalidInput + * @throws ThemeNotFound + * @throws ContentOverflow + * @throws MalformedTemplate + * @throws \MarkdownPresentation\Exception\Exception + * @throws \MarkdownPresentation\Exception\MalformedShape + * @throws \MarkdownPresentation\Exception\UnpatchedDependency + */ + private function draw(Deck $deck, LoadedTheme $loaded, string $path): string + { $roles = $this->roles($deck, $loaded); $this->theme = $loaded->theme(); @@ -462,8 +522,9 @@ private function blocks(Deck $deck, Theme $theme): array $source->content(), $source->index() + 1, $this->directory, - $this->files, + $this->scope, $theme, + $this->files, ); } @@ -491,11 +552,16 @@ private function blocks(Deck $deck, Theme $theme): array */ private function template(LoadedTheme $theme, array $roles): array { - if ($theme->template() === null) { + $master = $theme->template(); + + if ($master === null) { return []; } - $layouts = (new TemplateGeometry((string) $theme->template()))->boxes(); + // The name the master is known by, rather than the path it is in: a master the + // library staged is a file of its own making under a name of its own, and a reader + // told about that cannot tell which template it was. + $layouts = (new TemplateGeometry($master->path(), name: $master->name()))->boxes(); $boxes = []; foreach ($roles as $role) { diff --git a/src/Render/TemplateGeometry.php b/src/Render/TemplateGeometry.php index cfa8871..589e3c7 100644 --- a/src/Render/TemplateGeometry.php +++ b/src/Render/TemplateGeometry.php @@ -78,10 +78,14 @@ final class TemplateGeometry * @param string $path The theme's `template.pptx`. * @param int $maxPartBytes The largest one part of it may be once it is inflated, which is * what bounds the document built from it. + * @param string|null $name What a failure calls the template, where the file it is in is + * not the name it is known by — a master the library staged, which is a file of + * its own making under a name of its own. */ public function __construct( private readonly string $path, private readonly int $maxPartBytes = self::MAX_PART_BYTES, + private readonly ?string $name = null, ) { } @@ -109,7 +113,7 @@ public function boxes(): array $zip = new ZipArchive(); if ($zip->open($this->path) !== true) { - throw MalformedTemplate::notAPackage($this->path); + throw MalformedTemplate::notAPackage($this->label()); } try { @@ -119,6 +123,18 @@ public function boxes(): array } } + /** + * What a failure calls the template. + * + * The name it is known by where there is one, and the path it is in where there is + * not: a template the library staged is a file of its own making under a name of its + * own, and a reader told about that cannot tell which template it was. + */ + private function label(): string + { + return $this->name ?? $this->path; + } + /** * @return array}> * @@ -183,7 +199,7 @@ private function page(ZipArchive $zip): array $cy = (int) ($size?->getAttribute('cy') ?? 0); if ($cx < 1 || $cy < 1) { - throw MalformedTemplate::noPageSize($this->path); + throw MalformedTemplate::noPageSize($this->label()); } return ['cx' => $cx, 'cy' => $cy]; @@ -426,7 +442,7 @@ private function document(ZipArchive $zip, string $part): DOMDocument libxml_use_internal_errors($quiet); if ($loaded === false) { - throw MalformedTemplate::unreadable($this->path, $part); + throw MalformedTemplate::unreadable($this->label(), $part); } return $document; @@ -457,7 +473,7 @@ private function part(ZipArchive $zip, string $part): string fclose($stream); if (strlen($bytes) > $this->maxPartBytes) { - throw MalformedTemplate::partTooLarge($this->path, $part, $this->maxPartBytes); + throw MalformedTemplate::partTooLarge($this->label(), $part, $this->maxPartBytes); } return $bytes; diff --git a/src/Template/TemplatesFile.php b/src/Template/TemplatesFile.php index e5f8347..6ec6a01 100644 --- a/src/Template/TemplatesFile.php +++ b/src/Template/TemplatesFile.php @@ -6,6 +6,7 @@ use MarkdownPresentation\Exception\TemplatesFileNotFound; use MarkdownPresentation\Exception\TemplatesFileTooLarge; +use MarkdownPresentation\Files\FileFinder; use MarkdownPresentation\Filesystem\PathScope; /** @@ -14,6 +15,15 @@ * A path a deck writes, so it is resolved and held to the directories this render may * read — the same rule an image path has to pass, because both are paths a deck wrote and * neither is worth a way out of the directory the deck was given. + * + * **The templates file comes from the source the deck's files come from.** A source that + * holds it as a file answers a path, and the path is held to the scope as any other path + * is. A source that holds the contents answers no path, and the contents are taken on the + * caller's word: there is no path to hold to the scope, and a caller that brings a source + * of its own vouches for what is in it. + * + * The ceiling is where it was: with the read. A file over it is abandoned rather than + * read whole, which is what keeps the memory a deck's frontmatter can ask for bounded. */ final class TemplatesFile { @@ -30,11 +40,15 @@ final class TemplatesFile /** * @param string $directory The directory the deck is in, which is where a relative path * in the deck is written from. + * @param FileFinder|null $files Where the deck's files come from. Null for the disk, + * which is where a templates file was found before there + * was a source. */ public function __construct( private readonly string $directory, - private readonly PathScope $files, + private readonly PathScope $scope, private readonly int $maxBytes = self::MAX_BYTES, + private readonly ?FileFinder $files = null, ) { } @@ -50,6 +64,12 @@ public function read(string $written): string throw TemplatesFileNotFound::aUrl($written); } + $fromSource = $this->fromSource($written); + + if ($fromSource !== null) { + return $fromSource; + } + $joined = str_starts_with($written, '/') ? $written : rtrim($this->directory, '/') . '/' . $written; if (!is_file($joined)) { @@ -60,15 +80,66 @@ public function read(string $written): string // out of a directory that a prefix test says they are inside. $resolved = PathScope::canonical($joined); - if ($resolved === null || !$this->files->holds($resolved)) { - throw TemplatesFileNotFound::outside($written, $resolved ?? $joined, $this->files->roots()); + if ($resolved === null || !$this->scope->holds($resolved)) { + throw TemplatesFileNotFound::outside($written, $resolved ?? $joined, $this->scope->roots()); + } + + return $this->readFile($written, $resolved); + } + + /** + * The templates file in the source the deck's files come from, or null when it has none. + * + * A path the source answers is held to the scope, exactly as a path on disk is: a + * `templates-file:` is written by the deck, and what is allowed is where it lands. + */ + private function fromSource(string $written): ?string + { + if ($this->files === null) { + return null; } - // The read only happens where the file can be read: `file_get_contents()` warns - // when it cannot, and a warning is not a failure anybody can act on. - // - // With a length, so that a file over the ceiling is abandoned rather than read - // whole. + $path = $this->files->path($written, 'document'); + + if ($path !== null) { + if (!is_file($path)) { + throw TemplatesFileNotFound::at($path); + } + + $resolved = PathScope::canonical($path); + + if ($resolved === null || !$this->scope->holds($resolved)) { + throw TemplatesFileNotFound::outside($written, $resolved ?? $path, $this->scope->roots()); + } + + return $this->readFile($written, $resolved); + } + + $markdown = $this->files->read($written, 'document'); + + if ($markdown === null) { + return null; + } + + if (strlen($markdown) > $this->maxBytes) { + throw TemplatesFileTooLarge::of($written, strlen($markdown), $this->maxBytes); + } + + return $markdown; + } + + /** + * A file's Markdown, up to the ceiling. + * + * The read only happens where the file can be read: `file_get_contents()` warns when + * it cannot, and a warning is not a failure anybody can act on. And it happens with a + * length, so that a file over the ceiling is abandoned rather than read whole. + * + * @throws TemplatesFileNotFound + * @throws TemplatesFileTooLarge + */ + private function readFile(string $written, string $resolved): string + { $length = max(0, $this->maxBytes) + 1; $markdown = is_readable($resolved) diff --git a/src/Theme/LoadedTheme.php b/src/Theme/LoadedTheme.php index 0eb5572..7b372eb 100644 --- a/src/Theme/LoadedTheme.php +++ b/src/Theme/LoadedTheme.php @@ -26,7 +26,7 @@ public function __construct( private readonly Theme $theme, private readonly array $styles, private readonly array $layouts, - private readonly ?string $template, + private readonly ?Master $template, ) { } @@ -94,8 +94,8 @@ public function layout(string $role): string return $layout; } - /** The path of the theme's `.pptx`, or null when it ships without one. */ - public function template(): ?string + /** The theme's master, or null when it ships without one. */ + public function template(): ?Master { return $this->template; } diff --git a/src/Theme/Master.php b/src/Theme/Master.php new file mode 100644 index 0000000..169fe20 --- /dev/null +++ b/src/Theme/Master.php @@ -0,0 +1,95 @@ +path; + } + + /** What a failure calls the master. */ + public function name(): string + { + return $this->name; + } + + /** + * Give back the file the library staged, where it staged one. + * + * The theme's own master is not the library's to remove, so it stays where it is. + */ + public function release(): void + { + if ($this->owned) { + // Given back once: the render asks for it and the destructor asks again, and a + // second removal of a file that is gone is a warning nobody can act on. + $this->owned = false; + + if (is_file($this->path)) { + @unlink($this->path); + } + } + } + + public function __destruct() + { + $this->release(); + } +} diff --git a/src/Theme/ThemeLoader.php b/src/Theme/ThemeLoader.php index e6b7ed6..f976bdd 100644 --- a/src/Theme/ThemeLoader.php +++ b/src/Theme/ThemeLoader.php @@ -9,6 +9,7 @@ use MarkdownPresentation\Exception\ThemeNotFound; use MarkdownPresentation\Exception\UnknownLayoutRole; use MarkdownPresentation\Exception\UnknownStyleSlot; +use MarkdownPresentation\Exception\UnservedTemplate; use MarkdownPresentation\Files\FileFinder; use MarkdownPresentation\Filesystem\PathScope; use Symfony\Component\Yaml\Exception\ParseException; @@ -18,9 +19,10 @@ * Reads a theme out of a finder: the `theme.yml` and the files beside it. * * Only `theme.yml` is required. `styles.yml`, `layouts.yml` and `text-styles.yml` are - * read when they are there, and `template.pptx` is only a path: nothing here opens a - * `.pptx`, because a theme's template belongs to whoever renders the deck rather than to - * the loader that found it. + * read when they are there, and `template.pptx` is the theme's master: it is opened as a + * file, so a source that holds only its bytes has them staged into one, and nothing here + * reads what is inside it — a theme's master belongs to whoever renders the deck rather + * than to the loader that found it. * * The finder is asked by name — `house/theme.yml`, `house/styles.yml` — because the * library composes names and a finder answers them. A theme that never had a directory @@ -67,6 +69,7 @@ public function __construct(private readonly ?PathScope $scope = null) * @throws MalformedTheme when a file in it does not describe a theme * @throws UnknownStyleSlot when `styles.yml` names a slot that does not exist * @throws UnknownLayoutRole when `layouts.yml` names a role that does not exist + * @throws UnservedTemplate when the theme carries a master the source cannot serve * @throws Exception when a file in it cannot be read or parsed, or resolves out * of the scope */ @@ -103,6 +106,7 @@ public function load(string $name, ?string $written = null, ?FileFinder $files = * @throws MalformedTheme when the file does not describe a theme * @throws UnknownStyleSlot when `styles.yml` names a slot that does not exist * @throws UnknownLayoutRole when `layouts.yml` names a role that does not exist + * @throws UnservedTemplate when the theme carries a master the source cannot serve * @throws Exception when a file in it cannot be read or parsed, or resolves out * of the scope */ @@ -223,23 +227,46 @@ private function contained(string $path, string $written): ?string } /** - * The theme's template, held to the scope before it is named. + * The theme's master, and the refusal of one the source cannot serve. * - * The path is what the renderer opens, and it opens whatever it is given: a template - * that is a symlink out of the scope is a file outside it, read under a name that is - * inside. + * The master is asked for by name like the rest of the theme's files. A source that + * holds it as a file has it opened where it is — held to the scope first, because the + * renderer opens whatever it is given, and a master that is a symlink out of the scope + * is a file outside it read under a name that is inside. A source that holds only the + * bytes has them staged into a file of the library's own. * - * @throws ThemeNotFound + * A null answer is ambiguous on its own: the theme may carry no master, or the source + * may not be able to serve one. The listing of the theme's files tells the two apart, + * and a master the theme carries and the source cannot serve is refused rather than + * drawn without — a deck without the master's geometry is in the wrong design, with + * nothing in it saying so. + * + * @throws UnservedTemplate when the theme carries a master the source cannot serve + * @throws ThemeNotFound when the master resolves outside the directories this render may read */ - private function template(string $name, string $written, ?FileFinder $files): ?string + private function template(string $name, string $written, ?FileFinder $files): ?Master { - $path = $files?->path($name . '/' . self::TEMPLATE_FILE, 'theme'); + $file = $name . '/' . self::TEMPLATE_FILE; - if ($path === null) { - return null; + $path = $files?->path($file, 'theme'); + + if ($path !== null) { + $real = $this->contained($path, $written); + + return $real === null ? null : Master::at($real, $file); + } + + $bytes = $files?->read($file, 'theme'); + + if ($bytes !== null) { + return Master::of($bytes, $file); + } + + if ($files !== null && in_array(self::TEMPLATE_FILE, $files->files($name), true)) { + throw UnservedTemplate::named($written, self::TEMPLATE_FILE); } - return $this->contained($path, $written); + return null; } /** diff --git a/tests/Support/MemoryFinder.php b/tests/Support/MemoryFinder.php index db017da..a016c1f 100644 --- a/tests/Support/MemoryFinder.php +++ b/tests/Support/MemoryFinder.php @@ -22,6 +22,9 @@ final class MemoryFinder implements FileFinder /** @var array */ private array $reads = []; + /** @var array */ + private array $roles = []; + /** * @param array $files The files it holds, by name. */ @@ -32,6 +35,7 @@ public function __construct(private readonly array $files = []) public function read(string $name, ?string $role = null): ?string { $this->asked[$name] = true; + $this->roles[$name] = $role; $this->reads[$name] = ($this->reads[$name] ?? 0) + 1; return $this->files[$name] ?? null; @@ -40,6 +44,7 @@ public function read(string $name, ?string $role = null): ?string public function path(string $name, ?string $role = null): ?string { $this->asked[$name] = true; + $this->roles[$name] = $role; return null; } @@ -93,4 +98,15 @@ public function reads(string $name): int { return $this->reads[$name] ?? 0; } + + /** + * The role a name was asked for in. + * + * The role says which world the name is resolved in, so it is what tells a picture + * asked for as a picture from a theme asked for as a theme. + */ + public function askedFor(string $name): ?string + { + return $this->roles[$name] ?? null; + } } diff --git a/tests/Unit/exceptions.php b/tests/Unit/exceptions.php index 2c8dd6c..781a993 100644 --- a/tests/Unit/exceptions.php +++ b/tests/Unit/exceptions.php @@ -35,6 +35,7 @@ use MarkdownPresentation\Exception\UnsupportedColumn; use MarkdownPresentation\Exception\UnsupportedImageFormat; use MarkdownPresentation\Exception\UnsupportedLink; +use MarkdownPresentation\Exception\UnservedTemplate; use MarkdownPresentation\Render\NotesColourMapPass; use MarkdownPresentation\Render\PresentationRenderer; use MarkdownPresentation\Tests\Support\Doctest; @@ -134,12 +135,17 @@ function reportedExceptions(): array // // A PHPPresentation without the fixes is the caller's too: it is what the installer // put there, and no deck author can put it back. + // + // A master the source of a theme cannot serve is the caller's as well: the theme is + // right and the deck is right, and it is the program that brought the source that has + // to serve it. $callerSide = [ FileNotWritable::class, NothingToConvert::class, UnreadableDeck::class, UnreadablePackage::class, UnpatchedDependency::class, + UnservedTemplate::class, ]; if ($class === Exception::class || $class === InvalidInput::class) { diff --git a/tests/Unit/master-finder.php b/tests/Unit/master-finder.php new file mode 100644 index 0000000..9f6a7dd --- /dev/null +++ b/tests/Unit/master-finder.php @@ -0,0 +1,211 @@ + $files The files it holds, by name. + */ + public function __construct(private readonly array $files = []) + { + } + + public function read(string $name, ?string $role = null): ?string + { + return str_ends_with($name, 'template.pptx') ? null : ($this->files[$name] ?? null); + } + + public function path(string $name, ?string $role = null): ?string + { + return null; + } + + public function themes(): array + { + return ['house']; + } + + public function files(string $theme): array + { + return ['layouts.yml', 'template.pptx', 'theme.yml']; + } +} + +it('draws a deck in the geometry of a master the source holds as a file', function () { + // The source holds the theme's files where they are, so the master is opened where + // it is. The title lands where the template puts it rather than where this library + // would: that is the geometry the master states. + $path = Scratch::file('master.pptx', ''); + + (new PresentationRenderer(themes: masterResolver(new DiskFinder(theme: [dirname(masterTheme())])))) + ->render( + (new DeckParser())->parse("---\ntheme: templated\n---\n# One\n\nText.\n"), + $path, + ); + + expect(Pptx::open($path)->positionOf('title')['x'])->toBe(66.0); +}); + +it('draws a deck in the geometry of a master the source holds as bytes', function () { + // The source the finder exists for: a plugin that keeps its themes in a database and + // never had a directory to hand over. The master is a package, so its bytes are + // staged into one — and the deck is drawn in the same geometry all the same. + $finder = new MemoryFinder([ + 'house/theme.yml' => (string) file_get_contents(masterTheme() . '/theme.yml'), + 'house/layouts.yml' => (string) file_get_contents(masterTheme() . '/layouts.yml'), + 'house/template.pptx' => (string) file_get_contents(masterTheme() . '/template.pptx'), + ]); + + $path = Scratch::file('master-bytes.pptx', ''); + + (new PresentationRenderer(themes: masterResolver($finder))) + ->render( + (new DeckParser())->parse("---\ntheme: house\n---\n# One\n\nText.\n"), + $path, + ); + + expect(Pptx::open($path)->positionOf('title')['x'])->toBe(66.0); +}); + +it('leaves nothing behind when it stages a master', function () { + // The master was staged, so the staged file has to go: the render is over. The + // renderer is held — a caller keeps it — so the file is gone because the render gave + // it back and not because something was collected. + $before = glob(sys_get_temp_dir() . '/mdpres_master_*') ?: []; + + $finder = new MemoryFinder([ + 'house/theme.yml' => (string) file_get_contents(masterTheme() . '/theme.yml'), + 'house/layouts.yml' => (string) file_get_contents(masterTheme() . '/layouts.yml'), + 'house/template.pptx' => (string) file_get_contents(masterTheme() . '/template.pptx'), + ]); + + $renderer = new PresentationRenderer(themes: masterResolver($finder)); + + $renderer->render( + (new DeckParser())->parse("---\ntheme: house\n---\n# One\n\nText.\n"), + Scratch::file('master-staged.pptx', ''), + ); + + expect(glob(sys_get_temp_dir() . '/mdpres_master_*') ?: [])->toBe($before); +}); + +it('leaves nothing behind when the render fails', function () { + // A render that fails has staged a master all the same, and the file is the + // library's: a failure is no reason to leave it on the disk. + $before = glob(sys_get_temp_dir() . '/mdpres_master_*') ?: []; + + $finder = new MemoryFinder([ + 'house/theme.yml' => (string) file_get_contents(masterTheme() . '/theme.yml'), + 'house/layouts.yml' => (string) file_get_contents(masterTheme() . '/layouts.yml'), + 'house/template.pptx' => (string) file_get_contents(masterTheme() . '/template.pptx'), + ]); + + $renderer = new PresentationRenderer(themes: masterResolver($finder)); + + expect(fn () => $renderer->render( + (new DeckParser())->parse("---\ntheme: house\nlayout: nonsense\n---\n# One\n"), + Scratch::file('master-failed.pptx', ''), + )) + ->toThrow(UnknownLayoutRole::class); + + expect(glob(sys_get_temp_dir() . '/mdpres_master_*') ?: [])->toBe($before); +}); + +it('refuses a master the source cannot serve, by name', function () { + // The theme carries a master and the source cannot hand it over. Drawing the deck + // without it would draw it in the wrong design with nothing saying so, so it is + // refused — and the deck is not drawn at all. + $finder = new MasterlessSource([ + 'house/theme.yml' => (string) file_get_contents(masterTheme() . '/theme.yml'), + 'house/layouts.yml' => (string) file_get_contents(masterTheme() . '/layouts.yml'), + ]); + + $path = Scratch::file('master-unserved.pptx', ''); + + expect(fn () => (new PresentationRenderer(themes: masterResolver($finder))) + ->render( + (new DeckParser())->parse("---\ntheme: house\n---\n# One\n"), + $path, + )) + ->toThrow(UnservedTemplate::class, 'The theme "house" carries a "template.pptx" that the source of this render cannot serve'); + + // And no deck was written: the refusal came before anything was built. + expect(filesize($path))->toBe(0); +}); + +it('draws a deck in a theme that carries no master, and says nothing', function () { + // A theme without a master is a theme that works: the deck is drawn in the geometry + // this library puts it at, and nothing is said about a master that was never there. + $finder = new MemoryFinder([ + 'house/theme.yml' => (string) file_get_contents(masterTheme() . '/theme.yml'), + 'house/layouts.yml' => (string) file_get_contents(masterTheme() . '/layouts.yml'), + ]); + + $path = Scratch::file('no-master.pptx', ''); + + (new PresentationRenderer(themes: masterResolver($finder))) + ->render( + (new DeckParser())->parse("---\ntheme: house\n---\n# One\n"), + $path, + ); + + expect(Pptx::open($path)->positionOf('title')['x']) + ->toBe((float) (new SlideGeometry())->title(LayoutRole::Content)['box']->x); +}); diff --git a/tests/Unit/picture-finder.php b/tests/Unit/picture-finder.php new file mode 100644 index 0000000..de4d9c9 --- /dev/null +++ b/tests/Unit/picture-finder.php @@ -0,0 +1,157 @@ + + */ +function pictureMedia(Pptx $deck): array +{ + return array_values(array_filter( + $deck->parts(), + static fn (string $part): bool => str_starts_with($part, 'ppt/media/'), + )); +} + +it('draws a picture the source holds as bytes', function () { + $finder = new MemoryFinder(['logo.png' => (string) file_get_contents(finderPng())]); + + $path = Scratch::file('picture.pptx', ''); + + (new PresentationRenderer(files: $finder)) + ->render((new DeckParser())->parse("![A logo](logo.png)\n"), $path); + + $deck = Pptx::open($path); + $media = pictureMedia($deck); + + expect($media)->toHaveCount(1) + // And the picture is the one that went in, rather than a placeholder. + ->and($media[0])->toEndWith('.png') + ->and($deck->part($media[0]))->toStartWith("\x89PNG"); +}); + +it('draws a picture the source holds as a file', function () { + // A source that holds the picture as a file gives a path, and the picture is used + // where it is: the deck's own directory is the root it is read under. + $directory = Scratch::directoryPath('pictures'); + copy(finderPng(), $directory . '/logo.png'); + + $path = Scratch::file('picture-file.pptx', ''); + + (new PresentationRenderer($directory, files: new DiskFinder(picture: [$directory]))) + ->render((new DeckParser())->parse("![A logo](logo.png)\n"), $path); + + expect(pictureMedia(Pptx::open($path)))->toHaveCount(1); +}); + +it('asks the source for the picture by name, as a picture', function () { + // The name is the deck's own and the role says which world it is resolved in: a + // picture asked for as a picture is not a document asked for as a document. + $finder = new MemoryFinder(['logo.png' => (string) file_get_contents(finderPng())]); + + $path = Scratch::file('picture-asked.pptx', ''); + + (new PresentationRenderer(files: $finder)) + ->render((new DeckParser())->parse("![A logo](logo.png)\n"), $path); + + expect($finder->asked())->toContain('logo.png') + ->and($finder->askedFor('logo.png'))->toBe('picture'); +}); + +it('refuses a picture the source holds out of reach, in the library\'s words', function () { + // The finder answers the path; the library holds it to its own scope and refuses it. + // The message is the library's, because the library is what knows what this render + // may read. + $outside = Scratch::directoryPath('outside'); + copy(finderPng(), $outside . '/logo.png'); + + expect(fn () => (new PresentationRenderer(Scratch::directoryPath('deck'), files: new DiskFinder(picture: [$outside]))) + ->render((new DeckParser())->parse("![A logo](logo.png)\n"), Scratch::file('outside.pptx', ''))) + ->toThrow(ImageNotFound::class, 'rather than one this render may read'); +}); + +it('refuses a picture the source answers a path for but does not have', function () { + // A finder that answers a path for a file that has since gone: the picture is not + // there, and the library says so rather than reading nothing. + $finder = new class implements FileFinder { + public function read(string $name, ?string $role = null): ?string + { + return null; + } + + public function path(string $name, ?string $role = null): ?string + { + return '/mdpresentation-not-here/logo.png'; + } + + public function themes(): array + { + return []; + } + + public function files(string $theme): array + { + return []; + } + }; + + expect(fn () => (new PresentationRenderer(files: $finder)) + ->render((new DeckParser())->parse("![A logo](logo.png)\n"), Scratch::file('gone.pptx', ''))) + ->toThrow(ImageNotFound::class, 'which is not a file'); +}); + +it('leaves nothing behind when it stages a picture', function () { + // The picture was drawn from a staged file, so the staged file has to go: the render + // is over. The renderer is held — a caller keeps it — so the file is gone because the + // render gave it back and not because something was collected. + $before = glob(sys_get_temp_dir() . '/mdpres_pic_*') ?: []; + + $finder = new MemoryFinder(['logo.png' => (string) file_get_contents(finderPng())]); + + $renderer = new PresentationRenderer(files: $finder); + + $renderer->render( + (new DeckParser())->parse("![A logo](logo.png)\n"), + Scratch::file('picture-staged.pptx', ''), + ); + + expect(glob(sys_get_temp_dir() . '/mdpres_pic_*') ?: [])->toBe($before); +}); diff --git a/tests/Unit/templates.php b/tests/Unit/templates.php index 4120afc..b70f5b7 100644 --- a/tests/Unit/templates.php +++ b/tests/Unit/templates.php @@ -8,8 +8,11 @@ use MarkdownPresentation\Exception\MalformedFrontmatter; use MarkdownPresentation\Exception\UnknownFont; use MarkdownPresentation\Exception\UnknownTemplate; +use MarkdownPresentation\Files\DiskFinder; +use MarkdownPresentation\Files\FileFinder; use MarkdownPresentation\Render\Furniture; use MarkdownPresentation\Render\PresentationRenderer; +use MarkdownPresentation\Tests\Support\MemoryFinder; use MarkdownPresentation\Tests\Support\Pptx; use MarkdownPresentation\Tests\Support\Scratch; use MarkdownPresentation\Template\TemplatesFile; @@ -140,6 +143,36 @@ function templateFailure(string $markdown): string return ''; } +/** + * A rendered deck drawn with the source of the deck's files given. + * + * The source is the one a caller brings: a plugin that keeps its files in a database and + * never had a directory to hand over. + */ +function templateDeckFrom(?FileFinder $files, string $markdown): Pptx +{ + $path = Scratch::file('template.pptx', ''); + + (new PresentationRenderer(directory: Scratch::directory(), files: $files)) + ->render((new DeckParser())->parse($markdown), $path); + + return Pptx::open($path); +} + +/** + * A rendered deck drawn with the source given, and the message it failed with. + */ +function templateFailureFrom(?FileFinder $files, string $markdown): string +{ + try { + templateDeckFrom($files, $markdown); + } catch (MarkdownException $failure) { + return $failure->getMessage(); + } + + return ''; +} + /** * A rendered deck and the renderer that rendered it. * @@ -422,6 +455,117 @@ function templateRenderer(string $markdown, bool $strict = false): PresentationR expect(templateFailure($deck))->toContain('is a URL rather than a path'); }); +it('reads a templates file the source holds as bytes', function () { + // The source the finder exists for: a plugin that keeps its files in a database and + // never had a directory to hand over. The furniture comes from it and is drawn. + $finder = new MemoryFinder([ + 'templates.md' => "---\ntemplate: brand\n---\n::line{left=5% top=72.889% width=30% height=0.185%}\n", + ]); + + $deck = "---\ntemplates-file: templates.md\nlayout: blank\n---\n\n---\nlayout: blank\ntemplate: brand\n---\n"; + + expect(templateDeckFrom($finder, $deck)->shapeNames(2))->toBe(['line']); +}); + +it('asks the source for the templates file as a document', function () { + // The role says which world the name is resolved in: a templates file is a document's + // own file rather than a picture or a file of a theme. + $finder = new MemoryFinder([ + 'templates.md' => "---\ntemplate: brand\n---\n::line{}\n", + ]); + + $deck = "---\ntemplates-file: templates.md\nlayout: blank\n---\n"; + + templateDeckFrom($finder, $deck); + + expect($finder->asked())->toContain('templates.md') + ->and($finder->askedFor('templates.md'))->toBe('document'); +}); + +it('refuses a templates file the source holds that is larger than the library will read', function () { + // The ceiling is where it was, with the read: a file over it is refused rather than + // read whole, whichever source it came from. + $finder = new MemoryFinder([ + 'templates.md' => str_repeat('a', TemplatesFile::MAX_BYTES + 1), + ]); + + $deck = "---\ntemplates-file: templates.md\nlayout: blank\n---\n"; + + expect(templateFailureFrom($finder, $deck)) + ->toContain(sprintf( + 'is %d bytes, and a templates file may be at most %d', + TemplatesFile::MAX_BYTES + 1, + TemplatesFile::MAX_BYTES, + )); +}); + +it('refuses a templates file the source holds out of reach, in the library\'s words', function () { + // The finder answers the path; the library holds it to its own scope and refuses it. + // The message is the library's, because the library is what knows what this render + // may read. + $outside = sys_get_temp_dir() . '/templates-' . bin2hex(random_bytes(6)); + mkdir($outside); + file_put_contents($outside . '/templates.md', "---\ntemplate: brand\n---\n::line{}\n"); + + $deck = "---\ntemplates-file: templates.md\nlayout: blank\n---\n"; + + expect(templateFailureFrom(new DiskFinder(document: [$outside]), $deck)) + ->toContain('rather than one this render may read'); + + unlink($outside . '/templates.md'); + rmdir($outside); +}); + +it('reads a templates file the source holds as a file', function () { + // A source that holds the templates file as a file answers a path, and the file is + // read from where it is. + file_put_contents(Scratch::directory() . '/templates.md', "---\ntemplate: brand\n---\n::line{}\n"); + + $deck = "---\ntemplates-file: templates.md\nlayout: blank\n---\n\n---\nlayout: blank\ntemplate: brand\n---\n"; + + expect(templateDeckFrom(new DiskFinder(document: [Scratch::directory()]), $deck)->shapeNames(2))->toBe(['line']); +}); + +it('falls back to the disk when the source has no templates file', function () { + // A source that has nothing is asked all the same, and the file beside the deck is + // still where a templates file is found. + file_put_contents(Scratch::directory() . '/templates.md', "---\ntemplate: brand\n---\n::line{}\n"); + + $deck = "---\ntemplates-file: templates.md\nlayout: blank\n---\n\n---\nlayout: blank\ntemplate: brand\n---\n"; + + expect(templateDeckFrom(new MemoryFinder(), $deck)->shapeNames(2))->toBe(['line']); +}); + +it('refuses a templates file the source answers a path for but does not have', function () { + // A finder that answers a path for a file that has since gone: the templates file is + // not there, and the library says so rather than reading nothing. + $finder = new class implements FileFinder { + public function read(string $name, ?string $role = null): ?string + { + return null; + } + + public function path(string $name, ?string $role = null): ?string + { + return '/mdpresentation-not-here/templates.md'; + } + + public function themes(): array + { + return []; + } + + public function files(string $theme): array + { + return []; + } + }; + + $deck = "---\ntemplates-file: templates.md\nlayout: blank\n---\n"; + + expect(templateFailureFrom($finder, $deck))->toContain('There is no templates file at'); +}); + it('refuses a templates file that is larger than the library will read', function () { $templates = Scratch::file('templates.md', str_repeat('a', TemplatesFile::MAX_BYTES + 1)); diff --git a/tests/Unit/theme-loader.php b/tests/Unit/theme-loader.php index 796fc9b..b4e9d6c 100644 --- a/tests/Unit/theme-loader.php +++ b/tests/Unit/theme-loader.php @@ -190,7 +190,7 @@ ])); expect($loaded->layout('two-column'))->toBe('Two Content') - ->and($loaded->template())->toBe($loaded->directory() . '/template.pptx'); + ->and($loaded->template()?->path())->toBe($loaded->directory() . '/template.pptx'); }); it('refuses a file of a theme that resolves outside the scope', function () { From 901493e7fd4c6eefac2721ec990a495bf79796c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabian=20Gra=C3=9Fl?= Date: Thu, 29 Oct 2026 23:43:58 +0100 Subject: [PATCH 5/8] The theme is named in code, and the deck comes back as bytes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two things a program drawing decks could not do: name the theme itself, and take the deck away as bytes rather than as a path. `PresentationRenderer::withTheme(string $name): self` names the theme, and it outranks the deck's own `theme:` — the same rule the Word half follows, where a theme named in code answers for the document. The name is resolved through the finder when it is given and the theme that came back is kept, so the name is asked for once and not once for the name and once for the deck. The deck's frontmatter is still checked: this half refuses an unknown key or a wrong-typed value by name, and a deck writing `theme: 42` is still refused whatever theme it is drawn in. `PresentationRenderer::toString(Deck $deck): string` is the counterpart of the Word half's `toDocx()`: the deck, as the bytes of a `.pptx`. A `.pptx` is a package, and PHP's `ZipArchive` cannot write one to memory — `open('php://temp')` answers 9 and `open('php://temp', CREATE | OVERWRITE)` fails at `close()` — and PHPPresentation's writer takes a path. So the deck is written to a file of the library's own, read back and given back, at the mode of a file nobody else on the machine can read. Nothing of the library's is left on the disk: neither the file the deck was written to, nor the master or the pictures the render staged, whether the deck was written or the write failed. A theme kept across renders keeps its master too, and the master is written again when it is asked for: the file the library staged for it is given back at the end of every render, and the render after that needs it there. The rule the two halves' finders follow is this half's: a name that is already a path is taken as it stands, whatever the role it is asked for in. The Word half follows it now, and the tests here pin it for every role — the `theme` role above all, where a name is `/` and an absolute one used to be split on its first slash. --- CHANGELOG.md | 29 +++++++++ README.md | 32 +++++++++- src/Render/PresentationRenderer.php | 90 +++++++++++++++++++++++++- src/Render/Staging.php | 68 ++++++++++++++++++++ src/Theme/Master.php | 43 +++++++++---- tests/Unit/file-finder.php | 63 +++++++++++++++++++ tests/Unit/master-finder.php | 21 +++++++ tests/Unit/theme-finder.php | 53 ++++++++++++++++ tests/Unit/to-string.php | 98 +++++++++++++++++++++++++++++ 9 files changed, 483 insertions(+), 14 deletions(-) create mode 100644 src/Render/Staging.php create mode 100644 tests/Unit/to-string.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 11a2a79..3887c66 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,29 @@ All notable changes to this project are documented here. The format follows ### Added +- **A theme can be named in code, and it outranks the deck's own.** + `PresentationRenderer::withTheme(string $name): self` names the theme the decks are + drawn in, the way the Word half's `MarkdownToWord::withTheme()` does. + + A theme named in code answers for the deck, so the deck's own `theme:` is not read — + but it is still checked, because a value of the wrong type is a mistake in the deck + whatever theme the deck is drawn in: a deck writing `theme: 42` is refused. + + The name is resolved through the finder when it is given, and the theme that came back + is kept, so the name is asked for once and not once for the name and once for the + deck. A theme that does not resolve is refused when the name is given rather than at + the render, so a typo is reported before the deck is read. + +- **A deck can come back as bytes.** `PresentationRenderer::toString(Deck $deck): string` + is the counterpart of the Word half's `MarkdownToWord::toDocx()`: the deck, as the + bytes of a `.pptx`. + + A `.pptx` is a package, and PHP's `ZipArchive` cannot write one to memory — + `open('php://temp')` answers 9 and `open('php://temp', CREATE | OVERWRITE)` fails at + `close()` — and PHPPresentation's writer takes a path. The deck is therefore written + to a file of the library's own, read back and given back: nothing of the library's is + left on the disk, whether the deck was written or the write failed. + - **The furniture of a deck can be written once.** A wordmark, a rule and a page number were the same on every slide of a deck, and writing them out on every slide meant changing them on every slide: twenty-eight slides to find and twenty-eight edits to @@ -107,6 +130,12 @@ All notable changes to this project are documented here. The format follows ### Changed +- **A staged master is written again when it is asked for.** The file the library + staged for a theme's master is given back at the end of every render, so a theme kept + across renders — one the caller named in code — had no master to open the second + time. The master holds the source's bytes and writes them again when the file it + staged is gone, at the mode of a file nobody else on the machine can read. + - **`ThemeLoader::load()` and `loadManifest()` take a name and a finder.** The third parameter was the scope a theme's own files were held to and is now the finder they are read through; the first was a directory and is now the name the theme is known diff --git a/README.md b/README.md index 9d7efdc..3e35656 100644 --- a/README.md +++ b/README.md @@ -267,7 +267,8 @@ Slide 1 opens a "::note" that is not the last thing on the slide. A note is "::n ## Rendering `PresentationRenderer` takes a split deck and writes a `.pptx`. It resolves the theme -itself, from the deck's `theme:`, so there is nothing to resolve first. +itself, from the deck's `theme:` or from the name a caller gave it, so there is nothing to +resolve first. ```php use MarkdownPresentation\Document\DeckParser; @@ -303,6 +304,31 @@ A deck that names no theme gets the built-in `default`. That is the absence of a not a fallback from a directive that failed: a theme that does not resolve is still a failure, and it names the themes that do. +**A theme can be named in code, and it outranks the deck's own.** A deck names a theme in +its front matter, and a program that draws several decks may want one theme for all of +them: + +```php +$renderer = (new PresentationRenderer())->withTheme('corporate-dark'); +``` + +The name is resolved through the finder when it is given, and the theme that came back is +kept: the name is asked for once, and not once for the name and once for the deck. The +deck's own `theme:` is not read — but it is checked, because a value of the wrong type is +a mistake in the deck whatever theme the deck is drawn in. + +**A deck can come back as bytes.** `render()` writes the deck to a path and hands the path +back; `toString()` hands back the bytes of the same deck: + +```php +$bytes = (new PresentationRenderer())->toString($deck); +``` + +A `.pptx` is a package, and PHP's `ZipArchive` cannot write one to memory, so the deck is +written to a file of the library's own and read back from it. Nothing of the library's is +left on the disk: neither the file the deck was written to, nor the master or the pictures +the render staged. + ### What a slide becomes Every shape of a slide is named after the kind of thing it was drawn from, so the mapping can @@ -2639,6 +2665,10 @@ list of bases per role, tried in the order they are given. The directories of th on disk remain the fallback, so a program with a finder of its own does not lose the themes that ship with the package. +**A name that is already a path is the path, whatever the role.** A name beginning with +`/` names a file, and resolving it against a base would name a file nobody meant — so it +is taken as it stands, in the `theme` role as much as in any other. + **A theme's master is a file, and a file the library stages.** A `template.pptx` is a package, and a package is opened as a file: a source that holds the master as bytes rather than as a file has them staged into one, and the deck is drawn in the master's diff --git a/src/Render/PresentationRenderer.php b/src/Render/PresentationRenderer.php index bd4af89..bbc699f 100644 --- a/src/Render/PresentationRenderer.php +++ b/src/Render/PresentationRenderer.php @@ -203,6 +203,14 @@ final class PresentationRenderer */ private Theme $theme; + /** + * The theme {@see self::withTheme()} resolved, kept so that the name is resolved once. + * + * A source that counts its reads would say so: the name is asked for when it is given, + * and the theme that came back is the one the deck is drawn in. + */ + private ?LoadedTheme $resolved = null; + /** * The master the deck being rendered was drawn in. * @@ -261,6 +269,41 @@ public function __construct( $this->chrome = new Chrome($this->blocks, $this->directory, $this->scope, $files); } + /** + * A theme to draw this renderer's decks in, whatever the deck's own frontmatter says. + * + * The same rule the Word half follows: a theme named in code answers for the deck, so + * the frontmatter's `theme:` is not read — the name in the code is the one the deck is + * drawn in. The frontmatter's value is still checked, because a value of the wrong + * type is a mistake in the deck whatever theme the deck is drawn in: a deck writing + * `theme: 42` is refused. + * + * The name is resolved through the finder when it is given, and the theme that came + * back is kept: a source that counts its reads would say the name is asked for once and + * not once for the name and once for the deck. A theme that does not resolve is + * refused here rather than at the render, so a typo is reported before the deck is read. + * + * Mutating rather than returning a new renderer: the renderer holds the deck's + * directory and its sources, and there is no value in a copy that differs in a theme. + * + * @throws ThemeNotFound When the theme is not one that resolves. + * @throws \MarkdownPresentation\Exception\MalformedTheme When the theme's own files do + * not describe a theme. + * @throws UnknownLayoutRole When the theme names a layout role that does not exist. + * @throws \MarkdownPresentation\Exception\UnknownStyleSlot When the theme names a + * style slot that does not exist. + * @throws \MarkdownPresentation\Exception\UnservedTemplate When the theme carries a + * master the source of the theme cannot serve. + * @throws \MarkdownPresentation\Exception\Exception When one of the theme's files + * cannot be read or parsed. + */ + public function withTheme(string $name): self + { + $this->resolved = $this->themes->resolve($name); + + return $this; + } + /** * Every block the last render had to make smaller than it asked to be. * @@ -366,6 +409,41 @@ public function render(Deck $deck, string $path): string } } + /** + * The deck, as the bytes of a `.pptx`. + * + * The counterpart of the Word half's `MarkdownToWord::toDocx()`: a deck that comes + * back as bytes rather than as a path. + * + * A `.pptx` is a package, and PHP's `ZipArchive` cannot write one to memory, and + * PHPPresentation's writer takes a path — so the deck is written to a file of the + * library's own, read back and given back. Nothing of the library's is left on the + * disk: neither the file the deck was written to, nor the master or the pictures the + * render staged, whether the deck was written or the write failed. + * + * @throws InvalidInput When a slide names something this library cannot render. + * @throws ThemeNotFound When the deck names a theme that does not resolve. + * @throws ContentOverflow When a block does not fit and the renderer is strict. + * @throws MalformedTemplate When the theme's `template.pptx` cannot be read. + * @throws TooManySlides When the deck asks for more slides than {@see self::MAX_SLIDES}. + * @throws \MarkdownPresentation\Exception\Exception When the deck cannot be written. + * + * @throws \MarkdownPresentation\Exception\MalformedShape When a free-placement directive + * names an attribute it does not read, or gives one a value it cannot be. + * + * @throws \MarkdownPresentation\Exception\UnpatchedDependency When the installed + * PHPPresentation lacks what this library needs of it. + * + * @throws \MarkdownPresentation\Exception\UnservedTemplate When the theme carries a + * master the source of the theme cannot serve. + */ + public function toString(Deck $deck): string + { + return Staging::of(function (string $path) use ($deck): void { + $this->render($deck, $path); + }); + } + /** * The deck, drawn and written. * @@ -596,9 +674,15 @@ private function check(SourceSlide $slide): void } /** + * The theme the deck is drawn in. + * + * A theme named in code answers for the deck, so the frontmatter's `theme:` is not + * read — but it is checked, because a value of the wrong type is a mistake in the deck + * whatever theme the deck is drawn in. + * * @throws ThemeNotFound * @throws MalformedFrontmatter - */ + */ private function loaded(Deck $deck): LoadedTheme { $name = $deck->frontmatterValue('theme', self::DEFAULT_THEME); @@ -607,6 +691,10 @@ private function loaded(Deck $deck): LoadedTheme throw MalformedFrontmatter::wrongType(1, 'theme', 'a theme name'); } + if ($this->resolved !== null) { + return $this->resolved; + } + return $this->themes->resolve(trim($name)); } diff --git a/src/Render/Staging.php b/src/Render/Staging.php new file mode 100644 index 0000000..e667c1b --- /dev/null +++ b/src/Render/Staging.php @@ -0,0 +1,68 @@ +bytes !== null && !is_file($this->path)) { + @file_put_contents($this->path, $this->bytes); + @chmod($this->path, 0o600); + } + return $this->path; } @@ -77,14 +102,8 @@ public function name(): string */ public function release(): void { - if ($this->owned) { - // Given back once: the render asks for it and the destructor asks again, and a - // second removal of a file that is gone is a warning nobody can act on. - $this->owned = false; - - if (is_file($this->path)) { - @unlink($this->path); - } + if ($this->owned && is_file($this->path)) { + @unlink($this->path); } } diff --git a/tests/Unit/file-finder.php b/tests/Unit/file-finder.php index c40005e..410e47b 100644 --- a/tests/Unit/file-finder.php +++ b/tests/Unit/file-finder.php @@ -3,6 +3,7 @@ declare(strict_types=1); use MarkdownPresentation\Files\DiskFinder; +use MarkdownPresentation\Files\FileFinder; use MarkdownPresentation\Tests\Support\Scratch; /* @@ -87,6 +88,45 @@ function finderBase(string $name): string ->and($finder->read($base . '/nowhere.md', 'document'))->toBeNull(); }); +it('takes a name that is already a path as it stands, whatever the role', function () { + // The theme role is where this is easiest to get wrong, because a theme's name is + // split into a theme and a file: the role says how a name is resolved, and a name + // that is already a path is not resolved at all. + $base = Scratch::directoryPath('absolute-role'); + $outside = Scratch::directoryPath('absolute-outside'); + + mkdir($base . '/house'); + file_put_contents($base . '/house/theme.yml', "name: House\n"); + file_put_contents($outside . '/theme.yml', "name: Outside\n"); + + $finder = new DiskFinder(theme: [$base], picture: [$base], document: [$base]); + + expect($finder->read($outside . '/theme.yml', 'theme'))->toBe("name: Outside\n") + ->and($finder->path($outside . '/theme.yml', 'theme'))->toBe($outside . '/theme.yml') + ->and($finder->read($outside . '/theme.yml', 'picture'))->toBe("name: Outside\n") + ->and($finder->read($outside . '/theme.yml', 'document'))->toBe("name: Outside\n") + ->and($finder->read($outside . '/nowhere.yml', 'theme'))->toBeNull() + ->and($finder->path($outside . '/nowhere.yml', 'theme'))->toBeNull() + // And the theme of the base is still where it was. + ->and($finder->read('house/theme.yml', 'theme'))->toBe("name: House\n"); +}); + +it('does not split a name that is already a path into a theme and a file', function () { + // The leading slash is not a theme whose name is the empty string, and the rest of + // the name is not a file inside the first base: a name that is already a path is + // the path. + $base = Scratch::directoryPath('split'); + mkdir($base . '/house'); + file_put_contents($base . '/house/theme.yml', "name: House\n"); + + $finder = new DiskFinder(theme: [$base]); + + expect($finder->path('/house/theme.yml', 'theme')) + ->not->toBe($base . '/house/theme.yml') + // And the theme of that name is still the theme it is. + ->and($finder->read('house/theme.yml', 'theme'))->toBe("name: House\n"); +}); + it('lists the themes that hold a manifest, sorted', function () { $base = Scratch::directoryPath('themes'); @@ -244,3 +284,26 @@ function finderBase(string $name): string ->and($finder->read("logo.png\0.txt", 'picture'))->toBeNull() ->and($finder->path('../elsewhere/secret.txt', 'document'))->toBeNull(); }); + +it('gives the finder the shape an adapter is written against', function () { + // One adapter serves both halves of this suite, so this interface is the shape both + // of them promise, and a change to it is a change to both. + $shape = []; + + foreach ((new ReflectionClass(FileFinder::class))->getMethods() as $method) { + $parameters = []; + + foreach ($method->getParameters() as $parameter) { + $parameters[] = $parameter->getName() . ': ' . (string) $parameter->getType(); + } + + $shape[$method->getName()] = implode(', ', $parameters) . ' => ' . (string) $method->getReturnType(); + } + + expect($shape)->toBe([ + 'read' => 'name: string, role: ?string => ?string', + 'path' => 'name: string, role: ?string => ?string', + 'themes' => ' => array', + 'files' => 'theme: string => array', + ]); +}); diff --git a/tests/Unit/master-finder.php b/tests/Unit/master-finder.php index 9f6a7dd..f6aca84 100644 --- a/tests/Unit/master-finder.php +++ b/tests/Unit/master-finder.php @@ -209,3 +209,24 @@ public function files(string $theme): array expect(Pptx::open($path)->positionOf('title')['x']) ->toBe((float) (new SlideGeometry())->title(LayoutRole::Content)['box']->x); }); + +it('draws a deck in a theme the caller named, again', function () { + // The theme is kept, and so is the master it carries: the file the library staged for + // it is given back at the end of the render, and the render after that writes it again. + $finder = new MemoryFinder([ + 'house/theme.yml' => (string) file_get_contents(masterTheme() . '/theme.yml'), + 'house/layouts.yml' => (string) file_get_contents(masterTheme() . '/layouts.yml'), + 'house/template.pptx' => (string) file_get_contents(masterTheme() . '/template.pptx'), + ]); + + $renderer = (new PresentationRenderer(themes: masterResolver($finder)))->withTheme('house'); + + $first = Scratch::file('kept-first.pptx', ''); + $second = Scratch::file('kept-second.pptx', ''); + + $renderer->render((new DeckParser())->parse("# One\n"), $first); + $renderer->render((new DeckParser())->parse("# Two\n"), $second); + + expect(Pptx::open($first)->positionOf('title')['x'])->toBe(66.0) + ->and(Pptx::open($second)->positionOf('title')['x'])->toBe(66.0); +}); diff --git a/tests/Unit/theme-finder.php b/tests/Unit/theme-finder.php index a19c501..5c57147 100644 --- a/tests/Unit/theme-finder.php +++ b/tests/Unit/theme-finder.php @@ -2,11 +2,16 @@ declare(strict_types=1); +use MarkdownPresentation\Document\DeckParser; +use MarkdownPresentation\Exception\MalformedFrontmatter; use MarkdownPresentation\Exception\ThemeNotFound; use MarkdownPresentation\Files\DiskFinder; use MarkdownPresentation\Files\FileFinder; use MarkdownPresentation\Filesystem\PathScope; +use MarkdownPresentation\Render\PresentationRenderer; use MarkdownPresentation\Tests\Support\MemoryFinder; +use MarkdownPresentation\Tests\Support\Pptx; +use MarkdownPresentation\Tests\Support\Scratch; use MarkdownPresentation\Tests\Support\ThemeDirectory; use MarkdownPresentation\Theme\ThemeLoader; use MarkdownPresentation\Theme\ThemeResolver; @@ -179,3 +184,51 @@ public function files(string $theme): array expect(fn () => finderResolver($finder)->resolve('huse')) ->toThrow(ThemeNotFound::class, 'There is no theme named "huse". Available: house.'); }); + +it('draws a deck in the theme the caller named, not the one the deck named', function () { + // A theme named in code answers for the deck, so the deck's own `theme:` is not read. + // The two themes are the ones that ship, and they are told apart by the page behind + // every slide: one is light and one is dark. + $deck = (new DeckParser())->parse("---\ntheme: minimal\n---\n\n# One\n"); + + $path = Scratch::file('caller-theme.pptx', ''); + + (new PresentationRenderer())->withTheme('corporate-dark')->render($deck, $path); + + $master = Pptx::open($path)->part('ppt/slideMasters/slideMaster1.xml'); + + expect($master)->toMatch('#.*?#s') + ->and($master)->not->toMatch('#.*?#s'); +}); + +it('refuses a frontmatter theme of the wrong type, whatever the caller named', function () { + // The caller's theme answers for the deck, but a value of the wrong type is a mistake + // in the deck whatever theme the deck is drawn in. + $deck = (new DeckParser())->parse("---\ntheme: 42\n---\n\n# One\n"); + + expect(fn () => (new PresentationRenderer())->withTheme('corporate-dark')->render($deck, Scratch::file('wrong-type.pptx', ''))) + ->toThrow(MalformedFrontmatter::class, 'has a "theme" that is not a theme name'); +}); + +it('resolves the theme the caller named once', function () { + // The name is asked for when it is given, and the theme that came back is the one the + // deck is drawn in: a source that counts its reads says the name is asked for once and + // not once for the name and once for the deck. + $finder = new MemoryFinder([ + 'house/theme.yml' => "name: House\n", + 'house/layouts.yml' => "layouts:\n content: Title and Content\n", + ]); + + $renderer = (new PresentationRenderer(themes: finderResolver($finder)))->withTheme('house'); + + expect($finder->reads('house/theme.yml'))->toBe(1); + + // The deck names the theme as well, so a render that resolved the frontmatter's name + // would read it a second time. + $renderer->render( + (new DeckParser())->parse("---\ntheme: house\n---\n\n# One\n"), + Scratch::file('resolved-once.pptx', ''), + ); + + expect($finder->reads('house/theme.yml'))->toBe(1); +}); diff --git a/tests/Unit/to-string.php b/tests/Unit/to-string.php new file mode 100644 index 0000000..3269dcf --- /dev/null +++ b/tests/Unit/to-string.php @@ -0,0 +1,98 @@ + + */ +function stagedFiles(): array +{ + return glob(sys_get_temp_dir() . '/mdpres_*') ?: []; +} + +it('returns the bytes of a deck that opens as one', function () { + // The bytes are a deck, and they are the deck: compared by a part rather than by the + // archive, because a zip carries two-second timestamps and `docProps/core.xml` carries + // the clock. + $deck = (new DeckParser())->parse("# One\n\nSome text.\n"); + + $written = Scratch::file('written.pptx', ''); + (new PresentationRenderer())->render($deck, $written); + + $read = Scratch::file('read.pptx', ''); + file_put_contents($read, (new PresentationRenderer())->toString($deck)); + + expect(Pptx::open($read)->part('ppt/slides/slide1.xml')) + ->toBe(Pptx::open($written)->part('ppt/slides/slide1.xml')); +}); + +it('leaves nothing on the disk behind it', function () { + // The deck was written to a file of the library's own, and the file is given back. + $before = stagedFiles(); + + (new PresentationRenderer())->toString((new DeckParser())->parse("# One\n")); + + expect(stagedFiles())->toBe($before); +}); + +it('leaves nothing on the disk behind it when the deck is refused', function () { + // A deck that cannot be drawn is a failure with no file behind it — not even the + // temporary one the bytes would have been read from. + $before = stagedFiles(); + + expect(fn () => (new PresentationRenderer())->toString((new DeckParser())->parse("---\nlayout: nonsense\n---\n\n# One\n"))) + ->toThrow(UnknownLayoutRole::class); + + expect(stagedFiles())->toBe($before); +}); + +it('leaves nothing on the disk behind it, master and all', function () { + // The master the theme carries was staged for the render, and it goes with it: the + // render is over, and a file of the library's own does not outlive it. + $before = stagedFiles(); + + $finder = new MemoryFinder([ + 'house/theme.yml' => (string) file_get_contents(masterTheme() . '/theme.yml'), + 'house/layouts.yml' => (string) file_get_contents(masterTheme() . '/layouts.yml'), + 'house/template.pptx' => (string) file_get_contents(masterTheme() . '/template.pptx'), + ]); + + (new PresentationRenderer(themes: masterResolver($finder))) + ->toString((new DeckParser())->parse("---\ntheme: house\n---\n\n# One\n")); + + expect(stagedFiles())->toBe($before); +}); + +it('says so when the deck it wrote cannot be read back', function () { + // A deck that was not written is a failure and not an empty deck: bytes that open + // as nothing are the one answer a caller cannot act on. + expect(fn () => Staging::of(function (string $path): void { + unlink($path); + }))->toThrow(Exception::class, 'Unable to read the generated deck.'); +}); From b26432326102a6ec69f1b3e9cd301c00ab7e7891 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabian=20Gra=C3=9Fl?= Date: Fri, 30 Oct 2026 01:05:00 +0100 Subject: [PATCH 6/8] A method that does two things is split into two that each say what they do MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `php:S1142` fires on `DiskFinder::resolve()` and on `TemplatesFile::fromSource()`: both had four returns, and the rule allows three. Neither was one method doing one thing, so neither is closed by folding the returns into one. `DiskFinder::resolve()` asks three questions: is the name already a path, is it a theme's file, and which base holds it. The third now goes to a helper — `inWorlds()` — which is the shape the Word half's copy of this finder already has, so the two halves answer the same questions of the same names. The list of bases went with it: `bases()` is `worlds()` now, so the helper and the list it walks are named for the same thing. `TemplatesFile::fromSource()` asked a source for a path and, failing that, for the contents: two rules, two failures and two ceilings in one method. `fromPath()` holds the path to the scope and reads the file; `fromContents()` takes the contents on the caller's word and measures them against the ceiling. The path is asked for first, because a source that answers one holds the file as a file, and only a source that answers none holds the contents. Nothing about the answers changes. The same names resolve to the same paths, the same refusals carry the same words, and the two halves of the suite were set against each other over every role, every kind of name and every base — and every answer was the same. --- src/Files/DiskFinder.php | 15 ++++++++-- src/Template/TemplatesFile.php | 55 ++++++++++++++++++++++++++-------- 2 files changed, 55 insertions(+), 15 deletions(-) diff --git a/src/Files/DiskFinder.php b/src/Files/DiskFinder.php index 2e02635..d0435a1 100644 --- a/src/Files/DiskFinder.php +++ b/src/Files/DiskFinder.php @@ -157,7 +157,15 @@ private function resolve(string $name, ?string $role): ?string return $this->resolveTheme($name); } - foreach ($this->bases($role) as $base) { + return $this->inWorlds($name, $role); + } + + /** + * The file one of the worlds holds, or null when none of them does. + */ + private function inWorlds(string $name, ?string $role): ?string + { + foreach ($this->worlds($role) as $base) { $path = rtrim($base, '/') . '/' . $name; if (is_file($path)) { @@ -226,9 +234,12 @@ private function themeDirectory(string $theme): ?string * order the roles are declared, and is resolved as a plain name — the role is * what says how a name is resolved. * + * The theme role is not among them: a theme is resolved by its directory rather + * than by a file in a base, which {@see self::resolveTheme()} is where happens. + * * @return list */ - private function bases(?string $role): array + private function worlds(?string $role): array { return match ($role) { null => [...$this->theme, ...$this->picture, ...$this->document], diff --git a/src/Template/TemplatesFile.php b/src/Template/TemplatesFile.php index 6ec6a01..2f4734e 100644 --- a/src/Template/TemplatesFile.php +++ b/src/Template/TemplatesFile.php @@ -90,32 +90,61 @@ public function read(string $written): string /** * The templates file in the source the deck's files come from, or null when it has none. * - * A path the source answers is held to the scope, exactly as a path on disk is: a - * `templates-file:` is written by the deck, and what is allowed is where it lands. + * The path is asked for first, because a source that answers one holds the file as + * a file — read from where it is and held to the scope — and only a source that + * answers none holds the contents. */ private function fromSource(string $written): ?string { - if ($this->files === null) { + $source = $this->files; + + if ($source === null) { return null; } - $path = $this->files->path($written, 'document'); + $path = $source->path($written, 'document'); if ($path !== null) { - if (!is_file($path)) { - throw TemplatesFileNotFound::at($path); - } + return $this->fromPath($written, $path); + } + + return $this->fromContents($written, $source); + } - $resolved = PathScope::canonical($path); + /** + * The Markdown of the file the source holds at a path. + * + * The path is held to the scope, so a source may not reach a file this render may + * not read. + * + * @throws TemplatesFileNotFound + */ + private function fromPath(string $written, string $path): string + { + if (!is_file($path)) { + throw TemplatesFileNotFound::at($path); + } - if ($resolved === null || !$this->scope->holds($resolved)) { - throw TemplatesFileNotFound::outside($written, $resolved ?? $path, $this->scope->roots()); - } + $resolved = PathScope::canonical($path); - return $this->readFile($written, $resolved); + if ($resolved === null || !$this->scope->holds($resolved)) { + throw TemplatesFileNotFound::outside($written, $resolved ?? $path, $this->scope->roots()); } - $markdown = $this->files->read($written, 'document'); + return $this->readFile($written, $resolved); + } + + /** + * The Markdown the source holds as contents, or null when it holds none. + * + * There is no path to hold to the scope here, so the ceiling is the only rule the + * contents are measured against. + * + * @throws TemplatesFileTooLarge + */ + private function fromContents(string $written, FileFinder $source): ?string + { + $markdown = $source->read($written, 'document'); if ($markdown === null) { return null; From a5ca218134fd78e770747f5d175870e6343ff98b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabian=20Gra=C3=9Fl?= Date: Fri, 30 Oct 2026 01:14:19 +0100 Subject: [PATCH 7/8] The renderer is split, and the theme and the faces are classes of their own `php:S1448` says the renderer has too many methods, and the finding says what to do about it: split it. Twenty-three was over the ceiling of twenty. The theme a deck is drawn in is a thing the renderer works with rather than a step of the walk that draws it, and so are the faces the deck, its theme and its chrome name. `Render\DeckTheme` resolves the theme, reads the role of each slide and takes the boxes a theme's own `template.pptx` gives them; `Render\DeckFaces` finds the faces there are no advance widths for and records where each of them was named. Nothing a caller can see has changed: the same methods, the same messages and the same order. A deck over the ceiling is still refused before its theme is resolved, and nothing the library staged is left on the disk whether the deck was written or refused. A test pins the order, because a split that moved the theme ahead of the count would answer with the theme rather than the ceiling, and nothing else would say so. --- CHANGELOG.md | 15 ++ src/Render/DeckFaces.php | 151 ++++++++++++++ src/Render/DeckTheme.php | 201 ++++++++++++++++++ src/Render/PresentationRenderer.php | 313 +++------------------------- tests/Unit/layouts-and-steps.php | 16 ++ 5 files changed, 411 insertions(+), 285 deletions(-) create mode 100644 src/Render/DeckFaces.php create mode 100644 src/Render/DeckTheme.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 3887c66..94f24bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -130,6 +130,21 @@ All notable changes to this project are documented here. The format follows ### Changed +- **`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. + + `Render\DeckTheme` is the theme a deck is drawn in — the name resolved in code or in + the deck's own frontmatter, the role of each of its slides, and the boxes a theme's + own `template.pptx` gives those roles. `Render\DeckFaces` is the faces a theme, a + deck and a template's chrome name, and the report of the ones there are no advance + widths for. Neither is a step of the walk that draws the deck: they are the things + the walk works with. + + Nothing a caller can see has changed. The same methods, the same messages and the + same order — a deck over the ceiling is still refused before its theme is resolved, + and nothing the library staged is left on the disk whether the deck was written or + refused. + - **A staged master is written again when it is asked for.** The file the library staged for a theme's master is given back at the end of every render, so a theme kept across renders — one the caller named in code — had no master to open the second diff --git a/src/Render/DeckFaces.php b/src/Render/DeckFaces.php new file mode 100644 index 0000000..5aff3ce --- /dev/null +++ b/src/Render/DeckFaces.php @@ -0,0 +1,151 @@ +fonts()->get($slot); + + if ($face !== null && !$metric->has($face)) { + $this->fonts->record($face, sprintf("the theme's %s slot", $slot), $metric->faces()); + } + } + + foreach ($theme->textStyleFaces() as $name => $face) { + if (!$metric->has($face)) { + $this->fonts->record($face, sprintf('the text style "%s"', $name), $metric->faces()); + } + } + } + + /** + * The faces the deck named that the library has no table of advance widths for, + * recorded. + * + * A box the deck placed itself may name a face of its own with `font-family`, and that + * face is the one its words are drawn in and measured in. It is in the deck rather than + * in the theme, so it does not exist when the theme is resolved — it exists once the + * deck has been read, which is before anything is built and therefore before a byte + * reaches the disk. + * + * The box is named rather than the slide it is on, because a deck that writes two boxes + * of the same kind is two boxes and a reader who is told which one to change can find + * it; the slide is named as well, because two boxes of the same kind on two slides are + * two places and one of them is not the one meant. + * + * @param array $blocks The deck's slides, as they were read. + */ + public function deck(array $blocks, TextMetric $metric): void + { + foreach ($blocks as $index => $slide) { + foreach ($slide->shapes() as $shape) { + $face = self::faceOf($shape); + + if ($face !== null && !$metric->has($face)) { + $this->fonts->record( + $face, + sprintf('the "%s" box on slide %d', $shape->name(), $index + 1), + $metric->faces(), + ); + } + } + } + } + + /** + * The faces the chrome of a template named that the library has no table of advance + * widths for, recorded. + * + * The chrome of a template is held apart from the shapes of the slides: it is read once + * for the deck and drawn on every slide that names the template, so a walk over the + * slides never sees it. + * + * One entry for the whole template rather than one for each of the slides that draw it, + * because the face is named in one place: in the chrome. Twenty-eight slides carrying + * one wordmark is one name to change, and a report of twenty-eight entries would say so + * twenty-eight times and hide the fact that there is one thing to do. The place is + * named instead of the slide, so the reader is sent to the template rather than to one + * of the slides that happens to draw it. + * + * @param array> $drawn The chrome every template draws. + */ + public function chrome(array $drawn, TextMetric $metric): void + { + foreach ($drawn as $template => $shapes) { + foreach ($shapes as $shape) { + $face = self::faceOf($shape); + + if ($face !== null && !$metric->has($face)) { + $this->fonts->record( + $face, + sprintf('the "%s" box of the template "%s"', $shape->name(), $template), + $metric->faces(), + ); + } + } + } + } + + /** + * The face a shape names for the type in it, or null where it names none. + * + * Two classes carry a {@see \MarkdownPresentation\Render\Free\TextStyle}: the text a + * deck wrote and the number the renderer writes. Both are drawn in the face their style + * names and measured in it, so both are a place a face is named — and a walk that + * looked at one of them would miss the other. + */ + private static function faceOf(FreeShape $shape): ?string + { + return match (true) { + $shape instanceof FreeText => $shape->style->face, + $shape instanceof FreeSlideNumber => $shape->style->face, + default => null, + }; + } +} diff --git a/src/Render/DeckTheme.php b/src/Render/DeckTheme.php new file mode 100644 index 0000000..df3cb61 --- /dev/null +++ b/src/Render/DeckTheme.php @@ -0,0 +1,201 @@ +resolved = $this->themes->resolve($name); + } + + /** + * The theme the deck is drawn in. + * + * A theme named in code answers for the deck, so the frontmatter's `theme:` is not + * read — but it is checked, because a value of the wrong type is a mistake in the deck + * whatever theme the deck is drawn in. + * + * @throws ThemeNotFound + * @throws MalformedFrontmatter + */ + public function load(Deck $deck): LoadedTheme + { + $name = $deck->frontmatterValue('theme', self::DEFAULT_THEME); + + if (!is_string($name) || trim($name) === '') { + throw MalformedFrontmatter::wrongType(1, 'theme', 'a theme name'); + } + + if ($this->resolved !== null) { + return $this->resolved; + } + + return $this->themes->resolve(trim($name)); + } + + /** + * The role of each of the deck's slides, by index. + * + * Resolved before anything is built so that a role the theme does not map fails while + * there is still no file on disk to clean up. + * + * @return array + * + * @throws MalformedFrontmatter + * @throws UnknownLayoutRole + */ + public function roles(Deck $deck, LoadedTheme $theme): array + { + $roles = []; + + foreach ($deck->slides() as $slide) { + $named = $slide->frontmatterValue('layout', LayoutRole::Content->value); + + if (!is_string($named) || trim($named) === '') { + throw MalformedFrontmatter::wrongType( + $slide->index() + 1, + 'layout', + sprintf('a layout role — one of: %s', implode(', ', LayoutRole::names())), + ); + } + + $role = trim($named); + + if (LayoutRole::tryFrom($role) === null) { + throw UnknownLayoutRole::notARoleOn($slide->index() + 1, $role, LayoutRole::names()); + } + + // Asked of the theme rather than checked here, because a role the theme does not + // map is the theme's to fix and its message already says so. + $theme->layout($role); + + $roles[$slide->index()] = LayoutRole::from($role); + } + + return $roles; + } + + /** + * The roles the deck's slides use, in first-seen order and each of them once. + * + * Only the roles that are used are built. A theme that maps some of them would + * otherwise fail on the ones its deck never asked for, which is not the same failure as a + * deck that asks for one it did not map. + * + * Compared on the role's value rather than with `array_unique`, which casts each element + * to a string and an enum cannot be cast to one. + * + * @param array $roles + * + * @return list + */ + public function usedRoles(array $roles): array + { + $used = []; + + foreach ($roles as $role) { + if (!in_array($role->value, array_column($used, 'value'), true)) { + $used[] = $role; + } + } + + return $used; + } + + /** + * The boxes a theme's own `template.pptx` gives the roles the deck uses. + * + * A theme ships one or ships none, and this is where that decides anything: the roles a + * template covers are drawn in the template's geometry and the rest keep + * {@see SlideGeometry}'s, which is the precedence — a template fills in what it has and + * says nothing about what it does not, the same rule `layouts.yml` follows. Read before + * anything is built, so a template that cannot be read fails while there is still no file + * on disk to clean up. + * + * Keyed by the layout *name* rather than by the role, because that is what the template + * has: two roles a theme maps onto one name are one layout, and take the same boxes. + * + * @param list $roles + * + * @return array}> + * + * @throws MalformedTemplate + */ + public function template(LoadedTheme $theme, array $roles): array + { + $master = $theme->template(); + + if ($master === null) { + return []; + } + + // The name the master is known by, rather than the path it is in: a master the + // library staged is a file of its own making under a name of its own, and a reader + // told about that cannot tell which template it was. + $layouts = (new TemplateGeometry($master->path(), name: $master->name()))->boxes(); + $boxes = []; + + foreach ($roles as $role) { + $name = $theme->layout($role->value); + + if (isset($layouts[$name])) { + $boxes[$role->value] = $layouts[$name]; + } + } + + return $boxes; + } +} diff --git a/src/Render/PresentationRenderer.php b/src/Render/PresentationRenderer.php index bbc699f..877c602 100644 --- a/src/Render/PresentationRenderer.php +++ b/src/Render/PresentationRenderer.php @@ -32,8 +32,6 @@ use MarkdownPresentation\Render\CommonMark\BlockTranslator; use MarkdownPresentation\Render\CommonMark\InlineTranslator; use MarkdownPresentation\Render\Free\FreeShape; -use MarkdownPresentation\Render\Free\FreeSlideNumber; -use MarkdownPresentation\Render\Free\FreeText; use MarkdownPresentation\Template\Templates; use MarkdownPresentation\Template\TemplatesFile; use MarkdownPresentation\Theme\LayoutRole; @@ -78,12 +76,13 @@ * laid out, so it takes no column and is not measured against the layout's box — see * {@see SlideShapes}, which is the write order a `layer` decides. * - * Three things it does not do are somebody else's job, and all three classes that do them sit - * in this directory rather than being reached through a caller: {@see MasterLayouts} writes - * the master, the layouts and the document's metadata, {@see DeckWriter} writes the file and - * runs the passes over it, and {@see Furniture} reads the templates of the deck and the - * chrome they draw. What is left here is the walk: a deck's blocks, and the shapes they are - * drawn as. + * The work that is not the walk is somebody else's job, and the classes that do it sit in + * this directory rather than being reached through a caller: {@see MasterLayouts} writes the + * master, the layouts and the document's metadata, {@see DeckWriter} writes the file and + * runs the passes over it, {@see Furniture} reads the templates of the deck and the chrome + * they draw, {@see DeckTheme} resolves the theme and reads the role of each slide, and + * {@see DeckFaces} finds the faces that have no table of advance widths. What is left here + * is the walk: a deck's blocks, and the shapes they are drawn as. */ final class PresentationRenderer { @@ -102,7 +101,7 @@ final class PresentationRenderer public const DECK_DIRECTIVES = ['theme', 'author', 'description', 'templates-file']; /** The theme a deck that names none gets. */ - public const DEFAULT_THEME = 'default'; + public const DEFAULT_THEME = DeckTheme::DEFAULT_THEME; /** The step a slide with no marker in it is drawn at. */ private const FIRST_STEP = 1; @@ -162,8 +161,6 @@ final class PresentationRenderer */ private readonly ?FileFinder $files; - private readonly ThemeResolver $themes; - /** * The reader of the templates file a deck names. */ @@ -203,14 +200,6 @@ final class PresentationRenderer */ private Theme $theme; - /** - * The theme {@see self::withTheme()} resolved, kept so that the name is resolved once. - * - * A source that counts its reads would say so: the name is asked for when it is given, - * and the theme that came back is the one the deck is drawn in. - */ - private ?LoadedTheme $resolved = null; - /** * The master the deck being rendered was drawn in. * @@ -220,6 +209,16 @@ final class PresentationRenderer */ private ?Master $master = null; + /** + * The theme the deck is drawn in, and the roles of its slides. + */ + private readonly DeckTheme $deckTheme; + + /** + * The faces the deck, its theme and its chrome name. + */ + private readonly DeckFaces $faces; + /** * @param string $directory The directory a relative image path in the deck * resolves against, which is the directory the deck @@ -260,11 +259,12 @@ public function __construct( ) { // The theme of a deck sits beside it, and a relative image path in the deck // resolves against the same directory: a deck and its theme are one thing. - $this->themes = $themes->beside($this->directory); + $this->deckTheme = new DeckTheme($themes->beside($this->directory)); $this->scope = new PathScope([$this->directory, ...$imageRoots]); $this->files = $files; $this->overflows = new Overflows($strict); $this->fonts = new FontReport($strict); + $this->faces = new DeckFaces($this->fonts); $this->templatesFiles = new TemplatesFile($this->directory, $this->scope, files: $files); $this->chrome = new Chrome($this->blocks, $this->directory, $this->scope, $files); } @@ -299,7 +299,7 @@ public function __construct( */ public function withTheme(string $name): self { - $this->resolved = $this->themes->resolve($name); + $this->deckTheme->resolve($name); return $this; } @@ -393,7 +393,7 @@ public function render(Deck $deck, string $path): string $this->check($slide); } - $loaded = $this->loaded($deck); + $loaded = $this->deckTheme->load($deck); $this->master = $loaded->template(); try { @@ -460,14 +460,14 @@ public function toString(Deck $deck): string */ private function draw(Deck $deck, LoadedTheme $loaded, string $path): string { - $roles = $this->roles($deck, $loaded); + $roles = $this->deckTheme->roles($deck, $loaded); $this->theme = $loaded->theme(); // A theme naming a face there is no table for is measured with the reference face's // widths, and the strict renderer refuses the deck rather than measuring it with the // wrong face — so the faces are asked about before anything is built. $metric = new TextMetric(); - $this->recordUnknownFaces($this->theme, $metric); + $this->faces->theme($this->theme, $metric); // The templates before the slides are drawn, and the chrome before either: a slide // that names a template that is not there, and a template that cannot be drawn, are @@ -483,14 +483,14 @@ private function draw(Deck $deck, LoadedTheme $loaded, string $path): string $presentation = new PhpPresentation(); - $used = $this->usedRoles($roles); + $used = $this->deckTheme->usedRoles($roles); // The theme's type scale and spacing first, then the boxes a template gives for the // roles it covers: a template places, and placing is the later of the two answers. // The parentheses are load-bearing, not decoration: `new X()->y()` on one expression // is PHP 8.4 syntax, and `composer.json` promises 8.2, where it does not parse. Written // without them this file cannot be loaded at all, so every test that renders a deck // fails on the 8.2 job and nothing local says why. - $geometry = (new SlideGeometry(style: $this->theme->style()))->with($this->template($loaded, $used)); + $geometry = (new SlideGeometry(style: $this->theme->style()))->with($this->deckTheme->template($loaded, $used)); $layouts = MasterLayouts::build($presentation, $deck, $loaded, $used, $this->blocks->inline()); $writer = new ShapeWriter($this->theme, $geometry, $metric, $this->fonts); @@ -500,12 +500,12 @@ private function draw(Deck $deck, LoadedTheme $loaded, string $path): string // The faces the deck named, asked about once the deck has been read and before // anything is built: a box with a `font-family` of its own is drawn in it, and a box // measured with another face's widths is a box measured for type it does not hold. - $this->recordUnknownFacesOfTheDeck($blocks, $metric); + $this->faces->deck($blocks, $metric); // The faces the chrome named, asked about on the same terms: the chrome of a // template is drawn on the slides that name it, and the words in it are drawn in // the face it names. - $this->recordUnknownFacesOfTheChrome($metric); + $this->faces->chrome($this->drawn, $metric); $builds = []; @@ -609,50 +609,6 @@ private function blocks(Deck $deck, Theme $theme): array return $blocks; } - /** - * The boxes a theme's own `template.pptx` gives the roles the deck uses. - * - * A theme ships one or ships none, and this is where that decides anything: the roles a - * template covers are drawn in the template's geometry and the rest keep - * {@see SlideGeometry}'s, which is the precedence — a template fills in what it has and - * says nothing about what it does not, the same rule `layouts.yml` follows. Read before - * anything is built, so a template that cannot be read fails while there is still no file - * on disk to clean up. - * - * Keyed by the layout *name* rather than by the role, because that is what the template - * has: two roles a theme maps onto one name are one layout, and take the same boxes. - * - * @param list $roles - * - * @return array}> - * - * @throws MalformedTemplate - */ - private function template(LoadedTheme $theme, array $roles): array - { - $master = $theme->template(); - - if ($master === null) { - return []; - } - - // The name the master is known by, rather than the path it is in: a master the - // library staged is a file of its own making under a name of its own, and a reader - // told about that cannot tell which template it was. - $layouts = (new TemplateGeometry($master->path(), name: $master->name()))->boxes(); - $boxes = []; - - foreach ($roles as $role) { - $name = $theme->layout($role->value); - - if (isset($layouts[$name])) { - $boxes[$role->value] = $layouts[$name]; - } - } - - return $boxes; - } - /** * Every frontmatter key of every slide, checked before anything is built. * @@ -673,219 +629,6 @@ private function check(SourceSlide $slide): void } } - /** - * The theme the deck is drawn in. - * - * A theme named in code answers for the deck, so the frontmatter's `theme:` is not - * read — but it is checked, because a value of the wrong type is a mistake in the deck - * whatever theme the deck is drawn in. - * - * @throws ThemeNotFound - * @throws MalformedFrontmatter - */ - private function loaded(Deck $deck): LoadedTheme - { - $name = $deck->frontmatterValue('theme', self::DEFAULT_THEME); - - if (!is_string($name) || trim($name) === '') { - throw MalformedFrontmatter::wrongType(1, 'theme', 'a theme name'); - } - - if ($this->resolved !== null) { - return $this->resolved; - } - - return $this->themes->resolve(trim($name)); - } - - /** - * The faces the theme named that this renderer has no table of advance widths for, - * recorded. - * - * The heading and the body faces are the ones that decide how a line wraps, and a face - * with no table of its own is measured with the reference face's widths — which is what - * every face was measured with before there were tables of their own. The monospaced - * face is asked about as well: it is measured by its own advance, and which face it is - * says exactly how wide its text is. - * - * The slot is named rather than the face alone, because the face alone does not say - * where to change it. - * - * The faces the theme's text styles name are asked about here too, because - * `text-styles.yml` is a file of the theme and a style in it is a face the theme - * names — a face a word written in that style is drawn in, and therefore measured in. - * A style that names one of the theme's own slots is that slot's face and is not a - * finding: the theme has already said it. - */ - private function recordUnknownFaces(Theme $theme, TextMetric $metric): void - { - foreach (Theme::FONTS as $slot) { - $face = $theme->fonts()->get($slot); - - if ($face !== null && !$metric->has($face)) { - $this->fonts->record($face, sprintf("the theme's %s slot", $slot), $metric->faces()); - } - } - - foreach ($theme->textStyleFaces() as $name => $face) { - if (!$metric->has($face)) { - $this->fonts->record($face, sprintf('the text style "%s"', $name), $metric->faces()); - } - } - } - - /** - * The faces the deck named that this renderer has no table of advance widths for, - * recorded. - * - * A box the deck placed itself may name a face of its own with `font-family`, and that - * face is the one its words are drawn in and measured in. It is in the deck rather than - * in the theme, so it does not exist when the theme is resolved — it exists once the - * deck has been read, which is before anything is built and therefore before a byte - * reaches the disk. - * - * The box is named rather than the slide it is on, because a deck that writes two boxes - * of the same kind is two boxes and a reader who is told which one to change can find - * it; the slide is named as well, because two boxes of the same kind on two slides are - * two places and one of them is not the one meant. - * - * @param array $blocks The deck's slides, as they were read. - */ - private function recordUnknownFacesOfTheDeck(array $blocks, TextMetric $metric): void - { - foreach ($blocks as $index => $slide) { - foreach ($slide->shapes() as $shape) { - $face = self::faceOf($shape); - - if ($face !== null && !$metric->has($face)) { - $this->fonts->record( - $face, - sprintf('the "%s" box on slide %d', $shape->name(), $index + 1), - $metric->faces(), - ); - } - } - } - } - - /** - * The faces the chrome of a template named that this renderer has no table of advance - * widths for, recorded. - * - * The chrome of a template is held apart from the shapes of the slides: it is read once - * for the deck and drawn on every slide that names the template, so a walk over the - * slides never sees it. - * - * One entry for the whole template rather than one for each of the slides that draw it, - * because the face is named in one place: in the chrome. Twenty-eight slides carrying - * one wordmark is one name to change, and a report of twenty-eight entries would say so - * twenty-eight times and hide the fact that there is one thing to do. The place is - * named instead of the slide, so the reader is sent to the template rather than to one - * of the slides that happens to draw it. - */ - private function recordUnknownFacesOfTheChrome(TextMetric $metric): void - { - foreach ($this->drawn as $template => $shapes) { - foreach ($shapes as $shape) { - $face = self::faceOf($shape); - - if ($face !== null && !$metric->has($face)) { - $this->fonts->record( - $face, - sprintf('the "%s" box of the template "%s"', $shape->name(), $template), - $metric->faces(), - ); - } - } - } - } - - /** - * The face a shape names for the type in it, or null where it names none. - * - * Two classes carry a {@see \MarkdownPresentation\Render\Free\TextStyle}: the text a - * deck wrote and the number the renderer writes. Both are drawn in the face their style - * names and measured in it, so both are a place a face is named — and a walk that - * looked at one of them would miss the other. - */ - private static function faceOf(FreeShape $shape): ?string - { - return match (true) { - $shape instanceof FreeText => $shape->style->face, - $shape instanceof FreeSlideNumber => $shape->style->face, - default => null, - }; - } - - /** - * The role of each of the deck's slides, by index. - * - * Resolved before anything is built so that a role the theme does not map fails while - * there is still no file on disk to clean up. - * - * @return array - * - * @throws MalformedFrontmatter - * @throws UnknownLayoutRole - */ - private function roles(Deck $deck, LoadedTheme $theme): array - { - $roles = []; - - foreach ($deck->slides() as $slide) { - $named = $slide->frontmatterValue('layout', LayoutRole::Content->value); - - if (!is_string($named) || trim($named) === '') { - throw MalformedFrontmatter::wrongType( - $slide->index() + 1, - 'layout', - sprintf('a layout role — one of: %s', implode(', ', LayoutRole::names())), - ); - } - - $role = trim($named); - - if (LayoutRole::tryFrom($role) === null) { - throw UnknownLayoutRole::notARoleOn($slide->index() + 1, $role, LayoutRole::names()); - } - - // Asked of the theme rather than checked here, because a role the theme does not - // map is the theme's to fix and its message already says so. - $theme->layout($role); - - $roles[$slide->index()] = LayoutRole::from($role); - } - - return $roles; - } - - /** - * The roles the deck's slides use, in first-seen order and each of them once. - * - * Only the roles that are used are built. A theme that maps some of them would - * otherwise fail on the ones its deck never asked for, which is not the same failure as a - * deck that asks for one it did not map. - * - * Compared on the role's value rather than with `array_unique`, which casts each element - * to a string and an enum cannot be cast to one. - * - * @param array $roles - * - * @return list - */ - private function usedRoles(array $roles): array - { - $used = []; - - foreach ($roles as $role) { - if (!in_array($role->value, array_column($used, 'value'), true)) { - $used[] = $role; - } - } - - return $used; - } - /** * One source slide as the one written slide it is, and the shapes each of its steps * shows. diff --git a/tests/Unit/layouts-and-steps.php b/tests/Unit/layouts-and-steps.php index a916a01..73c7008 100644 --- a/tests/Unit/layouts-and-steps.php +++ b/tests/Unit/layouts-and-steps.php @@ -766,6 +766,22 @@ function slidesDeck(int $slides): string ->toBe(PresentationRenderer::MAX_SLIDES); }); +it('counts the slides of a deck before it resolves the theme', function () { + // The order the work happens in is the order of what can go wrong, and a deck over the + // ceiling is refused before anything is read. A deck that is over the ceiling and asks + // for a theme that does not resolve is therefore refused for the ceiling: the two + // refusals are about different things, and the one that costs the least to fix is the + // one that is reported first. + $deck = "---\ntheme: nowhere\n---\n\n" . slidesDeck(PresentationRenderer::MAX_SLIDES + 1); + + expect(fn () => slideDeck($deck)) + ->toThrow(TooManySlides::class, sprintf( + 'This deck asks for %d slides, and one presentation may carry %d.', + PresentationRenderer::MAX_SLIDES + 1, + PresentationRenderer::MAX_SLIDES, + )); +}); + it('reads a :::click region as one step of its own', function () { // The boundary and its fences are one thing: what is between them is read at the step the // opener names, through the same counter a bare marker moves, so there is no second From e96f7065bd9f96ce9e7e334c2828949fb98209e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabian=20Gra=C3=9Fl?= Date: Fri, 30 Oct 2026 07:52:04 +0100 Subject: [PATCH 8/8] A master the source cannot serve is refused, and the two loaders answer alike A finder that names a theme's master and cannot serve it was drawn without: the master came back as a path, the path did not resolve, and the deck was drawn in this library's geometry with nothing in the file saying so. That is the failure `UnservedTemplate` exists for, and the README claims the refusal without it. A source that answers a path for a master it cannot serve is now refused the same way as one whose listing names a master. The loader resolves the path of a theme's own files whether or not it has a scope, so this half and the Word half answer the same way: the path that reaches `LoadedTheme::directory()` and the failures that name one is the resolved path in both. The four public parameters that take a `PathScope` are named `$scope` rather than `$files`, which is what they are. The comments around them were wrong or missing. The writer replaces the file it writes into and keeps its mode, so the account of the window in which a staged deck's permissions could be narrowed was not the one that happens; a staged picture is narrowed like the other two staged files; a finder answers with the path as it was written; the theme a deck is drawn in comes from the resolver's finder and not the renderer's; and a `DiskFinder` is built for the themes of a render and for nothing else. --- CHANGELOG.md | 23 +++++- README.md | 22 +++--- src/Exception/UnservedTemplate.php | 10 +-- src/Files/DiskFinder.php | 25 ++----- src/Files/FileFinder.php | 5 +- src/Render/CommonMark/BlockTranslator.php | 3 +- src/Render/CommonMark/PictureReader.php | 20 +++-- src/Render/DeckTheme.php | 4 - src/Render/PresentationRenderer.php | 20 ++--- src/Render/Staging.php | 9 ++- src/Theme/Master.php | 7 +- src/Theme/ThemeLoader.php | 33 +++++---- src/Theme/ThemeResolver.php | 11 ++- tests/Unit/dead-code.php | 28 +++++++ tests/Unit/master-finder.php | 90 +++++++++++++++++++++++ tests/Unit/theme-loader.php | 17 +++++ 16 files changed, 237 insertions(+), 90 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 94f24bb..b586211 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,9 +16,9 @@ All notable changes to this project are documented here. The format follows but it is still checked, because a value of the wrong type is a mistake in the deck whatever theme the deck is drawn in: a deck writing `theme: 42` is refused. - The name is resolved through the finder when it is given, and the theme that came back - is kept, so the name is asked for once and not once for the name and once for the - deck. A theme that does not resolve is refused when the name is given rather than at + The name is resolved through the resolver's finder when it is given, and the theme that + came back is kept, so the name is asked for once and not once for the name and once for + the deck. A theme that does not resolve is refused when the name is given rather than at the render, so a typo is reported before the deck is read. - **A deck can come back as bytes.** `PresentationRenderer::toString(Deck $deck): string` @@ -151,6 +151,20 @@ All notable changes to this project are documented here. The format follows time. The master holds the source's bytes and writes them again when the file it staged is gone, at the mode of a file nobody else on the machine can read. +- **Four parameters are named `$scope` rather than `$files`.** The directories a render + may read are not a source of files, and a parameter named `$files` said the opposite: + `Render\CommonMark\BlockTranslator::parse()`, + `Render\CommonMark\SlideState::__construct()`, `Render\Chrome::__construct()` and + `Theme\ThemeLoader::__construct()` all take one. A caller using PHP 8's named + arguments has to say the new name. + +- **A theme's own files are resolved whether the loader has a scope or not.** The path + of a file was resolved before it was read, but only where a scope was given: a loader + without one returned the path as it was written, so `LoadedTheme::directory()` and the + failures that name a path said one thing in one half of the suite and another in the + other. The Word half's loader resolves the path either way, and this one does the + same now. + - **`ThemeLoader::load()` and `loadManifest()` take a name and a finder.** The third parameter was the scope a theme's own files were held to and is now the finder they are read through; the first was a directory and is now the name the theme is known @@ -197,7 +211,8 @@ All notable changes to this project are documented here. The format follows `Exception\UnservedTemplate` — rather than drawn without. The listing of a theme's files settles what a null answer means: the theme may carry no master, or the source may not be able to serve one, and a deck drawn without the master's geometry is drawn - in the wrong design with nothing saying so. + in the wrong design with nothing saying so. A source that answers a path for a master + it cannot serve is refused the same way, rather than drawn without. - **`LoadedTheme::template()` returns the master rather than its path.** It is a `Theme\Master` now: the path the master is opened at, the name a failure calls it, diff --git a/README.md b/README.md index 3e35656..4e07ba6 100644 --- a/README.md +++ b/README.md @@ -312,10 +312,10 @@ them: $renderer = (new PresentationRenderer())->withTheme('corporate-dark'); ``` -The name is resolved through the finder when it is given, and the theme that came back is -kept: the name is asked for once, and not once for the name and once for the deck. The -deck's own `theme:` is not read — but it is checked, because a value of the wrong type is -a mistake in the deck whatever theme the deck is drawn in. +The name is resolved through the resolver's finder when it is given, and the theme that +came back is kept: the name is asked for once, and not once for the name and once for the +deck. The deck's own `theme:` is not read — but it is checked, because a value of the wrong +type is a mistake in the deck whatever theme the deck is drawn in. **A deck can come back as bytes.** `render()` writes the deck to a path and hands the path back; `toString()` hands back the bytes of the same deck: @@ -2660,10 +2660,13 @@ file that is out of reach is answered like any other: the question of what a ren may read belongs to the library, which holds the path a finder answered to its own scope and refuses what is out of reach in its own words. -`Files\DiskFinder` is the default and answers out of directories on this machine, one -list of bases per role, tried in the order they are given. The directories of themes -on disk remain the fallback, so a program with a finder of its own does not lose the -themes that ship with the package. +`Files\DiskFinder` answers out of directories on this machine, one list of bases per +role, tried in the order they are given. A program that brings no finder of its own gets +one for its **themes** — the directories of themes beside the deck, beside the working +directory and the ones that ship with the package. A picture falls back to the deck's own +directory, and so does the templates file it imports. The directories of themes on disk +remain the fallback, so a program with a finder of its own does not lose the themes that +ship with the package. **A name that is already a path is the path, whatever the role.** A name beginning with `/` names a file, and resolving it against a base would name a file nobody meant — so it @@ -2681,7 +2684,8 @@ master is a theme that works, and nothing is said about it. A theme that carries and a source that cannot hand it over is refused with `MarkdownPresentation\Exception\UnservedTemplate` rather than drawn without: a deck without the master's geometry is in the wrong design, with nothing in the file saying -so. The listing of a theme's files is what tells the two apart. +so. The listing of a theme's files is what tells the two apart, and a source that +answers a path for a master it cannot serve is refused the same way. ## Loud failures diff --git a/src/Exception/UnservedTemplate.php b/src/Exception/UnservedTemplate.php index e44df3b..96f7339 100644 --- a/src/Exception/UnservedTemplate.php +++ b/src/Exception/UnservedTemplate.php @@ -7,11 +7,11 @@ /** * A theme's master that the source of a render cannot serve. * - * The theme carries a `template.pptx` — the listing of the theme's files says so — and the - * source cannot hand it over. That is a different thing from a theme that carries no - * master at all, and the difference is the whole of why this is a failure of its own: a - * deck drawn without the master's geometry is drawn in the wrong design, with nothing in - * the file saying so. + * The theme carries a `template.pptx` and the source cannot hand it over: the listing of + * the theme's files says the theme carries one, or the source answers a path for one it + * cannot serve. That is a different thing from a theme that carries no master at all, and + * the difference is the whole of why this is a failure of its own: a deck drawn without + * the master's geometry is drawn in the wrong design, with nothing in the file saying so. * * `Exception` rather than `InvalidInput`, because neither the author of the deck nor the * author of the theme can act on it: the file is right and the program that brought the diff --git a/src/Files/DiskFinder.php b/src/Files/DiskFinder.php index d0435a1..c544600 100644 --- a/src/Files/DiskFinder.php +++ b/src/Files/DiskFinder.php @@ -22,12 +22,9 @@ final class DiskFinder implements FileFinder { /** - * @param list $theme The directories a theme's files are looked for in, in - * the order they are tried. - * @param list $picture The directories a picture is looked for in, in the - * order they are tried. - * @param list $document The directories a deck's own file is looked for in, - * in the order they are tried. + * @param list $theme The directories a theme's files are looked for in. + * @param list $picture The directories a picture is looked for in. + * @param list $document The directories a deck's own file is looked for in. */ public function __construct( private readonly array $theme = [], @@ -47,9 +44,6 @@ public function read(string $name, ?string $role = null): ?string return null; } - // `@file_get_contents()` rather than a check first: a file that cannot be read is - // a file the finder has no contents for, and the warning it would print is not - // a failure anybody can act on. $contents = @file_get_contents($path); return $contents === false ? null : $contents; @@ -58,9 +52,8 @@ public function read(string $name, ?string $role = null): ?string /** * The path of a file a name identifies, when this finder holds it as a file. * - * The path as it was resolved rather than as the machine spells it: the library - * resolves what it is given and holds it to its own roots, and a path that arrives - * already resolved is a path it cannot check. + * The path as it was written, not as the file system resolved it: the library + * resolves what it is given and holds it to its own roots. */ public function path(string $name, ?string $role = null): ?string { @@ -182,8 +175,7 @@ private function inWorlds(string $name, ?string $role): ?string * A theme's file is named inside its theme, so the name is `/` and * the theme's directory is found first. The file is then looked for inside that * directory and nowhere else: a directory that exists but holds no manifest - * shadows a later one, which is what a theme resolved from a directory has always - * done. + * shadows a later one. */ private function resolveTheme(string $name): ?string { @@ -231,11 +223,10 @@ private function themeDirectory(string $theme): ?string * A role the finder was given no bases for looks in none of them: the role says * which world the name is resolved in, and a world it does not know about is not * one it can answer for. A name with no role is looked for in every world, in the - * order the roles are declared, and is resolved as a plain name — the role is - * what says how a name is resolved. + * order the roles are declared. * * The theme role is not among them: a theme is resolved by its directory rather - * than by a file in a base, which {@see self::resolveTheme()} is where happens. + * than by a file in a base, which {@see self::resolveTheme()} does. * * @return list */ diff --git a/src/Files/FileFinder.php b/src/Files/FileFinder.php index 8918610..e8b7b8f 100644 --- a/src/Files/FileFinder.php +++ b/src/Files/FileFinder.php @@ -18,8 +18,9 @@ * not what may be served. The three role names are the same in both halves of this * suite, so a finder written once serves both. * - * The names are the library's own: a theme's manifest is `corporate/theme.yml` and a - * picture is `logo.png`. A name is looked for in the places the finder was told about. + * The names are the library's own: a theme's manifest is `corporate/theme.yml`, its + * master `corporate/template.pptx` and a picture is `logo.png`. A name is looked for + * in the places the finder was told about. * * **A finder answers; it does not refuse.** A file that is not there is null, and * nothing else. Each half of this suite has its own exceptions, so a finder that diff --git a/src/Render/CommonMark/BlockTranslator.php b/src/Render/CommonMark/BlockTranslator.php index 02b64c1..5707d61 100644 --- a/src/Render/CommonMark/BlockTranslator.php +++ b/src/Render/CommonMark/BlockTranslator.php @@ -158,8 +158,7 @@ public function parse( * Give back the files this translator staged. * * A picture the source held as bytes is staged into a file of the library's own, and - * the file is the render's: it goes when the render is over, and the render asks for - * it back here. + * the file is the render's: it goes when the render is over. */ public function release(): void { diff --git a/src/Render/CommonMark/PictureReader.php b/src/Render/CommonMark/PictureReader.php index 3dec5d5..ed651f7 100644 --- a/src/Render/CommonMark/PictureReader.php +++ b/src/Render/CommonMark/PictureReader.php @@ -117,8 +117,7 @@ public function __destruct() * The source is asked first: a source that holds the picture as a file gives a path, * which is held to the scope as any other path is, and one that holds the bytes gives * the picture itself, which is staged into a file of the library's own. The disk is - * what a picture was found in before there was a source, and it is still where one is - * found when the source has none. + * where a picture is found when the source has none. * * @throws ImageNotFound */ @@ -178,10 +177,10 @@ private function held(string $path, string $written, SlideState $state): string * name as well as the bytes, and a `.svgz` is a gzipped drawing rather than a broken * one. * - * It is made with `tempnam()`, so its name is one nobody guessed and its mode is one - * nobody else on the machine can read; renaming it keeps that mode. A file that could - * not be written is left as it is: the reader that opens the picture refuses it and - * says so, which is a better sentence than one about a temporary directory. + * It is made with `tempnam()`, so its name is one nobody guessed, and its mode is + * narrowed here, so that it is one nobody else on the machine can read. A file that + * could not be written is left as it is: the reader that opens the picture refuses it + * and says so, which is a better sentence than one about a temporary directory. */ private function stage(string $bytes, string $written): string { @@ -193,12 +192,17 @@ private function stage(string $bytes, string $written): string $path = $extension === '' ? $temporary : $temporary . '.' . $extension; // Renamed rather than written under a second name, so that the file keeps the mode - // `tempnam()` gave it: a picture in flight through a shared temporary directory is - // not readable by every account on the machine. + // `tempnam()` gave it. @rename($temporary, $path); @file_put_contents($path, $bytes); + // The mode is narrowed here: renaming keeps the mode `tempnam()` gave the file, + // but a file created from scratch — the fallback path — is created at the process + // umask, and a picture in flight through a shared temporary directory is then + // readable by every account on the machine. + @chmod($path, 0o600); + // Both names are the library's to remove: the picture is under the second, and the // first is where the file is if the rename did not happen. $this->staged[] = $temporary; diff --git a/src/Render/DeckTheme.php b/src/Render/DeckTheme.php index df3cb61..19983fa 100644 --- a/src/Render/DeckTheme.php +++ b/src/Render/DeckTheme.php @@ -20,10 +20,6 @@ * than a step of the walk that draws it: the theme is resolved, the role of each slide is * read, and the boxes the theme's own template gives those roles are taken — and none of * that is the renderer's own work. - * - * A theme named in code answers for the deck, so the name is resolved once and kept: a - * source that counts its reads would say the name is asked for once and not once for the - * name and once for the deck. */ final class DeckTheme { diff --git a/src/Render/PresentationRenderer.php b/src/Render/PresentationRenderer.php index 877c602..6464ed0 100644 --- a/src/Render/PresentationRenderer.php +++ b/src/Render/PresentationRenderer.php @@ -156,7 +156,7 @@ final class PresentationRenderer /** * The source of the deck's files: its pictures and the templates file it names. * - * Null for the disk, which is where a deck's files were found before there was a + * Null for the disk, which is where a deck's files are found when there is no * source. A source that holds them as bytes is asked for them by name. */ private readonly ?FileFinder $files; @@ -223,7 +223,7 @@ final class PresentationRenderer * @param string $directory The directory a relative image path in the deck * resolves against, which is the directory the deck * itself is in. It is also the first of the roots an - * image may be read from: see {@see self::$files}. + * image may be read from: see {@see self::$scope}. * @param ThemeResolver $themes The resolver the themes are looked up in. It is * copied rather than used as it is: the renderer hands * it the deck's directory, and the copy is the one @@ -244,10 +244,9 @@ final class PresentationRenderer * arbitrary path read this constructor argument exists * to stop. * @param FileFinder|null $files Where the deck's files come from: the pictures it - * names and the templates file it imports. Null for the - * disk, which is where a deck's files were found before - * there was a source. The theme is not among them: the - * resolver the caller brought is where themes come from. + * names and the templates file it imports. The theme is + * not among them: the resolver the caller brought is + * where themes come from. */ public function __construct( private readonly string $directory = '.', @@ -278,10 +277,11 @@ public function __construct( * type is a mistake in the deck whatever theme the deck is drawn in: a deck writing * `theme: 42` is refused. * - * The name is resolved through the finder when it is given, and the theme that came - * back is kept: a source that counts its reads would say the name is asked for once and - * not once for the name and once for the deck. A theme that does not resolve is - * refused here rather than at the render, so a typo is reported before the deck is read. + * The name is resolved through the resolver's finder when it is given, and the theme + * that came back is kept: a source that counts its reads would say the name is asked + * for once and not once for the name and once for the deck. A theme that does not + * resolve is refused here rather than at the render, so a typo is reported before the + * deck is read. * * Mutating rather than returning a new renderer: the renderer holds the deck's * directory and its sources, and there is no value in a copy that differs in a theme. diff --git a/src/Render/Staging.php b/src/Render/Staging.php index e667c1b..8021f47 100644 --- a/src/Render/Staging.php +++ b/src/Render/Staging.php @@ -41,10 +41,11 @@ public static function of(callable $write): string try { $write($path); - // The writer unlinks the file `tempnam()` made and writes its own at the - // process umask: a deck in flight through a shared temporary directory is not - // readable by every account on the machine, and this is the only moment its - // permissions can be narrowed. + // The writer replaces the file and keeps the mode of the file it replaces, so + // the mode `tempnam()` gave it is the mode it keeps. The fallback path is the + // one that was not there to be replaced: the writer creates that file at the + // process umask, and a deck in flight through a shared temporary directory is + // then readable by every account on the machine. if (is_file($path)) { @chmod($path, 0o600); } diff --git a/src/Theme/Master.php b/src/Theme/Master.php index bb30a47..d6dbc14 100644 --- a/src/Theme/Master.php +++ b/src/Theme/Master.php @@ -64,9 +64,10 @@ public static function of(string $bytes, string $name): self @file_put_contents($path, $bytes); - // A master in flight through a shared temporary directory is not readable by every - // account on the machine, and the mode is narrowed here because the file may have - // been written at the process umask. + // The mode is narrowed here: the mode `tempnam()` gave the file is the mode it + // keeps, but a file created from scratch — the fallback path — is created at the + // process umask, and a master in flight through a shared temporary directory is + // then readable by every account on the machine. @chmod($path, 0o600); return new self($path, $name, true, $bytes); diff --git a/src/Theme/ThemeLoader.php b/src/Theme/ThemeLoader.php index f976bdd..23286c7 100644 --- a/src/Theme/ThemeLoader.php +++ b/src/Theme/ThemeLoader.php @@ -193,33 +193,30 @@ private function file(?FileFinder $files, string $theme, string $file, string $w } /** - * A path held to the scope, and the file it names. + * A path resolved and held to the scope. * * The path is resolved before it is held, because a prefix is not containment: `..` * and a symlink both defeat one, and a symlink pointing outside the roots is the * ordinary way a path that looks harmless turns into a read of the wrong file. * - * Without a scope there is nothing to hold the path to, and it is taken as the - * finder answered it: a theme this library ships is read on trust, and so is one a - * caller named a directory of its own. + * Without a scope there is nothing to hold the path to, and the file is read on + * trust: a theme this library ships is one, and so is one a caller named a directory + * of its own. The path is resolved all the same, because the Word half's loader + * resolves it and the two have to answer the same way. * - * @return string|null The path of the file, or null when there is none. + * @return string|null The resolved path, or null when there is no such file. * * @throws ThemeNotFound when the file is somewhere this render may not read */ private function contained(string $path, string $written): ?string { - if ($this->scope === null) { - return file_exists($path) ? $path : null; - } - $real = PathScope::resolve($path); if ($real === null) { return null; } - if (!$this->scope->holds($real)) { + if ($this->scope !== null && !$this->scope->holds($real)) { throw ThemeNotFound::outside($written, $real, $this->scope->roots()); } @@ -235,11 +232,11 @@ private function contained(string $path, string $written): ?string * is a file outside it read under a name that is inside. A source that holds only the * bytes has them staged into a file of the library's own. * - * A null answer is ambiguous on its own: the theme may carry no master, or the source - * may not be able to serve one. The listing of the theme's files tells the two apart, - * and a master the theme carries and the source cannot serve is refused rather than - * drawn without — a deck without the master's geometry is in the wrong design, with - * nothing in it saying so. + * A master the theme carries and the source cannot serve is refused rather than drawn + * without — a deck without the master's geometry is in the wrong design, with nothing + * in it saying so. The listing of the theme's files tells a theme that carries no + * master from a source that cannot serve one, and a path the source answers for a + * master it cannot serve is the same case said the other way round. * * @throws UnservedTemplate when the theme carries a master the source cannot serve * @throws ThemeNotFound when the master resolves outside the directories this render may read @@ -253,7 +250,11 @@ private function template(string $name, string $written, ?FileFinder $files): ?M if ($path !== null) { $real = $this->contained($path, $written); - return $real === null ? null : Master::at($real, $file); + if ($real === null) { + throw UnservedTemplate::named($written, self::TEMPLATE_FILE); + } + + return Master::at($real, $file); } $bytes = $files?->read($file, 'theme'); diff --git a/src/Theme/ThemeResolver.php b/src/Theme/ThemeResolver.php index f1e6ec2..00b40c5 100644 --- a/src/Theme/ThemeResolver.php +++ b/src/Theme/ThemeResolver.php @@ -54,11 +54,10 @@ * path as written cannot tell. The loader holds each file it reads, and the template it * names, and refuses the one that resolves elsewhere. * - * The themes themselves are read through a finder, which is given the directories of - * themes above: the finder resolves a name against them in the order they are given, - * and a theme directory that holds no manifest still shadows a later one. A finder the - * calling program brought is the source of themes in its own right, and the directories - * on disk remain the fallback rather than the only place a theme is looked for. + * The themes themselves are read through a finder: the one the calling program brought, + * or one over the directories of themes above. A theme directory that holds no manifest + * still shadows a later one, and the directories on disk remain the fallback rather than + * the only place a theme is looked for. */ final class ThemeResolver { @@ -247,7 +246,7 @@ private function themeDirectories(): array * The finder the themes are read through: the source the calling program brought, * or the disk default. * - * The default holds the directories of themes the six steps name, in the order they + * The default holds the directories of themes the resolver looks in, in the order they * are tried, so a theme is resolved by its directory and a theme directory that holds * no manifest still shadows a later one. */ diff --git a/tests/Unit/dead-code.php b/tests/Unit/dead-code.php index 08404f4..f4c5593 100644 --- a/tests/Unit/dead-code.php +++ b/tests/Unit/dead-code.php @@ -486,6 +486,34 @@ function pagesWithClaim(string $claim): array // text, so it is refused as well. The phrase is the claim as it was made, so that a // comment may not make it again. 'a label is refused on the shapes with no body of their own' => 'the shapes with no body of their own have nowhere to put it', + + // The three files a render stages are the library's own, and a file in a shared + // temporary directory is not readable by every account on the machine. The comments + // gave three different accounts of how the mode is arrived at, and two of them were + // false: PHPPresentation's writer replaces the file and keeps the mode of the file it + // replaces, and the file `tempnam()` made is 0600 to begin with. The phrases are what + // the comments said rather than the idea, so that a comment may not make them again. + // Each phrase is one line of the comment it came from, because a guard that a line + // break can walk past is not a guard. + 'a staged file is written at the process umask by the writer' => 'The writer unlinks the file `tempnam()` made', + 'a staged file keeps the mode the rename gave it' => 'renaming it keeps that mode', + // The path a finder answers is the path as it was written, and the library resolves it + // before it reads it: a path that arrives already resolved is a path the library + // cannot check. The phrase is what the docblock said, which said the opposite. + 'the path a finder answers is the path the machine spells' => 'as it was resolved rather than as the machine spells it', + // A loader without a scope reads the file it is given on trust, but it resolves the + // path all the same, because the Word half's loader does and the two have to answer + // the same way. The phrase is what the docblock said about the path it returned. + 'a loader without a scope takes the path as the finder answered it' => 'nothing to hold the path to, and it is taken as the', + // The theme a deck is drawn in comes from the resolver's finder, and the deck's own + // files come from the renderer's: they are different finders, and a docblock that says + // "the finder" names the wrong one half the time. + 'a theme named in code is resolved through the finder of the renderer' => 'resolved through the finder when it is given', + // A `DiskFinder` is built for the themes of a render and for nothing else: a picture + // falls back to the deck's own directory, and the templates file with it. + 'the disk finder is the default of every role' => '`Files\DiskFinder` is the default', + // A docblock that points at a property by name has to point at the property there is. + 'a docblock points at the property the rename left behind' => 'see {@see self::$files}', ]); it('has no decorative dividers in it', function () { diff --git a/tests/Unit/master-finder.php b/tests/Unit/master-finder.php index f6aca84..f43a839 100644 --- a/tests/Unit/master-finder.php +++ b/tests/Unit/master-finder.php @@ -13,6 +13,7 @@ use MarkdownPresentation\Tests\Support\Pptx; use MarkdownPresentation\Tests\Support\Scratch; use MarkdownPresentation\Theme\LayoutRole; +use MarkdownPresentation\Theme\ThemeLoader; use MarkdownPresentation\Theme\ThemeResolver; /* @@ -88,6 +89,61 @@ public function files(string $theme): array } } +/** + * A source that holds the theme's files as paths, and cannot serve the master. + * + * The other way a source cannot serve a master: it answers a path for one and the file is + * not there. The listing says the theme carries a master, and the path says where it + * should be. + */ +final class UnmountedSource implements FileFinder +{ + /** + * @param string $directory Where the theme's files are said to be. + */ + public function __construct(private readonly string $directory) + { + } + + public function read(string $name, ?string $role = null): ?string + { + return null; + } + + public function path(string $name, ?string $role = null): ?string + { + return $this->directory . '/' . $name; + } + + public function themes(): array + { + return ['house']; + } + + public function files(string $theme): array + { + return ['layouts.yml', 'template.pptx', 'theme.yml']; + } +} + +/** + * A theme's own files, written where a source says a theme's files are. + * + * The master is left out on purpose: the source cannot serve it, and the path it answers + * for it is the one that is not there. + */ +function unmountedTheme(string $prefix = 'mounted'): string +{ + $directory = Scratch::directoryPath($prefix); + + mkdir($directory . '/house'); + + file_put_contents($directory . '/house/theme.yml', (string) file_get_contents(masterTheme() . '/theme.yml')); + file_put_contents($directory . '/house/layouts.yml', (string) file_get_contents(masterTheme() . '/layouts.yml')); + + return $directory; +} + it('draws a deck in the geometry of a master the source holds as a file', function () { // The source holds the theme's files where they are, so the master is opened where // it is. The title lands where the template puts it rather than where this library @@ -190,6 +246,40 @@ public function files(string $theme): array expect(filesize($path))->toBe(0); }); +it('refuses a master the source answers a path for and cannot serve', function () { + // The source names the master and answers a path for it, and the file is not there: + // the same refusal as a source that answers nothing for it, said the other way + // round. A deck drawn without the master would be in the wrong design with nothing in + // it saying so. + $finder = new UnmountedSource(unmountedTheme()); + + expect(fn () => (new ThemeLoader())->load('house', 'house', $finder)) + ->toThrow(UnservedTemplate::class, 'The theme "house" carries a "template.pptx" that the source of this render cannot serve'); +}); + +it('draws no deck when the source answers a path for a master it cannot serve', function () { + // The refusal came before anything was built, so there is no file behind it. + $directory = unmountedTheme('unmounted'); + + $resolver = new ThemeResolver( + workingDirectory: Scratch::directoryPath('working'), + builtinDirectory: Scratch::directoryPath('builtin'), + roots: [$directory], + files: new UnmountedSource($directory), + ); + + $path = Scratch::file('master-unmounted.pptx', ''); + + expect(fn () => (new PresentationRenderer(themes: $resolver)) + ->render( + (new DeckParser())->parse("---\ntheme: house\n---\n# One\n"), + $path, + )) + ->toThrow(UnservedTemplate::class); + + expect(filesize($path))->toBe(0); +}); + it('draws a deck in a theme that carries no master, and says nothing', function () { // A theme without a master is a theme that works: the deck is drawn in the geometry // this library puts it at, and nothing is said about a master that was never there. diff --git a/tests/Unit/theme-loader.php b/tests/Unit/theme-loader.php index b4e9d6c..45f4e98 100644 --- a/tests/Unit/theme-loader.php +++ b/tests/Unit/theme-loader.php @@ -8,6 +8,7 @@ use MarkdownPresentation\Exception\UnknownLayoutRole; use MarkdownPresentation\Exception\UnknownStyleSlot; use MarkdownPresentation\Filesystem\PathScope; +use MarkdownPresentation\Tests\Support\Scratch; use MarkdownPresentation\Tests\Support\ThemeDirectory; use MarkdownPresentation\Theme\ThemeLoader; @@ -206,6 +207,22 @@ ->toThrow(ThemeNotFound::class, 'is at "' . realpath($outside . '/styles.yml') . '" rather than one this render may read'); }); +it('resolves the path of a theme\'s files, with a scope or without one', function () { + // The Word half's loader resolves the path whether or not it has a scope, and the two + // loaders are meant to be interchangeable: a theme read through a link is the theme + // the link reaches, whichever half reads it. + $real = ThemeDirectory::create(['theme.yml' => "name: Linked\n"], 'real'); + $holder = Scratch::directoryPath('holder'); + + if (!@symlink($real, $holder . '/house')) { + test()->markTestSkipped('symlinks are not available here'); + } + + $loaded = ThemeDirectory::load($holder . '/house'); + + expect($loaded->directory())->toBe(realpath($real)); +}); + it('takes the files of a theme on trust when it has no scope', function () { // The scope is what makes the check; without one the loader reads what it is // given, which is what a theme this library ships is and what a caller naming a