Skip to content

Conversation

@jayvdb
Copy link
Contributor

@jayvdb jayvdb commented Dec 20, 2015

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
Copy link
Owner

djc commented Dec 20, 2015

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
Copy link
Owner

djc commented Dec 20, 2015

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

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.
@jayvdb
Copy link
Contributor Author

jayvdb commented Dec 20, 2015

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
Copy link
Owner

djc commented Dec 20, 2015

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

@djc djc closed this Dec 20, 2015
@jayvdb
Copy link
Contributor Author

jayvdb commented Dec 20, 2015

much appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants