Skip to content

Conversation

@john-glover
Copy link

This is a fix for issue 943 that I reported. The root of the issue is that parsing sometimes surprisingly resulted in negative numbers and there was no way to specify some positive values with hexadecimal or octal scalars. The problem stems from trying to guess the width of the value because YAML does not provide a means of doing this (see issue 943 for details).

This changes the behavior for hexadecimal deserialization so I'm prepared for there to be some controversy here, but I think this is more in line with how a developer using the library might expect the deserialization to behave.

This also fixes the issue with octal parsing which was not functioning properly before.

@EdwardCooke
Copy link
Collaborator

I don't think an unisigned long is the most widely used number type. I would think that int is the most widely used as that is what the compiler seems to default as, example: var n = 0, n is an int. This would also end up being a breaking change due to changing the return types of the method being different. If you change this to doing an int, then long, then unsigned long I'd be happy to merge this in with the next breaking change build, but, I'm hoping that it will be a while before that happens again. The other alternative would be to change this up to be an option that can be set on the deserializerbuilder, something like a SmallestNumericDataType. Probably an option on the same method that enables the type conversion. Can't remember the name right off the top.

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.

2 participants