bootprint / bootprint-json-schema

Bootprint template to convert JSON-schema into readble HTML, this repository has moved to https://github.com/bootprint/bootprint-monorepo/tree/master/packages/bootprint-json-schema
MIT License
31 stars 12 forks source link

Support type property as an array, according to JSON Schema Validation #13

Open ramsey opened 7 years ago

ramsey commented 7 years ago

In JSON Schema, the "type" property may be a string or an array of strings. Previously, if type was set as array of strings (i.e., "type": ["null", "array"]), then bootprint-json-schema would render the following:

null,array

Additionally, the "items" property was not rendered.

The expected result for the given scenario is:

null, integer[]

And the "items" property should be rendered with its schema definition.

This supports the JSON Schema "type" as it is defined to allow an array of strings. If one of the strings in that array is "array," then items.type is used to render the datatype, and the "items" property is also rendered.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-9.3%) to 56.604% when pulling 0237974c16e860eed2cec008873fb17aa156aa7e on ramsey:fix-schema-type-array into 0225c10d325760cdb9b28d1986292937c8d7ba39 on bootprint:master.

nknapp commented 7 years ago

Interesting. I didn't know about multiple types. I have to read about that...

What about "null, object"? That should be dealt with as well, shouldn't it?

Would be great if you could add a test that covers the new lines. And it might take a few days until I have the time to review everything, but so far: Thanks for your PR

Am 28. März 2017 23:47:10 MESZ schrieb Ben Ramsey notifications@github.com:

In JSON Schema, the "type" property may be a string or an array of strings. Previously, if type was set as array of strings (i.e., "type": ["null", "array"]), then bootprint-json-schema would render the following:

null,array

Additionally, the "items" property was not rendered.

The expected result for the given scenario is:

null, integer[]

And the "items" property should be rendered with its schema definition.

This supports the JSON Schema "type" as it is defined to allow an array of strings. If one of the strings in that array is "array," then items.type is used to render the datatype, and the "items" property is also rendered. You can view, comment on, or merge this pull request online at:

https://github.com/bootprint/bootprint-json-schema/pull/13

-- Commit Summary --

  • Support type property as an array, according to JSON Schema Validation

-- File Changes --

M handlebars/helpers.js (11) M handlebars/partials/json-schema/body.hbs (4)

-- Patch Links --

https://github.com/bootprint/bootprint-json-schema/pull/13.patch https://github.com/bootprint/bootprint-json-schema/pull/13.diff

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/bootprint/bootprint-json-schema/pull/13

-- Diese Nachricht wurde von meinem Android-Mobiltelefon mit K-9 Mail gesendet.

ramsey commented 7 years ago

This should handle all cases, so null, object[] should work. I only used null, integer[] as an example.

I'll provide test cases soon. I thought I saw you mention on another PR that you hadn't yet set up the testing infrastructure, so I skipped over writing tests, since I wasn't sure the status of that. I could have been confused with another project I was working on. :-)

nknapp commented 7 years ago

You are right. There was such a PR. I set up the infrastructure before the merge and used it to write a test for that PR.

Am 29. März 2017 20:52:38 MESZ schrieb Ben Ramsey notifications@github.com:

This should handle all cases, so null, object[] should work. I only used null, integer[] as an example.

I'll provide test cases soon. I thought I saw you mention on another PR that you hadn't yet set up the testing infrastructure, so I skipped over writing tests, since I wasn't sure the status of that. I could have been confused with another project I was working on. :-)

-- You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/bootprint/bootprint-json-schema/pull/13#issuecomment-290189320

-- Diese Nachricht wurde von meinem Android-Mobiltelefon mit K-9 Mail gesendet.

owaaa commented 6 years ago

bump? I hit a roadblock and was happy to find this fork exists and fixed my issue. It solves the [array, null] issue well

nknapp commented 6 years ago

This change still needs a test. This can be something like the fstab-spec. Should not be too hard to write... If there is a test and the coverage does not go further down with the merge, I'll merge the PR, but I don't have time to write it at the moment.