Skip to content

DateTimeFormatter: Dynamically augment skeleton #604

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 5 commits into from
Closed

DateTimeFormatter: Dynamically augment skeleton #604

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e8ce923
Component: Date formatter
vectart Jul 15, 2015
5e7d5c3
Component: Travis CI
vectart Jul 15, 2015
d0fd2ed
Date formatter: code style fix
vectart Jul 15, 2015
da72d32
Date formatter: code style fix
Strate Apr 2, 2016
9db5ea0
DateTimeFormatter: augment skeleton refactoring
Strate Apr 3, 2016
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 .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ before_install:
install:
- npm install
- bower install
sudo: false
170 changes: 153 additions & 17 deletions src/date/expand-pattern.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
define([
"../common/format-message",
"../common/create-error/invalid-parameter-value"
], function( formatMessage, createErrorInvalidParameterValue ) {
"../common/create-error/invalid-parameter-value",
"./pattern-re"
], function( formatMessage, createErrorInvalidParameterValue, datePatternRe ) {

/**
* expandPattern( options, cldr )
Expand All @@ -26,7 +27,7 @@ define([
*/

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

function combineDateTime( type, datePattern, timePattern ) {
return formatMessage(
Expand All @@ -38,6 +39,128 @@ return function( options, cldr ) {
);
}

function getBestMatchPattern( path, skeleton ) {
var availableFormats, ratedFormats, format, pattern;

pattern = cldr.main([ path, skeleton ]);

if ( skeleton && !pattern ) {
availableFormats = cldr.main([ path ]);
ratedFormats = [];

for ( format in availableFormats ) {
ratedFormats.push({
format: format,
pattern: availableFormats[format],
rate: compareFormats( skeleton, format )
});
}

ratedFormats = ratedFormats
.filter( function( format ) {
return format.rate > -1;
} )
.sort( function( formatA, formatB ) {
return formatA.rate - formatB.rate;
});

if ( ratedFormats.length ) {
pattern = augmentFormat( skeleton, ratedFormats[0].pattern );
}
}

return pattern;
}

function repeatStr( str, count ) {
var i, result = "";
for ( i = 0; i < count; i++ ) {
result = result + str;
}
return result;
}
Copy link
Member

@rxaviers rxaviers Apr 4, 2016

Choose a reason for hiding this comment

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

Let's move this function into ./src/util/string/repeat-str.js. Maybe ./src/util/string/pad.js could reuse it.

Copy link
Contributor Author

@Strate Strate Apr 4, 2016

Choose a reason for hiding this comment

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

Maybe we could use String.prototype.repeat with some of es6-shim?


function normalizePatternType( char ) {
switch ( char ) {
case "e":
case "E":
case "c":
return "e";

case "M":
case "L":
return "L";

default:
return char;
}
}

function compareFormats( formatA, formatB ) {
var distance,
typeA,
typeB,
matchFound,
i,
j;

if ( formatA === formatB ) {
return 0;
}

formatA = formatA.match( datePatternRe );
formatB = formatB.match( datePatternRe );
if ( formatA.length === formatB.length ) {
distance = 1;
for ( i = 0; i < formatA.length; i++ ) {
typeA = normalizePatternType( formatA[i].charAt( 0 ) );
typeB = null;
matchFound = false;
for ( j = 0; j < formatB.length; j++ ) {
typeB = normalizePatternType( formatB[j].charAt( 0 ) );
if ( typeA === typeB ) {
break;
} else {
typeB = null;
}
}
if ( null === typeB ) {
return -1;
}
distance = distance + Math.abs( formatA[i].length - formatB[j].length );
if ( formatA[i].charAt( 0 ) !== formatB[j].charAt( 0 ) ) {
distance = distance + 1;
}
}
return distance;
}
return -1;
}

function augmentFormat( requestedSkeleton, bestMatchFormat ) {
var i, j, matchedType, matchedLength, requestedType, requestedLength;

requestedSkeleton = requestedSkeleton.match( datePatternRe );
bestMatchFormat = bestMatchFormat.match( datePatternRe );

for ( i = 0; i < bestMatchFormat.length; i++ ) {
matchedType = bestMatchFormat[i].charAt( 0 );
matchedLength = bestMatchFormat[i].length;
for ( j = 0; j < requestedSkeleton.length; j++ ) {
requestedType = requestedSkeleton[j].charAt( 0 );
requestedLength = requestedSkeleton[j].length;
if (
normalizePatternType( matchedType ) === normalizePatternType( requestedType ) &&
matchedLength < requestedLength
) {
bestMatchFormat[i] = repeatStr( matchedType, requestedLength );
}
}
}

return bestMatchFormat.join( "" );
}
Copy link
Member

@rxaviers rxaviers Apr 4, 2016

Choose a reason for hiding this comment

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

In general about the functions above, can .match(datePatternRe) and normalizePatternType be made in advance in the caller function so that you don't need to execute these over and over?

Copy link
Contributor Author

@Strate Strate Apr 4, 2016

Choose a reason for hiding this comment

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

You mean some sort of caching? But this could be a memory issue...


switch ( true ) {
case "skeleton" in options:
skeleton = options.skeleton;
Expand All @@ -48,25 +171,38 @@ return function( options, cldr ) {
if ( !result ) {
timeSkeleton = skeleton.split( /[^hHKkmsSAzZOvVXx]/ ).slice( -1 )[ 0 ];
dateSkeleton = skeleton.split( /[^GyYuUrQqMLlwWdDFgEec]/ )[ 0 ];
Copy link
Member

@rxaviers rxaviers Apr 4, 2016

Choose a reason for hiding this comment

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

Let's defer this initialization by moving them both inside the else below that will actually use them.

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";
dateTimeSkeleton = getBestMatchPattern(
"dates/calendars/gregorian/dateTimeFormats/availableFormats",
skeleton
);
if ( dateTimeSkeleton ) {
result = dateTimeSkeleton;
} else {
type = "short";
}
result = combineDateTime( type,
cldr.main([
dateSkeleton = getBestMatchPattern(
"dates/calendars/gregorian/dateTimeFormats/availableFormats",
dateSkeleton
]),
cldr.main([
);
timeSkeleton = getBestMatchPattern(
"dates/calendars/gregorian/dateTimeFormats/availableFormats",
timeSkeleton
])
);
);

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";
}

if ( dateSkeleton && timeSkeleton ) {
result = combineDateTime( type, dateSkeleton, timeSkeleton );
} else {
result = dateSkeleton || timeSkeleton;
}
}
Copy link
Member

@rxaviers rxaviers Apr 4, 2016

Choose a reason for hiding this comment

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

In general, the whole block above is getting too indented. Let's change it. Currently, we have something like this:

tryDirectMap()
if (!directMap) {
  tryToMatchIt()
  if (!match) {
    tryToMatchItsIndividualParts()
  }
}

Let's re-arrange it so we have:

tryDirectMap()
if (directMap) {
  break;
}

tryToMatchIt()
if (match) {
  break;
}

tryToMatchItsIndividualParts()

Copy link
Member

@rxaviers rxaviers Apr 4, 2016

Choose a reason for hiding this comment

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

About the directMap part of it... It's no longer needed, your getBestMatchPattern already checks for that.

}
break;

Expand Down
9 changes: 9 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 @@ -60,6 +60,15 @@ QUnit.test( "should return a formatter", function( assert ) {
assert.equal( Globalize.dateFormatter({ skeleton: "yQQQhm" })( date ), "Q3 2010, 5:35 PM" );
});

QUnit.test( "should augment a skeleton", function( assert ) {
extraSetup();

assert.equal( Globalize.dateFormatter({ skeleton: "yMMMMd" })( date ), "September 15, 2010" );
assert.equal( Globalize.dateFormatter({ skeleton: "MMMMd" })( date ), "September 15" );
assert.equal( Globalize.dateFormatter({ skeleton: "MMMM" })( date ), "September" );
assert.equal( Globalize.dateFormatter({ skeleton: "EEEE" })( date ), "Wednesday" );
});

QUnit.test( "should allow for runtime compilation", function( assert ) {
extraSetup();

Expand Down
70 changes: 64 additions & 6 deletions test/unit/date/expand-pattern.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,83 @@ define([
"src/date/expand-pattern",
"json!cldr-data/main/de/ca-gregorian.json",
"json!cldr-data/main/en/ca-gregorian.json",
"json!cldr-data/main/ru/ca-gregorian.json",
"json!cldr-data/supplemental/likelySubtags.json",

"cldr/event",
"cldr/supplemental"
], function( Cldr, expandPattern, deCaGregorian, enCaGregorian, likelySubtags ) {
], function( Cldr, expandPattern, deCaGregorian, enCaGregorian, ruCaGregorian, likelySubtags ) {

var de, en;
var de, en, ru;

Cldr.load( deCaGregorian, enCaGregorian, likelySubtags );
Cldr.load( deCaGregorian, enCaGregorian, ruCaGregorian, likelySubtags );

de = new Cldr( "de" );
en = new Cldr( "en" );
ru = new Cldr( "ru" );

/**
* Test actual patterns here
* @see https://ssl.icu-project.org/icu4jweb/flexTest.jsp
*/

QUnit.module( "Date Expand Pattern" );


QUnit.test( "should expand {skeleton: \"<skeleton>\"}", function( assert ) {
assert.equal( expandPattern( { skeleton: "GyMMMEd" }, en ), "E, MMM d, y G" );
assert.equal( expandPattern( { skeleton: "GyMMMEdhms" }, en ), "E, MMM d, y G, h:mm:ss a" );
assert.equal( expandPattern( { skeleton: "MMMMEdhm" }, de ), "E, d. MMMM 'um' h:mm a" );
var cldrs = {
en: en,
de: de,
ru: ru
};
var cases = {
en: {
"GyMMMEd": "E, MMM d, y G",
"GyMMMEdhms": "E, MMM d, y G, h:mm:ss a",
"MMMMEdhm": "E, MMMM d 'at' h:mm a",
"hhmm": "hh:mm a",
"HHmm": "HH:mm",
"EHmss": "E HH:mm:ss",
"MMMMh": "LLLL, h a"
},
de: {
"yMMMMd": "d. MMMM y",
"MMMMd": "d. MMMM",
"MMMM": "LLLL",
"MMMMy": "MMMM y",
"EEEE": "cccc",
"cccc": "cccc",
"EEEEMMMMd": "EEEE, d. MMMM",
"ccccMMMMd": "EEEE, d. MMMM",
"HHmm": "HH:mm",
"EEEEHHmm": "EEEE, HH:mm",
"EEEEHmm": "EEEE, HH:mm",
"ccccHmm": "EEEE, HH:mm",
"MMMMEdhm": "E, d. MMMM 'um' h:mm a"
},
ru: {
"yMMMMd": "d MMMM y 'г'.",
"MMMMd": "d MMMM",
"MMMM": "LLLL",
"MMMMy": "LLLL y 'г'.",
"EEEE": "cccc",
"cccc": "cccc",
"EEEEMMMMd": "cccc, d MMMM",
"ccccMMMMd": "cccc, d MMMM",
"HHmm": "HH:mm",
"EEEEHHmm": "EEEE HH:mm",
"EEEEHmm": "EEEE HH:mm",
"ccccHHmm": "EEEE HH:mm",
"ccccHmm": "EEEE HH:mm",
"MMMMEdhm": "ccc, d MMMM, h:mm a"
}
};
Object.keys( cases ).forEach( function( locale ) {
Object.keys( cases[locale] ).forEach( function( skeleton ) {
var expected = cases[locale][skeleton];
assert.equal( expandPattern( {skeleton: skeleton}, cldrs[locale] ), expected, locale + ", " + skeleton );
} );
} );
});

QUnit.test( "should expand {date: \"(full, ...)\"}", function( assert ) {
Expand Down