At CivicActions we've been reviewing internationalisation needs of various projects, and have a need for locale-appropriate date handling that doesn't seem to be covered anywhere yet. Couple of core issues: #11623: Localize date formats and #202891: format_date() doesn't return localized year correctly which touch on this to an extent.

Grugnog2 (not me!), has worked up an initial patch, which takes the following approach:

Stage #1: Allow admins to specify custom date formats alongside the ones offered by core. IMO this is a valid feature request in itself, since different formats aren't necessarily tied to particular locales.

Stage #2: allow these to be associated with locales, for either data input and/or display of date information.

Patch has a lot of TODOs etc., but we're posting it here for comment.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Owen Barton’s picture

Subscribing

stella’s picture

Status: Needs work » Needs review
FileSize
20.34 KB

Here's an updated patch, which expands on Grugnog2's work. This patch does the following:

  • Modifies admin/settings/date-time and removes the date formatting options. Instead two new tabs have been added here to allow users to add multiple formats (admin/settings/date-time/add-format) and edit existing ones, along with editing the default site formats (admin/settings/date-time/formats).
  • Creates a new table 'date_format' for storing the date formats entered.
  • Modifies admin/settings/language/edit/xx (where xx is the language code). It adds the ability for users to specify which of the configured date formats they want to use for each language / locale.
  • Creates a new table 'date_format_locale' for storing the date formats configured for each locale.
  • Implements hook_init() which will set the appropriate short, medium and long date formats to use based on that user's language. If there is no date format set for that language, then the site defaults will be used.
  • Provides hook_date_format() for any other modules that wish to provide additional date formats.

Cheers,
Stella

KarenS’s picture

Status: Needs review » Needs work

Well, +1 on the general idea of adding in better ways to adapt dates to international needs. I've had people requesting things like being able to present things like #219240: Jalali date and I've been trying to figure out ways to start opening this up to make those kinds of things possible.

If I read this right, this patch will really just address a specific part of the issue, making sure there are reasonable and customizable default formats available for different locales. It won't do anything about show dates in other calendar systems, and it is strictly limited to the PHP date format strings we use now (unlike some of the information in your links above that were talking about substituting in new strings like 'X' for a year that needs to be adjusted for a different calendar system).

That's OK, it's a starting point. My biggest beef is that we have a bunch of format string code in the Date API that is untouched by this and it seems like both concepts need to be incorporated together somehow. I don't have any idea how to do this, I'm just trying to get some ideas of how we can blend the 'core' handling of date formats and this new method together better. Maybe *all* date formats get pulled out of Date API and run through this new system. Maybe that won't work. Maybe it won't make sense. But I'd like you to do some thinking about how and if they can be blended together more seamlessly into a more global way of handling all date formats.

nedjo’s picture

Thanks for looking at this Karen.

You're right, this patch is just a first step. Basically it's intended to do three things:

1. Provide a means to provide user- or module- defined date formats.
2. Provide a means to link those formats with specific languages for short, medium, and long date formats.
3. Override the default variables for short, medium, and long date formats with locale-specific ones based on the current language.

I think you're probably right--if there's a way of designating formats, existing code that designates custom formats should now integrate with this system. I'm not sure either how this all fits together, but here are some thoughts.

First, it seems clear that date_format shouldn't be a required module in the date module set, because it's mainly concerned with UI issues. So, probably, a small part of this needs to go into date_api. Maybe that's a hook invocation that allows modules to return available date formats. Then date_format can implement that hook, adding to the list of available date formats.

And setting the default short, medium, and long date formats as done so far in the patch is only one need. We'll also want to enable locale-specific versions of other date formats as used in the date modules. (Which ones? I don't know as I'm not familiar enough with the code.)

The main problem looks to be deciding how date formats can be uniquely identified and where they should be stored. date_format introduces storage in a table with a unique ID field. Fine for its own purposes, but of course other modules returning date formats - and core's own date formats, as represented in the date format strings in the selects for setting default short medium and long dates - don't have IDs, just the format strings themselves.

So should date_format actually be separated into two parts? The first a set of APIs for handling date formats, which could go into date_api or could be a separate but also required module. The second the UI elements for e.g. setting default date formats, which calls the API methods for creating and updating date formats.

Then (again, thinking aloud here), maybe modules at the install phase create any date formats that they're responsible for, specifying custom 'format_type' values for these formats (that is, types beyond the short, medium, and long used in core). These are then added to the date_format_locale table for the locale 'en'. The date format UI module then provides the means to provide locale-specific overrides for these custom format types, just as it now does for the built-in short, medium, and long ones.

Then, since we now have the ability to provide locale-specific formats, modules can return format strings and their corresponding locales. In itself none of this answers questions like other calendar systems or extending the date formats that PHP provides. But it does open the way by providing the means of introducing custom formats and linking them with locales.

Or something like that?

KarenS’s picture

I think we're talking about adding a date_locale module rather than a date_formats module, which is probably a good idea. I'll need help with this because I don't do enough with locale.

I like the idea of making the default formats a hook and have the Date API module providing the usual suspects by default and use the something like the code in the patch to create alternative format lists. Then every place I create an options array of formats, I can grab the one for the current locale.

The core date_format() function now has a $language variable, but I haven't dug into how or where that is populated. One option is to focus on the date_format_date function in the date api and find a way to send it locale information and incorporate the locale into the results it creates. If we can create a system where that function can respond appropriately for the locale, I then just have to be sure that all formatting in Date and Calendar runs through that function (most does now, but I probably need to be sure it hasn't been bypassed anywhere).

Where would I expect to look to figure out what locale a user requires or get lists of available locales?

nedjo’s picture

Hi Karen,

I've sketched in some beginnings of documentation on multilanguage support for module developers here: http://drupal.org/node/303984. I just added a snippets page with sample code for finding enabled languages and user preferred language.

Looking at format_date(), it looks like the $langcode is used only to pass to t(), enabling translation "to a language other than what is used to display the page" (phpdoc for t()). So, seemingly, it's not immediately useful for the task of localizing date formats.

In terms of changes that may be needed:

Move existing formats to the DB.

date_short_formats() etc. currently define a set list of date formats. Presumably these shd be now either moved to the db or turned into a hook implementation returning available types. I suspect what we need is a single function that loads available formats. It can accept $type as an argument:


$formats = date_get_formats('medium');

Then e.g. date_format_options() would call on this new method.

How to individually identify format options? We need to link them to locales. So far, we're doing so by assigning an ID to each format--but that won't help when it comes to using predefined format strings. We need a way for a module to say--here is a format string, and here are the locale or locales it should be used for.

Like (a hypothetical example, I'm not saying this is appropriate for the given locales...):


/**
 * Implementation of hook_date_formats().
 */
function example_date_formats() {
  $formats = array();
  $formats[] = array(
    'type' =>'medium',
    'string' => 'D, Y-m-d H:i',
    'locales' => array(
      'fr',
      'de',
    ),
  );
  ...
  return $formats;
}

From here we would have two possible approaches.

1. All data are read in and stored in the DB.
2. Only the custom (manually entered) strings are entered into the DB. They, along with code-defined formats, are returned by a hook. I guess I'd lean toward this latter approach. The resulting method might look something like:


/**
 * Return available date formats.
 *
 * @param $type
 *   The type of date format to be returned. Values may include:
 *   'small', 'medium', 'large', 'custom'.
 * @return array An array of date formats.
 */
function date_get_formats($type = NULL) {
  static $formats = array();

  if (empty($formats)) {
    $modules = module_implements('date_formats');
    // Move date_api to the end of the list so it can override other module's settings.
    unset($modules[array_search('date_api', $modules)]);
    $modules[] = 'date_api';
    foreach ($modules as $module) {
      $module_formats = module_invoke($module, 'date_formats');
      foreach ($module_formats as $module_format) {
        $module_format['module'] = $module;
        // If no format type is specified, assign 'custom'.
        if (!isset($module_format['type'])) {
          $module_format['type'] = 'custom';
        }
        if (!isset($formats[$module_format['type']])) {
          $formats[$module_format['type']] = array();
        }
        // If another module already set this format, merge in the new settings.
        if (isset($formats[$module_format['type']][$module_format['string']])) {
          $formats[$module_format['type']][$module_format['string']] = array_merge($formats[$module_format['type']][$module_format['string']], $format);
        }
        else {
          $formats[$module_format['type']][$module_format['string']] = $module_format;
        }
      }
    }
  }

  return $type ? (isset($formats[$type]) ? $formats[$type] : FALSE) : $formats;
}

Then we'd need a way to determine which format string to use in the case that more than one is set to be used for a given locale for a given format type. I guess the last declared. So:


function date_format_locale($type, $langcode) {
  $formats = date_get_formats($type);
  foreach (array_reverse($formats) as $format) {
    if (in_array($format['locales'], $langcode)) {
      return $format;
    }
  }
  return FALSE;
}

I'm looking again at Date and the CCK implementation. Currently, date fields save a custom format string. There can be four formats set: custom, short, medium, and long. This provides flexibility. Individual fields don't have to have the site default; they can instead set their own formats.

But this won't work at all with localized formats.

For localized formats, what we need to know for a given field is what type or types of format should be used--not what specific format string. Then we can localize that type--e.g., provide the locale-appropriate version of a short date.

Offhand, it seems to me we might need:

* cck date field formats are short, medium, long, and custom (rather than default).
* for short, medium, and long, they stay the same as they currently are except that they get an additional option: "Use the default", which triggers use of the default short, medium, and long format. In the help or field description, note that only if you keep the default do you get localization support. Custom never supports localization--which is why it is no longer the default.


define('DATE_API_DEFAULT', 'default');

* Maybe (optional), provide a way to register or create additional types of date formats, which then show up alongside short, medium, long, and custom as formats that can be selected for a field. These format types are also configurable by locale.

nedjo’s picture

* We'd want to drupal_alter() the list of formats, to allow modules e.g. simply to add locales to existing formats rather than having to declare their own.

* Probably much of this would need to be cached via cache_set().

nedjo’s picture

Thinking on this a bit more....

Maybe we need a set of date format types, with the ability for modules to add new ones.


/**
 * Implementation of hook_date_format_types().
 */
function date_date_format_types() {
  return array(
    'custom' => t('Custom'),
    'large' => t('Long'),
    'medium' => t('Medium'),
    'small' => t('Short'),
  );
}

For CCK, admins can set a display for each of these display types. This looks very similar to the existing "Additional display settings" fieldset. However, the fields are initially hidden. The admin is presented with radios:

________________________________

- Medium ---------
o Default
o Preset
o Custom (advanced)

Select a Medium date display format. Select "Default" to use the site-wide default Medium display. Select Preset to select one of the preset date formats. Select Custom to enter your own custom format (advanced use only). Note that only the Default setting can be localized (provide different formats depending on the current language).
________________________________

Default is the default. If Preset is selected, a select list appears. If custom is selected, the custom textfield appears.

Then the "Default display" is no longer a specific date format. Rather, it is the display type to use as a default:

________________________________

- Default display ---------
o Short
o Medium
o Long
o Custom

Select a default display format for this field. The display formats can be customized below.
________________________________

What I'm hoping we can do through an approach something like this is retain the full flexibility of the existing implementation while enabling localization of formats.

nedjo’s picture

Talked with KarenS about needed approaches. We identified a new requirement. In many cases, the appropriate date/time format will require knowledge of both country and language. E.g., for US English, a 12-hour clock is used, whereas some other English-speaking countries may use a 24-hour clock.

We could link data formats with country-specific language codes, e.g., en-us. However, in practice, very few sites will install custom languages. Therefore, there is a need for a country setting for sites. A country setting could be used as follows:

If a two-digit language is set, look first for a date format appropriate to the country-language. E.g., if country is 'us', look for a date format for 'en-us'. Finding none, default to a date format for 'en'.

Such a setting would also be useful in other cases, e.g., locale appropriate number formats and

stella has produced a core patch #333156: Add ability to configure site default country, and also an interim Drupal 6 solution, http://drupal.org/project/site_country.

The functionality discussed above can be reworked to include this new country approach.

Specifically, it seems that data formats should follow the same general logic we use for node types in core. That is:

1. Modules may define a set of voting tags by implementing a hook, hook_date_formats().

These hook implementations define the module-defined formats.

Like node types, module-defined formats are read into the database.

2. An admin UI can be used both to customize module-defined formats (e.g., to give them a custom title) and to add new formats. This works like the node types system. Admin-defined formats may be deleted. Module-defined formats may be disabled but not deleted.

3. We store date format data in the tables introduced above.

4. A method, date_get_formats(), returns available date formats from the database, comparably to node_get_types(). Arguments may be fed to get only enabled formats or only formats for a given locale/language.

westwesterson’s picture

subscribe

Owen Barton’s picture

I just came across http://www-01.ibm.com/software/globalization/locales/index.jsp which is a fantastic resource for digging into localization in depth, including POSIX and Java examples for each - there is a summary at http://en.wikipedia.org/wiki/Calendar_date although it misses some of the details.

Overall it is clear that pretty much anything we do will have exceptions. Linking with countries only will probably hit 90%, but there are clear cases where the date and/or number formats are related to languages within that country (e.g. Belgum) or the script used, even with the same language (e.g. Bosnia-Herzegovina).

The short/medium/long formats also probably hit 90% of the cases, especially since admins can tweak them per-country.

stella’s picture

Status: Needs work » Needs review
FileSize
56.76 KB

Here's an initial version of the patch. I've a few more changes to make though. If people could test, that would be great, thanks!

Cheers,
Stella

catch’s picture

Did a visual review.

There's a few whitespace in date.css, date_field_formatter_info() etc.
Can date_update_6002() use drupal_get_schema_unprocessed('date_api') to pull in the schema rather than declaring it in full twice?
No need to use strtoupper() for country codes
$ctrycode we don't abbreviate variable names, so should be $country_code/$country_lang
+ $is_existing = FALSE; db_result() returns false if there's no record, so this isn't needed

These are just nitpicks, and theres very few in quite a big patch. Will hopefully come back with a proper test and some more substantive feedback later on.

catch’s picture

Some quick testing, will do more tomorrow:

this hunk:

        if (drupal_strlen($langcode) == 2 && !in_array($ctry_lang, array_keys($languages))) {
          $languages[$ctry_lang] = "$name ($ctrycode)";
        }

Gives me "recoverable fatal error: Object of class stdClass could not be converted to string in /home/catch/www/6/sites/all/modules/date/date_api.module on line 1676." when installing.

The date format and date_locale tabs both seem to be working very well, nice work! Minor point that we should add descriptions to 'name' and 'type' since unless you map them to node type equivalents they're not very self-explanatory.

stella’s picture

I've applied all the changes you suggested, except for the removing of strtoupper() on country code. The country_code module capitalizes the country code in the language string, so the date module needs to do the same when verifying that the language-country combination doesn't already exist in the configured list of languages.

I'm in the middle of some additional changes to the module for configuring new date formats. Will post a new patch tomorrow.

Cheers,
Stella

catch’s picture

stella: I just committed #328194: Switch to all lower case language codes which moves to lowercase (or case agnostic) language strings, I should've mentioned that in the feedback.

stella’s picture

FileSize
60.7 KB
50.81 KB

I've attached two patches. The first (v1) is basically the same as the last one with some additional changes. The second one (v2) differs in that it allows users to add additional date format strings to the list of available formats. These are then available in the drop down list. It means the "Custom format" is removed from the selection list, as is the custom format entry box. There's also a slight difference the in the db structure.

Which method do people prefer?

quicksketch’s picture

Adding to my review queue.

hass’s picture

We should remove the t'ifying from all schema strings. This has already been done for D7 and is planned to be backported to D6 soon.

stella’s picture

@hass: do you have an issue number about removing t() from all schema strings?

I would prefer to see which method people prefer before making any further changes.

hass’s picture

stella’s picture

FileSize
59.92 KB
55.18 KB

Re-rolled the patches against the latest version of the code. I also made a few minor changes, including the removal of t() on schema descriptions.

I've also made some screencasts of the two patches in action.

Cheers,
Stella

KarenS’s picture

Nice work guys! Does anyone want to weigh in on preferences between version 1 and version 2? My initial thought is mostly that the screen feels less cluttered in Version 2 by moving the custom strings elsewhere. I wasn't able to figure out if there are differences between them other than the way the setup screen looks, i.e. do they basically provide the same functionality or are they different?

I'd like to move this along fairly soon so stella doesn't have to keep re-rolling the patch to keep up with my changes, and I have a number of updates to make to the Date module.

Thanks for the blip.tv files, that's very helpful :)

hass’s picture

Translatable strings review:

'No custom formats configured.'
-> 'No custom formats have been configured.'

'Configuration saved.'
-> 'Configuration has been saved.'

'Removed date format %format.'
-> 'Date format %format has been removed.'

'The machine-readable name of this format type. <br>This name must contain only lowercase letters, numbers, and underscores and must be unique.'
-> Don't add a blank in front of BR. Aside - I the BR really required?

'This format type already exists. Please enter a unique type.'
-> two spaces should be changed to one space in front of second sentence (before "Please...").

'Additional Display Settings'
-> 'Additional display settings'

stella’s picture

Status: Needs review » Needs work

@KarenS: they provide the same functionality, only the UI and db schemas are different. One thing you probably should review (same in both patches) is the integration with the CCK date field. Particularly the 'additional date display' settings under the CCK date field configuration page. This is where you can override the system settings for 'short', 'medium', etc date formats. Part of me thinks this could be removed altogether since users can now add custom date format types. However I've left it there and it will still be used unless the 'date_locale' module is enabled, in which case the one appropriate to the current language is displayed. If you still want the date override feature, then maybe I'll modify the patch slightly so a there is a 'use system setting' option in the drop down lists too. Let me know if you want me to make any changes here.

@hass: thanks, will add those changes.

Cheers,
Stella

nedjo’s picture

stella:

Very nice work! This is indeed getting close.

Regarding v1 or v2 of the patch, I lean towards v2. I think the UI makes more sense--keep new format entry distinct. More importantly, I prefer the data module in v2. If I follow correctly, in v1 the custom formats entered by users are saved as default formats for the given format_type values, but no corresponding record is created in the date_format table. In v2, custom formats are saved as distinct records alongside code-defined formats.

I've done a review of the code for the patch v2, comments below, mostly very small issues. I haven't yet tested.

From the screencasts - very useful, thanks! - I found the UI clear and well designed. The only detail I noted was that, when editing a language's date format settings, we don't see what language we're editing. Maybe e.g. set a form prefix with something like 'Date format settings for %language_name' ?

Code review comments

function date_api_date_formats_form()

* Move these two lines to a helper function, since they're used repeatedly?
+ drupal_add_js(drupal_get_path('module', 'system') .'/system.js', 'module');
+ drupal_add_js(array('dateTime' => array('lookup' => url('admin/settings/date-time/lookup'))), 'setting');

function date_api_configure_custom_date_formats()

* drupal_set_message(t('No custom formats configured.'), 'warning');

Maybe just output this rather than set message. Is 'warning' needed?

* reverse these two lines to match their row order:

+ $delete_link = l(t('remove'), 'admin/settings/date-time/formats/delete/' . $format_info['dfid']);
+ $display_text = format_date(time(), 'custom', $format);

function date_api_date_format_select_field()

* I found this line confusing until I read on:

+ '#suffix' => ($show_remove == 1 && $type_info['locked'] == 0) ? '

' : '

',

Maybe precede it with a comment:

// Leave the date-container div open if we are going to be adding to and
// then closing it below.

function date_api_schema()

+ 'langcode' => array(

Generally $langcode is used for a language code variable name, but 'language' is used as the field value, e.g., in node table. Probably we should follow that and name this field 'language'.

+ 'description' => 'Language code, can be 2 or 5 characters, e.g. en, en-US.',

There are other forms, e.g., zh-hans. We could follow what is done in node.schema, e.g., 'A {languages}.language for this format to be used with.'

function date_api_update_6002()

+ $schema = drupal_get_schema_unprocessed('date_api');

I think we need to repeat the schema here, rather than loading it. Otherwise, if changes to the schema are made in future, sites may get them twice--once in this update and again in future updates.

function date_api_menu_alter()

I don't immediately follow why we need to load the form here or what the 'date_api_menu_callback' callback is needed for. Some code comments would be good.

+ $form = $callbacks['admin/settings/date-time']['page arguments'][0];

$form should be $form_id for clarity

function date_get_formats()

* This line looks unnecessary?

+ $_date_formats = array();

date_get_format()

+ while ($object = db_fetch_object($result)) {
+ $format['dfid'] = $dfid;
+ $format['format'] = $object->format;
+ $format['type'] = $object->type;
+ $format['locked'] = $object->locked;
+ }
+
+ return $format;

Could this be simplified to

return db_fetch_array($result);

date_api_delete_format_form_submit() and date_api_delete_format_type_form_submit()

Since we have API functions for save and read, it would be good to move the delete SQL to API functions.

function date_get_format_types()

+ * The format type: 'short', 'medium', 'long', 'custom'. If empty, then all

Since custom ones are valid, maybe:

+ * The format type, e.g., 'short', 'medium', 'long', 'custom'. If empty, then all

date_api_form_system_date_time_settings_alter()

+ $form['date_formats'] = NULL;

Why is this needed? A comment would be good. Also the existing comment, "+ * Add in additional date format handling", doesn't seem to fit, since it looks like we're removing rather than adding.

function _date_api_date_formats_list()

Ideally we'd be able to assign at least a few default locales here. en-US?

function date_field_formatter_info()

+ 'label' => $type_info['title'], // @todo translate this?

Obviously it would be nice to provide translation, but relatively hard to do so probably best left out for now.

What we should do, though, is determine if this is locked. If so, we pass the title through t(), since it's a code-based string.

+ 'multiple values' => CONTENT_HANDLE_CORE

For consistency, should have a comma at the end of the line.

date_locale_install()

The _install function is empty. Maybe because the table was moved to data_api.install? Probably we need to either have the table managed here in date_locale or move the API function date_locale_locale_format_save() to date_api.

date_locale_write_locale_record()

Looks like this is a leftover, now replaced by date_locale_locale_format_save() ?

function date_locale_format_form()

+ date_api_date_format_select_field(&$form, $type, $type_info, $default, $choices);

& not needed on &$form

nedjo’s picture

One thing to check is to ensure that we're treating the language codes as case-neutral.

The relevant specs specify that language codes are case neutral. Drupal core ships with all lower case language codes (e.g,. pt-br for Brazilian Portuguese), but some sites may add languages in forms like en-US. We need to support both.

stella’s picture

Status: Needs work » Needs review
FileSize
61.44 KB

Here's a revised patch. It takes Nedjo's comments into account and also adds a 'System default' setting to the date format drop-down lists under 'additional display settings' on the CCK date field configuration page.

Cheers,
Stella

catch’s picture

FileSize
74.44 KB
58.37 KB

Looks like nedjo already gave this a good going over, I can only find minor nit-picks:

+  // the 'admin/settings/date-time'.  Also set it to be the default local task -
+  // need to make tabs work as expected.

Should be 'needed'.

+ * Get the format details for a paritcular id.

*particular

+    $_date_format_types = array();
+    $_date_format_types = _date_format_types_build();

Doesn't need to be initialised to an array since _date_format_types_build() always returns an array.

Also one small UI issue -

from http://d6.6/admin/settings/date-time/locale I can search for date settings per locale, which is great. However after searching, the only way to get back to the search page is to click on the tab - this could probably do with a reset button or something - or with a list of locales instead of a select box. Attached screenshots to show what I mean.

None of these should hold up further reviews so leaving at CNR.

stella’s picture

FileSize
61.89 KB

Here's a re-rolled patch with those few changes.

Regarding UI issue, I added a "Cancel" button to the per locale formats page.

Cheers,
Stella

KarenS’s picture

This is looking very nice! The UI to add the formats works well and the js to show you what the format will look like is a very nice touch.

The integration into the Date module needs some work though. I'm getting PHP notices and the logic looks wrong to me (although I may just have not dug deep enough). I'm also not clear on what should happen if Date Locale is enabled vs if it is not enabled, your thought it that the custom values wouldn't be needed if it is enabled? I don't think we can remove those options mid-cycle since that would be a huge unexpected change to people who have configured values already, so it must stay there and anything currently set up must continue to work without breaking (unless you see a safe upgrade path for existing values, which I don't).

I'm going to see what I can come up with, ideas are welcome.

KarenS’s picture

Also, should Date Locale have a dependence on the Locale module? It doesn't now, but I assume it should.

KarenS’s picture

I like this new system better than the current system for setting formats in the Date module -- you can create formats that you can use on lots of different dates instead of exactly four custom formats for each field, much more flexible, plus I'd rather not have two different ways of handling date formats for locale and non-locale sites. So I'm playing with ways to get rid of the options in the Date field settings page and move everything to the new system without breaking any existing formatting.

One idea is to build an update that will scan through the current date format settings for each field and create a custom format and type for any that don't match the system settings (i.e. field_date's 'long' format becomes a new custom format type called 'field_date_long'). We would also need temporary wrapper code in the Date formatter that is smart enough to figure out what to do to keep Views and custom code from breaking since the format names are changed. Once the new formats are created, the user could remove or consolidate them as they like.

I'm also trying to check that things work right if you have Date format on and Date off, or only Date API, or Date on and Date format off. Did you test those things?

Also, we should add more explanation about how this works, probably in Advanced help, since I've been trying to move most of the documentation there. Advanced Help just uses regular html snippets so it's pretty easy to write. If someone who understands Locale better than I do could work on some explanations for the Locale part of things, I'll put together something for the Date settings part.

stella’s picture

FileSize
61.91 KB

Yes! Date locale should have a dependency on Locale, well spotted! I've attached a new version of the patch which adds in this dependency.

I don't know why you are seeing PHP notices - what are they? I presume you ran update.php? I'm using the patch with a clean install of Drupal / Date, and am not having any problems. Maybe the update functions are incorrect or missing something, I'll double check that now. It would help though if you could post the PHP errors you're seeing and when / where they happen.

Regarding my comments in #25. I changed the integration slightly in the following patch.

Particularly the 'additional date display' settings under the CCK date field configuration page. This is where you can override the system settings for 'short', 'medium', etc date formats. Part of me thinks this could be removed altogether since users can now add custom date format types. However I've left it there and it will still be used unless the 'date_locale' module is enabled, in which case the one appropriate to the current language is displayed.

In newer versions of the patch, you can override the system settings for 'short', 'medium', 'long', etc as normal - just like you could before, with no interference from the Date Locale module. So ignore that comment :) However I added a new option to the drop-downs called 'System default'. When this is selected, the default date format for 'long', etc, as configured on admin/settings/date-time/formats is used. If the date_locale module is enabled, and if there is a locale specific setting, then the locale specific setting will be used - but again only if the 'System default' option is chosen. Hope that clarifies the integration with Date.

Cheers,
Stella

stella’s picture

FileSize
62.02 KB

Oh, here's another slightly altered version - just improves the 'no custom formats configured' message on admin/settings/date-time/formats/custom

I've tested with Date + Date Locale both on, both off, and both combinations of one on, one off. Haven't seen any problems yet.

KarenS’s picture

The PHP notices were in the Date admin area where I was getting things like 'undefined index: output_format_date_0'. But don't worry about re-rolling the patch because the more I look at this the more I think what I really want to do with this is remove all the date format settings from there other than an option to choose a default format type, and move all the format string handling to the new place in date and time settings. Then everywhere I now show options to choose those formats (in Views and in the Display fields screen) I will instead show a drop-down list of all the available format types. That will clean up the Date settings quite a bit (that screen is already too cumbersome) and move all the format handling into a central location.

I'm working on an update that will take all the current format settings and move them to the new system as custom format types and I think that is going to work fine.

stella’s picture

That sounds great! If you need me to test or review anything, just let me know.

Cheers,
Stella

KarenS’s picture

I've got this mostly working but have run into a problem and maybe you can help. We have to change things like format_date(time(), 'custom', $format) to date_format_date(date_now(), 'custom', $format) because the core function doesn't handle all the format strings that the Date API can (like 'e' to display the long timezone name). So I made that change but I still kept getting an error when saving a custom format on line 1317 of common.inc, which is in format_date(). It turns out something (the javascript?) is calling system_date_time_lookup() which still uses format_date() and it chokes if there are any escape characters in the format string.

Try to input something like 'Y-m-d\TH:i:s' as a custom format and you'll see it. Obviously we have to allow escape characters and if you double them, as the core function seems to expect, they don't work right when saved.

I ran into this when trying to convert existing formats to become custom formats in the new system. I can guarantee people are using escape characters and all kinds of odd things in their format strings, I get support questions all the time when they add so much garbage to the format strings that they won't work. So I have to be able to store those custom strings, with all their escape characters.

Maybe Date API needs its own version of system_date_time_lookup() that uses date_format_date instead of format_date? I'm not quite sure where that comes into play though.

stella’s picture

FileSize
63.77 KB

Ok, try the attached patch. It should fix the problem you're having. It includes the following changes:

  • I've changed all instances of format_date(time(), 'custom', $format) in the patch to date_format_date(date_now(), 'custom', $format)
  • Instead of using Drupal core's system.js file, which is the script that fetches admin/settings/date-time/lookup (which uses format_date()), I created a date_api.js file which includes a similar function. This function gets the correctly formatted date using date_format_date() from page callback 'admin/settings/date-time/formats/lookup'.

Cheers,
Stella

KarenS’s picture

Status: Needs review » Fixed

OK I finally got this all working. The upgrade path turned out to be more complicated than I expected. I have to find all the current formats, decide if they already exist in the custom formats list and if not create them, decide if the field is using a system format type and if not create a custom format type, go into the display settings for the field and update their values, go into Views stored in the database to see if there are date fields in them that need the formatter updated, and then store the new field values and clear all the caches. And then it will try to update the date formats before it has created the new tables, so you have to abort the date update and set a message to tell the user they have to re-run update.php to get the rest of the updates.

But after doing all that I was able to rip big chunks of code out of the Date module, which I always love to do :) It ends up as a much nicer, cleaner system for creating flexible date formats even if you aren't using locale, so it's a big win.

Thanks a bunch everyone!!

Please take the committed code through its paces to be sure nothing got broken for existing fields, but I'm pretty sure it's working right.

stella’s picture

Great, thanks KarenS!

stella’s picture

I tested the latest version of the module, with the changes checked in, and it all works great. The simplified date cck field configuration page is much easier to use. Great work!

Cheers,
Stella

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.