The hook_date_formats_alter() sample code:

function hook_date_formats_alter(&$formats) {
  foreach ($formats as $id => $format) {
    $formats[$id]['locales'][] = 'en-ca';
  }
}

is broken, as $formats is an array keyed by date format type, and then by date format. That is, $formats looks like

Array
(
    [short] => Array
        (
            [Y-m-d H:i] => Array
                (
                    [type] => short
                    [format] => Y-m-d H:i
                    [locales] => Array
                        (
                        )

                    [module] => system
                    [locked] => 1
                    [is_new] => 
                    [dfid] => 1
                )
...
CommentFileSizeAuthor
#8 hook_date_formats_alter-1030788-8.patch1.24 KBbfroehle
#7 hook_date_formats_alter-1030788-7.patch1.22 KBbfroehle
#5 1030788-5.patch1.83 KBbfroehle
#1 1030788-1.patch888 bytesbfroehle

Comments

bfroehle’s picture

bfroehle commented
Status: Active » Needs review
StatusFileSize
new 1030788-1.patch888 bytes

And a patch.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Needs work

This patch references _system_date_formats_build(), but I can't find that function on api.drupal.org. Must be something wrong?

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs work » Needs review

Whoops, that must have been a glitch on api.drupal.org.

The patch looks OK, except I don't think I'm familiar with doing
foreach ($types as $type => &$formats)
Does that work?

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Needs work

Also, $id is not really an "ID" per se, but the format's machine name. So I think for clarity it would be better to do:

foreach ($types as $type => $formats) {
  foreach ($formats as $name => $format) {
     $types[$type][$name]['locales'][] = 'en-ca';
bfroehle’s picture

bfroehle commented
Status: Needs work » Needs review
StatusFileSize
new 1030788-5.patch1.83 KB

I've taken jhodgdon's suggestion in #4 into account --- yes, using the fancy foreach value as reference is legal in PHP 5 --- but your suggestion adds to readability. I'm not too thrilled with $name, as it's not a readable name in any sense but really more a $format. So I changed it to foreach ($formats as $format => $value) { ?

Also added some documentation about the contents of $types, which was blatantly copied from _system_date_formats_build().

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Needs work

This looks pretty good. The only thing that seems a bit confusing in the text is that you say the secondary arrays are keyed by format, and then there is a format component just below. These are referring to two different things (format machine name vs. php date format), but that isn't clear from the text. I think the first reference should say "format machine name" instead of just "format"? Maybe for consistency, the outer array should reference machine name of the type also?

Also, I think the 5-letter codes are "locale" codes rather than "language" codes, and aren't the country portions capitalized, like en-US, en-CA, etc.?

bfroehle’s picture

bfroehle commented
Version: 7.x-dev » 8.x-dev
Status: Needs work » Needs review
StatusFileSize
new hook_date_formats_alter-1030788-7.patch1.22 KB

Taking another stab at this. Changes:

  1. Parameter to hook_date_formats_alter renamed to $date_formats. This is consistent with the drupal_alter call in system.module.
  2. Parameter docstring in #5 omitted, since the descriptions are all available in hook_date_formats already.
  3. In the example, the foreach iteration becomes:
      foreach ($date_formats as $type => $formats) {
    foreach ($formats as $format => $date_format)

    Not the prettiest, but I couldn't do better. :(

bfroehle’s picture

bfroehle commented
StatusFileSize
new hook_date_formats_alter-1030788-8.patch1.24 KB

Lol and now with proper PHP syntax. Doh!

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Needs work

The @param section from the patch in #5 is missing from the patch in #8. I see that you did that intentionally, but you at least need to put in the @param and say something like "See xyz() for a description blah blah blah".

bfroehle’s picture

bfroehle commented

Is a simple "@see hook_date_formats()" line enough?

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

You can't really put a @see inside a @param section, because @see makes a separate See Also section on api.drupal.org. So instead, write a short description of the parameter... something like:

 * @param $types
 *   An associative array of date formats. See whatever_function() for a description.
tr’s picture

tr commented
Version: 8.x-dev » 7.x-dev
Issue summary: View changes

hook_date_formats_alter() no longer exists in Drupal 8, so moving this issue back to D7 which still has the problem.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Related issues: +#2196557: hook_date_formats_alter() and related hooks removed, no searchable change notice!

Huh. I don't know what happened to it, as there is no change notice. Filed:
#2196557: hook_date_formats_alter() and related hooks removed, no searchable change notice!

Status: Needs work » Closed (outdated)

Automatically closed because Drupal 7 security and bugfix support has ended as of 5 January 2025. If the issue verifiably applies to later versions, please reopen with details and update the version.