The purpose of this feature request is to improve the wording of the "repeat rule" display text to make it more concise wherever possible, enhance the information provided, and ensure the accuracy, while making it generally easier to read.

    This patch takes care of several fixes and enhancements:
  1. #460776: Repeating Date Rule Display missing Ordinal word (e.g. “Second”) when displayed in View, e.g. Upcoming
  2. Remove duplicate "every" in the phrase "every week every Thursday"
  3. Limit the need for the word "every" in general, preferring words such as daily, weekly, monthly, and annually, or eliminate entirely such as in the following case: "Repeats on the first Sunday of the month" rather than "Repeats every month on the first Sunday"
  4. Use commas and conjunctions in all lists of repeat rule parts.
  5. Reuse and clarify wording for ordinal by-day parts, i.e. "first Sunday and Saturday of the month" instead of "first Sunday, first Saturday"
  6. Change "on" to "during" when referring to months, i.e. "during January, February, and March" instead of "on January, February, March"
  7. Show the by-month-day dates in the repeat rule, i.e. "Repeats on days 2, 3, and 10 of the month"
  8. Works on the "every (number)" cases (every 2 days, etc.)
    Still remaining to be addressed are:
  1. Use consistent formatting for the "Until" date(s); currently the Date format used is hard-coded and does not necessarily matched the preferred date format for the field display.
  2. Include the start date in the repeat rule if they do not coincide with one another, i.e. "Starts on Tuesday, Jan 1 2009 and repeats on the second Wednesday of the month..." to clearly indicate that the first instance of the date is an exception to the rule.
  3. Include more information for weekly and monthly recurring dates with no by-day, or by-month-day information, i.e. "Repeats every Thursday" instead of "Repeats every week", or "Repeats on the 15th day of every month" instead of "Repeats monthly".
  4. Further enhance readability of the by-month-day dates using the "nth" form of the numbers; i.e. "on the 2nd, 3rd and 10th" instead of "on days 2, 3 and 10"
  5. Ensure that invalid repeat rules are either rejected entirely or are treated the same way by both the _date_repeat_calc function and the date_repeat_rrule_description function; i.e. if the by-day part and the by-month-day part are both in use in an invalid way, the dates shown in the list do not match the wording of the display rule
  6. Ensure that the existing translations provided are all working correctly and that any additional translations will work correctly

I think I've addressed most of the easy stuff already, and have tested it against several examples. The remainder of the tasks have the need to get some more context about the date field "from" value and "to" value, which doesn't seem to be available within the date_repeat module as it stands now, and some more significant code changes that I thought I might wait for. I am not at the point of creating an official patch for this yet, but I wanted to share what I've done so far.

I've been at Drupal for over 2 years and been programming for decades but new to PHP, and this is my first official code contribution to Drupal, so please be kind. :)

Here is the code for date_repeat.module:

function date_repeat_rrule_description($rrule, $format = 'D M d Y') {
  // Empty or invalid value.
  if (empty($rrule) || !strstr($rrule, 'RRULE')) {
    return;
  }
  
  require_once('./'. drupal_get_path('module', 'date_api') .'/date_api_ical.inc');
  require_once('./'. drupal_get_path('module', 'date_repeat') .'/date_repeat_calc.inc');
  
  // Make sure there will be an empty description for any unused parts.
  $description = array(
    '!interval' => '', 
    '!byday' => '', 
    '!bymonth' => '', 
    '!count' => '',
    '!until' => '', 
    '!except' => '',
    '!week_starts_on' => '',
    );
  $parts = date_repeat_split_rrule($rrule);
  $exceptions = $parts[1];
  $rrule = $parts[0];
  $interval = INTERVAL_options();

  if ($rrule['INTERVAL'] < 1) {
    $rrule['INTERVAL'] = 1;
  }

    if (empty($rrule['BYDAY']) && empty($rrule['BYMONTHDAY']) && empty($rrule['BYMONTH']) || $rrule['FREQ']=='DAILY' || $rrule['FREQ']=='WEEKLY') {
        switch ($rrule['FREQ']) {
        case 'WEEKLY':
          $description['!interval'] = format_plural($rrule['INTERVAL'], 'weekly', 'every @count weeks') .' ';
          break;
        case 'MONTHLY':
          $description['!interval'] = format_plural($rrule['INTERVAL'], 'monthly', 'every @count months') .' ';
          break;
        case 'YEARLY':
          $description['!interval'] = format_plural($rrule['INTERVAL'], 'annually', 'every @count years') .' ';
          break;
        default:
          $description['!interval'] = format_plural($rrule['INTERVAL'], 'daily', 'every @count days') .' ';
          break;
        }
  }

    $days = date_repeat_dow_day_options();
    $counts = date_repeat_dow_count_options();

    //Sort the by-day results if any into two categories, 
    //those occurring weekly, and those occurring on the nth ordinal per month, grouped by ordinal
    $ordinalResults = array();
    $weeklyResults = array();
    if (!empty($rrule['BYDAY'])) {
        foreach ($rrule['BYDAY'] as $byday) {
            $day = substr($byday, -2);
            $strCount = substr($byday, 0, 2);
            if ($count = intval($strCount)) {
                $instance = trim(t($days[$day]));
                if (empty($ordinalResults[$count]))
                {
                    $ordinalResults[$count] = array();
                }
                $ordinalResults[$count][] = $instance;
            }
            else {
                $weeklyResults[] = trim(t($days[$day]));
            }
        }
    
        $byDayResults = array();
        if (!empty($weeklyResults))
        {
            $byDayResults[] = t('on').' '.trim(t(implodeGrammar($weeklyResults)));
        }
        if (!empty($ordinalResults))
        {
            $resultsByOrdinal = array();
            for ($i=-5; $i<=5; $i++)
            {
                if (!empty($ordinalResults[$i]))
                {
                    $ordinal = $i < 0 ? '-'.$i : ($i > 0 ? '+'.$i : '');
                    $resultsByOrdinal[] = t('the').' '.strtolower(trim(t($counts[$ordinal]))).' '.implodeGrammar($ordinalResults[$i]);
                }
            }
            $byDayResults[] = t('on').' '.trim(t(implodeGrammar($resultsByOrdinal))).' '.t('of the month');
        }
        $description['!byday'] = implodeGrammar($byDayResults);
  }

if (!empty($rrule['BYMONTH'])) {
    if (sizeof($rrule['BYMONTH']) < 12) {
      $results = array();
      $months = date_month_names();
      foreach ($rrule['BYMONTH'] as $month) {
        $results[] = $months[$month];
      }
      if (!empty($rrule['BYMONTHDAY'])) {
        $description['!bymonth'] = trim(t('!repeats_every_interval on days !month_days of !month_names', array('!repeats_every_interval ' => '', '!month_days' => implodeGrammar($rrule['BYMONTHDAY']), '!month_names' => implodeGrammar($results))));
      }
      else {
        $description['!bymonth'] = trim(t('!repeats_every_interval during !month_names', array('!repeats_every_interval ' => '', '!month_names' => implodeGrammar($results))));
      }
    }
  }
  if (!empty($rrule['COUNT'])) {
    $description['!count'] = trim(t('!repeats_every_interval !count times', array('!repeats_every_interval ' => '', '!count' => $rrule['COUNT'])));
  }
  if (!empty($rrule['UNTIL'])) {
    $until = date_ical_date($rrule['UNTIL']);
    $description['!until'] = trim(t('!repeats_every_interval until !until_date', array('!repeats_every_interval ' => '', '!until_date' => date_format_date($until, 'custom', $format))));
  }
  if ($exceptions) {
    $values = array();
    foreach ($exceptions as $exception) {
      $values[] = date_format_date(date_ical_date($exception), 'custom', $format);
    }
    $description['!except'] = trim(t('!repeats_every_interval except on !except_dates', array('!repeats_every_interval ' => '', '!except_dates' => implodeGrammar($values))));
  }
  if (!empty($rrule['WKST'])) {
    $day_names = date_repeat_dow_day_options();
    $description['!week_starts_on'] = trim(t('!repeats_every_interval where the week start on !day_of_week', array('!repeats_every_interval ' => '', '!day_of_week' => $day_names[trim($rrule['WKST'])])));
  }
  return t('Repeats !interval !byday !count !bymonth !until !except.', $description);
}

 /**
 * Implodes an array of strings using punctuation and/or a conjunction, depending on the number of items in the array
 * Two items are joined by a conjunction, whereas three or more are joined by commas with the conjunction preceding the last item
 */
 function implodeGrammar($terms, $punctuation = ',', $conjunction = 'and') {
    $conjunction = trim(t($conjunction));
    $count = count($terms);
    if ($count == 1)
    {
       return $terms[0];
    }
    elseif ($count == 2)
    {
        return implode(' '.$conjunction.' ', $terms);
    }
    else
    {
        $result = array();
        for ($i = 0; $i<$count-1; $i++)
        {
            $result[] = $terms[$i];
        }
        $result[] = ' '.$conjunction.' '.$terms[$count-1];
        return implode($punctuation.' ', $result);
    }
    return '';
 }
CommentFileSizeAuthor
#51 date_repeat-583192-51.patch10.2 KBfchandler
#50 date_repeat-583192-50.patch10.2 KBfchandler
#49 date_repeat-583192-49.patch5.98 KBfchandler
#46 fields.png18.3 KBaitala
#43 date_repeat-583192-43.patch9.82 KBjackbravo
#36 date_repeat-583192-36.patch12.05 KBjackbravo
#32 date_repeat-583192-32.patch13.16 KBBWPanda
#31 date_repeat-583192-31.patch12.84 KBBWPanda
#28 date_repeat-583192-28.patch12.67 KBBWPanda
#25 date_repeat-583192-25.patch12.75 KBBWPanda
#5 date_repeat.module.patch14.01 KBpauldawg
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

NancyDru’s picture

NancyDru
she/her/hers
Primary language English
Location Boston
CreditAttribution: NancyDru commented

This looks better for "Repeats on the first Saturday of the month until Fri Dec 31 2010 ." Note a space between year and period. [I really wish I didn't have to specify an ending date.]

But "Repeats on days 31 of October until Mon Dec 31 2012 ." is terrible! I would much rather see "Repeats on the 31st of October until Mon Dec 31 2012."

Code style: "{" does not begin a row in Drupal. And we put spaces around operators (even in looping statements)

    if ($count == 1) {
       return $terms[0];
    }
    elseif ($count == 2) {
        return implode(' '.$conjunction.' ', $terms);
    }
    else {
        $result = array();
        for ($i = 0; $i < $count - 1; $i++) {
            $result[] = $terms[$i];
        }
        $result[] = ' '.$conjunction.' '.$terms[$count - 1];
        return implode($punctuation.' ', $result);
    }

Very minor: I have been told that "++$i" is faster than "$i++".

pauldawg’s picture

pauldawg CreditAttribution: pauldawg commented

Thanks NancyDru,

I'll take note of your formatting guidelines and will instruct my IDE to do the same! I usually prefer the opening bracket at the end of a line rather the beginning as well, since it saves space. As for the ++i$ preference, I haven't heard of any performance differences here, but have always followed the $i++ construct, as this is the usual style seen in C++ and Java. In actuality, there is of course a functional difference between the two, but it doesn't matter here since we aren't actually doing anything with the result. Do you have any articles you can point me to describing the performance difference?

As for your formatting example, I agree that it is terrible, and didn't actually catch the plural issue. I would personally avoid the lazy "(s)" approach and just check the value instead, so it would read "on day 31 of October" which is grammatically correct, if not quite as elegant as I'd like. However, I would agree that it would be better if is used the "nth" format instead. The reason I haven't yet done this is that I wanted to do it in a way that was more easily translatable. For example, French has a version of this: 1iere (for premiere), 2ieme (for deuxieme), 3ieme (for troisieme). The question is how best to allow for this while still allowing translations to be created. Since I haven't yet looked into the translations approach, I went for an approach that was straightforward, easy to translate, and grammatically correct.

If you have any guidelines on how translations should be done, I'd be very happy to continue with this. Also, if you or KarenS has any thoughts on how we might be able to better integrate the repeat rule with the start date, I'd be very interested in working on that as well. My plan is to roll this out on my own site soon. I'm happy to finally be able to start contributing back to this great community!

Paul

pauldawg’s picture

pauldawg CreditAttribution: pauldawg commented

Two additional points:

1) I hadn't noticed the space before the period and will fix that as well.

2) I am thinking that translating might involve more actual hook implementations rather than simple string substitution, in order to support the semantics, structure and syntax of each particular language, although we may be able to do something adequate with externalized formatting strings. I am not yet familiar with the translation support in Drupal so I am hoping someone with translation experience in Drupal will contribute to this.

In case I haven't mentioned it yet, this Date module as well as the Calendar and related modules, has come a long way in the last 2 years and is quite commendable. This should become part of Drupal core at some point, especially as CCK is already there in D7.

NancyDru’s picture

NancyDru
she/her/hers
Primary language English
Location Boston
CreditAttribution: NancyDru commented

I suspect translation is going to be a nightmare here, because dynamic translation is not very good right now. The core t() function should not be used on variable strings, and especially not on partial strings [e.g. t('the')].

As for the difference between $i++ and ++$i, I wouldn't worry much about it. I suspect the difference is not worth remembering which one does what differently. I suspect it is much less than the difference between single and double quotes.

I have to agree and the progress. I had always been told that Calendar was hard to set up and problematic after that. Other than issues with the upcoming block (which I scrapped), and mostly due to my lack of expertise with Views, it was a piece of cake. I suspect it took no more time, and maybe less, than using the Event & ER modules (which are falling into disrepair).

IIRC, once upon a time, someone submitted a format_ordinals function to the Helpers module; you might want to go look for it. I don't know for sure, but it might work in the translation area. No use re-inventing the wheel.

pauldawg’s picture

pauldawg CreditAttribution: pauldawg commented
Status: Needs work » Needs review
FileSize
date_repeat.module.patch14.01 KB

Okay I think I have a patch worth reviewing. I have fixed a couple more issues (including the plural issue and the space before the period) and I am thinking that perhaps I could leave out the translation piece for now and submit the patch for review. This would leave the following remaining items which could be split into other issues. The reason I think we could leave out the translation piece is that based on the previous explanation of the t() function and my observations on how it was used, my sense is that the existing repeat rules were likely not translated properly anyway.

The following need to request information from elsewhere in the date module and could be submitted as one issue or multiple issues:

  • Use consistent formatting for the "Until" date(s); currently the Date format used is hard-coded and does not necessarily matched the preferred date format for the field display.
  • Include the start date in the repeat rule if they do not coincide with one another, i.e. "Starts on Tuesday, Jan 1 2009 and repeats on the second Wednesday of the month..." to clearly indicate that the first instance of the date is an exception to the rule.
  • Include more information for weekly and monthly recurring dates with no by-day, or by-month-day information, i.e. "Repeats every Thursday" instead of "Repeats every week", or "Repeats on the 15th day of every month" instead of "Repeats monthly".
  • Ensure that invalid repeat rules are either rejected entirely or are treated the same way by both the _date_repeat_calc function and the date_repeat_rrule_description function; i.e. if the by-day part and the by-month-day part are both in use in an invalid way, the dates shown in the list do not match the wording of the display rule

The following items may be related to translations and could be submitted in a separate issue:

  • Further enhance readability of the by-month-day dates using the "nth" form of the numbers; i.e. "on the 2nd, 3rd and 10th" instead of "on days 2, 3 and 10"
  • Ensure that the existing translations provided are all working correctly and that any additional translations will work correctly
pauldawg’s picture

pauldawg CreditAttribution: pauldawg commented

Oh and I also reformatted the file based on what I think are decent rules for formatting. I will double-check them against the Drupal coding standards and send it through the Coder module.

pauldawg’s picture

pauldawg CreditAttribution: pauldawg commented

Nancy, have you been able to test this patch? Also, do you know if this patch was created correctly to get picked up by the "test bot", and whether this should indeed be merged with the original issue?

NancyDru’s picture

NancyDru
she/her/hers
Primary language English
Location Boston
CreditAttribution: NancyDru commented

No I haven't yet. Real Life™ has been in the way.

I don't think the test bot works on contribs.

pauldawg’s picture

pauldawg CreditAttribution: pauldawg commented

Thanks for the quick reply, Nancy. Wasn't sure exactly how the test bot worked, or whether it worked for Drupal 6.

NancyDru’s picture

NancyDru
she/her/hers
Primary language English
Location Boston
CreditAttribution: NancyDru commented

"Hunk #5 failed at 127."

BTW:

  require_once ('./' . drupal_get_path('module', 'date_api') . '/date_api_ical.inc');
  require_once ('./' . drupal_get_path('module', 'date_repeat') . '/date_repeat_calc.inc');

can be Drupal-ized with:

  module_load_include('inc', 'date_api', 'date_api_ical');
  module_load_include('inc', 'date_repeat', 'date_repeat_calc');
NancyDru’s picture

NancyDru
she/her/hers
Primary language English
Location Boston
CreditAttribution: NancyDru commented

I can't recall if I still had the previous patch there, but I don't think so.

pauldawg’s picture

pauldawg CreditAttribution: pauldawg commented

Hi Nancy,

I did the patch based on 6--2, using the patch capability built into Eclipse (Galileo) with CVS. It's my first patch (yet another first) so it's probably my fault if it didn't work. :) As for the require_once, I saw someone else also point this out in another thread. That is the code that is in 6--2, so I can also include that fix as well. If I can find the other comment I'll give a reply to that one as well. Thanks for the hint!

Hoping to see more activity in Date in the next few weeks, as this is one of the most important modules for me right now. I am also troubleshooting a critical bug with the inability to include multiple Date fields in a single View Filter due to incorrect SQL being created, but no one has chimed in on that yet. If you or anyone you know has experience with the Date SQL API, please let me know or send them over to #592796: Date Filters not working in Views when multiple date fields are used.

I'm excited to be a part of the Drupal developer community and hope to increase my experience and start contributing some of my own modules soon.

Paul

NancyDru’s picture

NancyDru
she/her/hers
Primary language English
Location Boston
CreditAttribution: NancyDru commented

I have plenty of modules you are welcome to help on.

pauldawg’s picture

pauldawg CreditAttribution: pauldawg commented

Great! I'd be interested in starting with the RealName module if you've got any work there, since I use it on my site. You've got enough open in RealName so I should be able to find something to work on there. Unfortunately I am not having much luck getting by ZendDebugger or XDebug to work in Eclipse, so things are still a little slow-going... if you know of any good resources talking about debugging PHP and Drupal, it would be greatly appreciated. Let's take that conversation offline from here. You can reach me through my contact tab and I'll share my email address with you there if you'd like.

Thanks!

OnlineWD™’s picture

OnlineWD™ CreditAttribution: OnlineWD™ commented

Subscribing, am here because I was wondering why the repeat dates aren't formatted the same as the start or to date. Obviously it's not straight forward, I would go for a long format i.e. full day (with th, nd, st, rd) names, full month names and full four digit years.

Also the repeat dates are above the From date which doesn't seem right. It ought to be From then Repeat until/Except or To.

harpa413’s picture

harpa413 CreditAttribution: harpa413 commented

like OnlineWD, i too am looking for a way to display the repeat rule _after_ the From date. sounds like you've done a lot of work already, so would be more sensible to implement your patch than to start tweaking the same code myself. is this a piece you have included or could easily add?

what i've got now comes out as

Repeats every week until Tue Nov 17 2009 .
Tuesday, 11/10/09, 6:00pm - 9:00pm

if i were writing it manually, i would choose something more like

2 consecutive Tuesdays, 11/10/09 & 11/17/09, 6:00pm - 9:00pm

but i'd be pleased with

Tuesday, 11/10/09, 6:00pm - 9:00pm
Repeats every Tuesday until 11/17/09

pauldawg’s picture

pauldawg CreditAttribution: pauldawg commented

@harpa413, @OnlineWD:

My opinion is that there should be better integration between the start date and the repeat rule, but currently this is not how it is designed. The repeat rule is displayed independently from the actual start date, and it is followed by some number of dates that fall into the schedule, but the number of dates displayed is configurable. I am fairly new to this, but so far I haven't been able to figure out how to actually get access to the start date value. Note that it is NOT the same as the first occurrence in the repeating schedule. Try this: schedule an event for a Tuesday and add a repeat rule occurring every Wednesday. According to the IEEE iCal standards upon which this module is largely based, this is a valid condition, and there is no requirement to force a start date to be on the same date as one of repeating days. Unfortunately, in this example, there is no mention of the Tuesday in the repeat rule. If I had my druthers I'd put a lot more into this, but unfortunately I am up against some design issues that I don't know how to get around. If a co-maintainer of this module were able to assist me, I'd happily take some direction and run with it, but so far I haven't found a way to integrate the start date with the repeat rule, and thus I don't know how to accomplish what you are looking to do. If you want to poke around with the code and see what you come up with, please let me know what you find out!

OnlineWD™’s picture

OnlineWD™ CreditAttribution: OnlineWD™ commented 
div.field-type-date div.field-item div {
display:none;
}
function themename_date_display_range($date1, $date2, $timezone = NULL) {
  return '<span class="date-display-start">'. $date1 .'</span>'.
      '<span class="date-display-separator"> until ..<br /></span>' .
      '<span class="date-display-end">'. $date2 . $timezone. '.</span>';
}

This does me fine, don't need to read the overview - the list of dates and times is sufficient i.e..

Event dates and times:
Thursday 12:00am, November 11th, 2009 until ..
Monday 12:00am, November 11th, 2009.
Thursday 12:00am, December 12th, 2009 until ..
Monday 12:00am, December 12th, 2009.
Thursday 12:00am, December 12th, 2009 until ..
Monday 12:00am, December 12th, 2009.

eojthebrave’s picture

eojthebrave
he/him
Primary language English
Location Minneapolis, MN
CreditAttribution: eojthebrave commented

Applied the patch from #5 and it appears to be working as expected. For future reference patches should be rolled from the Drupal root directory. Not a big deal though.

Couple of things I did notice.

I think this line

if (!empty($rrule['BYMONTH']) || $rrule['FREQ'] == 'MONTHLY') {

should be

if (!empty($rrule['BYMONTH']) && $rrule['FREQ'] == 'MONTHLY') {

And the formatting used for some of the big arrays is not consistent with Drupal coding standards. http://drupal.org/coding-standards#array

Thanks.

pauldawg’s picture

pauldawg CreditAttribution: pauldawg commented

Thanks Joe for reviewing this. I will give this another look later but I believe the line you referenced is correct as-is. I'll either correct it or add comments to explain it and submit another patch from the Drupal root.

the formatting used for some of the big arrays is not consistent with Drupal coding standards

Can you give me an example of what you are referring to? I haven't yet added the Coding module and I'll do that as well. It's been on my to-do list. But I also found some other issues with the existing code (such as potential misuse of the t() function, which was pointed out by another user) and I didn't want to introduce any non-contiguous changes, as per the coding best practices.

Thanks again!

eojthebrave’s picture

eojthebrave
he/him
Primary language English
Location Minneapolis, MN
CreditAttribution: eojthebrave commented

I was referring to the formatting of some of the large mutli-dimensional arrays in the patch. As an example your patch has this hunk:

 /**
  * Helper function for FREQ options.
  */
 function FREQ_options() {
-  return array(
-    'NONE' => t('-- Period'),
-    'DAILY' => date_t('Days', 'datetime_plural'),
-    'WEEKLY' => date_t('Weeks', 'datetime_plural'),
-    'MONTHLY' => date_t('Months', 'datetime_plural'),
-    'YEARLY' => date_t('Years', 'datetime_plural'),
-  );
+  return array('NONE' => t('-- Period'), 'DAILY' => date_t('Days', 'datetime_plural'), 'WEEKLY' => date_t('Weeks', 'datetime_plural'), 'MONTHLY' => date_t('Months', 'datetime_plural'), 'YEARLY' => date_t('Years', 'datetime_plural'));
 }

The above function should be formatted as such.

 function date_repeat_theme() {
  return array(
    'NONE' => t('-- Period'),
    'DAILY' => date_t('Days', 'datetime_plural'),
    'WEEKLY' => date_t('Weeks', 'datetime_plural'),
    'MONTHLY' => date_t('Months', 'datetime_plural'),
    'YEARLY' => date_t('Years', 'datetime_plural')
  );
}

And, it looks like in this case the only change made in your patch is removing this formatting. So unless you're actually changing something in the function this change isn't needed.

With regards to the if statement I mentioned above. Without the change I'm getting a bunch of notices from the foreach loop just below it since it is possible for $rrule['BYMONTH'] to not exist and the loop is still executed. It's working in my situation, haven't had a chance to do any extensive testing yet.

pauldawg’s picture

pauldawg CreditAttribution: pauldawg commented

Thanks for the clarification. That formatting was performed by Eclipse's auto formatting. It looked good on first glance but I guess I missed something. Still searching for an IDE which will enforce PHP formatting rules correctly, but so far no luck. I will fix the formatting and test that rule by the end of the week. If I recall correctly there was a reason for the OR condition, but I cannot remember what it was. In any case it could be clearer.

pauldawg’s picture

pauldawg CreditAttribution: pauldawg commented

#18 - Thanks for sharing!

I think I need to understand theming a little bit more. Didn't even occur to me to theme my way out of this. Hoping to get a copy of Drupal Pro for Christmas!!

YK85’s picture

YK85 CreditAttribution: YK85 commented

subscribing

BWPanda’s picture

BWPanda CreditAttribution: BWPanda commented
Version: 6.x-2.4 » 6.x-2.x-dev
FileSize
date_repeat-583192-25.patch12.75 KB

Cleaned up patch code, ran through coder and added:

5. Further enhance readability of the by-month-day dates using the "nth" form of the numbers; i.e. "on the 2nd, 3rd and 10th" instead of "on days 2, 3 and 10"

from the original post.

Needs reviewing.

And BTW, patches for contrib modules don't need to be rolled from Drupal's root directory, just that module's root directory.

Geijutsuka’s picture

Geijutsuka CreditAttribution: Geijutsuka commented

Definitely subscribing.

NancyDru’s picture

NancyDru
she/her/hers
Primary language English
Location Boston
CreditAttribution: NancyDru commented

FYI: "nth form" is commonly referred to as an ordinal.

What is the value in trim operating on the results of a t? Do you see problems with translators adding extra space?

BWPanda’s picture

BWPanda CreditAttribution: BWPanda commented
FileSize
date_repeat-583192-28.patch12.67 KB

FYI: "nth form" is commonly referred to as an ordinal.

Yes, I added a function called date_repeat_ordinal_suffix().

What is the value in trim operating on the results of a t? Do you see problems with translators adding extra space?

No idea. They're present in the latest version of the module, I imagine pauldawg just wanted to keep everything the same...
I've attached a new patch less the trims.

NancyDru’s picture

NancyDru
she/her/hers
Primary language English
Location Boston
CreditAttribution: NancyDru commented

Thanks. I realize now that my post sounded overly critical, which was not my intent. I did look through the patch and, other than my question about trim, it looks good. I am rather busy this week, hopefully I can test it this weekend.

jackalope’s picture

jackalope CreditAttribution: jackalope commented

Tried the patch in #28 out; it seems to work (i.e. the date repeats are certainly more readable) but I get the following errors:

# warning: Invalid argument supplied for foreach() in /home/members/user/sites/site.org/web/sites/all/modules/date/date_repeat/date_repeat.module on line 244.
# warning: Invalid argument supplied for foreach() in /home/members/user/sites/site.org/web/sites/all/modules/date/date_repeat/date_repeat.module on line 249.
BWPanda’s picture

BWPanda CreditAttribution: BWPanda commented
FileSize
date_repeat-583192-31.patch12.84 KB

Here's an updated patch that fixes some issues with the output, specifically removing extra spaces.

I'm not getting any errors jackalope, so maybe someone else can test and see what they get...

BWPanda’s picture

BWPanda CreditAttribution: BWPanda commented
FileSize
date_repeat-583192-32.patch13.16 KB

I think I found jackalope's errors. The attached updated patch should fix them.

crutch’s picture

crutch CreditAttribution: crutch commented

no patch errors but drupal error

Fatal error: Call to undefined function date_repeat_rrule_description() in C:\TWAMPd\htdocs\sites\all\modules\date\date\date.theme on line 229

Ellen Dee’s picture

Ellen Dee CreditAttribution: Ellen Dee commented

subscribing! (And will be back to test.)

aidanlis’s picture

aidanlis CreditAttribution: aidanlis commented

This doesn't apply cleanly to -dev:

aidanlister:date aidan$ patch -p0 < date_repeat-583192-32.patch
patching file date_repeat/date_repeat.module
Hunk #1 FAILED at 1.
Hunk #2 succeeded at 42 (offset -1 lines).
Hunk #3 succeeded at 81 (offset -1 lines).
Hunk #4 succeeded at 133 (offset -1 lines).
Hunk #5 succeeded at 144 (offset -1 lines).
Hunk #6 FAILED at 170.
Hunk #7 FAILED at 266.
3 out of 10 hunks FAILED -- saving rejects to file date_repeat/date_repeat.module.rej

jackbravo’s picture

jackbravo CreditAttribution: jackbravo commented
FileSize
date_repeat-583192-36.patch12.05 KB

Here is a re-roll of the patch for latest dev.

Seems to be working great =). I was specifically looking for a solution to problem #2 on the description:

2. Remove duplicate "every" in the phrase "every week every Thursday"

jackbravo’s picture

jackbravo CreditAttribution: jackbravo commented
Status: Needs review » Reviewed & tested by the community

If I just re-rolled the patch, can I mark it as RTBC?

arlinsandbulte’s picture

arlinsandbulte CreditAttribution: arlinsandbulte commented
Version: 6.x-2.x-dev » 7.x-2.x-dev
Status: Reviewed & tested by the community » Needs work

6.x-2.x is almost in big-fix only mode, so this might be "won't fix."

But instead of setting it as such, I am going to push it into the 7.x-2.x queue for work and review.
After this issue is addressed in 7.x-2.x, we will decide if it should be back-ported to D6.x-2.x.

carsato’s picture

carsato CreditAttribution: carsato commented

The rerolled patch in #36 (http://drupal.org/node/583192#comment-5798296) doesn't work any more because it's outdated. There are new dev versions.

If I have some time at the end of the day I'll try to reroll.

Katrina B’s picture

Katrina B CreditAttribution: Katrina B commented

I'd like to get rid of the space before the period, which I'm seeing on a Drupal 6 site. I'm not a programmer or coder, so messing around with code is not my idea of fun. If you have any suggestions for a solution, please keep that in mind. Thanks!

Michael_Lessard_micles.biz’s picture

Michael_Lessard_micles.biz CreditAttribution: Michael_Lessard_micles.biz commented

Shouldn't the D7 Date Repeat components
simply use the formats we can set in /admin/config/regional/date-time/date-views

As noted in this issue, the displayed field for the repeat rule does not respect its own Date module settings.

Probably easy to fix, but I'm looking ... I have not deciphered the fix in the patches here.

nb: seems to be in date_repeat.module between lines 272 and 295 or maybe in date.theme

jackbravo’s picture

jackbravo CreditAttribution: jackbravo commented
Status: Needs work » Needs review

This is a re-roll of the patch on 36 without adding the comments on 41. But I guess this was needed first. As a note on that, the function being modified (date_repeat_rrule_description) has a $format parameter that uses every time it calls the date_format_date function.

jackbravo’s picture

jackbravo CreditAttribution: jackbravo commented
FileSize
date_repeat-583192-43.patch9.82 KB

Ups! forgot to add patch =P.

Here it is.

jackbravo’s picture

jackbravo CreditAttribution: jackbravo commented
Issue summary: View changes

Updating the summary, because it does work with the case of "every (number)" cases (every 2 days, etc.).

troybthompson’s picture

troybthompson CreditAttribution: troybthompson commented

This is great. Would improving accuracy solve a problem I've been having with this text respecting time zone settings?
Without this patch, if you have an event from, say daily from the 24-31st with times 10pm-11pm and I'm on EST and the field is to adjust for timezone, the long list of dates shows up correctly, but the repeating rule explanation shows the dates as October 25th - 31st, 2015. Or is this something separate?

aitala’s picture

aitala CreditAttribution: aitala commented
FileSize
fields.png18.3 KB

HI,

I really like this patch... what would be very useful for me is to be able to have the date first, then the repeat rule.

Would that be possible?

Thanks,

Eric

fchandler’s picture

fchandler CreditAttribution: fchandler as a volunteer commented

I have applied #43 on my local dev to the 7x-2.9 release, and all works well. However, some commas would still be nice.

Before patch output
Repeats every week every Saturday 30 times except Sat Mar 11 2017.

with commas
Repeats every week, every Saturday, 30 times, except Sat, Mar 11, 2017.

After patch output
Repeats every week on Saturday 30 times except on Sat Mar 11 2017.

with commas
Repeats every week, on Saturday, 30 times, except on Sat, Mar 11, 2017.

sorry if i am being the grammar police.

fchandler’s picture

fchandler CreditAttribution: fchandler as a volunteer commented

for the patch #43 version of date_repeat.module these lines will add commas.

/**
 * Build a description of an iCal rule.
 *
 * Constructs a human-readable description of the rule.
 */
function date_repeat_rrule_description($rrule, $format = 'D, M d, Y') {
  // Empty or invalid value.
  if (empty($rrule) || !strstr($rrule, 'RRULE')) {
    return;
  }

  module_load_include('inc', 'date_api', 'date_api_ical');
  module_load_include('inc', 'date_repeat', 'date_repeat_calc');

  $parts = date_repeat_split_rrule($rrule);
  $additions = $parts[2];
  $exceptions = $parts[1];
  $rrule = $parts[0];
  if ($rrule['FREQ'] == 'NONE') {
    return;
  }

  // Make sure there will be an empty description for any unused parts.
  $description = array(
    '!interval' => '',
    '!byday' => '',
    '!bymonth' => '',
    '!count' => '',
    '!until' => '',
    '!except' => '',
    '!additional' => '',
    '!week_starts_on' => '',
  );
  $interval = date_repeat_interval_options();

  if ($rrule['INTERVAL'] < 1) {
    $rrule['INTERVAL'] = 1;
  }

  if ($rrule['FREQ'] == 'DAILY'
      || $rrule['FREQ'] == 'WEEKLY'
      || $rrule['INTERVAL'] > 1
      || (empty($rrule['BYDAY']) && empty($rrule['BYMONTHDAY']) && empty($rrule['BYMONTH']))
  ) {
    switch ($rrule['FREQ']) {
      case 'WEEKLY':
        $description['!interval'] = format_plural($rrule['INTERVAL'], 'every week,', 'every @count weeks') . ' ';
        break;

      case 'MONTHLY':
        $description['!interval'] = format_plural($rrule['INTERVAL'], 'every month,', 'every @count months') . ' ';
        break;

      case 'YEARLY':
        $description['!interval'] = format_plural($rrule['INTERVAL'], 'every year,', 'every @count years') . ' ';
        break;

      default:
        $description['!interval'] = format_plural($rrule['INTERVAL'], 'every day,', 'every @count days') . ' ';
        break;
    }
  }

  if (!empty($rrule['BYDAY'])) {
    $days = date_repeat_dow_day_options();
    $counts = date_repeat_dow_count_options();
    $ordinalResults = array();
    $weeklyResults = array();
    foreach ($rrule['BYDAY'] as $byday) {
      // Get the numeric part of the BYDAY option, i.e. +3 from +3MO.
      $day = substr($byday, -2);
      $strCount = drupal_substr($byday, 0, 2);
      if ($count = intval($strCount)) {
        $instance = t($days[$day]);
        if (empty($ordinalResults[$count])) {
          $ordinalResults[$count] = array();
        }
        $ordinalResults[$count][] = $instance;
      }
      else {
        $weeklyResults[] = t($days[$day]);
      }
    }

    $byDayResults = array();
    if (!empty($weeklyResults)) {
      $byDayResults[] = t('on') .' '. t(implodeGrammar($weeklyResults)) . t(',');
    }
    if (!empty($ordinalResults)) {
      $resultsByOrdinal = array();
      // TODO: Put this instances in the correct sequence. Currently they are
      // 'fifth from last' to 'last', followed by 'first' to 'fifth'. They
      // should probably be in order from earliest to latest so that 'first'
      // comes before 'last'.
      for ($i = -5; $i <= 5; $i++) {
        if (!empty($ordinalResults[''. $i])) {
          $ordinal = $i < 0 ? $i : ($i > 0 ? '+'. $i : '');
          $resultsByOrdinal[] = t('the') .' '. drupal_strtolower(t($counts[$ordinal])) .' '. implodeGrammar($ordinalResults[$i]);
        }
      }
      $byDayResults[] = t('on') .' '. t(implodeGrammar($resultsByOrdinal)) .' '. t('of the month');
    }
    $description['!byday'] = implodeGrammar($byDayResults);
  }

  if (!empty($rrule['COUNT'])) {
    $description['!count'] = t('!count times,', array('!count' => $rrule['COUNT']));
  }
  if (!empty($rrule['BYMONTH'])) {
    if (count($rrule['BYMONTH']) < 12) {
      $results = array();
      $months = date_month_names();
      foreach ($rrule['BYMONTH'] as $month) {
        $results[] = $months[$month];
      }
      if (!empty($rrule['BYMONTHDAY'])) {
        $monthdays = $rrule['BYMONTHDAY'];
        // Add ordinal suffix to each day
        foreach ($monthdays as $id => $day) {
          $monthdays[$id] = date_repeat_ordinal_suffix($day);
        }
        if (empty($results)) {
          $results[0] = "the month";
        }
        $description['!bymonth'] = t(
          'on the !month_days, of !month_names,',
          array(
            '!month_days' => implodeGrammar($monthdays),
            '!month_names' => implodeGrammar($results),
          )
        );
      }
      else {
        $description['!bymonth'] = t(
          'during !month_names,',
          array('!month_names' => implodeGrammar($results))
        );
      }
    }
  }
  if (!empty($rrule['UNTIL'])) {
    $until = date_ical_date($rrule['UNTIL'], 'UTC');
    date_timezone_set($until, date_default_timezone_object());
    $description['!until'] = t(
      'until !until_date,',
      array('!until_date' => date_format_date($until, 'custom', $format))
    );
  }
  if ($exceptions) {
    $values = array();
    foreach ($exceptions as $exception) {
      $except = date_ical_date($exception, 'UTC');
      date_timezone_set($except, date_default_timezone_object());
      $values[] = date_format_date($except, 'custom', $format);
    }
    $description['!except'] = t(
      'except on !except_dates.',
      array('!except_dates' => implodeGrammar($values))
    );
  }
  if (!empty($rrule['WKST'])) {
    $day_names = date_repeat_dow_day_options();
    $description['!week_starts_on'] = t(
      'where the week starts on !day_of_week.',
      array('!day_of_week' => $day_names[trim($rrule['WKST'])])
    );
  }
  if ($additions) {
    $values = array();
    foreach ($additions as $addition) {
      $add = date_ical_date($addition, 'UTC');
      date_timezone_set($add, date_default_timezone_object());
      $values[] = date_format_date($add, 'custom', $format);
    }
    $description['!additional'] = trim(t('Also includes !additional_dates.',
      array('!additional_dates' => implode(', ', $values))));
  }

  $description = array_filter($description);
  unset($description['!week_starts_on']); // Don't output this value

  $output = implode(' ', $description);
  $output = t('Repeats @repeat', array('@repeat' => $output));

  return $output;
}

For the current 7.x-2.9 version this will add commas:

/**
 * Build a description of an iCal rule.
 *
 * Constructs a human-readable description of the rule.
 */
function date_repeat_rrule_description($rrule, $format = 'D, M d, Y') {
  // Empty or invalid value.
  if (empty($rrule) || !strstr($rrule, 'RRULE')) {
    return;
  }

  module_load_include('inc', 'date_api', 'date_api_ical');
  module_load_include('inc', 'date_repeat', 'date_repeat_calc');

  $parts = date_repeat_split_rrule($rrule);
  $additions = $parts[2];
  $exceptions = $parts[1];
  $rrule = $parts[0];
  if ($rrule['FREQ'] == 'NONE') {
    return;
  }

  // Make sure there will be an empty description for any unused parts.
  $description = array(
    '!interval' => '',
    '!byday' => '',
    '!bymonth' => '',
    '!count' => '',
    '!until' => '',
    '!except' => '',
    '!additional' => '',
    '!week_starts_on' => '',
  );
  $interval = date_repeat_interval_options();

  if ($rrule['INTERVAL'] < 1) {
    $rrule['INTERVAL'] = 1;
  }

  if ($rrule['FREQ'] == 'DAILY'
      || $rrule['FREQ'] == 'WEEKLY'
      || $rrule['INTERVAL'] > 1
      || (empty($rrule['BYDAY']) && empty($rrule['BYMONTHDAY']) && empty($rrule['BYMONTH']))
  ) {
    switch ($rrule['FREQ']) {
      case 'WEEKLY':
        $description['!interval'] = format_plural($rrule['INTERVAL'], 'every week,', 'every @count weeks') . ' ';
        break;

      case 'MONTHLY':
        $description['!interval'] = format_plural($rrule['INTERVAL'], 'every month,', 'every @count months') . ' ';
        break;

      case 'YEARLY':
        $description['!interval'] = format_plural($rrule['INTERVAL'], 'every year,', 'every @count years') . ' ';
        break;

      default:
        $description['!interval'] = format_plural($rrule['INTERVAL'], 'every day,', 'every @count days') . ' ';
        break;
    }
  }

  if (!empty($rrule['BYDAY'])) {
    $days = date_repeat_dow_day_options();
    $counts = date_repeat_dow_count_options();
    $ordinalResults = array();
    $weeklyResults = array();
    foreach ($rrule['BYDAY'] as $byday) {
      // Get the numeric part of the BYDAY option, i.e. +3 from +3MO.
      $day = substr($byday, -2);
      $strCount = drupal_substr($byday, 0, 2);
      if ($count = intval($strCount)) {
        $instance = t($days[$day]);
        if (empty($ordinalResults[$count])) {
          $ordinalResults[$count] = array();
        }
        $ordinalResults[$count][] = $instance;
      }
      else {
        $weeklyResults[] = t($days[$day]);
      }
    }

    $byDayResults = array();
    if (!empty($weeklyResults)) {
      $byDayResults[] = t('on') .' '. t(implodeGrammar($weeklyResults)) . t(',');
    }
    if (!empty($ordinalResults)) {
      $resultsByOrdinal = array();
      // TODO: Put this instances in the correct sequence. Currently they are
      // 'fifth from last' to 'last', followed by 'first' to 'fifth'. They
      // should probably be in order from earliest to latest so that 'first'
      // comes before 'last'.
      for ($i = -5; $i <= 5; $i++) {
        if (!empty($ordinalResults[''. $i])) {
          $ordinal = $i < 0 ? $i : ($i > 0 ? '+'. $i : '');
          $resultsByOrdinal[] = t('the') .' '. drupal_strtolower(t($counts[$ordinal])) .' '. implodeGrammar($ordinalResults[$i]);
        }
      }
      $byDayResults[] = t('on') .' '. t(implodeGrammar($resultsByOrdinal)) .' '. t('of the month');
    }
    $description['!byday'] = implodeGrammar($byDayResults);
  }

  if (!empty($rrule['COUNT'])) {
    $description['!count'] = t('!count times,', array('!count' => $rrule['COUNT']));
  }
  if (!empty($rrule['BYMONTH'])) {
    if (count($rrule['BYMONTH']) < 12) {
      $results = array();
      $months = date_month_names();
      foreach ($rrule['BYMONTH'] as $month) {
        $results[] = $months[$month];
      }
      if (!empty($rrule['BYMONTHDAY'])) {
        $monthdays = $rrule['BYMONTHDAY'];
        // Add ordinal suffix to each day
        foreach ($monthdays as $id => $day) {
          $monthdays[$id] = date_repeat_ordinal_suffix($day);
        }
        if (empty($results)) {
          $results[0] = "the month";
        }
        $description['!bymonth'] = t(
          'on the !month_days, of !month_names,',
          array(
            '!month_days' => implodeGrammar($monthdays),
            '!month_names' => implodeGrammar($results),
          )
        );
      }
      else {
        $description['!bymonth'] = t(
          'during !month_names,',
          array('!month_names' => implodeGrammar($results))
        );
      }
    }
  }
  if (!empty($rrule['UNTIL'])) {
    $until = date_ical_date($rrule['UNTIL'], 'UTC');
    date_timezone_set($until, date_default_timezone_object());
    $description['!until'] = t(
      'until !until_date,',
      array('!until_date' => date_format_date($until, 'custom', $format))
    );
  }
  if ($exceptions) {
    $values = array();
    foreach ($exceptions as $exception) {
      $except = date_ical_date($exception, 'UTC');
      date_timezone_set($except, date_default_timezone_object());
      $values[] = date_format_date($except, 'custom', $format);
    }
    $description['!except'] = t(
      'except on !except_dates.',
      array('!except_dates' => implodeGrammar($values))
    );
  }
  if (!empty($rrule['WKST'])) {
    $day_names = date_repeat_dow_day_options();
    $description['!week_starts_on'] = t(
      'where the week starts on !day_of_week.',
      array('!day_of_week' => $day_names[trim($rrule['WKST'])])
    );
  }
  if ($additions) {
    $values = array();
    foreach ($additions as $addition) {
      $add = date_ical_date($addition, 'UTC');
      date_timezone_set($add, date_default_timezone_object());
      $values[] = date_format_date($add, 'custom', $format);
    }
    $description['!additional'] = trim(t('Also includes !additional_dates.',
      array('!additional_dates' => implode(', ', $values))));
  }

  $description = array_filter($description);
  unset($description['!week_starts_on']); // Don't output this value

  $output = implode(' ', $description);
  $output = t('Repeats @repeat', array('@repeat' => $output));

  return $output;
}
fchandler’s picture

fchandler CreditAttribution: fchandler as a volunteer commented
FileSize
date_repeat-583192-49.patch5.98 KB

I made this patch for the current 7x-2.9 or dev version that adds commas to the current version. This is my first time creating a patch. I hope I did it correctly. It worked on my local machine.

fchandler’s picture

fchandler CreditAttribution: fchandler as a volunteer commented
FileSize
date_repeat-583192-50.patch10.2 KB

I then rolled this patch with #43 patch applied. It adds commas to the #43 patch.

fchandler’s picture

fchandler CreditAttribution: fchandler as a volunteer commented
FileSize
date_repeat-583192-51.patch10.2 KB

and there was a correction needed, so here is #43 again. There was an extra period at the end of patch #50.

echoz’s picture

echoz CreditAttribution: echoz commented

patch #51 applies cleanly to Date 7.x-2.10

I haven't tested it thoroughly, nor did I patch dev, so not setting status to RTBC, although I have been running #43 for a long time on 7.x-2.9 without issue.

echoz’s picture

echoz CreditAttribution: echoz commented

I'm unclear for what's needed to get this to RTBC in hopes to include it with #2867810: Plan for Date 7.x-2.11 release

steinmb’s picture

steinmb CreditAttribution: steinmb as a volunteer commented

We need to test it against latest dev. Manually test the patch. Create screenshots of before and after patch would also save the maintainer time. The code in #51 also need to be reviewed, that goes without saying.

Chris Matthews’s picture

Chris Matthews CreditAttribution: Chris Matthews as a volunteer commented
Status: Needs review » Needs work
Issue tags: +Needs reroll

The 2 year old patch in #51 to date_repeat.module does not apply to the latest date 7.x-2.x-dev and needs a reroll.