Repository navigation
Warn and ignore a brand file that holds only comments - #15101
Conversation
A commented-out `_brand.yml` is useful as a template, but it parses to no YAML value, so brand validation failed with `YAML value is missing` and the render stopped. Both brand loaders now check for an empty YAML value first, warn once, and treat the file as absent. Invalid YAML still goes through validation so its error is reported as before. Fixes quarto-dev#14891
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
cderv
left a comment
cderv
left a comment
There was a problem hiding this comment.
Thanks for the PR. I tested it locally and the fix works for the cases you listed: comment-only _brand.yml next to the document, brand: file.yml in front matter, and brand: {light, dark} with a comment-only dark file, both in front matter and in _quarto.yml. An empty (0 byte) _brand.yml is also handled now. The new test fails without the change to project-shared.ts and passes with it.
I'll push a small commit on your branch before merging: merge main to solve the conflict in news/changelog-1.11.md, and adjust the changelog entry (author credit and wording). No need to push anything on your side meanwhile.
One thing I noticed in the light/dark case. With a comment-only dark file, the page now renders with the light/dark toggle but no dark stylesheet, so the toggle does nothing. The toggle is decided from the dark key in the brand metadata and not from the brand that was loaded. Before this PR the same input stopped the render, so this is still better. I'll open a separate issue for it, as the fix is in the dark mode logic shared by several formats.
About the TypeError on broken YAML in a brand file: yes, please open a separate issue for it. I can reproduce it too with a _brand.yml containing color: [, and I did not find an existing issue.
Resolve the changelog conflict with the quarto-dev#14882 entry, keeping entries in issue order. The quarto-dev#14891 entry now starts with "Fix" and carries the author credit, as for external contributions.
Since #15101, a brand file that holds only comments is ignored with a warning, but the page still behaved as if that dark brand existed. The key-order check in darkModeDefault read the raw brand: metadata, which still listed dark:, so the page got a color-scheme toggle with no dark styles, and a dark-only brand started in dark mode with light styles. Honour brand: key order only when a dark brand was actually loaded (format.render.brand?.dark). theme: key order is unchanged and still wins, so an explicit theme: {light, dark} keeps its toggle. darkModeDefaultMetadata behaves as before because it runs before the brand is loaded. - Fix darkModeDefault in src/format/html/format-html-info.ts - Add smoke-all tests for the ignored dark brand cases, each with its own comment-only fixture (warnOnce would hide the warning in a shared fixture) - Document the case in llm-docs/html-dark-mode-architecture.md Fixes #15131
3 participants
Description
Fixes #14891.
A
_brand.ymlthat is all comments parses to no YAML value, so the brand schema rejects it withYAML value is missingand the render stops. Both loaders inprojectResolveBrandnow check for an empty value first, print a warning (once per file) and carry on as if the file wasn't there. Invalid YAML still goes through validation, so its error is reported the same as before.What I checked on Windows 11:
_brand.ymlnext to a document: warning, document rendersbrand: my-brand.ymlin front matter pointing at a comment-only file: warning, rendersbrand: {light, dark}with a comment-only dark file: warning, the light brand is still appliedThe new smoke-all test (
2026/10/13/issue-14891) fails without the change and passes with it, and the existing brand tests (126 documents, 197 checks) pass.One thing I noticed along the way but left out of this PR: a brand file with broken YAML, e.g. an unclosed
[, fails withTypeError: Cannot read properties of undefined (reading 'value')rather than a validation message. That happens onmaintoo. Happy to open a separate issue for it.Checklist
I have (if applicable):
AI-assisted PR