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.

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

Add c14n for node and document #138

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add c14n for node and document #138

wants to merge 1 commit into from

Conversation

saks
Copy link

@saks saks commented Sep 6, 2024

There are not too many choices when it comes to "Canonical XML" implementation for rust. This PR is a compilation of ideas, approaches and test files from the following projects:

Work items

  • add canonicalize method for Node and Document
  • add Node::ancestors method (I personally find it very useful when using "nokogiri", plus it help in tests a lot). Although I don't feel too strongly about, can remove if it doesn't fit.
  • add Node::at_xpath method. Yes, it is similar to findnodes (and can ultimately be implemented using Context), but with design from "nokogiri", I find it a lot more flexible when navigating a tree. Same as for the previous item, can update PR to remove it if necessary.

Any feedback is welcome! I don't feel particularly attached to any approach here. Please let me know how to make this PR better.

@dginev
Copy link
Member

dginev commented Sep 6, 2024

@saks interesting, there is a very high-level question I have first.

Why do you think this wrapper crate is the right place to introduce this method?
rust-libxml mostly tries to follow the libxml2 coverage, and probably should not grow much further, as a general-purpose XML utility library.

As you mention xml_c14n exists as a separate Rust crate. Maybe your work could also be packaged as a separate crate?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants