Fix duplicate schema definitions causing YAML validation issues#14136
Fix duplicate schema definitions causing YAML validation issues#14136jdonaldson wants to merge 3 commits intoquarto-dev:mainfrom
Conversation
…to-dev#14122) When the same option name is defined in multiple schema YAML files, objectRefSchemaFromGlob() silently overwrites — last writer wins. This caused validation errors for valid values matching a "losing" variant. - Add `enum: [false]` to `logo-light-dark-specifier-path-optional` so `logo: false` validates for all formats (dashboard, revealjs, typst) - Consolidate `subject` schema into document-metadata.yml as `anyOf: [string, object]` covering both simple and structured epub forms - Remove duplicate `subject` entry from document-epub.yml - Add unit tests for both cases
✅ 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. |
|
Just adding a small comment that there's a human behind this PR. I used Claude here to make some small changes, which were reviewed before submitting. Tests are also added. |
cderv
added
the
triaged-to
Now that logo-light-dark-specifier-path-optional accepts `false`, return `undefined` explicitly for all falsy values rather than leaking `false` through to resolveLogo.
|
This pointed out a minor (possibly harmless) bug in |
Verifies that the consolidated subject schema in document-metadata.yml correctly renders structured subject values (text/authority/term) into EPUB3 OPF metadata.
gordonwoodhull
left a comment
gordonwoodhull
left a comment
There was a problem hiding this comment.
I've reviewed the logo part (my bug) - looks good, adding a tiny fix.
The subject resolution also makes sense to me.
Claude did not think the structured epub subject test was a rigorous enough test, so we've also added a smoke-all test. I'll let it comment on that.
There was a problem hiding this comment.
End-to-end epub test for structured subject — The subject-structured.yml schema test inlines a copy of the schema rather than referencing it (unlike the logo-false.yml test which uses ref:). This means it validates the shape of the anyOf but not the actual schema Quarto uses — if someone later changes document-metadata.yml, this test would still pass against its own detached copy. Added subject-structured-epub.qmd as a smoke-all test that renders an actual epub with structured subject metadata and verifies the OPF output contains the expected dc:subject, authority, and term elements. This exercises the full pipeline: schema validation → metadata resolution → Pandoc rendering.
|
Hello, and thanks for the PR! We will need you to fill out the original template for the PR, specifically because we need you to attest you've signed the CLA for this project. |
5 participants
Summary
Fixes #14122.
When the same option name (e.g.,
logo,subject) is defined in multiple schema YAML files,objectRefSchemaFromGlob()silently overwrites — last writer wins. This causes YAML validation errors when users specify valid values that match a "losing" schema variant.logo: Addenum: [false]tologo-light-dark-specifier-path-optionalsologo: falsevalidates for all formats (dashboard, revealjs, typst), matching the existinglogo-light-dark-specifierbehaviorsubject: Consolidate intodocument-metadata.ymlasanyOf: [string, object]covering both simple (pdf/office) and structured (epub) forms; remove duplicate entry fromdocument-epub.ymljson-schemas.json,schema-types.ts, editor tooling)Test plan
schema-schema.test.ts— 3/3 pass (schema YAML files well-formed)schema-files.test.ts— 4/4 pass (2 existing + 2 new test cases)logo-false.yml— validatesfalseaccepted bylogo-light-dark-specifier-path-optionalsubject-structured.yml— validates structured object form passes schema validationlogo: falsein dashboard format renders without validation errorsubjectin epub format renders without validation error