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

Fixing a minor bug in Number validation. #1329

Merged
merged 1 commit into from
Feb 26, 2017

Conversation

coder4life
Copy link
Contributor

@coder4life coder4life commented Feb 25, 2017

The function validateNumber only checks for numeric values, regardless of int or float contexts.

By PHP isNumeric

"42",
1337,
0x539,
02471,
0b10100111001,
1337e0,
"not numeric",
array(),
9.1

Will evaluate respectively

'42' is numeric
'1337' is numeric
'1337' is numeric
'1337' is numeric
'1337' is numeric
'1337' is numeric
'not numeric' is NOT numeric
'Array' is NOT numeric
'9.1' is numeric

Grav though does not support all value types for a variety of reasons. One being YAML Blueprint definitions, where it makes sense to make a new type value for a more specialized format. Specifically for numbers if a more advance number format it would make sense to make a new type for that number format.

YAML spec specifically allows both Integer or Float contexts. As seen in Grav the validate class validateInt and validateFloat. This is useful when the output formats explicitly needs to be a certain format.

However, in the case of generic numeric contexts, which numbers could be floats or ints dynamically, the values are cast back to an int currently in Grav numeric validation. Having dynamic primitive number formats is important, true in an interpreted language like JavaScript where 1 and 1.0 will both work for a number value.

The reason to cast the type of the variable still is to prevent wide selection of number formats, and to keep Grav in line with primitive YAML field formats.

The function validateNumber only checks for numeric values.

**By PHP isNumeric**

"42",
1337,
0x539,
02471,
0b10100111001,
1337e0,
"not numeric",
array(),
9.1

**Will evaluate respectively**

'42' is numeric
'1337' is numeric
'1337' is numeric
'1337' is numeric
'1337' is numeric
'1337' is numeric
'not numeric' is NOT numeric
'Array' is NOT numeric
'9.1' is numeric

Grav though does not support all value types for a variety of reasons. One being YAML Blueprint definitions, where it makes sense to make a new type value for a more specialized format. Specifically for numbers if a more advance number format it would make sense to make a new type for that number format.

YAML spec specifically allows both Integer or Float contexts, as seen in the validate class validateInt and validateFloat. This is useful when the output formats explicitly needs to be a certain format.

However, in the case of generic numeric contexts, which numbers could be floats or ints dynamically, the values are cast back to an int currently in Grav numeric validation. Having dynamic primitive number formats is important, true in an interpreted language like JavaScript where 1 and 1.0 will both work for a number value.

The reason to cast the type of the variable still is to prevent wide selection of number formats, and to keep Grav in line with primitive YAML field formats.
@flaviocopes flaviocopes merged commit 08e2bb5 into getgrav:develop Feb 26, 2017
@flaviocopes
Copy link
Contributor

Thanks!

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