djc / rnc2rng

RELAX NG Compact to regular syntax conversion library
MIT License
16 stars 13 forks source link

Do not emit grammar node inside root node #6

Closed jayvdb closed 8 years ago

jayvdb commented 8 years ago

XMLSerializer always emits a grammar node as the root node. As a result, no other grammar nodes may be emitted as children.

For each included rnc, validate the included root attributes are present in the main root document, and process the included rnc without its root attribute.

djc commented 8 years ago

This looks sensible, thanks!

I tried to get some more detail on how to handle includes from the spec, but it doesn't become very clear to me. In any case, this seems like a straightforward improvement (plus, it fixes your issue!).

I presume you've looked at trang output to see how it handles includes?

With the minor nit I pointed out fixed, I'm happy to merge this.

(One thought that came up during review of the patch is that we should probably strip the quotes and spaces from namespace and types declarations eagerly, during parsing. That would make this code a bit simpler, and is probably a more sensible separation of concerns. Do you agree?)

djc commented 8 years ago

Also, I think you'd probably like to get a new release on PyPI after this is merged?

jayvdb commented 8 years ago

wrt to includes, I was mostly guided by lxml and trang, but I looked at the specs enough to confirm the grammar node may only appear once in each document.

One aspect I am not confident about is how many start elements are allowed in the grammar node, and whether each include can also have its own start element.

I'd love a new release before the end of January, but earlier is better.

Stripping quotes and spaces during parsing sounds good. better to remove that early so it doesn't become a feature of your data model that people rely on.

A new release would be lovely.

djc commented 8 years ago

Merged with very slight modifications in d92f4a9c8e9c26c8eee970577bec3f1a469c0b25. Moved the stripping of quotes (and spaces) to the parser in 68f8eafac03c74b544b292898aee2c34eecd94dc and uploaded the resulting 2.2 version to PyPI. Thanks!

jayvdb commented 8 years ago

much appreciated.