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

Document to string with xmlSaveOptions #58

Closed
jangernert opened this issue Sep 24, 2019 · 2 comments · Fixed by #59
Closed

Document to string with xmlSaveOptions #58

jangernert opened this issue Sep 24, 2019 · 2 comments · Fixed by #59

Comments

@jangernert
Copy link
Contributor

I would very much welcome some way to serialize a Document that is able to take xmlSaveOptions into account.
I hacked together a branch of rust-libxml locally that does exactly what I need for my application:

  pub fn to_string_custom(&self) -> String {
    unsafe {
      // allocate a buffer to dump into
      let buf = xmlBufferCreate();
      let c_utf8 = CString::new("UTF-8").unwrap();
      let options = xmlSaveOption_XML_SAVE_NO_EMPTY + xmlSaveOption_XML_SAVE_AS_HTML;

      let save_ctx = xmlSaveToBuffer(buf, c_utf8.as_ptr(), options as i32);
      let _size = xmlSaveDoc(save_ctx, self.doc_ptr());

      let result = xmlBufferContent(buf);
      let c_string = CStr::from_ptr(result as *const c_char);
      let node_string = c_string.to_string_lossy().into_owned();
      xmlBufferFree(buf);

      node_string
    }
  }

It would be cool to somehow generalize this and upstream it. Since there is already to_string() that uses xmlDocDumpFormatMemoryEnc I'm not sure how something that uses the xmlSaveCtxt would fit into the picture.

Is the functionality something that could find its way into the code base? And if so, how would you implement it?
I'd like to submit a PR, but think I need some guidance on how this should be implemented.

@dginev
Copy link
Member

dginev commented Sep 24, 2019

Certainly possible and welcome! The only reason it hasn't been added is that no one needed it yet.

On a related note, I have been recurrently reminded by clippy that the libxml Document::to_string is actually not compliant with the rust conventions, due to the optional boolean argument. So we may combine your feature with a refactor that implements Display on Document with a simple default to_string that takes no arguments, while evolving the "advanced" serialization as you mention.

I could see a publicly visible struct Document::SaveOptions that can internally map into the variety of xmlSaveOption_XML_* constants. And then you would have a signature

pub struct SaveOptions {
 save_no_empty_tags: bool,
 save_as_html: bool,
  // ... 
}
pub fn to_string_custom(&self, options: SaveOptions) -> String {...}

You could also implement a default for the option set, and then make the simple to_string be an alias for using it:

impl Default for SaveOptions {
 fn default() -> Self {
  SaveOptions {
    save_no_empty_tags: false,
    save_as_html:false, 
    // ...
 }
}
impl fmt::Display for Document {
  fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
    write!(f, "{}", self.to_string_custom(SaveOptions::default()))
  }
}

If you feel upto creating a PR and adding a couple of tests with the default serialization, as well as one or two advanced serialization setups, I would be happy to assist+merge.

@jangernert
Copy link
Contributor Author

Thank you for that quick reply. Don't expect something in the next 1-2 days, but I will definitely try to come up with a PR :)

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

Successfully merging a pull request may close this issue.

2 participants