search

d3x0r / JSON6

JSON for Humans (ES6)
Other
236 stars 14 forks source link

Leading 0 for octal is a mistake #16

Closed rmunn closed 4 years ago

rmunn commented 5 years ago

The C syntax used a leading 0 for octal numbers, and until recently every other language copied that syntax because C syntax was familiar. But nearly everyone when they see a number like 020 will first read that as twenty, and only then will they say, "No, wait, there's a leading 0 so that's octal: that's sixteen, not twenty". This has bitten many, many people over the years, so modern languages are starting to change the syntax to either forbid leading zeroes, or to treat them as decimal numbers. In all such cases, the prefix 0o is used for octal numbers, which parallels 0x for hex and 0b for binary.

I suggest that you follow the trend of modern languages, and either forbid leading 0s entirely in numeric literals, or treat them as decimals (which IMHO is the better option as it's the path of least surprise). If octal numbers are desired (and they are quite useful for specifying Unix permissions), a 0o prefix will be very widely understood. (And the spec should either require, or strongly recommend, that the octal 0o prefix use a lowercase o, because 0O looks too much like two leading zeroes).

d3x0r commented 5 years ago

0o or 0O are both taken for octal numbers (as are 0[bB,xX] ). captials work for JSON on most systems except apple Safari. The leading 0's interpretation as octal was re-added (it was originally removed) because dealing with existing configuration settings there were times that the leading 0xxx was used. I'm certainly open to hearing more arguments for or against this...

Was there a place where a leading 0 number couldn't also use a '.' to indicate it's decimal? 0123. for instance. Was it a situation that you ran into? Stringification only generates decimal outputs.

Actually the C version originally didn't, but all I did for JS was took the collected string as pass it to Number(..), so I inherited ES6+ behavior... and then I added it to the C version. It is still possible to catch it and treat it differently.

fdwr commented 4 years ago

(just came to add the same comment, only to see an existing issue on it) Leading zero meaning octal was a faux pas, as people have been bitten by this repeatedly, writing a leading decimal number '0' to pad values out. I'd really rather see this blight stamped out rather propagated into the future for any languages :( for a nearly useless radix on modern machines (the very rare exception being lingering *nix permissions), preferring to be explicit: 0o123.

https://medium.com/techmacademy/leading-zeros-in-python-a-hidden-mystery-revealed-ee3e3065634d https://contributors.scala-lang.org/t/bring-back-leading-zero-for-decimal-literals-not-for-octal-uses/2102/3

@rmunn Those are some great links you shared - hindsight is clarity...

d3x0r commented 4 years ago

I did start to add an option... SUPPORT_LEAD_ZERO_OCTAL which could be used to throw an error... the current implementation is really just assuming that Number(val.string) didn't handle 0 leading octal numbers, which it does... so I sort of have to reverse the handling and make sure to throw out such a thing... I would, myself, prefer to see explicit 0o or 0O prefixes used...(safari didn't support 0O, 0B or 0X, btw) but it also didn't matter a lot, because often JSO(N/X) content is built through a stringify which would generate a decimal number anyway. I'm not at all certain how I would implement setting the option for library consumers...

mathiasrw commented 4 years ago

Just saying: Chrome is working on phasing out the octal notation from the Es standard.

d3x0r commented 4 years ago

Hmm I see. Plus 'strict' removes it. https://www.w3schools.com/js/js_strict.asp

Octal numeric literals are not allowed: Octal escape characters are not allowed:

With the availability of binary, especially with underscore support is a good alternative. (that and literal 0O or 0o)

so leading 0 octal should just be decimal? I don't beleive it should throw an error (sort of wish I could throw like caution markers ... )

mathiasrw commented 4 years ago

I agree. Just decimal.

d3x0r commented 4 years ago

fixed by https://github.com/d3x0r/JSON6/commit/72bcada81785919ef39e712ba7386e29541fc1b0 removes custom format checks and just passes to Number() which results 0001234 is 1234d.