Refactoring for the style

[javagl]

The style currently is a set of functions that serve the purpose of formatting/styling the output. A considerable number of functions use some if (isAdoc()) check to generate different outputs, depending on whether the output should be Markdown or AsciiDoc, and this is configured where the style is initialized based on the command line parameters.

It might be a matter of ... style (...), but when I see a structure like

boolean flag;
function functionA() { if (flag) doThis(); else doThat(); }
function functionB() { if (flag) doThis(); else doThat(); }
function functionC() { if (flag) doThis(); else doThat(); }
...

then this just screams "interface" to me.

This is a draft of converting style into such an interface/abstract base class, with specific implementations styleMd and styleAdoc.

One could consider going further with that, in a larger refactoring, and turning the current style into something like a "generator backend" that just receives the respective parts of the schema, and generates output. This might be generalized so that it could create everything between a single MD file or a set of HTML files, transparently. This would imply defining the best interface for this "backend", and right now, there are things in the generateMarkdown.js that could reasonably be part of such a more generic backend. But I wanted to create this DRAFT PR to gather feedback of whether any efforts in this direction are worth pursuing.

---

Edit:

One technical detail that is related to this: There are currently some hard-wired prefixes for the #anchor names that are generated. This could or should be configurable for the case that multiple property references (with possibly duplicate names) are supposed to be merged into one larger document.

[emackey]

This seems good. I vaguely remember trying to start down a path similar to this when I first added ADoc, but there was so much code duplication between the two similar styles that I switched to the isADoc thing halfway through. But this architecture you have here seems better, and would allow additional output styles to be added in the future much more easily.

[javagl]

This is obsolete due to a larger refactoring at https://github.com/javagl/wetzel/tree/generate-3dtiles

[emackey]

It's a little disappointing if glTF and 3D Tiles end up with forked-and-different copies of the same tool to generate the same kind of documentation. Seems like double the maintenance burden, in the long run. But I don't have the time budget for reviewing 3D Tiles' changes to wetzel, unfortunately, so it appears we're stuck on that path.

[javagl]

It's indeed unfortunate. But there have been too many things for the 3D Tiles JSON schema that wetzel choked on.

Roughly corresponding to the draft PRs that I just closed, but maybe incomplete:

• Multiple search paths
• Multiple input files (!)
• Documentation for additionalProperties (beyond true)
• Circular references (replaceRef didn't play well with that)
• Support for definitions (that's a big one - the whole structure (e.g. the typeName) was not laid out for that)
• Support for oneOf - or more generally, ""inheritance"" (yeah, it's not really "inheritance", but at some point, has to be treated like that)
• ... + many smaller ones

The state in the linked branch is far from perfect, but works for 3D Tiles right now. The glTF output is 99% equal (I omitted some page breaks, really trivial things). And it actually includes fixes that will be required for the upcoming "glTF 2.0.x specification". But in terms of the output, these are really small fixes, so I assume that they'd rather be done based on the main state of wetzel (tagging this as "wetzel 0.2.4", to be included in the glTF-spec-Docker container and such), and ... later we can see what to do with the 3D Tiles branch...

[emackey]

The glTF output is 99% equal (I omitted some page breaks, really trivial things).

As you mentioned in a separate email, the output is all that matters to glTF. The tool could be completely rewritten from scratch if you like, so long as the output was >99% the same. The output doesn't even need to match byte-for-byte exactly; we just need to avoid output changes that would be meaningful to Khronos & ISO (lost descriptions, large changes to HTML/PDF formatting, etc.)

It would be great to have only one tool to maintain instead of two. The tests could contain examples from both glTF and 3D Tiles, to make sure it's working correctly for both.

[javagl]

Apart from the desired changes, there are no semantic changes in the glTF output.

(The "desired" ones being: The properties of extensions are of type object instead of Extension, and the minProperties are mentioned in the generated documentation).

But...

• I haven't run the unit tests yet (only checked the glTF output, by having a diff-viewer open during refactoring and re-generating it after each change)
• There still are things of which I know that they are "wrong" - i.e. they will not work for certain JSON schemas - even though they work for 3D Tiles and glTF right now
• The overall structure changed significantly, and I don't know whether the new structure is acceptable

The changes also refer to the CLI. Some parameters have been changed, added, and removed. However, the command line is quite long-ish and a bit clumsy anyhow, so I considered to just put all these parameters into some sort of wetzel-gltf-config.json that can just be read at startup...

For the updates of the glTF spec that I mentioned above, there could be "small" fixes in main. Depending on the available time and schedule, I'll try to do some minor cleanups in the refactored state, and maybe we can think about a merge then.

[emackey]

The "desired" ones being: The properties of extensions are of type object instead of Extension

Just so I'm clear, does this mean for example this properties table would list image.extensions as being type object (or any) instead of type Extension?

I think that part is incorrect, as it would allow something like:

    "images" : [{
        "extensions" : {
            "EXT_single_number" : 42
        }
    }]

This is disallowed in glTF. Each extension must itself be an object. The extensions type is a dictionary of objects, not a simple object.

For extras the situation is a bit more murky. For legacy reasons we can't rule out type any, for example "extras" : 42 is technically legal glTF. But if you follow the link to the extras type, it will use the keyword "SHOULD" to say that you should be treating extras as an object, not any other type. The contents of that object are unspecified though, unlike extensions. But for these reasons, it's preferable to keep the links to specified types for extensions and extras.

[javagl]

Sorry, that probably wasn't clear: This specifically referred to KhronosGroup/glTF#2158 - the issue contains more details.

(EDIT: Referring to your example of image: It is about https://www.khronos.org/registry/glTF/specs/2.0/glTF-2.0.html#_image_extensions saying "Type of each property: Extension")

(The point about the minProperties is tracked in KhronosGroup/glTF#2165 )

[emackey]

Ah, thanks for the clarification! And apologies for the misunderstanding.

So, this seems all the more reason we should be merging these updates into the main copy of wetzel.

[javagl]

The point of "'Extension' vs. object" is actually already addressed in #79 .

The minProperties change could also be a similarly trivial fix: Just copy these lines to the right place (and I'd probably do this in the near future, just to have a state that can be used to generate the next release of the glTF spec).

But beyond that, I'm hesitating to create a PR and suggest to merge the refactored state. On the one hand, the proof is in the pudding, and the output is all that counts. On the other hand: It's not all that counts: wetzel has many stars, forks and users, and the changes are everything but "backward compatible" (also referring to the CLI).

(Apart from the usual things that cause some programmer's weltschmerz and hesitation: I don't know whether it's 'acceptable' in terms of the code style (because I'm not a JavaScript expert). There is some legacy code. There are some undocumented functions. There is not (yet) enough coverage on 'unit-test' level. There still are some TODOs...).