Improve drupal theme system

A big -1 on a lot of the string changes. I am not a fan of changing to single quotes just to change to single quotes. We should pick what is more readable which is what Dries has told me in the past. Changes like this are not a step forward:

-    return "<img src=\"$path\" $attr alt=\"$alt\" title=\"$title\" />";
+    return '<img src="'. $path .'" '. $attr .' alt="'. $alt .'" title="'. $title .'" />';

That is just really hard to read. The escaped double quotes aren't the best, but it is much easier to read than a lot of concatenation with mixed single and double quotes.

I am also opposed to all of the newlines that were removed. It may make the output HTML a tad smaller, but it makes the resulting HTML much harder to read and debug. Example of a bad change:

-      $output .= "<div class=\"messages $type\">\n";
+      $output .= '<div class="messages '. $type .'">';
       if (count($messages) > 1) {
-        $output .= " <ul>\n";
+        $output .= '<ul>';
         foreach($messages as $message) {
-          $output .= '  <li>'. $message ."</li>\n";
+          $output .= '<li>'. $message .'</li>';
         }
-        $output .= " </ul>\n";
+        $output .= '</ul>';
       }
       else {
         $output .= $messages[0];
       }
-      $output .= "</div>\n";
+      $output .= '</div>';

I completely agree with jhriggs on this. So much so, in fact, that I've come out of hiding to say as much. And if your argument for making the code much less readable by putting in single quotes is "it's faster", you'll find there are plenty of places in Drupal where your time would be better spent optimising big chunks of slowness rather than imperceptible improvements like how your string substitution works.

-1 on these sorts of string changes.

Eh, what's wrong with using:

$output .= "<div class='messages $type'>\n";

It works just fine from both the PHP and HTML perspectives.

I dislike mange of the quote-related changes as well. It makes the code harder to read/study. Won't commit as is.

Chris Johnson wrote:
> Eh, what's wrong with using:
> $output .= "

Al, are you talking about the newline appended to the end of the string? I just quoted the code that was previously quoted from the patch, and changed the quoting of the string and the (X)HTML attributes. As far as I know (having quickly glanced at the W3 documents), single and double quotes are valid for attributes, which is what I was trying to demonstrate. You might be right about the use of newline, but that wasn't really part of my intended point.

Note also: In PHP, '$foo' is a string while "$foo" is an interpolated variable. However, "some '$foo' text" results in $foo being interpolated as a variable, that is, PHP is smart. This characteristic allows general use of single quotes inside double quote strings for attributes.

As a result, the \" mess can be cleaned up without resorting to the concatenation mess proposed in the original patch.

But it's just a suggestion, or point of information. Anything that makes code easier to read makes it easier to maintain and results in fewer bugs.

I believe you are right, Chris. I looked into it too. It seems XML attributes can be double or single quoted. I guess my only reservation with using single-quoted attributes would be consistency. Most everywhere we use double-quoted attributes. It might reduce readability from a consistency standpoint.

It seems to me that double quotes should always be used for quoting attributes in XHTML because most of my Javascript function calls use single quotes and using single quotes around them might cause problems.