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

RFC: Support for other calendars and date conversion in localization process #6536

Closed
wants to merge 9 commits into from

Conversation

zoghal
Copy link
Contributor

@zoghal zoghal commented May 11, 2015

cakephp3-all-calendar-date-localization
Before the push the tests was wondering if you have any suggestions for this feature? these changes is ok?

@dereuromark dereuromark added this to the 3.1.0 milestone May 11, 2015
@zoghal zoghal changed the title Support for other calendars and date conversion in localization process RFC: Support for other calendars and date conversion in localization process May 11, 2015
*/
public static function getCalendarName($locale = null)
{
if (is_null($locale)) {
Copy link
Member

Choose a reason for hiding this comment

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

I usually stick to === null to avoid a function call.

@zoghal zoghal force-pushed the 3.1-multiple-date-localization branch from 65ec434 to 5edbb3d Compare May 11, 2015 12:56
@@ -52,7 +52,7 @@ class DateTimeType extends Type
*
* @var string|array|int
*/
protected $_localeFormat;
protected $_localeFormat = 'yyyy-MM-dd hh:mm:ss';
Copy link
Member

Choose a reason for hiding this comment

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

The locale format does not have to be this, it is actually going to be different per locale

@zoghal zoghal force-pushed the 3.1-multiple-date-localization branch from 1571068 to f209ad5 Compare May 12, 2015 22:08
@markstory
Copy link
Member

@zoghal Could you update this into a state where it can be merged again?

$timezone = isset($value['timezone']) ? $value['timezone'] : null;

if (isset($value['locale'])) {
$format = $this->_parseValue($format, $timezone, 'fa_IR@calendar=persian');
Copy link
Member

Choose a reason for hiding this comment

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

Should this line use the locale value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😄 😄 this for test and tracing.Please ignore this codes.

@zoghal
Copy link
Contributor Author

zoghal commented Jun 4, 2015

@markstory I'm working on it. but on other branch.
in current state supporting of:

  • several parameter optional for setting localization in formHelper Without dependence on the global settings in the app.
  • orm detect if field is use localization, if used automatic translate to gregorian calendar.

just code of this branch is a bit dirty. I will cleaning and transfer these branches on the next few days

@zoghal zoghal force-pushed the 3.1-multiple-date-localization branch 2 times, most recently from 6cc41e4 to e760f75 Compare June 7, 2015 08:53
@zoghal zoghal force-pushed the 3.1-multiple-date-localization branch from e760f75 to c5fff71 Compare June 10, 2015 01:02
@zoghal
Copy link
Contributor Author

zoghal commented Jun 10, 2015

@markstory Changes and updates are finished, please check

*/
public static function detectCalendarType($locale = null)
{
if (preg_match('/@calendar=(japanese|buddhist|chinese|persian|indian|islamic|hebrew|coptic|ethiopic)/', $locale)) {
Copy link
Member

Choose a reason for hiding this comment

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

you dont need the preg_match if null is passed, maybe adding

if ($locale && ...)

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

*/
public static function setDefaultCalendar($calendar)
{
static::$defaultCalendar = $calendar;
Copy link
Member

Choose a reason for hiding this comment

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

Ahouldd you check that this method gets a string? Right now other methods may misbehave if 0/false is provided.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes yes , You are right

@zoghal zoghal force-pushed the 3.1-multiple-date-localization branch from 8ff208e to c50ff9a Compare June 25, 2015 00:39
-ORM support  localizion Date and time
@markstory markstory modified the milestones: 3.1.0, 3.2.0 Aug 26, 2015
@markstory
Copy link
Member

Closing as this branch can no longer be merged. If you have time to update it, please reopen the pull request 😄.

@markstory markstory closed this Sep 5, 2015
@zoghal
Copy link
Contributor Author

zoghal commented Sep 5, 2015

Yes it is closed. Unfortunately I am unaware 6 months of updates. And this should be implemented.
Very, very sorry for the delay. I'll come back and do it soon. 😋
thanks dear mark

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

Successfully merging this pull request may close these issues.

5 participants