minidom pretty xml output improvement

[teajayfr]

BPO	1777134
Nosy	@devdanzin, @ezio-melotti
Files	minidom_output_improvement.diff: Improves the minidom pretty xml output minidom_output_improvement.patch: Improves the minidom pretty xml output. This replaces the previous patch. minidom_output_improvement_v3.patch: teajay_fr's patch updated against trunk

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2013-01-27.04:01:16.951>
created_at = <Date 2007-08-19.08:42:36.000>
labels = ['easy', 'type-feature', 'library']
title = 'minidom pretty xml output improvement'
updated_at = <Date 2013-01-27.04:01:16.949>
user = 'https://bugs.python.org/teajayfr'

bugs.python.org fields:

activity = <Date 2013-01-27.04:01:16.949>
actor = 'ezio.melotti'
assignee = 'none'
closed = True
closed_date = <Date 2013-01-27.04:01:16.951>
closer = 'ezio.melotti'
components = ['Library (Lib)']
creation = <Date 2007-08-19.08:42:36.000>
creator = 'teajay_fr'
dependencies = []
files = ['8177', '9482', '13042']
hgrepos = []
issue_num = 1777134
keywords = ['patch', 'easy']
message_count = 12.0
messages = ['53043', '62345', '62435', '62636', '81487', '81707', '81708', '104801', '109955', '110589', '130753', '180741']
nosy_count = 10.0
nosy_names = ['tseaver', 'ggenellina', 'ajaksu2', 'jmg', 'teajay_fr', 'vdupras', 'ezio.melotti', 'rrwood', 'BreamoreBoy', 'D\xc3\xa1vid.G\xc3\xa1bor.Bodor']
pr_nums = []
priority = 'normal'
resolution = 'out of date'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue1777134'
versions = ['Python 3.2']

[teajayfr]

This patch provides a fix to the minidom pretty xml output. In the initial version when outputing textnodes additional line breaks were added at the beginning and the end of the textnode block thus output false textnode content. If you load and save an xml document with textnodes the size of the textnodes increases every time.

To fix this, a simple logic has been added to the textnode output function in order to prevent this behavior.

This is my first patch submission so I hope I haven't done anything wrong. I so please let me know I'll be glad to improve it.

Cheers and keep up the good work.

Teajay

[jmg]

I think this is a good patch. It gives more useful pretty XML output.
I would suggest that possibly this routine be moved to xml.dom or
xml.dom.utils instead of being part of minidom since it should not be
minidom specific.

There is one bug in the patch in that:
node.writexml(writer, (%s%s) % (indent,addindent)

the parens around the %s%s should be quotes instead.

[vdupras]

If the patch would have better styling ("if(onetextnode == True):"), correct the test it breaks, and even better, add new ones, I guess it
would be an acceptable one.

[teajayfr]

Thanks for the feedback. I looked at the issues you mentionned and tried
to sort that out. You might want to have a look it this new patch. I ran
the tests, added a new test case and hopefully managed to get the code
style right this time :-)

Let me know if there is anything else I need to improve.

[rrwood]

This patch would be very useful to me, so I'm sad to see it's been
languishing for so long. :-(

Is there any way to encourage the maintainer to merge this into the
current branch?

[devdanzin]

@roy: we can try :)

Patch updated, tests pass. However, keeping the default output and
adding an option to prettyprint should keep lots of current users sane,
while those wanting prettier output could have it.

[rrwood]

Thanks! :-)

On Wed, Feb 11, 2009 at 9:28 PM, Daniel Diniz <report@bugs.python.org>wrote:

Daniel Diniz <ajaksu@gmail.com> added the comment:

@roy: we can try :)

Patch updated, tests pass. However, keeping the default output and
adding an option to prettyprint should keep lots of current users sane,
while those wanting prettier output could have it.

----------
keywords: +easy
nosy: +ajaksu2
stage: -> patch review
type: -> feature request
versions: +Python 2.6, Python 2.7
Added file:
http://bugs.python.org/file13042/minidom_output_improvement_v3.patch

---

Python tracker <report@bugs.python.org>
<http://bugs.python.org/issue1777134\>

---

[tseaver]

The patch applies cleanly to the 2.6 branch, and with minimal fuzz to
the trunk. Exsting tests pass in both cases, as does the new test.

The added testcase seems plainly correct.

The first hunk of the diff to Lib/xml/dom/minidom.py is a little messy
/ hard to follow:

• It makes the module less PEP-8 conformant (line length > 80, operator
  spacing)

• The 'onetextnode' flag is set, but never used.

The second hunk is somewhat problematic: it changes 'Text.writexml' to
ignore any explicit 'indent' or 'newl' argument passed, which is a
backwards-incompatible behavior change. Perhaps changing the default
values for those arguments to 'None', and then using the new code path
in that case, would be the better course, while falling back to the old
one if either is passed.

I'm not sure about the justification for the third hunk at all (removing
the newline at the end of the XML prologue). There *can't* be any
meaningful whitespace outside of the root element, so we shouldn't have
round-trip issues if we leave it.

[terryjreedy]

Removed junk copy of message

[BreamoreBoy]

Looking at the comments here msg104801 seems like more work needs to be done on the patch.

[DvidGborBodor]

I would prefer to see this improvement as an option, rather than the default, because I believe that 'bpo-4147' satisfies "pretty printing" better.

While leaving out whitespace from text-only elements is benefical for compatibility and roundtripping, there are certain situation where it hurts the prettyness of the xml really hard, an example:

<root>
<element1>
TextNode1
<childnode2 />
<childnode3 />
TextNode4
</element1>
<element2>TextNode</element2>
</root>

vs.

<root>
<element1>TextNode1<childnode2 />
<childnode3 />TextNode4</element1>
<element2>TextNode</element2>
</root>

[ezio-melotti]

I think this can be closed, since bpo-4147 already improved tpprettyxml().