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

Update ScalarNodeDeserializer.cs #464

Closed
wants to merge 1 commit into from
Closed

Update ScalarNodeDeserializer.cs #464

wants to merge 1 commit into from

Conversation

ETSD
Copy link

@ETSD ETSD commented Feb 5, 2020

Removed support for Octal numbers as this is no longer supported in the YAML specification.

Removed support for Octal numbers as this is no longer supported in the YAML specification.
@aaubry
Copy link
Owner

aaubry commented Feb 9, 2020

What's the point of removing this support ? YamlDotNet implements the YAML 1.1 specification, which specifies octal numbers. Even the 1.2 specification mentions octal numbers, although the format is different.

I don't know if anyone is relying on octal numbers but we can't just remove that support.
I could accept a way to disable octal support, although ideally that would be driven by the version of the YAML document.

@ETSD
Copy link
Author

ETSD commented Feb 10, 2020

We have some coding guidelines that we are required to adhere to. Included in these is that all numbers be prefixed with '0' characters (usually to three or four characters). Multiple developers have run into this "feature" from an outdated YAML specification. Given that there is no way within C# to explicitly identify a number as Octal (unlike Binary and Hex), and that this is a library specifically for C#, it is an odd choice to assume that numbers prefixed with a '0' character (if not explicitly identified as Binary or Hex) should be treated as Octal.

@aaubry
Copy link
Owner

aaubry commented Feb 11, 2020

I understand your need, but this needs to be configurable. I'd like to avoid introducing breaking changes unless they are inevitable.

@EdwardCooke
Copy link
Collaborator

This PR appears to be abandoned, it's over a year old. I'm going to close it.

1 similar comment
@EdwardCooke
Copy link
Collaborator

This PR 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

3 participants