longbill / jquery-date-range-picker

A jQuery plugin that allows user to select a date range
MIT License
1.12k stars 579 forks source link

Fix ordinal suffixes format #418

Open kl23 opened 6 years ago

kl23 commented 6 years ago

update /Do/ format for ordinal suffix "rd"

see #415

holtkamp commented 6 years ago

Thanks, but what "problem" does it fix? You have an actual usecase where the "rd" is used in a date? Never seen it...

kl23 commented 6 years ago

till an hour ago, I supposed that moment claims a format with ordinal suffix an invalid date.

the part below ___format calls getValidDate() which depends on moment but moment accepts the following in its isValid() function and returns true for some weird cases:

e.g. moment('5xxworhnou42ntub20485c234c23nrewst Mar 2018', 'Do MMM YYYY').toDate() returns a JS date object of 5 Mar 2018

I am not sure how it defines "a valid date" and "a valid date format". according to its doc,

moment already "support" Do format, so removal is theoretically not necessary any more.

___format is either useless (from now), or, if we need a more specific validation, the validating mechanism in getValidDate() may need some changes.

holtkamp commented 6 years ago

ok, so the best approach seems to be to put this on hold for now. We might even decide to remove the intermediate ___format approach since it is not even needed (anymore)...