classification
Title: minidom pretty xml output improvement
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.2
process
Status: closed Resolution: out of date
Dependencies: Superseder:
Assigned To: Nosy List: BreamoreBoy, Dávid.Gábor.Bodor, ajaksu2, ezio.melotti, ggenellina, jmg, rrwood, teajay_fr, tseaver, vdupras
Priority: normal Keywords: easy, patch

Created on 2007-08-19 08:42 by teajay_fr, last changed 2013-01-27 04:01 by ezio.melotti. This issue is now closed.

Files
File name Uploaded Description Edit
minidom_output_improvement.diff teajay_fr, 2007-08-19 08:42 Improves the minidom pretty xml output
minidom_output_improvement.patch teajay_fr, 2008-02-21 18:00 Improves the minidom pretty xml output. This replaces the previous patch.
minidom_output_improvement_v3.patch ajaksu2, 2009-02-12 02:28 teajay_fr's patch updated against trunk
Messages (12)
msg53043 - (view) Author: Teajay (teajay_fr) Date: 2007-08-19 08:42
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
msg62345 - (view) Author: John-Mark Gurney (jmg) Date: 2008-02-13 01:07
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.
msg62435 - (view) Author: Virgil Dupras (vdupras) (Python triager) Date: 2008-02-15 19:31
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.
msg62636 - (view) Author: Teajay (teajay_fr) Date: 2008-02-21 18:00
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.
msg81487 - (view) Author: Roy Wood (rrwood) Date: 2009-02-09 20:45
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?
msg81707 - (view) Author: Daniel Diniz (ajaksu2) (Python triager) Date: 2009-02-12 02:28
@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.
msg81708 - (view) Author: Roy Wood (rrwood) Date: 2009-02-12 02:32
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>
> _______________________________________
>
msg104801 - (view) Author: Tres Seaver (tseaver) * Date: 2010-05-02 20:16
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 PEP8 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.
msg109955 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2010-07-11 02:16
Removed junk copy of message
msg110589 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2010-07-17 18:12
Looking at the comments here msg104801 seems like more work needs to be done on the patch.
msg130753 - (view) Author: Dávid Gábor Bodor (Dávid.Gábor.Bodor) Date: 2011-03-13 18:10
I would prefer to see this improvement as an option, rather than the default, because I believe that 'Issue4147' 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>
msg180741 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2013-01-27 04:01
I think this can be closed, since #4147 already improved tpprettyxml().
History
Date User Action Args
2013-01-27 04:01:16ezio.melottisetstatus: open -> closed

nosy: + ezio.melotti
messages: + msg180741

resolution: out of date
stage: patch review -> resolved
2011-03-13 18:10:54Dávid.Gábor.Bodorsetnosy: + Dávid.Gábor.Bodor
messages: + msg130753
2010-11-12 04:59:34terry.reedysetnosy: - terry.reedy
2010-07-17 18:12:21BreamoreBoysetnosy: + BreamoreBoy
messages: + msg110589
2010-07-11 02:16:59terry.reedysetnosy: + terry.reedy

messages: + msg109955
versions: + Python 3.2, - Python 2.6, Python 2.7
2010-07-11 02:16:28terry.reedysetfiles: - unnamed
2010-05-02 20:16:45tseaversetnosy: + tseaver
messages: + msg104801
2009-02-12 02:32:27rrwoodsetfiles: + unnamed
messages: + msg81708
2009-02-12 02:28:45ajaksu2setfiles: + minidom_output_improvement_v3.patch
versions: + Python 2.6, Python 2.7
nosy: + ajaksu2
messages: + msg81707
keywords: + easy
type: enhancement
stage: patch review
2009-02-09 21:11:58ggenellinasetnosy: + ggenellina
2009-02-09 20:45:18rrwoodsetnosy: + rrwood
messages: + msg81487
2008-02-21 18:00:08teajay_frsetfiles: + minidom_output_improvement.patch
messages: + msg62636
2008-02-15 19:31:05vduprassetnosy: + vdupras
messages: + msg62435
2008-02-13 01:07:46jmgsetnosy: + jmg
messages: + msg62345
2007-08-19 08:42:36teajay_frcreate