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

Add support for non-Gregorian calendars #447

Open
wants to merge 62 commits into
base: master
from

Conversation

Projects
None yet
9 participants
@dwachss

dwachss commented May 8, 2015

As discussed in globalizejs/globalize#320 , this creates a generalized date class and allows for the use of non-gregorian calendars. This is more of a proof-of concept; only Gregorian dates are implemented but other calendar algorithms could be added to Globalize.calendars.

Some thoughts:

Gdate should be an independent class; having calendars in Globalize makes the dependency graph circular (core.js depends on parse.js which depends on core.js for the calendar algorithms).

start-of.js is ugly; it has both the date and Gdate parameters, which must refer to the same date. That ought to be fixed.

I'm getting an error with the functional tests:

    .dateFormatter( pattern ) - should return a formatter
    Actual: null
    Expected: undefined
    Error: E_MISSING_CLDR: Missing required CLDR content
    'main/en/dates/calendars/gregorian/dateTimeFormats/availableFormats/dhms'.

But that looks like a problem with the cldr data.

I'm also getting errors from commitplease, but I don't know how to correct the messages on old commits.

@jquerybot jquerybot added the CLA: Valid label May 8, 2015

@dwachss

This comment has been minimized.

Show comment
Hide comment
@dwachss

dwachss May 8, 2015

The latest commit (@d8f6e02) now completes the grunt tasks without error.

dwachss commented May 8, 2015

The latest commit (@d8f6e02) now completes the grunt tasks without error.

Show outdated Hide outdated src/core.js
Show outdated Hide outdated src/date.js
Show outdated Hide outdated src/date.js
@rxaviers

This comment has been minimized.

Show comment
Hide comment
@rxaviers

rxaviers May 11, 2015

Member

Great job so far!

Coding style... Your code uses arbitrary style. I'm not concerned with this right now. But, we'll have to fix all that at some point. Indentation, spaces, etc should follow http://contribute.jquery.org/style-guide/js/.

cldr.attributes... Please, do not change cldr.attributes (code). Instead, let's make something analogous to the numberingSystem (for numbers). In practice, this means instead of using
cldr.main("dates/calendars/{calendar}/dateTimeFormats"), use cldr.main([ "dates/calendars", calendar, "dateTimeFormats"), where calendar = fn( cldr ) (similar to this code).

Member

rxaviers commented May 11, 2015

Great job so far!

Coding style... Your code uses arbitrary style. I'm not concerned with this right now. But, we'll have to fix all that at some point. Indentation, spaces, etc should follow http://contribute.jquery.org/style-guide/js/.

cldr.attributes... Please, do not change cldr.attributes (code). Instead, let's make something analogous to the numberingSystem (for numbers). In practice, this means instead of using
cldr.main("dates/calendars/{calendar}/dateTimeFormats"), use cldr.main([ "dates/calendars", calendar, "dateTimeFormats"), where calendar = fn( cldr ) (similar to this code).

Show outdated Hide outdated src/date/parse.js
Show outdated Hide outdated src/date/parse.js
Show outdated Hide outdated src/date/parse.js
Show outdated Hide outdated src/date/parse.js
@rxaviers

This comment has been minimized.

Show comment
Hide comment
@rxaviers

rxaviers May 11, 2015

Member

I left inline comments.

Variable names... There are a lot of abbreviated names, e.g., cal, d, m (especially in the gdate code). All variable names should use full words (names should be descriptive but not excessively so).

Third party code... If there is any code borrowed from different libraries, we must mention them. So, please let me know.

Member

rxaviers commented May 11, 2015

I left inline comments.

Variable names... There are a lot of abbreviated names, e.g., cal, d, m (especially in the gdate code). All variable names should use full words (names should be descriptive but not excessively so).

Third party code... If there is any code borrowed from different libraries, we must mention them. So, please let me know.

@rxaviers

This comment has been minimized.

Show comment
Hide comment
@rxaviers

rxaviers May 11, 2015

Member

Thanks so far and just let me know on any questions.

Member

rxaviers commented May 11, 2015

Thanks so far and just let me know on any questions.

@dwachss

This comment has been minimized.

Show comment
Hide comment
@dwachss

dwachss May 11, 2015

Thanks for all the comments and code review. I'm out of town for the next week (new granddaughter!) but I will go through it all when I get back.

Sent from my iPad

On May 11, 2015, at 11:12 AM, Rafael Xavier de Souza notifications@github.com wrote:

Thanks so far and just let me know on any questions.


Reply to this email directly or view it on GitHub.

dwachss commented May 11, 2015

Thanks for all the comments and code review. I'm out of town for the next week (new granddaughter!) but I will go through it all when I get back.

Sent from my iPad

On May 11, 2015, at 11:12 AM, Rafael Xavier de Souza notifications@github.com wrote:

Thanks so far and just let me know on any questions.


Reply to this email directly or view it on GitHub.

@rxaviers

This comment has been minimized.

Show comment
Hide comment
@rxaviers

rxaviers May 11, 2015

Member

Congratulations and thanks for letting me know.

Member

rxaviers commented May 11, 2015

Congratulations and thanks for letting me know.

);
} else {
// record the possible expansion for numeric months

This comment has been minimized.

@rxaviers

rxaviers Feb 11, 2016

Member

let's indent this according to the below.

@rxaviers

rxaviers Feb 11, 2016

Member

let's indent this according to the below.

This comment has been minimized.

@dwachss

dwachss Apr 6, 2016

I'm not sure what you mean.

@dwachss

dwachss Apr 6, 2016

I'm not sure what you mean.

var match, monthPatterns, reSource, type;
if ( length <= 2 ) {
// Unicode equivalent to \d+

This comment has been minimized.

@rxaviers

rxaviers Feb 11, 2016

Member

Let's indent this according to the below.

@rxaviers

rxaviers Feb 11, 2016

Member

Let's indent this according to the below.

This comment has been minimized.

@dwachss

dwachss Apr 6, 2016

I'm not sure what you mean.

@dwachss

dwachss Apr 6, 2016

I'm not sure what you mean.

Show outdated Hide outdated src/date/format.js
Show outdated Hide outdated src/date/format.js
// fastest (native) way to clone an object
months = JSON.parse( JSON.stringify( months ) );
}

This comment has been minimized.

@rxaviers

rxaviers Feb 11, 2016

Member

I'm wondering if we could avoid this.

@rxaviers

rxaviers Feb 11, 2016

Member

I'm wondering if we could avoid this.

This comment has been minimized.

@dwachss

dwachss Apr 6, 2016

are you talking about the clone? months is returned from cldr.main('dates/calendar') and is the actual data from the global CLDR object. I'm hesitant to change that directly; I don't know if any other part of the program relies on the month names in their original form.
You know cldr.js; if it's OK to modify it, we can drop that line.

@dwachss

dwachss Apr 6, 2016

are you talking about the clone? months is returned from cldr.main('dates/calendar') and is the actual data from the global CLDR object. I'm hesitant to change that directly; I don't know if any other part of the program relies on the month names in their original form.
You know cldr.js; if it's OK to modify it, we can drop that line.

case "month":
date.setDate( 1 );
/* falls through */
case "day":
date.setHours( 0 );
/* falls through */

This comment has been minimized.

@rxaviers

rxaviers Feb 11, 2016

Member

Do we still need the switch?

@rxaviers

rxaviers Feb 11, 2016

Member

Do we still need the switch?

This comment has been minimized.

@dwachss

dwachss Apr 6, 2016

I don't think we need the startOf function at all. I haven't eliminated it yet, but it should be.

@dwachss

dwachss Apr 6, 2016

I don't think we need the startOf function at all. I haven't eliminated it yet, but it should be.

ashensis pushed a commit to ashensis/globalize that referenced this pull request Mar 17, 2016

Merge pull request #447 from thomasmaas/ordering
ordering by translated attributes
@Ashamandi

This comment has been minimized.

Show comment
Hide comment
@Ashamandi

Ashamandi Apr 21, 2016

Hello @dwachss and @rxaviers , Could you please tell me what is done at supporting Hijri calender so far ? because I am interesting in contributing in this support, and as I see you don`t add it yet , Am I right?

Ashamandi commented Apr 21, 2016

Hello @dwachss and @rxaviers , Could you please tell me what is done at supporting Hijri calender so far ? because I am interesting in contributing in this support, and as I see you don`t add it yet , Am I right?

@dwachss

This comment has been minimized.

Show comment
Hide comment
@dwachss

dwachss Apr 21, 2016

Hijiri is the traditional Japanese calendar, right? I haven't done that, only Hebrew, Islamic and Chinese. Look at the gdate documentation and feel free to contribute.
The file astronomy.js has routines for new moons and solstices if you need that.
Danny

On Apr 21, 2016, at 9:11 AM, Ashamandi notifications@github.com wrote:

Hello @dwachss and @rxaviers , Could you please tell me what is done at supporting Hijri calender so far ? because I am interesting in contributing in this support, and as I see you don`t add it yet , Am I right?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

dwachss commented Apr 21, 2016

Hijiri is the traditional Japanese calendar, right? I haven't done that, only Hebrew, Islamic and Chinese. Look at the gdate documentation and feel free to contribute.
The file astronomy.js has routines for new moons and solstices if you need that.
Danny

On Apr 21, 2016, at 9:11 AM, Ashamandi notifications@github.com wrote:

Hello @dwachss and @rxaviers , Could you please tell me what is done at supporting Hijri calender so far ? because I am interesting in contributing in this support, and as I see you don`t add it yet , Am I right?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

@Ashamandi

This comment has been minimized.

Show comment
Hide comment
@Ashamandi

Ashamandi Apr 21, 2016

Sorry but I mean by Hijri calendar the Islamic one which is a lunar calendar consisting of 12 months in a year of 354 or 355 days, Do you support it ??

Ashamandi commented Apr 21, 2016

Sorry but I mean by Hijri calendar the Islamic one which is a lunar calendar consisting of 12 months in a year of 354 or 355 days, Do you support it ??

@dwachss

This comment has been minimized.

Show comment
Hide comment
@dwachss

dwachss Apr 22, 2016

I see. I have not implemented the astronomic Islamic calendar, but your help would be appreciated. As I said, the astronomy.js file has routines for new moons, but the details of the algorithm and adjusting for Mecca local time needs to be done.

Sent from my iPod

On Apr 21, 2016, at 11:18 AM, Ashamandi notifications@github.com wrote:

I go to /gdate/Islamic-date.js
I find it is about tabular calender
+// Islamic tabular calendar (http://en.wikipedia.org/wiki/Tabular_Islamic_calendar)

but I want to support muslium hijri calender
https://en.wikipedia.org/wiki/Islamic_calendar


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

dwachss commented Apr 22, 2016

I see. I have not implemented the astronomic Islamic calendar, but your help would be appreciated. As I said, the astronomy.js file has routines for new moons, but the details of the algorithm and adjusting for Mecca local time needs to be done.

Sent from my iPod

On Apr 21, 2016, at 11:18 AM, Ashamandi notifications@github.com wrote:

I go to /gdate/Islamic-date.js
I find it is about tabular calender
+// Islamic tabular calendar (http://en.wikipedia.org/wiki/Tabular_Islamic_calendar)

but I want to support muslium hijri calender
https://en.wikipedia.org/wiki/Islamic_calendar


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

@Ashamandi

This comment has been minimized.

Show comment
Hide comment
@Ashamandi

Ashamandi Jul 17, 2016

@dwachss, Sorry for this misunderstanding, I found that you had already implemented what I was looking for so I would like to thank you for your great work,
and I have a question "When will this PR be ready and released so we can use this version of Globalize at Jquery-ui and create an islamic datepicker widget?" ... Thanks in advance

Ashamandi commented Jul 17, 2016

@dwachss, Sorry for this misunderstanding, I found that you had already implemented what I was looking for so I would like to thank you for your great work,
and I have a question "When will this PR be ready and released so we can use this version of Globalize at Jquery-ui and create an islamic datepicker widget?" ... Thanks in advance

@scottgonzalez

This comment has been minimized.

Show comment
Hide comment
@scottgonzalez

scottgonzalez Jul 17, 2016

Contributor

This will be released as part of jQuery UI 1.13.0, but we never assign or publicly forecast release dates.

Contributor

scottgonzalez commented Jul 17, 2016

This will be released as part of jQuery UI 1.13.0, but we never assign or publicly forecast release dates.

@JSFOwner JSFOwner removed the CLA: Valid label Nov 2, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment