XML Prolog support #34

laubstein · 2016-08-16T15:21:12Z

This commit fixes #33 .

The code supports only version, encoding and standalone xml prolog attributes (here).
The new special attribute (xmlProlog) is using the default attrPrefix, is this ok?

I have added two new configs:

forceXmlProlog: false,
defaultXmlProlog: '<?xml version="1.0" encoding="UTF-8"?>'

Unit test

  describe('xml prolog', function() {
    it('should be consistent', function() {
      var originalString = '<?xml version="1.0"?><a>1</a>';
      var xmlObject = JXON.stringToXml(originalString);
      var jsonFromXml = JXON.xmlToJs(xmlObject);
      var jsonFromString = JXON.stringToJs(originalString);

      assert.equal(originalString, JXON.jsToString(jsonFromXml));
      assert.equal(originalString, JXON.jsToString(jsonFromString));

      // PhantomJS do not serialize the prolog, so we are forcing a default
      JXON.config({
        forceXmlProlog: true,
        defaultXmlProlog: '<?xml version="1.0"?>'
      });
      assert.equal(originalString, JXON.xmlToString(xmlObject));
    });
  });

The new JSON representation from

<?xml version="1.0"?>
<a>foo</a>

will be...

{
    "a": "foo",
    "$xmlProlog": {
        "version":"1.0"
    }
}

Firefox always serialize XML with '<?xml version="1.0" encoding="UTF-8"?>'

tyrasd · 2016-08-17T15:42:31Z

jxon.js

  };
  var aCache = [];
  var rIsNull = /^\s*$/;
  var rIsBool = /^(?:true|false)$/i;
+  var rXmlProlog = /^(<\?xml.*?\?>[\n]?)/;
+  var rXmlPrologAttributes = /\b(version|encoding|standalone)="([^"]+?)"/g;


these attributes could also be quoted in single quotes ' according to the specs

My mistake... that's easy to fix.

tyrasd · 2016-08-17T16:02:03Z

supports only version, encoding and standalone

that's ok, because these are the only three allowed pseudo-attributes of the xml declaration (see https://www.w3.org/TR/2004/REC-xml-20040204/#sec-prolog-dtd)

using the default attrPrefix

Yeah, that's fine, because the top-most level in the JXON representation cannot currently have any attributes, as it would reference the "parent" of the xml root element. The analogy that this should apply to the pseudo-attributes of the xml document suggests itself.

But I think your current implementation is a bit over-complicated, if you ask me. What do you think about the following proposals:

I think the more commonly used name for the xml prolog is xml declaration, let's use a nomenclature that's a bit closer to that.
You're adding a couple of helpers only to parse something that's than immediately stringified again. What if we screw all that and just save everything that matches the xml declaration regexp into a string (i.e. as if it were a full xml node)? That would save us a lot of code and for also most (or all?) of the problems I've highlighted in the PR.
make forceXmlProlog default to true.

laubstein · 2016-08-17T16:41:21Z

I will try to fix all points and them submit a new commit. 👍

laubstein added 4 commits August 16, 2016 12:00

Fixes tyrasd#33 - Added support to xml prolog

6849751

Minified version

e8add2c

Fixed a problem with firefox

ba6a86a

Firefox always serialize XML with '<?xml version="1.0" encoding="UTF-8"?>'

Minified version

a7aa48e

tyrasd reviewed Aug 17, 2016
View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

XML Prolog support #34

XML Prolog support #34

laubstein commented Aug 16, 2016

tyrasd Aug 17, 2016

laubstein Aug 17, 2016 •

edited

Loading

tyrasd commented Aug 17, 2016

laubstein commented Aug 17, 2016

XML Prolog support #34

Are you sure you want to change the base?

XML Prolog support #34

Conversation

laubstein commented Aug 16, 2016

Unit test

tyrasd Aug 17, 2016

Choose a reason for hiding this comment

laubstein Aug 17, 2016 • edited Loading

Choose a reason for hiding this comment

tyrasd commented Aug 17, 2016

laubstein commented Aug 17, 2016

laubstein Aug 17, 2016 •

edited

Loading