Skip to content

Fixed issue 272 Handle DateTime in same skeleton. #426

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.

Sign up for GitHub

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

Closed
wants to merge 22 commits into from
Closed

Fixed issue 272 Handle DateTime in same skeleton. #426

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
f624add
Fixed issue 272 Handle DateTime in same skeleton.
manrajgrover Mar 18, 2015
ddd187a
Fixed code with new approach.
manrajgrover Mar 22, 2015
ca8489a
Minor fixes.
manrajgrover Mar 22, 2015
0e459b7
Fixed few style errors.
manrajgrover Mar 22, 2015
93cb191
Some more style fixes.
manrajgrover Mar 22, 2015
3b50fcd
Some more fixes.
manrajgrover Mar 22, 2015
f325299
Even more fixes.
manrajgrover Mar 22, 2015
306b57e
Style fixes.
manrajgrover Mar 22, 2015
45a2ed8
Build style fixes.
manrajgrover Mar 22, 2015
2919ab6
Some more build fixes.
manrajgrover Mar 22, 2015
c3056de
Re-coded with new inference.
manrajgrover Mar 28, 2015
00673c1
Added new skip path to validateRequiredCldr.
manrajgrover Mar 28, 2015
d7d6136
Fixes styling.
manrajgrover Mar 28, 2015
dd4236d
Added functional test for datetime skeletons.
manrajgrover Mar 28, 2015
e573063
Fixes issues and alternatives.
manrajgrover Mar 30, 2015
4876f8d
Fixes minor error.
manrajgrover Mar 30, 2015
d64b3dc
Fixes scope of function.
manrajgrover Mar 30, 2015
e9984fa
Fixes whitespaces.
manrajgrover Mar 30, 2015
5c096d5
Changed Alphabetical Order.
manrajgrover Apr 4, 2015
6d0f96c
Removes duplicate block of code
manrajgrover Apr 5, 2015
6fce167
Fixes code styling and issues related to getDateTime function.
manrajgrover Apr 13, 2015
8929507
Fixes styling.
manrajgrover Apr 13, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/date.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ define([
function validateRequiredCldr( path, value ) {
validateCldr( path, value, {
skip: [
/dates\/calendars\/gregorian\/dateTimeFormats\/availableFormats/,
/dates\/calendars\/gregorian\/days\/.*\/short/,
/supplemental\/timeData\/(?!001)/,
/supplemental\/weekData\/(?!001)/
Copy link
Member

@rxaviers rxaviers Apr 4, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alphabetical order is:

/dates\/calendars\/gregorian\/dateTimeFormats\/availableFormats/,
/dates\/calendars\/gregorian\/days\/.*\/short/,
/supplemental\/timeData\/(?!001)/,
/supplemental\/weekData\/(?!001)/

Copy link
Contributor Author

@manrajgrover manrajgrover Apr 4, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oopsie. Fixing.

Expand Down
59 changes: 41 additions & 18 deletions src/date/expand-pattern.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,54 @@ define([
* - { datetime: "full" } returns "EEEE, MMMM d, y 'at' h:mm:ss a zzzz";
* - { pattern: "dd/mm" } returns "dd/mm";
*/

return function( pattern, cldr ) {
var result;
var dateSkeleton, result, skeleton, timeSkeleton, type;

function combineDateTime( type, datePattern, timePattern ) {
return formatMessage(
cldr.main([
"dates/calendars/gregorian/dateTimeFormats",
type
]),
[ timePattern, datePattern ]
);
}
Copy link
Member

@rxaviers rxaviers Mar 18, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and if the availableFormats data does not include a dateFormatItem whose skeleton matches the same set of fields

First, check for availableFormats. Therefore, keep the above. If result === undefined, keep going...

Copy link
Contributor Author

@manrajgrover manrajgrover Mar 26, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rxaviers sir,

Sir here it is throwing error E_MISSING_CLDR error from cldr.main itself and not returning undefined for skeletons containing both date and time part. Hence we first have to check if skeleton entered has dateSkeleton or timeSkeleton or both before getting result. I am attaching a screenshot that will explain better.
untitled213

Copy link
Member

@rxaviers rxaviers Mar 26, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hence we first have to check if skeleton entered has dateSkeleton or timeSkeleton or both before getting result. I am attaching a screenshot that will explain better.

Nope. There may be skeletons containing both date and time parts. So, we need to follow what's in the specification, which says "if the availableFormats data does not include a dateFormatItem whose skeleton matches the same set of fields" that we should look for date and time separately.

We need to find a way to technically do that. cldr.main itself doesn't throw E_MISSING_CLDR error. The error is thrown by our validation. We could skip that path from the validation. See https://github.com/jquery/globalize/blob/master/src/date.js#L63.

Copy link
Member

@rxaviers rxaviers Mar 26, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example, otherwise you wouldn't get EHms.

Copy link
Contributor Author

@manrajgrover manrajgrover Mar 27, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rxaviers sir,

There may be skeletons containing both date and time parts. So, we need to follow what's in the specification, which says "if the availableFormats data does not include a dateFormatItem whose skeleton matches the same set of fields" that we should look for date and time separately.

Yeah. Some cases will get missed through this method.

We could skip that path from the validation. See https://github.com/jquery/globalize/blob/master/src/date.js#L63.

By skipping the path you mean adding the path for availableFormats here?
https://github.com/jquery/globalize/blob/master/src/date.js#L29

Copy link
Member

@rxaviers rxaviers Mar 27, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By skipping the path you mean adding the path for availableFormats here?
https://github.com/jquery/globalize/blob/master/src/date.js#L29

Correct.


if ( typeof pattern === "string" ) {
pattern = { skeleton: pattern };
}

switch ( true ) {
case "skeleton" in pattern:
skeleton = pattern.skeleton;
result = cldr.main([
"dates/calendars/gregorian/dateTimeFormats/availableFormats",
pattern.skeleton
skeleton
]);
if ( !result ) {
timeSkeleton = skeleton.split( /[^hHKkmsSAzZOvVXx]/ ).slice( -1 )[ 0 ];
dateSkeleton = skeleton.split( /[^GyYuUrQqMLlwWdDFgEec]/ )[ 0 ];
if ( /(MMMM|LLLL).*[Ec]/.test( dateSkeleton ) ) {
type = "full";
} else if ( /MMMM/g.test( dateSkeleton ) ) {
type = "long";
} else if ( /MMM/g.test( dateSkeleton ) || /LLL/g.test( dateSkeleton ) ) {
type = "medium";
} else {
type = "short";
}
result = combineDateTime( type,
cldr.main([
"dates/calendars/gregorian/dateTimeFormats/availableFormats",
dateSkeleton
]),
cldr.main([
"dates/calendars/gregorian/dateTimeFormats/availableFormats",
timeSkeleton
])
);
}
break;

case "date" in pattern:
Expand All @@ -49,22 +84,10 @@ return function( pattern, cldr ) {
break;

case "datetime" in pattern:
result = cldr.main([
"dates/calendars/gregorian/dateTimeFormats",
pattern.datetime
]);
if ( result ) {
result = formatMessage( result, [
cldr.main([
"dates/calendars/gregorian/timeFormats",
pattern.datetime
]),
cldr.main([
"dates/calendars/gregorian/dateFormats",
pattern.datetime
])
]);
}
result = combineDateTime( pattern.datetime,
cldr.main([ "dates/calendars/gregorian/dateFormats", pattern.datetime ]),
cldr.main([ "dates/calendars/gregorian/timeFormats", pattern.datetime ])
);
break;

case "pattern" in pattern:
Expand Down
4 changes: 4 additions & 0 deletions test/functional/date/date-formatter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ QUnit.test( "should return a formatter", function( assert ) {
extraSetup();

assert.equal( Globalize.dateFormatter( "GyMMMEd" )( date ), "Wed, Sep 15, 2010 AD" );
assert.equal( Globalize.dateFormatter( "dhms" )( date ), "15, 5:35:07 PM" );
assert.equal( Globalize.dateFormatter( "GyMMMEdhms" )( date ), "Wed, Sep 15, 2010 AD, 5:35:07 PM" );
assert.equal( Globalize.dateFormatter( "Ems" )( date ), "Wed, 35:07" );
assert.equal( Globalize.dateFormatter( "yQQQhm" )( date ), "Q3 2010, 5:35 PM" );
});

});