Problem/Motivation

The date_format_date() function in date_api.module assumes you pass it an object of type DateObject (as it accesses $date->granularity), but the documentation simply states the parameter type is "object" and has the description "A date object.". In fact, the function docblock doesn't even mention the DateObject class at all.

This caused me confusion, when I tried to pass in a DateTime instead of a DateObject. I'm guessing this has / will trip up others too.

Proposed resolution

Update the documentation to make it clear that a DateObject should be passed in, not the PHP DateTime object.

Proposed docblock:

/**
 * Formats a date, using a date type or a custom date format string.
 *
 * Reworked from Drupal's format_date function to handle pre-1970 and
 * post-2038 dates, and accept a \DateObject (from date_api.module) instead of
 * a timestamp as input. Translates formatted date results, unlike PHP function
 * date_format(). Should only be used for display, not input, because it can't
 * be parsed.
 *
 * @param \DateObject $date
 *   The date object to format. Note this cannot be an instance of the PHP 
 *   \DateTime class: you must use date_api's wrapper class, \DateObject.
 * @param string $type
 *   (optional) The date format to use. Can be 'small', 'medium' or 'large' for
 *   the preconfigured date formats. If 'custom' is specified, then $format is
 *   required as well. Defaults to 'medium'.
 * @param string $format
 *   (optional) A PHP date format string as required by date(). A backslash
 *   should be used before a character to avoid interpreting the character as
 *   part of a date format. Defaults to an empty string.
 * @param string $langcode
 *   (optional) Language code to translate to. Defaults to NULL.
 *
 * @return string
 *   A translated date string in the requested format.
 *
 * @see format_date()
 */

Remaining tasks

  1. Come up with a proposed docblock
  2. Write patch.
  3. Review and RTBC.
  4. Commit.

User interface changes

None.

API changes

None.

Data model changes

None.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mparker17’s picture

Issue summary: View changes

Added proposed docblock to issue description.

mparker17’s picture

Assigned: mparker17 » Unassigned
Status: Active » Needs review
FileSize
1.53 KB

Here's a patch containing the docblock suggested in #1.

francoisb’s picture

Agree with your proposition.

I see a second problem with the current docblock. 'small' and 'large' for $type should read instead 'short', resp. 'long' like in Drupal's format_date().

I tested 'small' and 'large' with date_format_date(), and it doesn't work. It returns default medium format, not the expected small or large.

darrenwh’s picture

I would have thought putting some type hinting into the function parameter would help?

darrenwh’s picture

Chris Matthews’s picture

The 2 year old patch in #5 to date_api.module applied cleanly to the latest 7.x-2.x-dev. Is anyone subscribed to this issue able to confirm the status is ready for RTBC?

Checking patch date_api/date_api.module...
Hunk #1 succeeded at 1708 (offset 2 lines).
Hunk #2 succeeded at 1732 (offset 2 lines).
Applied patch date_api/date_api.module cleanly.

Status: Needs review » Needs work

The last submitted patch, 5: explicitly-document-2527582-5-D7.patch, failed testing. View results