Skip to content

Commit

Permalink
Don't wrap arbitrary nodes as elements, fixes libxmljs#226
Browse files Browse the repository at this point in the history
* Added `type()` to namespaces
  • Loading branch information
Riley Martinez-Lynch committed Nov 16, 2015
1 parent 6dd5848 commit 95bf50d
Show file tree
Hide file tree
Showing 7 changed files with 43 additions and 11 deletions.
1 change: 1 addition & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ module.exports.features = bindings.features;
module.exports.Comment = require('./lib/comment');
module.exports.Document = Document;
module.exports.Element = require('./lib/element');
module.exports.Namespace = require('./lib/namespace');
module.exports.Text = require('./lib/text');

// Compatibility synonyms
Expand Down
4 changes: 4 additions & 0 deletions lib/comment.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,9 @@ var Comment = function(doc, content) {

Comment.prototype = bindings.Comment.prototype;

Comment.prototype.name = function() {
return "comment";
};

module.exports = Comment;

14 changes: 14 additions & 0 deletions lib/namespace.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
var bindings = require('./bindings');

var Namespace = function() {
throw new Error('cannot be directly instantiated');
};

Namespace.prototype = bindings.Namespace.prototype;

Namespace.prototype.type = function() {
return "namespace_decl";
};

module.exports = Namespace;

4 changes: 4 additions & 0 deletions lib/text.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,8 @@ function Text(doc, content) {

Text.prototype = bindings.Text.prototype;

Text.prototype.name = function() {
return "text";
};

module.exports = Text;
2 changes: 1 addition & 1 deletion src/xml_element.cc
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ XmlElement::get_child(int32_t idx) {
if (!child)
return scope.Escape(Nan::Null());

return scope.Escape(XmlElement::New(child));
return scope.Escape(XmlNode::New(child));
}

v8::Local<v8::Value>
Expand Down
24 changes: 19 additions & 5 deletions src/xml_node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -218,14 +218,28 @@ XmlNode::New(xmlNode* node)
{
Nan::EscapableHandleScope scope;
switch (node->type) {

case XML_ATTRIBUTE_NODE:
return scope.Escape(XmlAttribute::New(reinterpret_cast<xmlAttr *>(node)));

case XML_TEXT_NODE:
return scope.Escape(XmlText::New(node));

case XML_COMMENT_NODE:
return scope.Escape(XmlComment::New(node));

case XML_DOCUMENT_NODE:
case XML_HTML_DOCUMENT_NODE:
#ifdef LIBXML_DOCB_ENABLED
case XML_DOCB_DOCUMENT_NODE:
#endif
return scope.Escape(XmlDocument::New(reinterpret_cast<xmlDoc *>(node)));

case XML_NAMESPACE_DECL:
return scope.Escape(XmlNamespace::New(reinterpret_cast<xmlNs *>(node)));

case XML_ELEMENT_NODE:
default:
// if we don't know how to convert to specific libxmljs wrapper,
// wrap in an XmlElement. There should probably be specific
// wrapper types for text nodes etc., but this is what existing
// code expects.
return scope.Escape(XmlElement::New(node));
}
}
Expand Down Expand Up @@ -360,7 +374,7 @@ XmlNode::get_parent() {
Nan::EscapableHandleScope scope;

if (xml_obj->parent) {
return scope.Escape(XmlElement::New(xml_obj->parent));
return scope.Escape(XmlNode::New(xml_obj->parent));
}

return scope.Escape(XmlDocument::New(xml_obj->doc));
Expand Down
5 changes: 0 additions & 5 deletions test/text.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,6 @@ module.exports.setters = function(assert) {
module.exports.getters = function(assert) {
var doc = libxml.Document();
var elem = libxml.Text(doc, 'getters');

assert.throws(function() {
elem.name();
}, "text nodes should NOT expose a name");

assert.equal('text', elem.type());
assert.done();
};
Expand Down

0 comments on commit 95bf50d

Please sign in to comment.