cast icon indicating copy to clipboard operation
cast copied to clipboard

fixes expected cast.ToInt behavior with prefixed "0" in string

Open cbajohr opened this issue 5 years ago • 5 comments

Fixes issue #74

It fixes the expected behavior of cast.ToInt() with a string prefixed with "0".

Before this fix cast.ToInt("01234") results into 668 instead of 1234. Now it converts strings like "1234" and "01234" to 1234.

There should be a separate cast method for hex integers like ("0x1234") like cast.ToHexInt("0x1234") e.g. if that is a must have.

cbajohr avatar Mar 12 '19 16:03 cbajohr

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Mar 12 '19 16:03 CLAassistant

If base == 0, the base is implied by the string's prefix: base 16 for "0x", base 8 for "0", and base 10 otherwise.

I did not originally implement this, but I don't think we can change the existing behavior, even if it may sound odd/wrong. A 0-prefixed integer means base 8. So if you want base 10 I think you need to trim those strings yourself.

@spf13 may chime in.

bep avatar Mar 18 '19 12:03 bep

Maybe you are right, this could be a breaking change on existing implementations. The thing is, that it's not obvious that this method converts the string to a base 8 instead of a base 10. At least it led to a misbehavior on one of my apps.

An optional base parameter for cast.ToInt could be a compromise. Or another method like cast.ToInt10() or something like this.

But yes, I also could cast the string myself.

cbajohr avatar Mar 18 '19 13:03 cbajohr

@cbajohr I'm pretty sure we have user-facing issues in Hugo regarding this, so we should definitively fix it ... somehow.

/cc @moorereason

bep avatar Mar 18 '19 13:03 bep

This feature/issue has come up before (#56). Hugo docs are pretty clear: https://gohugo.io/functions/int/.

This PR would break the existing API by forcing all inputs to base 10, so I'm against merging it without a major semver release.

The "bug" is in the godoc comments. I'd recommend updating the godoc comments for one function (ToIntE) and then add a note to every relevant function to see ToIntE about base parsing.

moorereason avatar Mar 18 '19 17:03 moorereason