Debug log improvements

+++ includes/rules.core.inc
@@ -2094,7 +2094,11 @@ class RulesLog {
+      $head = '<h4 class="rules-debug-log-head rules-debug-open rules-action-link">';
+      $head .= '<span unselectable="on" class="ui-icon ui-icon-triangle-1-e rules-debug-icon-open">&nbsp;</span>';
+      $head .= t($this->log[$line][0], $this->log[$line][1]);
+      $head .= '</h4>';
+      $output[] = '<div class="rules-debug-block">' . $head . $this->renderHelper($line) . '</div>';

Hm, are we sure that it is ok to generate HTML here? Shouldn't this be done in a theme function?

+++ rules.module
@@ -994,32 +997,36 @@ function rules_menu_add_element_title($array) {
+    global $theme_key;
+  ¶
+    // Invoke a the page redirect, in case the action has been executed.

trailing white space, also elsewhere.

Marked #935754: Control debug info visibility as duplicate.

-  // Invoke a the page redirect, in case the action has been executed.
-  // @see rules_action_drupal_goto()
-  if (isset($GLOBALS['_rules_action_drupal_goto_do'])) {
-    list($url, $force) = $GLOBALS['_rules_action_drupal_goto_do'];
-    drupal_goto($url);
-  }

Watch out - you may not move that part into the if!

+    '#title' => t('Debug log settings.'),

There should not be a trailing point.

<i>%theme</i>

Do not use and %, % suffices.

+        } else {
+          Drupal.rules.changeDebugIcon(icon, false);
+        }

Is that conform to the drupal js coding standards?

Also, does the js generally work on all major browsers?

+})(jQuery);
\ No newline at end of file

This shouldn't be there. Add a new line at the end of file.

+  /* Prevent text selection. */

Let's improve that comment to explain why.

+.rules-action-link {
+  position: relative;
+  cursor: pointer;

The class name is not specific to the debug-log, but the CSS. So let's use a specific selector.

+
+  drupal_add_js(drupal_get_path('module', 'rules') . '/ui/rules.debug.js');

Let's add all necessary JS at the same place in the function.

+.rules-debug-block {
+  border: 1px solid #CCCCCC;
+}

the color code should be lower-case

@h4:
With nested lists, we might end up with nested h4. Let's better just stay with divs.

+        $head = '<h4 class="rules-debug-log-head rules-debug-open rules-action-link">';
+        $head .= '<span unselectable="on" class="ui-icon ui-icon-triangle-1-e rules-debug-icon-open">&nbsp;</span>';
+        $head .= t($this->log[$line][0], $this->log[$line][1]);
+        $head .= '</h4>';

We should move all that tags into theme function. Best renderHelper() should produce a render-array containing appropriate #theme keys.

@themes:
I'm not totally sure whether it's fine to have the setting for the default-theme and the admin theme only. What is if a site uses multiple themes? Maybe we have to show the setting for all enabled themes?

+++ includes/rules.core.inc
@@ -2094,7 +2094,9 @@ class RulesLog {
+      $vars['head'] = t($this->log[$line][0], $this->log[$line][1]);

You should never use t() to translate variables.

+++ rules.module
@@ -1000,26 +1007,35 @@ function rules_page_build(&$page) {
+      // If there is no setting for the curren theme, will try to use the
+      // default themes setting.

spelling error, should be "current". should be "If there is no setting for the current theme, use the
default theme setting."

+++ rules_admin/rules_admin.inc
@@ -95,12 +95,48 @@ function rules_admin_components_overview($form, &$form_state, $base_path) {
+      // Hide the regions settings, when the debug log is disabled.

remove that comma

+++ ui/rules.ui.css
@@ -70,17 +70,54 @@ ul.rules-operations-add li {
+  cursor: pointer;
+  ¶
+  /* The span element with the icon which opens the log, has a whitepsace.

trailing white space

subscribing

+++ b/rules_admin/rules_admin.inc
@@ -95,12 +95,48 @@ function rules_admin_components_overview($form, &$form_state, $base_path) {
+    '#description' => t('Once the debug log is enabled, it appears every time a rule is evaluated. It is only visible for users having the permission <a href="!url">%link</a>.', array('%link' => $link, '!url' => url('admin/people/permissions'))),

you can directly link to the permission section with "admin/people/permissions#module-rules"

I'm an impatient developer, so I prefer that the debug log expands immediately without delay.

It would be nice to link the debug statements to the configurations, so that I can easily go to the according Rules configuration page.

And the overlay still eats up the debug log when it closes (e.g. after submitting a node form).

+ * Clear the debug log only, if the debug log will be definitely rendered.

Wrong grammar, there should be no comma before this if.

-    'label' => t('Unblock a user'),
+    'label' => t('Unblock a use'),

This reverts another patch?

+      'title' => t('Access the Rules debug log.'),

There should be no trailing point.

+  // If there is no setting for the current theme, will try to use the
+  // default themes setting.
+  $region = variable_get('rules_debug_region_' . $theme_key, variable_get('rules_debug_region_' . $theme_default, 'help'));

I think we should do a small helper function (maybe private) for that.

+      $('.rules-debug-open-all').click(function(){

Missing whitespace.

+    $region = rules_debug_log_region();
+    // If there is no setting for the current theme, will try to use the
+    // default themes setting.

I guess the comment needs to be removed here, as it is in the function now. Also it doesn't try to use the default theme settings, it does so.

>+ * Helper function which returns the current region for the debug log.

Minor, but it's clear that it is a function ;) Just start the comment with "Returns the ...".

+/**
+ * Clear the debug log only if the debug log will be definitely rendered.
+ */
+function rules_debug_log_pre_render($elements) {

The comment is wrong, the function does a lot more.

+  return  (class_exists('RulesLog', FALSE) && RulesLog::logger()->hasErrors() && user_access('administer rules'));

Double whitespace.

 function theme_rules_log($variables) {

As already noted, all js in that function should be added at the same place.

'<span class="rules-debug-open-all rules-debug-collapsible-link">-Open all-<span>';
"Open all" should go to translators [eg: t()]

Just some minor nitpicks:

+++ b/rules.module
@@ -938,6 +945,17 @@ function rules_menu_add_element_title($array) {
+  // If there is no setting for the current theme, will try to use the
+  // default themes setting.

better: "If there is no setting for the current theme use the default theme setting."

+++ b/rules_admin/rules_admin.inc
@@ -133,12 +133,48 @@ function rules_admin_components_overview($form, &$form_state, $base_path) {
+  $theme_default = variable_get('theme_default', 'bartik');
+  $admin_theme = variable_get('admin_theme');

if we use bartik as fallback, we should use seven as fallback, too.

+++ b/rules_admin/rules_admin.inc
@@ -133,12 +133,48 @@ function rules_admin_components_overview($form, &$form_state, $base_path) {
+    '#description' => t('The region, where the debug log should be displayed on the default theme %theme. For other themes, Rules will try to display the log on the same region, or hide it in case it doesn\'t exist.', array('%theme' => $theme_default)),

Better use double quotes (") here as such escaping may not be handled properly by .pot file generators for text translation http://drupal.org/node/318#quotes

+++ b/ui/ui.theme.inc
@@ -204,11 +204,26 @@ function theme_rules_data_selector_help($variables) {
+  drupal_add_js(drupal_get_path('module', 'rules') . '/ui/rules.debug.js');

I recommend a faster way:
dirname(__FILE__) . '/rules.debug.js'

I recommend a faster way:
dirname(__FILE__) . '/rules.debug.js'

Indeed, that might be faster but a bit more inflexible if the code is moved around. Anyway, let's stay with core if we have no good cause to do not so, and core uses drupal_get_path() in such situations.

dirname(__FILE__) is great for, in case of file-includes that have to run when drupal might not be fully bootstrapped....

Works for me.

thanks, committed!

Impressive team work. This is Drupal at its best...