CesiumGS / wetzel

Generate Markdown documentation from JSON Schema
Apache License 2.0
133 stars 54 forks source link

Fix glTF property reference details #88

Open javagl opened 11 months ago

javagl commented 11 months ago

Fixes https://github.com/KhronosGroup/glTF/issues/2165

Adding the minProperties information was trivial: If it is there, just add the bullet point:

Khronos Wetzel minProperties

(It also handles maxProperties. This is not used in the glTF schema, but not handling it would feel wrong...)

--

Fixes https://github.com/KhronosGroup/glTF/issues/2158

Properly handling that may be a bit more tricky, depending on the exact structure of the input schema, and the (too) important role that the title plays for the current state of wetzel. The corresponding line in the code contained a // TODO: additionalProperties is really a full schema and I'm not sure what to do with that one. I had investigated some options back when this issue first came up, and suggested one possible (!) solution there, which is now implemented here, and which does achieve the desired result:

Khronos Wetzel extension properties

Whether it always yields the desired result for all possible inputs....? I don't know. (Does it matter? Probably not...). However, someone might want to review this.


An aside: The minProperties change did not require an update of the 'golden' output, so there seems to be a lack of test coverage for that. The ... "pragmatic" point of view here is that the linked issues should be fixed in the context of glTF, and this goal should be achieved.

I locally reviewed the outputs that are generated for glTF (i.e. the JsonSchemaReference.adoc and PropertiesReference.adoc), and the looked correct for me. Having another look at these files (and a check whether all this works when it is run from within the glTF spec build process) could be part of a review.

EDIT: @emackey I think that you are the main maintainer of wetzel, maybe you want to have a look ....