Repository navigation
minidom pretty xml output improvement #45325
Description
| BPO | 1777134 |
|---|---|
| Nosy | @devdanzin, @ezio-melotti |
| Files |
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']
Activity
teajayfr commentedon Aug 19, 2007
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 commentedon Feb 13, 2008
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 commentedon Feb 15, 2008
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 commentedon Feb 21, 2008
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 commentedon Feb 9, 2009
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 commentedon Feb 12, 2009
@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 commentedon Feb 12, 2009
Thanks! :-)
On Wed, Feb 11, 2009 at 9:28 PM, Daniel Diniz <report@bugs.python.org>wrote:
tseaver commentedon May 2, 2010
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 commentedon Jul 11, 2010
Removed junk copy of message
BreamoreBoy commentedon Jul 17, 2010
Looking at the comments here msg104801 seems like more work needs to be done on the patch.
DvidGborBodor commentedon Mar 13, 2011
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 commentedon Jan 27, 2013
I think this can be closed, since bpo-4147 already improved tpprettyxml().