CesiumGS / wetzel

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

Items missing from Schema with array type #64

Open theory opened 2 years ago

theory commented 2 years ago

I have a JSON schema named things.schema.json that looks like this:

{
    "$schema": "https://json-schema.org/draft/2020-12/schema",
    "$id": "things.schema.json",
    "title": "Things",
    "description": "A list of things.",
    "type": "array",
    "uniqueItems": true,
    "minItems": 1,
    "items": { "$ref": "thing.schema.json" }
}

It references thing.schema.json, which looks like this:

{
    "$schema": "https://json-schema.org/draft/2020-12/schema",
    "$id": "thing.schema.json",
    "title": "Thing",
    "description": "A thing.",
    "type": "object",
    "properties": {
      "name": { "type": "string" },
      "age": { "type": "number" }
    },
    "required": ["name", "age"]
}

Then I run wetzel like so:

wetzel.js --autoLink=cqo things.schema.json > things.md

The resulting file looks like this:

# Objects
* [`Thing`](#reference-thing)
* [`Things`](#reference-things) (root object)

---------------------------------------
<a name="reference-thing"></a>
## Thing

A thing.

**`Thing` Properties**

|   |Type|Description|Required|
|---|---|---|---|
|**name**|`string`|| &#10003; Yes|
|**age**|`number`|| &#10003; Yes|

Additional properties are allowed.

### thing.name

* **Type**: `string`
* **Required**:  &#10003; Yes

### thing.age

* **Type**: `number`
* **Required**:  &#10003; Yes

---------------------------------------
<a name="reference-things"></a>
## Things

A list of things.

Note that the Things reference at the bottom is missing any information about the items. The same is true even if I set "items": { "type": "string" }; the resulting Markdown is simply:

# Objects
* [`Things`](#reference-things) (root object)

---------------------------------------
<a name="reference-things"></a>
## Things

A list of things.

If I add minItems uniqueItems, or any of the rest, none of it appears, either. Shouldn't there be more details on what constitutes an an array schema?

theory commented 2 years ago

I assume it needs to handle arrays in getSchemaMarkdown():

--- a/lib/generateMarkdown.js
+++ b/lib/generateMarkdown.js
@@ -221,6 +221,8 @@ function getSchemaMarkdown(schema, fileName, headerLevel, suppressWarnings, sche
         var title = defaultValue(schema.title, suppressWarnings ? '' : 'WETZEL_WARNING: title not defined');
         md += createPropertiesDetails(schema, title, headerLevel + 1, knownTypes, autoLink);
         md += createExamples(schema, headerLevel + 1);
+    } else if (schemaType === 'array') {
+        md += 'Add information about the array constraints and types here'
     }

     return md;

That does go to the right place in the Markdown, though I don't know exactly how you'd like it formatted; perhaps something like for animation channels?

emackey commented 2 years ago

Sounds great, but I would love to reuse the same code that generates this for arrays that are object properties. A quick glance doesn't make it look like the code is easily reusable, which is a shame. But I don't think we would want two separate implementations of this feature, if that's avoidable.

theory commented 2 years ago

Yeah. I hacked this into my current copy so I could at least get a couple links, but it's copied and modified from elsewhere. Not pretty.

diff --git a/lib/generateMarkdown.js b/lib/generateMarkdown.js
index 961381c..365b15f 100644
--- a/lib/generateMarkdown.js
+++ b/lib/generateMarkdown.js
@@ -221,6 +221,28 @@ function getSchemaMarkdown(schema, fileName, headerLevel, suppressWarnings, sche
         var title = defaultValue(schema.title, suppressWarnings ? '' : 'WETZEL_WARNING: title not defined');
         md += createPropertiesDetails(schema, title, headerLevel + 1, knownTypes, autoLink);
         md += createExamples(schema, headerLevel + 1);
+    } else if (schemaType === 'array') {
+        var summary = getPropertySummary(schema, knownTypes, autoLink);
+        var eachElementInTheArrayMust = 'Each element in the array' + style.mustKeyword
+
+        if (schema.items) {
+            var itemSum = getPropertySummary(schema.items, knownTypes, autoLink)
+            md += style.bulletItem(style.propertyDetails('Type') + ': ' + itemSum.formattedType, 0);
+            if (schema.uniqueItems) {
+                md += style.bulletItem(eachElementInTheArrayMust + 'be unique.', 1);
+            }
+        }
+
+        // Schema reference
+        if (embedMode === enums.embedMode.referenceIncludeDocument) {
+            md += style.bulletItem(style.bold('JSON schema') + ': ' + style.getSchemaEmbedLink(fileName, schema)) + '\n';
+        } else if (defined(schemaRelativeBasePath)) {
+            if (!schemaRelativeBasePath.endsWith('/')) {
+                schemaRelativeBasePath += '/';
+            }
+            md += style.bulletItem(style.bold('JSON schema') + ': ' + style.getLinkMarkdown(fileName, schemaRelativeBasePath.replace(/\\/g, '/') + fileName)) + '\n';
+        }
+
     }

     return md;
meganrm commented 2 years ago

Hi, I'm using your library to make my documentation and I ran into this issue. Is this going to be fixed anytime soon? Thanks!

theory commented 2 years ago

@meganrm unless and until the glTF schema uses an array types, I think it will only be added if someone else contributes it. The project seems pretty amenable to merging PRs tho.