Add pluggable calendar backend to core and centralize date translation

Add tag

Added a note to this issue at http://groups.drupal.org/node/10607, #242965: Add support for pluggable calendars in Date API, #166234: Support non-Gregorian Calendars (once supported in core). and #1178342: Allow contributed modules to alter the format_date() function result.. I think the last one is probably a duplicate of this issue. Also posted in the Calendar systems queue at #1827978: Swappable calendars proposed for core. I don't feel experienced in this field to comment but I'm pretty sure there are people around those issues and projects that would be very qualified to help / review, etc.

Great! thanks Karen for working on this issue , this is indeed an important issue,
Generally i like this approach , as a matter of fact it's very similar to what i planed for the next major version of calendar systems.
The only difference perhaps is that i wanted to propose a similar patch to date module :)
I'm also working on a new submodule for calendar systems which makes it possible to do calculations by different calendar systems
considering that we have unified api for all entities in D8, implementing a generic solution is within reach
i'll notify you once i finished (probably tommorow)
I think i'll need to try it and see how well it works. i'll report back soon. How much time do we have till feature freeze?

Ok then i'll give you a review right now ;)

Calendar systems comes with a core patch in order to work, it does exactly what you're looking for i think. it passes the control to calendar system when the format_date is called and it decides which calendar to use for formatting the date.
http://drupalcode.org/project/calendar_systems.git/blob/ec71923e4790ef63...

However there are problems :
- Drupal uses format_date for everything including formatting date text field values and there is no way to tell where it's been used. This can cause trouble when using non Gregorian calendars because when the form saves, it can't convert the formatted date back to it's original value. It's addressed in calendar systems by altering forms submitted values and convert them back to Gregorian before they reach the original date validation. Yet it's not a total solution since currently it doesn't work for all kind of date fields specially the custom ones.

Solutions considering that date component is RTBC #1802278: Add a Date component to core :
- It's possible to automatically detect date fields and convert the values back to Gregorian using alteration. this alteration i think should happen by default even for Gregorian.
- Similar to previous solution but a simpler one is if the date component also has a separate hook for when it converts the date strings submitted by forms
- And an even simpler one is that core should always use strtotime method provided by date component. this way wherever something converts back to timestamp it gets converted using the methods provided by the same calendar system

If we want to make it more flexible and allow different fields to have different calendar systems or have separate calendar system than the default one. A new attribute can be introduced by date component, meaning that we can override site's default calendar system by adding a new parameter to indicate he in use calendar system, this way we can accurately and safely convert it back to timestamp

reverting the removed Datein8 tag

Fixing tags...

I think you didn't get my point :)
By strotime i didn't mean to actually using it, what i meant was a general "something to time conversion". it can even be a dropdown multi part date widget. but the problem still stands. When we represent the date in non Gregorian in ANYWAY and in forms we also need a generic method to convert it back to time (machine readable)
Drupal already has a text date field on node forms, it won't be converted back to time correctly if format_date returns anything other than a standard Gregorian date string
The solutions i proposed were to address this problem and nothing more.

As i'm sure you already know the different between calendar systems is not just the name of months and days, the whole algorithm is different. some calendar systems even have more parts than the regular year, month, day

Does your patch contain a way make it possible to contributed modules like calendar systems to change the default calendar system ?
Also if this patch is only about new plugin architecture for date then it should probably be address after this one has landed

Correcting the tags again

Correcting the tags again

Karen S #1802278: Add a Date component to core is committed , whould you plz re-roll the patch? it fails on latest 8.x

#9: 1811912-calendar-translation-9.patch queued for re-testing.

Thanks,
I reviewed the patch, generally i like what i saw , well done :)
I have some of questions and then i'm going to write an Iranian plugin for it to try it out in action.
- How does the strings (month names, week names) are going to be extracted from the plugins
- Shouldn't calendar name have a prefix like GregorianCalendar or CalendarGregorian?
- Also i think it's better to use context for all week,month,etc names in Gregorian calendar plugin
- Regarding format_date, i couldn't find any clear information about International Formatter, but i guess you're referring to ISO date formats, plz correct me if i'm wrong? if that's the case then i'm agree with you. But right now all that needs to be done is using the new date classes instead of the default one in format_date function, i already did this in calendar systems module
Also this change causes a problem in forms with text date field like "Authored on" because when the form is submitted Drupal always uses the default Date class to validate the field and since the date is not in Gregorian , validation always fails. also even if the validation passes it can't be converted to timestamp because again Drupal uses the default date class.

I started writing the Drupal8 version of calendar system by depending on this patch.
How do i override the default calendar system from within a module?

I implemented the following hook, but when i change the calendar to Iranian it throws "Drupal\Component\Plugin\Exception\PluginException: Plugin (iranian) instance class "Iranian" does not exist"
Class is in module's root folder
i tried giving it absolute path as well, same result
Any idea?

function calendar_systems_datetime_calendar_info() {
    return array(
    'iranian' => array(
      'label' => t('Iranian'),
      'class' => 'Iranian',
    ),
  );
}

Classes use PSR-0 standards, with namespaces.

See core/lib/Drupal/Core/Datetime/Plugin/Type/Gregorian.php

----
One question, why are these using hooks and not annotations?
If you'd rather switch to using annotations in a follow-up, can that be opened?

This is the implementation of this hook for Gregorian in core, the question is
what the value of "class" should be if Gregorian class was inside a module
I'm aware that Drupal 8 is using PSR but i couldn't find any documentation
regarding this use case

function system_datetime_calendar_info() {
  return array(
    'gregorian' => array(
      'label' => t('Gregorian'),
      'class' => '\Drupal\Core\Datetime\Plugin\Type\Gregorian',
    ),
  );
}

function calendar_systems_datetime_calendar_info() {
    return array(
    'iranian' => array(
      'label' => t('Iranian'),
      'class' => '\Drupal\calendar_systems\Iranian',
    ),
  );
}

That would work if your file was lib/Drupal/calendar_systems/Iranian.php inside your module folder.

Thanks tim.plunkett i appreciate the help
I added exit; inside the Iranian.php file to make sure that it's included and it does now.
However it get this error now "instance class "\Drupal\calendar_systems\Iranian" does not exist"
This much i could figure out myself, i'm sharing it for anyone else having the same problem :)
New name space should be added to the new Plugin as well.
And it should be something similar to this
Iranian.php :

namespace Drupal\calendar_systems;

First real review,
It seems that it has some serious issues or don't know how to use it at all :)
1. Plugins should a base class like DrupalDateTime because when i started using Iranian plugin i encountered several missing methods like canUseIntl()

2. Using DrupalDateTime constructor as factory might be easier to use. so can just do this to get the default calendar object ,
also new $calendar variable can also be added to DrupalDateTime constructor to override the default one when nessassary

$date_time = new DrupalDateTime($timestamp, $timezones[$timezone]);
$date_time = new DrupalDateTime($time = 'now', $timezone = NULL, $format = NULL, $settings = array(), 'Iranian');

3. I tried adding new calendar to format_date function , but i didn't know how to pass required parameters to the constructor

use Drupal\Component\Datetime\DateTimePlus;
use Drupal\Core\Datetime\Plugin\DateCalendar;
function format_date($timestamp, $type = 'medium', $format = '', $timezone = NULL, $langcode = NULL) {
  ...
  $calendar = config('system.calendar')->get('calendar');
  $plugin = new DateCalendar();
  $date_time = $plugin->createInstance($calendar, array($timestamp, $timezones[$timezone]));

It seems impossible to pass them due to way createInstance works

  public function createInstance($plugin_id, array $configuration) {
    $plugin_class = $this->getPluginClass($plugin_id);
    return new $plugin_class($configuration, $plugin_id, $this->discovery);
  }

1) Your plugin should implement DateCalendarInterface. How is that related to canUseIntl()?

2) I don't see how a factory would be more useful

3) You would need to swap out the calendar plugin, it has nothing to do with how format_date() or createInstance() work.

I would recommend reading through http://drupal.org/node/1637614 and its subpages.

1) Different calendar systems use different algorithms, to be able to represent the date in selected calendar systems format, calendar plugin like the Iranian needs to implement its own date formater. so i copied the format function in DrupalDateTime and placed it in Iranian calendar plugin. however as you mentioned it seems that it's not the way it works. There are many multi calendar libraries out there and all of them work like this. one of the popular one is this http://keith-wood.name/calendarsPicker.html
The usually have gregorianToX and xToGregorian method as well. how do i achieve this using the current architecture ?

2) Using factory like that makes it transparent to anyone uses the new date plugin system. module developer wouldn't even need to know that there is a multi calendar support.

3 ) I read that already, that's why i'm asking. as also described here http://drupal.org/node/1637730 , it's not possible to pass constructor parameters via createInstance method. Unless we create a new factory specially for date that supports that.
However i managed to do it using the following code and it seems to be working even thought it might no be the correct way of doing it :

  $calendar = config('system.calendar')->get('calendar');
  $plugin = new DateCalendar();
  $plugin_definition = $plugin->getDefinition($calendar);
  $date_time = new $plugin_definition['class']($timestamp, $timezones[$timezone]);

This issue has been stated to be a blocker/dependency for #501428: Date and time field type in core, but I cannot really see, read, or understand why that is. @KarenS, any chance to clarify why this is a dependency and cannot be dealt with independently from a Date field type?

Thanks for the explanation @KarentS
To me the whole point of this patch is to eliminate the need for a core patch for supporting multi calendars
Currently in Drupal 7 a small patch is required.
However according to what you explained even a bigger patch will be required in Drupal8 because if we want to use the new date plugin system it also has to be hacked
IMHO it doesn't need to be simple because it's not something that we don't know about, there are already very nice multi calendar implementation like jquery world calendar by keith, that address the same issue perfectly. We just need to implement something similar. You actually did most of the job only some changes are required.
Also Intl is not enabled by default since it's a PECL extension. I'm not sure if it's a good idea to rely on that. We should at least be able to implement standalone calendar plugins when such a extension does on exist on server (specially on shared hostings)

Right now in order to have a full multi calendar system (at least for representing dates) there are some limitations that can be overcome without too much trouble, let me know if this makes sense to you (you can consider it the second step) :

1) format_date function to use new date plugin system
2) making it possible for contributed modules to override default calendar system
3) making it possible for date plugins to format date, allowing them to provide alternative even when IntlDateFormatter is not available
4) making it possible for date plugins to validate the submitted dates
5) converting dates back to Gregorian upon submission for core date fields (the rest can be handled by contributed modules)

I've already done all this in calendar systems module, so it's clear how it can be done.
P.S : It these things can be done after the feature freeze then we can focus on the basic functionality only

Where are we with this patch? It's a MUST HAVE for Persian users. I can imagine this being useful for many others

Hadi, Read comment #36 to #41 to findout what's going on

@KarenS , since there is only 9 days till feature freeze and i don't have time to write a proper patch till then i move on to this issue #1178342: Allow contributed modules to alter the format_date() function result.

FWIW, I believe the progress demonstrated here is sufficiently substantial to qualify for the feature freeze extension: http://buytaert.net/drupal-8-feature-freeze-extended This is a really important feature, and it's also important to get it right.

Is there a tag signifying "qualifies for the feature freeze extension"?
Might be useful.

Sometime tomorrow / over the weekend I'm hoping to create a 9.x release node, which we can then start classifying things that are out of scope. That seems a bit more clear than a tag to me.

That's great, now that we have time with some minor changes i think it will be done

Any news on this?

Though this was given an exception for feature freeze in #46, no progress was made in the last three months, and the feature freeze extension window closed yesterday.

pity :/

Damn :-/

This is quit an important feature.

I wish KarenS would have gone with her original plan.

If you're interested join this issue #1178342: Allow contributed modules to alter the format_date() function result. (which is the required patch for calendar systems module to work) and help out.
Unfortunately i couldn't find enough time to rewrite the patch in a way that it could also be used for other calendar systems. Karen's patch was mostly useful for multilingual purposes and not multi calendar purpose, i was afraid that if it gets in in that shape we will probably needed several more core patches to have multicalendar instead of only a tiny one.
But not to worry If #1178342: Allow contributed modules to alter the format_date() function result. gets in you can use calendar systems module for all your multi calendar needs without a need to patch the code and i'll see if i can work with Karen on date module for proper multi calendar architecture we both agree. as a matter of fact i already look into date module and it seemed quite possible
And always remember that you can always help by joining in anyway you can (documenting , patching , reviewing, testing, reporting bugs, etc)

related(?): #1345758: META: Provide locale (regional) formats framework for automated translation of non textual data

Adjust the description to match the current state of the patch.

API addition.