Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add .nsDecls API #271

Closed
wants to merge 3 commits into from
Closed

Add .nsDecls API #271

wants to merge 3 commits into from

Conversation

hildjj
Copy link
Contributor

@hildjj hildjj commented Nov 3, 2014

Fixes #270. New API:

  var str = '<html xmlns="urn:example" xmlns:ex1="urn:example:1"><body/></html>';
  var doc = libxml.parseXmlString(str);
  var root = doc.root();
  var decls = root.nsDecls()

decls now includes two namespace objects. Applying to the body element yields an empty array.

Tests included. I'll add docs to the wiki if you take the patch. I'm open to renaming the API anything you like.

@hildjj
Add .nsDecls API
c1d09f9
@hildjj
Merge branch 'master' of https://github.com/polotek/libxmljs into nam…
ced66a1 
…espace-declarations

* 'master' of https://github.com/polotek/libxmljs:
  travis: change node 0.11 to 0.12
  travis: simplify before_install
  Upgrade to NAN 1.7.0
  Add .namespace() to Attribute
  0.13.0
  update nan to 1.5.1 and fix issues with the latest v8
  Added test for RELAX NG validation
  Added missing include for RELAX NG schema validation
  Added support for RELAX NG schema validation
@hildjj
Copy link
Contributor Author

hildjj commented Apr 8, 2015

This PR does still work. Would you mind either merging this, giving me something to change, or proposing another approach for #270?

@teleological
Copy link
Collaborator

Is there a use case to prefer this to namespaces() i.e. xmlGetNsList ? In any case the node will need to be type checked since attributes don't include an nsDef member.

@hildjj
Copy link
Contributor Author

hildjj commented Nov 9, 2015

.namespaces() gives the set of namespaces that are currently in scope for a given node. For the example given above:

var body = root.childNodes(0);
body.namespaces();

still yields two namespaces. If you're iterating through a document, keeping track of the current state of the namespace stack just to watch for changes is somewhat painful.

Returning to this code, I kind of hate the string nsDecls, and wouldn't mind changing it. I'll be happy to add a node check if there's a shot you're going to take this.

@rchipka
Copy link
Member

rchipka commented Nov 16, 2015

Maybe we can make it namespace([local]) and namespaces([local]) instead, where local is an optional boolean argument specifying if we should return all namespaces or local namespace declarations.

If you can change this to a boolean argument for namespace() and update the NAN calls to version 2, I see no reason why we can't merge this.

hildjj added a commit to hildjj/libxmljs that referenced this pull request Nov 17, 2015
@hildjj
Fixes libxmljs#270, replaces libxmljs#271
f3aae8f
hildjj added a commit to hildjj/libxmljs that referenced this pull request Nov 17, 2015
@hildjj
Fixes libxmljs#270, replaces libxmljs#271
34d55d2
@hildjj hildjj mentioned this pull request Nov 17, 2015
Merged
@hildjj
Copy link
Contributor Author

hildjj commented Nov 17, 2015

Started a new PR at #363, with the suggested API.

@hildjj hildjj closed this Nov 17, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

No API for namespace declarations
3 participants
@hildjj @teleological @rchipka