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

string numbers should be quoted #33

Closed
bj0 opened this issue Jun 3, 2013 · 18 comments
Closed

string numbers should be quoted #33

bj0 opened this issue Jun 3, 2013 · 18 comments
Assignees

Comments

@bj0
Copy link

bj0 commented Jun 3, 2013

You get the same result if you serialize 5 or "5" or 5.0. As a result, type information is lost:

var sw = new StringWriter();
(new Serializer()).Serialize(sw, "5");
(new Serializer()).Serialize(sw, 5);
(new Serializer()).Serialize(sw, 5.0);
sw.ToString().Dump("result"); // output: "5\n...\n5\n...\n5\n..."

Since the result can be explicitly deserialized into int, string, or double it's not a serious bug.

I'm not even sure what the yaml spec says on the issue, as I usually just see what the PyYaml implementation does:

>>> import yaml
>>> yaml.dump(1)
'1\n...\n'
>>> yaml.dump(1.0)
'1.0\n...\n'
>>> yaml.dump('1')
"'1'\n"

Anyway, great work on the library so far, the last release fixed a bug I was having trouble with :)

@aaubry
Copy link
Owner

aaubry commented Jun 3, 2013

The YAML specification defines different schemas that specify how scalars should be treated by default. It is funny that you mention this because I just started working on schemas yesterday :)
I expect to have a working implementation soon.

@ghost ghost assigned aaubry Jun 3, 2013
@Untit1ed
Copy link

Untit1ed commented Jan 26, 2017

Any progress on this?

@gabriel-samfira
Copy link

gabriel-samfira commented Dec 21, 2018

I am also curious about progress on this. The problem is that not all strings should be serialized as Plain scalars. This adds ambiguity. For example:

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

$obj = @{"IAmAString" = "200"; "IAmAnInt" = 200}
$serializer.Serialize($obj)

which outputs:

IAmAString: 200
IAmAnInt: 200

When you deserialize this, you loose the type. This is why the Python yaml module, adds single quotes around values that might cause ambiguity.

I think this could be fixed here:

https://github.com/aaubry/YamlDotNet/blob/master/YamlDotNet/Serialization/EventEmitters/TypeAssigningEventEmitter.cs#L83

By testing if the value can be cast to any type other than string, and if it can be, just set:

eventInfo.IsPlainImplicit = false;
eventInfo.IsQuotedImplicit = true;

@gabriel-samfira
Copy link

gabriel-samfira commented Dec 21, 2018

So, using something like this:

diff --git a/YamlDotNet/Serialization/EventEmitters/TypeAssigningEventEmitter.cs b/YamlDotNet/Serialization/EventEmitters/TypeAssigningEventEmitter.cs
index dae028e..d78aff5 100644
--- a/YamlDotNet/Serialization/EventEmitters/TypeAssigningEventEmitter.cs
+++ b/YamlDotNet/Serialization/EventEmitters/TypeAssigningEventEmitter.cs
@@ -81,6 +81,21 @@ namespace YamlDotNet.Serialization.EventEmitters
                     break;
 
                 case TypeCode.String:
+                    decimal dec;
+                    int number;
+                    float flt;
+                    long lng;
+                    double dbl;
+                    var val = eventInfo.Source.Value.ToString();
+                    if (Int32.TryParse(val, out number) ||
+                        Int64.TryParse(val, out lng) ||
+                        Double.TryParse(val, out dbl) ||
+                        Decimal.TryParse(val, out dec) ||
+                        float.TryParse(val, out flt))
+                    {
+                        eventInfo.Style = ScalarStyle.DoubleQuoted;
+                    }
+                    goto case TypeCode.Char;
                 case TypeCode.Char:
                     eventInfo.Tag = "tag:yaml.org,2002:str";
                     eventInfo.RenderedValue = eventInfo.Source.Value.ToString();
@@ -146,4 +161,4 @@ namespace YamlDotNet.Serialization.EventEmitters
             }
         }
     }
-}
\ No newline at end of file
+}

Results in:

$builder = [YamlDotNet.Serialization.SerializerBuilder]::new()
$serializer = $builder.Build()
$obj = @{
    "IAmAString" = "200"
    "IAmAnInt" = 200
    "11" = "KeyAsIntAndShouldBeDisambiguated"
    "1sa1" = "JustAStringStartingWithANumber"
}
$serializer.Serialize($obj)                                                                                                           
IAmAnInt: 200
IAmAString: "200"
"11": KeyAsIntAndShouldBeDisambiguated
1sa1: JustAStringStartingWithANumber

Which uses quotes around values that are at risk to loose their type when serialized and deserialized by other parsers.

@ridhoq
Copy link

ridhoq commented Apr 19, 2020

Just checking in on the progress of this. It looks like @gabriel-samfira has fixed this downstream in the following PR: https://github.com/cloudbase/powershell-yaml/pull/41/files#diff-e4c14acd05e286e165f6b75e7a30d165. Is it possible to bring this change (or something similar) upstream in this library?

@Roadrunner67
Copy link

@aaubry I noticed you accepted my PR #477, are you up to a version release soon?
What do you need to include this fix from @gabriel-samfira ?

@aaubry
Copy link
Owner

aaubry commented Apr 21, 2020

@Roadrunner67 I'll do a release today. Regarding the fix, I don't think it should be included because I don't think that it is correct. The YAML language uses tags to represent types. Whether a scalar is plain or quoted is a presentation detail. The spec states that two scalars are equal if they have the same tag and content, regardless of their style.

Nevertheless, if someone wants @gabriel-samfira 's behaviour, it is just a matter of registering a implementation of IEventEmitter. There's no need to change the existing code.

@gabriel-samfira
Copy link

Nevertheless, if someone wants @gabriel-samfira 's behaviour, it is just a matter of registering a implementation of IEventEmitter

That is actually how we implemented it in the end in powershell-yaml. At least for now. Tags should be the long term solution. Just need to find the time to do it.

@aaubry not sure if I ever mentioned this before, but thanks for all your work!

@aaubry
Copy link
Owner

aaubry commented Apr 21, 2020

Thanks @gabriel-samfira, you're welcome!

@Ronan-Lenor
Copy link

Ronan-Lenor commented Jul 20, 2020

is it normal that it don't work in 8.1.2?
do i have to wait the version 8.1.3?

for the moment i still use something like that in my *.yaml files:
key: !!int 1000

@EdwardCooke
Copy link
Collaborator

I know this is pretty old, but it will be resolved in the next release so I'm closing this.

@Mrgaton
Copy link

Mrgaton commented Jan 26, 2023

Please fix it

@Mrgaton
Copy link

Mrgaton commented Jan 26, 2023

Nevermine i just writed myself a fixer of the output because I know that this is gonna take forever

@EdwardCooke
Copy link
Collaborator

It’s actually released. On the serializer builder call with quoting necessary strings.

@Mrgaton
Copy link

Mrgaton commented Jan 26, 2023

It’s actually released. On the serializer builder call with quoting necessary strings.

It highlights everything except the strings

@edwardcookemacu
Copy link

This works, quoting string when they are numbers

using YamlDotNet.Serialization;

var test = new { Item = "123" };

var serializer = new SerializerBuilder()
    .WithQuotingNecessaryStrings()
    .Build();
var actual = serializer.Serialize(test);

Console.WriteLine(actual);

Results in:

Item: "123"

@Mrgaton
Copy link

Mrgaton commented Jan 27, 2023

yes but i dont want to quote numbers or bools I want to quote strings

@EdwardCooke
Copy link
Collaborator

Currently you can use the yamlmember attribute to specify you want double or single quotes.

Currently there is no feature for defaulting to using quotes. Wouldn’t be hard to do though. If that is what you are looking for, open a new issue.

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

No branches or pull requests

10 participants