-
-
Notifications
You must be signed in to change notification settings - Fork 148
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
FIX #180: YAML Generator now quotes strings containing special chars #181
Conversation
…al chars When using the YAMLGenerator in the MINIMIZE_QUOTES mode, the generator serializes strings in plain style, i.e. without quoting them. However, as stated in the YAML Spec [1] such plain style strings cannot contain some special characters such as ':', ',', and others. [1] https://yaml.org/spec/1.2/spec.html#id2788859 As with the serialization of boolean-like strings, the YAMLGenerator should be adjusted to always quote strings containing those special characters. I will shortly try to provide a pull request to fix this issue.
Hello everybody, unfortunately, the build is breaking for my PR due to test failues. However, I had exactly the same test failues locally already when just checking out the 2.11 branch. Does anyone know if that is expected behavior, i.e. that the 2.11 branch is currently broken? I would very much appreciate any help or hint. Thank you everyone! |
First of all: thank you for reporting the issue, proposing a fix (and basing it on 2.11 which is the right branch). My only concern is that the implementation os bit inefficient (although I don't have a benchmark to show it, and granted YAML handling is not super efficient to begin with). But just one more thing before merging: unless I have asked and received CLA (apologies if so), I'd need one from: https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf (or, Corporate CLA which is in same repo). Looking forward to merging! |
Hey cowtowncoder, thank you so much for your response. I signed the CLA and sent it to the given address. Concerning my PR: I can understand that you have doubts about the performance of my contribution as it scans the strings to be serialized with the String.contains() method. Unfortunately, I do not completely understand how the use of Java.util.Pattern could make that more performant. If you tell me a bit more about your idea, I can of course adjust my PR so that you don't have additional work with it. I am absolutely fine with further discussing my solution and improve it before you merge it. I am looking forward to contribute the well-known FasterXML/jackson libraries. Best regards |
@trohrberg No problem wrt PR; I can play with that aspect after merging -- I just mentioned it as a sidenote (I often make small tweaks but want to let author know). :) I'll see when the CLA comes to the account; if not (not sure if gmail account has issues), I can give you my personal address. But will wait for a bit. Thank you again for quick follow up & contribution! |
@trohrberg For some reason, I don't see your email to |
@cowtowncoder - No problem, I sent the CLA again to info@fasterxml.com and also to your personal address. Please check if you received it this time. I sometimes have problems delivering eMails to some mail servers from my self-hosted mail server. And thank you for merging my PR already. I am looking forward to a new release of fasterxml/jackson-dataformats-text. |
(fix for #180)
When using the YAMLGenerator in the MINIMIZE_QUOTES mode, the generator serializes strings in plain style, i.e. without quoting them. However, as stated in the YAML Spec [1] such plain style strings cannot contain some special characters such as ':', ',', and others.
[1] https://yaml.org/spec/1.2/spec.html#id2788859
As with the serialization of boolean-like strings, the YAMLGenerator should be adjusted to always quote strings containing those special characters.
I will shortly try to provide a pull request to fix this issue.