Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should the Document constructor use a different parser for collapse_whitespace? #54

Open
JKatzwinkel opened this issue Jul 28, 2022 · 6 comments
Labels
design Proposals and discussion of API changes
Milestone

Comments

@JKatzwinkel
Copy link
Contributor

JKatzwinkel commented Jul 28, 2022

lxml.etree.tostring with pretty_print=True has this caveat:

If lxml cannot distinguish between whitespace and data, it will not alter your data. Whitespace is therefore only added between nodes that do not contain data. This is always the case for trees constructed element-by-element, so no problems should be expected here. For parsed trees, a good way to assure that no conflicting whitespace is left in the tree is the remove_blank_text option [...]

Now instantiating a delb.Document with the collapse_whitespace flag somewhat feels like it should do away with whitespaces in a way that makes the parsed XML suitable for custom formatting, e.g. calling:

  lxml.etree.tostring(document.root._etree_obj, pretty_print=True)

...or something like this. However, in order to be able to pretty print delb content, it is still necessary to use a custom parser on instantiation, e.g.

  document = Document(source, parser=etree.XMLParser(remove_blank_text=True))

...in which case the collapse_whitespace flag of the Document constructor isn't even relevant.

I feel like wanting to pretty-print delb objects as a usecase is somewhat justified (I needed it today in order to simplify a test), and think that this behaviour is somewhat obscured right now and should at least be documented in some way. But maybe this could even be handled in a more user-friendly way. Is there a point in using delb.Document with collapse_whitespace without an lxml parser that also removes whitespace or could the use of such a parser perhaps be implied by collapse_whitespace in general?

Should TagNode have a tostring method with an optional pretty_print flag as well?

@funkyfuture
Copy link
Contributor

after taking a few superficial glances, just some open ended threads:

  • the _collapse_whitespace method implements the recommendations linked in Document.collapse_whitespace, iirc
  • i assume that these recommendations generally make sense for text documents
  • i also assume that remove_blank_text is rather harsh in its doings, i guess i would have used that option if it was compatible with the aforementioned recommendations
  • does lxml's pretty serialization comply with these recommendations?
  • a serialization must not add whitespace here, for example: la<hi rendition="u">la</hi>la
  • can we agree that a pretty serialization must yield a result that is, when again parsed with the collapse_whitespace option, identical to the parsing of an unpretty Serialisat?

@funkyfuture funkyfuture added the design Proposals and discussion of API changes label Jul 30, 2022
@funkyfuture
Copy link
Contributor

i've been thinking that whitespace would make a good major topic for the 0.5 version. and, instead of relying on libxml specifics, we can implement serialization natively. that's to be engineered for the Rust implementation anyway and we can look at the API design (pretty-formatted string representations? always move all namespace declarations to the root node when serializing a document?).

@funkyfuture funkyfuture added this to the 0.5 milestone Oct 9, 2022
@funkyfuture
Copy link
Contributor

the last cell in the docs/getting_started.ipynb also gives a great example where pretty is broken. so that demo-case could be taken as one test.

@funkyfuture
Copy link
Contributor

i started looking into this which led me to realise that the serialization doesn't consider the xml:space attribute yet.

@funkyfuture
Copy link
Contributor

quick update: yesterday i was honest enough to meself to realize that i'm actually traumatized by the task of producing properly placed whitespace. but i still think the target is in eye's sight. let's hope the XML Foundation covers rehab.

@funkyfuture
Copy link
Contributor

currently right now i'm in a manic phase (yes diggin to solve the problem got me to new experiences) and i imagine that the implementation will produce the most beautifullest XML that the world has ever seen and only the radiated überhumen on Mars will be able to deliver something better. anyway after a few hints by @zed-g i have the idea to compile an appendix for the documentation that compares "pretty" XML serialisat productions by different serializers for a small variety of samples.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Proposals and discussion of API changes
Projects
None yet
Development

No branches or pull requests

2 participants