gr2m / moment-parseformat

A moment.js plugin to extract the format of a date/time string
http://gr2m.github.io/moment-parseformat
Apache License 2.0
45 stars 30 forks source link

3/15 → D/YY #17

Closed ghost closed 9 years ago

ghost commented 9 years ago

moment.parseFormat('3/15') parsed to D/YY, I expected: M/D

Although D/YY is a valid interpretation, it's not the most intuitive one.

gr2m commented 9 years ago

That's a tough one. It could as well be M/YY. Can't think of any arguments in favor of the one or the other at the moment, but D/YY definitely doesn't make any sense.

onecreative commented 9 years ago

I was going to say it should default to M/YY, simply because that's what goes on a credit card. But then I realized that credit card expires don't need to be interpreted or reformatted, and if you take them out of the equation most scenarios probably would be M/DD (birthdays and upcoming events), followed by M/YY.

Sent from my iPhone

On Jan 18, 2015, at 5:18 AM, Gregor Martynus notifications@github.com wrote:

That's a tough one. It could as well be M/YY. Can't think of any arguments in favor of the one or the other at the moment, but D/YY definitely doesn't make any sense.

— Reply to this email directly or view it on GitHub.

ghost commented 9 years ago

Maybe special case it with a comment in the source that the assigned format string literal might be changed depending on the programmer's imagined use cases? In my situation, I was just testing out the functionality with birthdays, but I could imagine another programmer going the other way.

On the whole, that seems like a good solution for any ambiguous parsing.

zoepage commented 9 years ago

So, basically it should be D/MM or M/YY right? Because D/YY really doesn't make sense here. How should we handle this @gr2m ?

gr2m commented 9 years ago

I'd suggest the following:

  1. if the last number is > 12, then we do Month/Year
  2. If the first number is > 12, then we do Day/Month
  3. If both is > 12, then ... somethings wrong
  4. If both is < 12, then default to Month/Year
zoepage commented 9 years ago

I agree with you except here:

If both is < 12, then default to Month/Year

I think, this would be a great idea for Day/Month, because this use case is much more common (birthdays). As far as we use Month/Year primary for things like e.g. exp. dates on credit cards, last number should be > 12 anyways.

gr2m commented 9 years ago

Okay, yeah, I don't mind, we should just document it. Maybe let's add an "Special Cases" section in the README?

In theory, "month/day" would also be possible, but I'd say lets ignore this until someone brings it up with a real use case :)

zoepage commented 9 years ago

Sounds great. Will do. Thanks :)

zoepage commented 9 years ago

fixed with #32

gr2m commented 9 years ago

please don't close until PR is merged