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 escape_all_strings option to Emitter #129

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

Conversation

paulhauner
Copy link

@paulhauner paulhauner commented May 14, 2019

First of all, thanks for this library. It's been great and has got me out of some sticky situations where serde is not flexible enough. Much appreciated.

This PR makes a change to ensure typing between loading and emitting.

Problem

Consider the following YAML:

example: "0x00"

After one round of YamlLoader -> YamlEmitter the YAML is now:

example: 0x00

After a second round the YAML is then:

example: 0

The YAML has now changed significantly and information has been lost.

Solution

I added an emitter.escape_all_strings(bool) function which forces all YAML::String nodes to be wrapped in double-quotes.

I would be interested on your opinion on two points:

  • I would have used single-quotes, however it seemed much simpler to use double-quotes. I am not a YAML expert and I would be interested to hear of any side-effects.
  • The key is now wrapped in double-quotes too (e.g., "example": "0x00"). I am not sure if this has side-effects too.

Thank you for you time, happy to make changes as you require.

This ensures typing between loading an emitting.
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.

1 participant