Merge lp:~gz/bzr-xmloutput/relocate__escape_cdata into lp:bzr-xmloutput

[Martin Packman]

Import xml escaping function through local module without relying on bzrlib elementtree monkey-patched version

In fixing bug 614522 I removed the monkey-patching artefact of a copy of the elementtree _escape_cdata function from bzrlib.xml_serializer, which bzr-xmloutput is relying on.

Despite being blasé about how easy this is to fix to Vincent, I got pretty stuck last night trying to create a branch that didn't change code all over the place.

It was sort of a choice between:
* Expand every import of _escape_cdata to look in all *three* locations elementtree may live.
* Stop using relative imports everywhere and just do the import dance in the base package, and import from there using bzrlib.plugins.xmloutput as the name.
* Write a new escaping function somewhere and use that, the elementtree one isn't really sufficient anyway.
* Import from elementtree via bzrlib.xml_serializer everywhere.
* Make a new module to put the import dance in and import from there.

In the end, this branch does the last two, plus pokes a few imports statements that I couldn't resist nearby.

There are some bits of code I had in this branch, but I have not included here:
* Fixing the Python 2.5ism in test_info_xml.
* Teasing out the needlessly lazy imports from the helpful ones in the base package.
* Importing from bzrlib.plugins.xmloutput rather than using relative imports.
* Some more import cleanup.
* Creating xml prolog function rather than having the same user encoding inserting logic in lots of places.
* Making the prolog use an encoding expat can actually parse.

I can put up branches for any of these that there's interest in landing, but what I'd really like to see, and haven't coded, is a sane abstraction for writing xml that actually ensures well-formedness.

Still unresolved, ElementTree 1.3 (used in Python 2.7) makes the encoding parameter non-optional, for good reason, so this wants changing to something better sooner rather than later.

[Guillermo Gonzalez (verterok)]

Wow, this looks great.

Thanks a lot for putting this together.

Yes, there is interest in any improvement/fix/code cleanup you might have in mind (or already coded ;-))
And I agree with you on most of the exposed above, mostly on the need of a unified way to generate the xml (or json, or protobuf or <another format of your choice>).

I'm a bit short on time (as the commit log shows) :-(
But I hope to get back to work on bzr-xmloutput and bzr-eclipse in the short term again.

review: Approve

[Guillermo Gonzalez (verterok)]

Please set a commit message and I'll merge this in trunk.

Thanks!

[Martin Packman (gz)]

> Yes, there is interest in any improvement/fix/code cleanup you might have in
> mind (or already coded ;-))

I'll put up another branch or two with the less invasive bits.

> I'm a bit short on time (as the commit log shows) :-(

I understand completely...

=== modified file '__init__.py'
--- __init__.py	2010-02-08 04:26:45 +0000
+++ __init__.py	2010-09-05 18:09:43 +0000
@@ -41,7 +41,6 @@
     option,
     log,
     workingtree,
-    xml_serializer,
     errors
     )
 
@@ -278,7 +277,7 @@
         self.outf.write('<?xml version="1.0" encoding="%s"?>' % \
                 bzrlib.osutils.get_user_encoding())
         self.outf.write('<plugins>')
-        from bzrlib.xml_serializer import _escape_cdata
+        from writer import _escape_cdata
         for name, plugin in bzrlib.plugin.plugins().items():
             self.outf.write('<plugin>')
             self.outf.write('<name>%s</name>' % name)

=== modified file 'annotatexml.py'
--- annotatexml.py	2009-09-23 03:29:47 +0000
+++ annotatexml.py	2010-09-05 18:09:43 +0000
@@ -35,11 +35,12 @@
     # to support bzr < 1.8
     from bzrlib.annotate import _annotate_file
 
-
 from bzrlib.annotate import _annotations
-from bzrlib.xml_serializer import _escape_cdata
 from bzrlib import osutils
 
+from writer import _escape_cdata
+
+
 empty_annotation = 'revno="" author="" date=""'
 
 def annotate_file_xml(branch, rev_id, file_id, to_file=None,

=== modified file 'logxml.py'
--- logxml.py	2009-07-02 16:12:44 +0000
+++ logxml.py	2010-09-05 18:09:43 +0000
@@ -1,5 +1,7 @@
 # -*- encoding: utf-8 -*-
 
+import os
+
 from bzrlib.lazy_import import lazy_import
 lazy_import(globals(), """
 import bzrlib
@@ -8,10 +10,10 @@
     osutils,
     log,
     )
-from bzrlib.xml_serializer import _escape_cdata
-import os
 """)
 
+from writer import _escape_cdata
+
 
 class XMLLogFormatter(log.LogFormatter):
     """ add a --xml format to 'bzr log'"""

=== modified file 'lsxml.py'
--- lsxml.py	2009-11-15 19:27:23 +0000
+++ lsxml.py	2010-09-05 18:09:43 +0000
@@ -19,14 +19,15 @@
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA
 #
 
+import os
 
 from bzrlib.lazy_import import lazy_import
 lazy_import(globals(), """
-import os
-from bzrlib import bzrdir, errors, osutils, xml_serializer
-from bzrlib.xml_serializer import _escape_cdata
+from bzrlib import bzrdir, errors, osutils
 """)
 
+from writer import _escape_cdata
+
 
 def show_ls_xml(outf, revision=None, non_recursive=False,
             from_root=False, unknown=False, versioned=False,

=== modified file 'statusxml.py'
--- statusxml.py	2009-07-05 19:59:00 +0000
+++ statusxml.py	2010-09-05 18:09:43 +0000
@@ -26,19 +26,18 @@
 import os, sys, time
 from bzrlib import (
     diff,
-    trace,
     errors,
+    osutils,
     revision as _mod_revision,
     status,
-    osutils,
+    trace,
     )
 
-from bzrlib.osutils import terminal_width
-from bzrlib.xml_serializer import _escape_cdata
-from bzrlib.trace import warning
 import logxml
 """)
 
+from writer import _escape_cdata
+
 
 def show_tree_status_xml(wt, show_unchanged=None,
                      specific_files=None,
@@ -119,7 +118,7 @@
         new_is_working_tree = True
         if revision is None:
             if wt.last_revision() != wt.branch.last_revision():
-                warning("working tree is out of date, run 'bzr update'")
+                trace.warning("working tree is out of date, run 'bzr update'")
             new = wt
             old = new.basis_tree()
         elif len(revision) > 0:

=== modified file 'versionxml.py'
--- versionxml.py	2009-09-23 03:29:47 +0000
+++ versionxml.py	2010-09-05 18:09:43 +0000
@@ -35,7 +35,8 @@
     trace,
     )
 from bzrlib.version import _get_bzr_source_tree
-from bzrlib.xml_serializer import _escape_cdata
+
+from writer import _escape_cdata
 
 
 def show_version_xml(show_config=True, show_copyright=True, to_file=None):

=== added file 'writer.py'
--- writer.py	1970-01-01 00:00:00 +0000
+++ writer.py	2010-09-05 18:09:43 +0000
@@ -0,0 +1,10 @@
+"""Tools for writing xml
+
+Suggestion for this module: create a simple xml writer here and expurgate all
+other modules of angle brackets entirely.
+"""
+
+import bzrlib.xml_serializer
+
+# Use xml_serializer to avoid duplicating the elementtree location logic
+_escape_cdata = bzrlib.xml_serializer.elementtree.ElementTree._escape_cdata

=== modified file 'xml_errors.py'
--- xml_errors.py	2009-09-23 03:29:47 +0000
+++ xml_errors.py	2010-09-05 18:09:43 +0000
@@ -1,15 +1,16 @@
 """ XMLError handling module """
+
 from bzrlib.lazy_import import lazy_import
 lazy_import(globals(), """
-import bzrlib
 from bzrlib import (
     errors,
-    debug,
+    osutils,
+    trace,
     )
 """)
 
-from bzrlib.xml_serializer import _escape_cdata
-from bzrlib import trace
+from writer import _escape_cdata
+
 
 class XMLError(errors.BzrError):
     """ A class that wraps an BzrError and 'serialize' it as xml."""
@@ -21,7 +22,7 @@
     def __str__(self):
         """__str__"""
         xml = '<?xml version="1.0" encoding="%s"?>' % \
-                bzrlib.osutils.get_user_encoding()
+                osutils.get_user_encoding()
         try:
             xml += '<error>%s</error>' % self.get_cause_xml()
         except Exception, e: