Datepicker widget should respect drupal date mask

With pleasure - that would be super awesome. Just note that we must also be able to parse it correctly.

Here is a first in-depth review:

+++ b/birthdays.moduleundefined
@@ -223,8 +224,15 @@ function birthdays_field_widget_form(&$form, &$form_state, $field, $instance, $l
+          'dateformat' => $instance['settings']['dateformat'], ¶

Trailing space.

+++ b/birthdays.moduleundefined
@@ -223,8 +224,15 @@ function birthdays_field_widget_form(&$form, &$form_state, $field, $instance, $l
+          'dateformat_noyear' => str_ireplace(array('/yy', '-yy', 'yy-', 'yy/'), '', $instance['settings']['dateformat']),

As a German I have special intrest in dd.mm.yy.

+++ b/birthdays.moduleundefined
@@ -280,6 +288,8 @@ function birthdays_field_textfield_validate($element, &$form_state) {
+dsm($birthday);
+dsm($birthday->toArray());

Of course those shouldn't be in the final version. (Btw: dsm in depracted in favor of dpm.)

+++ b/birthdays.moduleundefined
@@ -302,13 +312,25 @@ function birthdays_field_instance_settings_form($field, $instance) {
+    '#type' => 'textfield',

As discussed, this should be a select widget.

+++ b/birthdays.moduleundefined
@@ -302,13 +312,25 @@ function birthdays_field_instance_settings_form($field, $instance) {
+    '#description' => ('Type the date format you want, examples: yy/mm/dd, dd/mm/yy, mm-dd-aa'),

The description will be changed anyway. Out of intrest: What would aa be?

+++ b/birthdays.moduleundefined
@@ -606,7 +628,7 @@ function _birthdays_get($trigger, $instance, $day, $month, $year = NULL) {
-  // The the cronjob run at most once a day.
+  // The cronjob run at most once a day.

I changed this to Run cron at most once a day. in another commit. Thanks thanks for spotting this ;)

+++ b/birthdays_birthday.incundefined
@@ -47,19 +47,39 @@ class BirthdaysBirthday {
-  public static function fromString($text, $allow_future = FALSE) {
+  public static function fromString($text, $allow_future = FALSE, $mask = 'dd/mm/yyyy') {

I suggested the default value myself. Maybe yy/mm/dd is a better one, because we're using that above.
We should also decide if we use yy or yyyy in masks. I'd prefer yy. Example: yy/mm/dd matches 11/03/5 and should also match 2011/03/5, I think.

+++ b/birthdays_birthday.incundefined
@@ -47,19 +47,39 @@ class BirthdaysBirthday {
+    // find out which separator we are using

All comments should start with a capital letter and have a trailing . (dot).

+++ b/birthdays_birthday.incundefined
@@ -47,19 +47,39 @@ class BirthdaysBirthday {
+    switch ($mask) {
+    case "dd{$sep}mm{$sep}yyyy":

Indent switch(...) statements like this:

switch ($mask) {
  case 'foo':
    bar();
    break;
}

+++ b/birthdays_birthday.incundefined
@@ -47,19 +47,39 @@ class BirthdaysBirthday {
+      $date = explode($sep, $text);

Here is a problem: 11/11/11/1/2/3 shouldn't be accepted as a valid date.

+++ b/birthdays_birthday.incundefined
@@ -47,19 +47,39 @@ class BirthdaysBirthday {
+      return self::fromDate(isset($date[2]) ? $date[2] : 0, $date[1], $date[0], $allow_future);      ¶

Trailing spaces.

+++ b/birthdays_birthday.incundefined
@@ -47,19 +47,39 @@ class BirthdaysBirthday {
+      break;

No break; after return;

+++ b/birthdays_birthday.incundefined
@@ -47,19 +47,39 @@ class BirthdaysBirthday {
+    case "mm{$sep}dd{$sep}yyyy": ¶

Trailing space.

+++ b/birthdays_birthday.incundefined
@@ -47,19 +47,39 @@ class BirthdaysBirthday {
+    case "yyyy{$sep}mm{$sep}dd": ¶

Trailing space.

+++ b/birthdays_birthday.incundefined
@@ -47,19 +47,39 @@ class BirthdaysBirthday {
+    ¶

Trailing spaces.

+++ b/birthdays_birthday.incundefined
@@ -47,19 +47,39 @@ class BirthdaysBirthday {
     // Match dd.mm(.yyyy).
     if (preg_match('/^(\d{1,2})\.(\d{1,2})\.?(\d{4}|\d{2})?$/', $text, $matches)) {
       return self::fromDate(isset($matches[3]) ? $matches[3] : 0, $matches[2], $matches[1], $allow_future);
     }
 
-    // Match (yyyy-)mm-dd or (yyyy/)mm/dd.
-    foreach (array('\/', '-') as $sep) {
+    // Match (yyyy-)mm-dd or (yyyy/)mm/dd or (yyyy.)mm.dd
+    foreach (array('/', '-', '.') as $sep) {
       if (preg_match('/^((?P<year>\d{4}|\d{2})' . $sep . ')?(?P<month>\d{1,2})' . $sep . '(?P<day>\d{1,2})/', $text, $matches)) {
         return self::fromArray($matches, $allow_future);

Can we convert this to case-branches of the switch(...) statement above?

We must make use of the new $mask argument there: https://github.com/niklasf/birthdays/blob/7.x-1.x/birthdays.module#L263.

Other than those details it's pretty good. Thank you so much for working on this.

Update: I forgot that we will also need a $mask parameter for toString().

Will this change allow the Birthdays Module to select a Date Format?
I would like to be able to create a new custom format and have the Birthdays module use that format in all locations.

Almost: It will allow you to choose any date format you want for displaying the birthday - however the one on the edit pages is a little more strict and only uses certain date formats (for now) - you can select from a list. Which date format do you need?

Thank you for the response.
I would like a format that is Month Day, without the year.
e.g. Dec 10 for 10 December.

So currently I need to create a custom Date Format of Mon Day.
Then go to Date and Time and set the Medium Date format to the new custom format.
Then the birthdays module is hardcoded to use the medium format?

I would hope that soon you could choose a custom date format instead of being forced to use medium which impacts all other modules and/or functionality which depends on that format.

Please let me know if I am not understanding correctly.

Merged in #1355268: Weekday added to date format, making it invalid.

Committed http://drupalcode.org/project/birthdays.git/commitdiff/0f5ed19 with a few coding style fixes. I think we can release it after solving #1355810: Regression: Entering a year is no longer optional caused by this.