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

Su_2016_Aug_3 → Su_YYYY_MMM_D #68

Closed jedwards1211 closed 7 years ago

jedwards1211 commented 7 years ago

moment.parseFormat('Su_2016_Aug_3') parsed to Su_YYYY_MMM_D, I expected: dd_YYYY_MMM_D

jedwards1211 commented 7 years ago

\b doesn't really cut it in the regex you're using because it doesn't treat _ as a word boundary. We should use ([^a-z]|^|$) instead as the boundary for names and abbreviations.

gr2m commented 7 years ago

similar to #67, I think you can easily fix this outside of parseFormat, replace all _ with spaces before passing it to momentParseformat.

jedwards1211 commented 7 years ago

@gr2m why not just support it here though? I already fixed it and added passing tests in my PR

jedwards1211 commented 7 years ago

@gr2m I get your point about #67, but for this issue, discriminating against _ seems very arbitrary to me, especially since it's so commonly used to replace spaces, for instance in filenames. It's no harder to support than -.

jedwards1211 commented 7 years ago

@gr2m I should also point out that moment-parseformat tries to identify numeric timestamps (x and X), which are almost never human-entered.

gr2m commented 7 years ago

I know this is frustrating but please understand the perspective of the maintainers of this module and other Open Source libraries: for you it's a one time commitment, for us it's maintaining every edge feature until the end of the lifetime. I recommend you create issues first, discuss with the maintainers, then create a PR once everyone agrees it's a good idea, especially when it comes to adding features.

In years since this library exists you are the first to ask to add support for this feature, so I'm afraid I'm not willing to add it at this point. It's in the interest of everyone else depending on this library, and myself

jedwards1211 commented 7 years ago

Okay, I understand. For the record, replacing _ in 2015_01_05 doesn't work: parsing 2015 01 05 yields YYYY DD D instead of YYYY MM DD. Replacing _ with - works though, even if I wind up with a string like 2015-03-05-4:00-PM.

gr2m commented 7 years ago

The last sounds like a bug, can you make a test case for it? I'm mobile today, I can have a look over the weekend myself

jedwards1211 commented 7 years ago

okay