Internationalization?

Thank you for your reassurance.

I can't use 1.x-dev and opened a dedicated issue: #649530: Creating the cool graph hangs until PHP times out

Should I post patches against 1.0?

Thanks, I will.

It's not at the top of my list right now, but I have a definite need for it.

What's your preference: make a separate am/pm vs. 24-hour time setting or take it from the default site setting like #462762: Make date field internationally compatible?

Here's a patch that fixes a few things:

1. Actually use the minute part of the opening hours (it was using the seconds part instead, snapping to whole hours.
2. Intelligently support fractional hours in the cool graph:
   1. round to quarters
   2. label the full hours
   3. omit the caption for fractional hours to avoid fewer quarters getting wider.
3. Take the desired hours:minutes format from the site settings.
4. Avoid endless loop when opening hours extend to 24:00.

P.S. I've tried to mimic your chaotic indentation and tab use...

Sure, no problem. Having the code cleaned up is definitely worth it for me to reroll.

I'll integrate the new functionality (such as merci_hours_admin), too...

Here's the rerolled patch. See the comments in #5.

Thanks for looking into it!

I understand your wishful thinking, but this is not going to happen. The number of known installed sites of my combined contribs is about 200 times the number of MERCI's, and never in two years has anyone reviewed a patch unless they had a very keen interest in the feature/fix (read: begged for it before-hand). If we progress at this rate, I may as well start from scratch...

Most of the people we work with have consistently said they'd like to contribute in some way

Ok, give them a chance.

Thanks!

There are a few things that you haven't committed — I guess I should have explained them in more detail:

1. Are you not indenting on purpose or is it your editor that does funny things? For example:
   

        else {
          $start_time = date_format($start_object, 'H:i');
          if ($start_time < $start_hours['open']) {
          form_set_error('field_merci_date][0][value][time', t('Reservations cannot start on a %day before %start.', array('%day' => $start_name, '%start' => merci_format_time($start_hours['open']))));
          }
          elseif ($start_time > $start_hours['close']) {
          form_set_error('field_merci_date][0][value][time', t('Reservations cannot start on a %day after %end.', array('%day' => $start_name, '%end' => merci_format_time($start_hours['close']))));
          }
        }
   

   I indented the lines with the form_set_error() calls and you removed two leading spaces for each.

   Code like the following is hard to read and hard to maintain:

          form_set_error("existing_items][placeholders][$did", $message);
          }
          // Check to make sure the currently assigned bucket item is still
          // reservable with the submitted dates.
          elseif (!is_numeric($value)) {
          $bucket_items = array_keys(merci_get_available_bucket_items($node, $value));
          $assigned_item = (int) $node->existing_items['items'][$did];
          if ($assigned_item && !in_array($assigned_item, $bucket_items)) {
            $title_name = db_fetch_object(db_query("SELECT n.title, nt.name FROM {node} n INNER JOIN {node_type} nt ON n.type = nt.type WHERE n.nid = %d", $assigned_item));
            form_set_error("existing_items][placeholders][$did", t("The assignment of %item for the %bucket reservation is no longer reservable with your current date settings.", array('%item' => $title_name->title, '%bucket' => $title_name->name)));
          }
          }
        }
        }
   

    

2.            $time = $hours_date['open'];
              if ($minutes = (int) substr($time, 3)) {
                $hour = (substr($time, 0, 2) + 1);
                $quarters = (int)((60-$minutes)/15);
                $time = ($hour < 10 ? '0'. $hour : $hour) .':00';
                if ($quarters > 0) {
                  $message.= '<th colspan="'. $quarters .'">&nbsp;</th>';
                }
              }
   

   Above I've solved the following problem: If opening hours are at 9:30, then you label "9:30", "10:30", "11:30" — this isn't so great. There should be labels at the full hours only.
   If opening hours are at 9:30, then there should be two 15-minute blocks ahead of 10:00. These two blocks should be the same width as all other 15-minute blocks, meaning there's not enough space for a label. Thus labeling should start only at "10:00". Fractional opening hours will not work correctly without this.
   
    

3.            $end_time = substr($hours_date['close'], 0, 2) .':00';
              
              while ( $time < $end_time ) {
   

   Above I'm doing something similar for fractional closing hours, except it's much simpler. If closing is at 11:15, then there should be only one 15-minute block beginning at 11:00. I'm suppressing the "11:00" label, because it would likely force that last block to be wider than the other 15-minute blocks.
   
    

4.              $message .= '<th colspan="4">' . merci_format_time($time) . '</th>';
   

   Oops, you left a date( 'g:i a' , strtotime( $time ) ) here. Eliminating the hard-coded am/pm's is what this was all about.
   
    

5.              if (substr($time, 0, 2) == '00') {
                  $time = '24'. substr($time, 2);
                }
   

   This is absolutely essential for supporting opening hours until midnight. After 23:00, strtotime( $time . ' +1 hour' ) will roll over to 00:00 and the loop won't terminate.
   
    

6.              $time = $hours_date['open'];
                if ($minutes = (int) substr($time, 3)) {
                  $hour = substr($time, 0, 2);
                  $quarters = (int)(($minutes+14)/15);
                  if ($quarters == 4) {
                    $time = ($hour <= 8 ? '0' : '') . ($hour + 1) .':00';
                  }
                  else {
                    $time = $hour .':'. ($quarters ? $quarters*15 : '00');
                  }
                }
   
   

   Above I'm supporting fractional opening hours. 9:30 and 9:40 should have two 15-minute slots ahead of 9:00. 9:45 and 9:59 should have one slot. (The 9:59 case is degenerate, of course, but MERCI should work properly for all inputs that are formally correct.)
   
    

7.                if ($time == '00:00') {
                    $time = '24:00';
                  }
   

   Again, we have to be able to generate blocks all the way to midnight, but then we need to stop rather than looping forever.
   
    

8. ?>
   Don't put this at the bottom of any of your Drupal code files. You risk outputting newlines ahead of the HTTP headers, which causes problems. And it's simply unnecessary.

How do we proceed?

It's partially applied, but the sections I listed aren't.

I don't know why, and whether it would make sense to prepare another patch with the missing pieces...

The primary motivation here was to get MERCI to use the site's time format in the cool graph instead of hard-coding the am/pm format. In the process I fixed all the other bugs that I encountered, so this got a bit bigger than expected.

You committed part of this patch, but BETA2 still shows the am/pm format in the cool graph.

All occurrences of 'g:i a' should be replaced by the appropriate calls to merci_format_time(). For example

date( 'g:i a' , strtotime( $time ) ) ===>> merci_format_time($time)

If it really helps I can create another patch, but given the history of this issue and the fact that this is a trivial change, it probably makes more sense if you just search your current code for 'g:i a' and make the change yourself.

Oh, I see, the U.S. standard format is 'g:ia' rather than 'g:i a'! And you're using 'g:i' now, without the am/pm? That's what you're showing in wellformatted_merci.png, right?

The 24h format should continue through 13:00, 14:00, and beyond. (Just makeing sure...)

Thanks!

The hour format looks good now, thanks.

MERCI hangs if I enter admin opening hours through midnight (e.g. 07:00-24:00) and try to preview a reservation with a conflict. That's one of the bugs that I fixed, see #16.5/7 above.

Again? After #5, #9, and #16 I have some doubts whether submitting patches fits into your workflow...