search

tonsky / tongue

Do-it-yourself i18n library for Clojure/Script
Eclipse Public License 1.0
307 stars 19 forks source link

NPE when formatting a ratio in clojure jvm #15

Closed madstap closed 3 years ago

madstap commented 5 years ago

Repro:

((formatter {:group "."}) 1/3) => NullPointerException

It only happens when :group is not the empty string. Looks like it's because the regex on this line doesn't match, which then makes rem on this line be called with (rem nil 3).

Are ratios considered valid input? A fix could be calling double or float on the input. If that's the case though, it should do it consistently regardless of the contents of opts IMHO.

I'd be happy to supply a PR if you agree.

tonsky commented 5 years ago

Ratios are not supported. They are usually hard to deal with. If you figure out a nice way to convert them to decimal representation (while leaving integers integers—those should not be converted), feel free to send a PR

madstap commented 5 years ago

Not necessarily a "nice" way to do it, but there could be a ratio? check in a:clj conditional, which, if true, calls double. Dunno what impact that'd have on perf though, but it would only impact the jvm version. My use-case for all this is avoiding that confusing NPE when testing my frontend code on the jvm.

tonsky commented 5 years ago

that might work. Why only JVM though? Does CLJS not have ratios?

madstap commented 5 years ago

No, it doesn't.