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 save options #59

Merged
merged 1 commit into from
Sep 26, 2019
Merged

Document to string with save options #59

merged 1 commit into from
Sep 26, 2019

Conversation

jangernert
Copy link
Contributor

@jangernert jangernert commented Sep 26, 2019

  • fixes Document to string with xmlSaveOptions #58
  • Add a second method to serialize a document with xmlSaveOption: to_string_with_options
  • remove format option from to_string since it is covered by the new method
  • small whitespace and ordering changes were the result of a cargo fmt and can be reverted if needed

@@ -171,26 +213,51 @@ impl Document {
}

/// Serializes the `Document`
pub fn to_string(&self, format: bool) -> String {
pub fn to_string(&self) -> String {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can entirely remove the to_string definition here, it is auto-generated by the fmt::Display implementation

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

(*xml_indent_tree_output_ptr) = indent;
}
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent to see this removed, thanks!

@dginev
Copy link
Member

dginev commented Sep 26, 2019

Thanks @jangernert ! This looks good to merge after you simply delete the explicit to_string implementation and let fmt::Display add it in for us.

@dginev dginev merged commit 51197e7 into KWARC:master Sep 26, 2019
@dginev
Copy link
Member

dginev commented Sep 26, 2019

Thanks, glad to have the upgrade!

@jangernert jangernert deleted the save_options branch September 26, 2019 16:49
@jangernert
Copy link
Contributor Author

Cool! Happy to help.

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.

Document to string with xmlSaveOptions
2 participants