Skip to content

Add a JSONSchemaWriter to Inlet#490

Merged
joshessman-llnl merged 16 commits intodevelopfrom
feature/essman/json_schema_writer
Mar 22, 2021
Merged

Add a JSONSchemaWriter to Inlet#490
joshessman-llnl merged 16 commits intodevelopfrom
feature/essman/json_schema_writer

Conversation

@joshessman-llnl
Copy link
Member

@joshessman-llnl joshessman-llnl commented Mar 4, 2021

Summary

  • This PR is a feature
  • It does the following:
    • Fixes Inlet: Add writer for VSCode code completion (IntelliSense) #483
    • Adds a JSON schema writer for VSCode integration with JSON/YAML input files
    • Adjusts the Inlet hierarchy traversal logic to visit only one element of a container instead of all of them (since their schemas are guaranteed to be identical) - this was implemented at the Writer level in Document function callbacks in SphinxWriter #450 but after having implemented a second Writer I think it's general enough to apply to all Writers
    • Moves collectionIndices and collectionIndicesWithPaths from Container methods to nonmembers in the detail namespace, since they don't need access to private members (and so they can be used elsewhere)

This PR also adds some tests that cover a small subset of what the new SchemaWriter produces - there's a lot of specialized logic associated with structs/collections of structs/primitive arrays that was tested manually. If the test approach makes sense, I can add more tests in this PR or a follow-up. I happened to have a tool called jsonschema on my machine that can check a JSON file against a JSON schema, so I've used this to verify that the schema verification agrees with Inlet's verification. These tests are skipped when jsonschema is not available.

I've also added a demo GIF showing what the VSCode integration looks like, though I've crunched it quite a bit to avoid cluttering the repo. If the resolution is too low I can increase it a bit.

New doc snippet: https://axom.readthedocs.io/en/feature-essman-json_schema_writer/axom/inlet/docs/sphinx/quick_start.html#generating-documentation

@joshessman-llnl joshessman-llnl added enhancement New feature or request Inlet labels Mar 4, 2021
@white238
Copy link
Member

white238 commented Mar 4, 2021

Copy link
Member

@kennyweiss kennyweiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great @joshessman-llnl !

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems brittle.
Is this typically available in unix? What about windows?

Is there a way we can add this via the build system, e.g. via axom's devtools and pass the path in through a #define in axom/config.hpp?

Perhaps as a follow-up PR?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have the path hardcoded here for security reasons (to avoid PATH injection attacks, though I'm not sure if this is a threat vector that Axom cares about).

I don't recall installing jsonschema (it just happened to be present), but I think it's available as a PyPI package that can be installed fairly easily. I think it could also be made available through spack.

{
if(!hasSchemaUtility())
{
GTEST_SKIP() << "jsonschema tool was not found";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


const bool jsonVerified = validateString(inlet, testString);

EXPECT_FALSE(jsonVerified);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool!

inlet.writeDoc();
std::unique_ptr<inlet::JSONSchemaWriter> schemaWriter(
new inlet::JSONSchemaWriter("nested_structs.json"));
inlet.registerWriter(std::move(schemaWriter));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(It will be easy for me to check, but) is there only one writer at a time?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes (I had to look it up myself) - I don't see any reason why we couldn't just store a vector of them, though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#499

logged so we don't forget

Copy link
Member

@white238 white238 Mar 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be fixed for the non-WSL windows build.

Possibly removing the path completely?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This executable isn't there on OSX (at least mine)

protected:
void SetUp() override
{
if(!hasSchemaUtility())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 at least it just skips if its not there...

This allows for integration with text editors like Visual Studio Code, which allows you to associate a JSON schema
with an input file and subsequently provides autocompletion, linting, tooltips, and more.

Using the same ``documentation_generation.cpp`` example, the automatically generated schema can be used to assist
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added an issue so I don't forget that we should have a dedicated page for Writer classes.

#500

@joshessman-llnl joshessman-llnl deleted the feature/essman/json_schema_writer branch March 22, 2021 22:25
@joshessman-llnl joshessman-llnl linked an issue Mar 23, 2021 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request Inlet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inlet: Add writer for VSCode code completion (IntelliSense) inlet::SphinxDocWriter should ignore internal container groups

3 participants

Comments