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

Fixes issue #237 - Adds serialization support to treat floating point numbers defined in… #239

Closed

Conversation

replaysMike
Copy link

… strings as quoted strings.

Previously, floating point numbers were treated as normal numbers even if defined as a string in the passed in object.
Adds test case.

… strings as quoted strings.

Previously, floating point numbers were treated as normal numbers even if defined as a string in the passed in object.
Adds test case.
@replaysMike
Copy link
Author

I've reviewed what I could about the Yaml spec, and in addition to some behaviors seen in other uses of Yaml. This library treats all numbers as unquoted elements even when serializing from string properties in objects passed in. I viewed this as being incorrect, and breaks swagger generated schema as their spec requires the version number to be quoted.

#237

…o make it consistent with AlphaNumeric naming, but the nomenclature would be incorrect in this case as it only looks at single digit characters.
@aaubry
Copy link
Owner

aaubry commented Feb 6, 2017

Thanks for taking the time to look into this. While I agree that the current behavior is buggy, I think that your proposed fix is too specific for floating point numbers. Also, I don't think that this should be solved directly by the parser. The specification says that the parser should produce non-specific tags, that can then be resolved according to a schemas](http://www.yaml.org/spec/1.2/spec.html#Schema).

I have started working on an implementation of schemas on the feat-schema branch. I am not completely satisfied with the current implementation, but you could take a look if you are interested.

@dargmuesli
Copy link

What makes you feel not completely satisfied with the current implementation? I have a similar problem with the docker compose.yml file. There too needs the version number to be enclosed in quotes. Why are those schemas not a thing yet? Can I support you in any way?

@aaubry
Copy link
Owner

aaubry commented Oct 4, 2017

Regarding schemas, I feel that the whole tag resolution process needs to be reworked to comply with the specification. When parsing, the parser should assign non-specific tags to the nodes. Then a schema could be applied to resolve such tags. I have implemented part of that, but it does not allow to use a more advanced schema, such as a .NET type to resolve the tags.

But your problem seems to have more to do with serialization, am I correct? In that case, you probably only need to add [YamlMember(ScalarStyle = ScalarStyle.DoubleQuoted)] to the properties that you need to be quoted. I don't think that schemas would make any difference.

@dargmuesli
Copy link

I played with serialization for a few hours now and got your suggestion working. But I don't know how this gets me any further.

I am using powershell-yaml which uses YamlDotNet. I pass a hashtable with all my data to the serializer. Within this hashtable all strings that should be strings are strings and all numbers that should be numbers are numbers. What YamlDotNet outputs is: numbers are numbers, strings - that contain numbers - are numbers.

The strings I give YamlDotNet are strings on purpose! Even if they contain only numbers. I cannot work with the current output, because I need strings to be strings, no matter what their content is!

How can I achieve that?

@aaubry
Copy link
Owner

aaubry commented Oct 4, 2017

The thing is that your strings are being serialized as strings, but in YAML, strings are usually not quoted, unless they contain special characters. But if the parser interprets them as a number, that will indeed be a problem. This has convinced me that schemas are also necessary when emitting documents. I will have to reflect more on this.

As a workaround, if you are using a sufficiently recent version of YamlDotNet, you can try to substitute your strings with instances of YamlScalarNode. This will allow you to specify the scalar style. I am away from the computer at the moment, but I can try to give you an example later.

@dargmuesli
Copy link

If you could give me an example, that would be really great!

@aaubry
Copy link
Owner

aaubry commented Oct 8, 2017

I checked powershell-yaml, and it is using a really old version of YamlDotNet. This means that my suggestion won't work, and also that even if the schemas are implemented, you probably won't be able to use them from that wrapper.

I also tried creating a class in Powershell that implemented IYamlSerializable, but that fails because powershell-yaml seems to be doing some kind of conversion of the objects that it receives, which causes an exception.

You will probably be better off if you use YamlDotNet directly. Just install the nuget package or extract YamlDotNet.dll somewhere and load it:

Add-Type -Path './lib/YamlDotNet.dll'

function New-DoubleQuotedString($value)
{
    $scalar = [YamlDotNet.RepresentationModel.YamlScalarNode]::new($value)
    $scalar.Style = [YamlDotNet.Core.ScalarStyle]::DoubleQuoted
    return $scalar
}


$serializer = [YamlDotNet.Serialization.SerializerBuilder]::new().Build()

$serializer.Serialize(@{"hello"="world"; "quoted"=New-DoubleQuotedString("bar"); "anArray"=@(1,2,3); "nested"=@{"array"=@("this", "is", "an", "array")}})

This gives the following output (notice the double quotes):

hello: world
quoted: "bar"
anArray:
- 1
- 2
- 3
nested:
  array:
  - this
  - is
  - an
  - array

@dargmuesli
Copy link

That's a great example, thank you so much! I'll ditch powershell-yaml and use your implementation. Thank you once more, it would have taken me very long to figure this out.

@julealgon
Copy link

julealgon commented May 28, 2021

@aaubry , what's the state of this change? Is is still a problem in the latest versions, or has it been fixed some other way (and if so, in which version)? Is the change from @replaysMike still needed?

EDIT: I see the related issue (#237) has been closed. I assume this means this is not a problem anymore?

@aaubry aaubry mentioned this pull request Apr 29, 2022
@aaubry aaubry force-pushed the master branch 8 times, most recently from a0f8359 to 78b1ab3 Compare December 2, 2022 22:39
@EdwardCooke
Copy link
Collaborator

I believe this has been fixed in version 12.x. I would give the latest version a try. There were a lot of special strings that needed to be quoted for it to work.
This PR also appears to be abandoned, it's over a year old. I'm going to close it.

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.

None yet

6 participants