Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Date parse: lenient about zero-padding date fields #695

Closed
rxaviers opened this issue Feb 24, 2017 · 6 comments
Closed

Date parse: lenient about zero-padding date fields #695

rxaviers opened this issue Feb 24, 2017 · 6 comments
Labels

Comments

@rxaviers
Copy link
Member

Date parser is strict about parsing zero-padded date fields, for example parsing yMd skeleton for pt (that means pattern dd/MM/y), we have:

Globalize('pt').parseDate('23/02/2017');
// > 2017-02-23T03:00:00.000Z

Globalize('pt').parseDate('23/2/2017');
// > null

Though it's impossible to set parser to parse non-zero-padded day and month for pt unless it's used raw to pass the pattern itself (which is not recommended). Therefore, parser should be lenient about parsing those date fields, specially the ones below for the analogous reason.

  • year: date: "short" may include y or yy, also y isn't the non-zero-padded for yy, what makes it even more mandatory to have a way to parse non-zero-added yy.
  • month: skeleton: "yMd" (i.e., one M) may include MM as the pattern); similarly L.
  • day: skeleton: "yMd" (i.e., one d) may include d or dd
  • hour: skeleton: "hms" (i.e., one h) may include h or hh; similarly HKk.
  • minute: skeleton: "hms" (i.e., one m) always includes mm
  • second: skeleton: "hms" (i.e., one s) always includes ss
@rxaviers
Copy link
Member Author

rxaviers commented Feb 24, 2017

Some ideas... We could add an option to parseDate() to modify the lenient behavior, e.g., options.lenientZeroPadding that takes true or false (default). Or the inverse, e.g., options.strictZeroPadding that also takes a boolean where false is default. Or even to make it lenient by default and don't care with the strict zero padding parsing for now.

@jzaefferer
Copy link
Contributor

Or even to make it lenient by default and don't care with the strict zero padding parsing for now.

Would be interesting how the current test suite "reacts" to that. Even if that comes up without regressions, it might justify a major release.

@rxaviers
Copy link
Member Author

rxaviers commented Mar 2, 2017

  • Or (a) options.lenientZeroPadding that takes true or false (default) - This is backward compatible.
  • Or (b) options.strictZeroPadding that also takes a boolean where false is default - This isn't backward compatible, although could be the desired behavior.

@rxaviers
Copy link
Member Author

rxaviers commented Mar 2, 2017

The question is should we favor backward compatibility (a) or to consider it a bug an eventually implement a options.strictZeroPadding if it's clear we have both use cases (b).

@jzaefferer
Copy link
Contributor

Options for behaviour that looks like a bug vs breaking backwards compatibility? Not a great choice either way.

I still wonder how likely a bugfix is to actually break anything. That would be the case if an app expects Globalize('pt').parseDate('23/2/2017'); to return null and would break otherwise, right?

@rxaviers
Copy link
Member Author

rxaviers commented Mar 10, 2017

Yeap, I also can't think if a case where one would expect Globalize('pt').parseDate('23/2/2017') to return null. Some additional points…

  1. We already do the opposite, i.e., we parse zero-padded numberic dates for non-zero-padded numbers formats (e.g., en-US):

    Globalize('en').parseDate('1/31/09', {date: 'short'});
    // > 2009-01-30T16:00:00.000Z
    
    // Works fine too
    Globalize('en').parseDate('01/31/09', {date: 'short'});
    // > 2009-01-30T16:00:00.000Z
  2. CLDR’s “Parsing Dates and Times” http://www.unicode.org/reports/tr35/tr35-dates.html#Parsing_Dates_Times suggests an even more loose parsing, so it should be ok to be lenient about the zero-padded numbers.

I’m inclined to consider it a bug and make a patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants