ordinal.js
ordinal.js copied to clipboard
Instead of using `indicator`, pass a `boolean` as the second param.
Maybe something like this:
// Method definition: ordinal( number, includeNumber = true ).
ordinal(2); // "2nd"
ordinal(2, false); // "nd"
ordinal(2, true); // "2nd"
Defaults to include the number but you can pass false
to the second param and it will not include the number.
Thoughts?
@joshuapinter I suppose, what is the usecase/advantage though?
Just more intuitive, instead of having to import something else called "indicator" which is not very self-explanatory.
In my case, which probably isn't too uncommon, sometimes you need to include the number and sometimes you don't.
Happy to do a PR if you are open to the idea.
indicator
is truly a terrible name, it is more a "suffix-only" option - alas this API was mimicking the previous versions of this module (which I inherited from someone else).
I'm not against the idea that the existing is bad, but, I don't think the proposed syntax above is anymore intuitive.
Defaulted positional arguments can be messy too if users aren't careful, think parseInt
and Array.prototype.map
.
Ha. We agree on that.
I'm not a fan of unnamed params passed in either, actually.
What about an options Object as the second params with suffixOnly
, like this:
ordinal( 2 ); // "2nd"
ordinal( 2, {suffixOnly: true} ); // "nd"
ordinal( 2, {suffixOnly: false} ); // "2nd"
@joshuapinter I like it. It also easily allows for language extensions if anyone ever needs it.
PR accepted, as 2.0.0
👍 I'll take a look when I get time.