Issue status update for http://drupal.org/node/29283 Post a follow up: http://drupal.org/project/comments/add/29283 Project: Drupal Version: cvs Component: theme system Category: bug reports Priority: normal Assigned to: moshe weitzman Reported by: moshe weitzman Updated by: Uwe Hermann Status: patch (code needs review) Attachment: http://drupal.org/files/issues/logo_0.patch (791 bytes) The patch is broken. Here's a fixed one. Uwe Hermann Previous comments: ------------------------------------------------------------------------ Sun, 21 Aug 2005 01:49:53 +0000 : moshe weitzman Attachment: http://drupal.org/files/issues/logo.patch (830 bytes) if admin chooses no logo in theme config, bluemarine still lays down an img tag which points back to the site home page. thats a sure way to increase your load and bandwidth. yikes! ------------------------------------------------------------------------ Sun, 21 Aug 2005 01:53:19 +0000 : moshe weitzman pushbutton already avoids this mistake so i copied its code.

Issue status update for http://drupal.org/node/29283 Post a follow up: http://drupal.org/project/comments/add/29283 Project: Drupal Version: cvs Component: theme system Category: bug reports Priority: normal Assigned to: moshe weitzman Reported by: moshe weitzman Updated by: fajerstarter Status: patch (code needs review) My mistake :) I meant: Shouldn't <div class='site-slogan'>... </div> , <div id="secondary">... </div> etc. be wrapped with to remove the empty when not in use. Cleaner XHTML output. Or is it too expesive to check that for each div? fajerstarter Previous comments: ------------------------------------------------------------------------ Sun, 21 Aug 2005 01:49:53 +0000 : moshe weitzman Attachment: http://drupal.org/files/issues/logo.patch (830 bytes) if admin chooses no logo in theme config, bluemarine still lays down an img tag which points back to the site home page. thats a sure way to increase your load and bandwidth. yikes! ------------------------------------------------------------------------ Sun, 21 Aug 2005 01:53:19 +0000 : moshe weitzman pushbutton already avoids this mistake so i copied its code. ------------------------------------------------------------------------ Sun, 21 Aug 2005 11:51:27 +0000 : Uwe Hermann Attachment: http://drupal.org/files/issues/logo_0.patch (791 bytes) The patch is broken. Here's a fixed one. ------------------------------------------------------------------------ Sun, 21 Aug 2005 12:01:01 +0000 : fajerstarter When your at it: shouldn't .., .. etc. be wrapped with to remove the empty when not in use. Cleaner XHTML output. Or is it too expesive to check that for each div?

Issue status update for http://drupal.org/node/29283 Post a follow up: http://drupal.org/project/comments/add/29283 Project: Drupal Version: cvs Component: theme system Category: bug reports Priority: normal Assigned to: moshe weitzman Reported by: moshe weitzman Updated by: Uwe Hermann Status: patch (code needs review) Attachment: http://drupal.org/files/issues/logo_1.patch (1.43 KB) Sure, why not. I don't think performance will suffer too much. Here's an updated patch. Uwe Hermann Previous comments: ------------------------------------------------------------------------ Sun, 21 Aug 2005 01:49:53 +0000 : moshe weitzman Attachment: http://drupal.org/files/issues/logo.patch (830 bytes) if admin chooses no logo in theme config, bluemarine still lays down an img tag which points back to the site home page. thats a sure way to increase your load and bandwidth. yikes! ------------------------------------------------------------------------ Sun, 21 Aug 2005 01:53:19 +0000 : moshe weitzman pushbutton already avoids this mistake so i copied its code. ------------------------------------------------------------------------ Sun, 21 Aug 2005 11:51:27 +0000 : Uwe Hermann Attachment: http://drupal.org/files/issues/logo_0.patch (791 bytes) The patch is broken. Here's a fixed one. ------------------------------------------------------------------------ Sun, 21 Aug 2005 12:01:01 +0000 : fajerstarter When your at it: shouldn't .., .. etc. be wrapped with to remove the empty when not in use. Cleaner XHTML output. Or is it too expesive to check that for each div? ------------------------------------------------------------------------ Sun, 21 Aug 2005 12:06:46 +0000 : fajerstarter My mistake :) I meant: Shouldn't <div class='site-slogan'>... </div> , <div id="secondary">... </div> etc. be wrapped with to remove the empty when not in use. Cleaner XHTML output. Or is it too expesive to check that for each div? ------------------------------------------------------------------------ Sun, 21 Aug 2005 12:08:33 +0000 : fajerstarter ... wrapped with <?php if ... ?> ... sorry... BTW, the preview function doesn't work :)

Issue status update for http://drupal.org/node/29283 Post a follow up: http://drupal.org/project/comments/add/29283 Project: Drupal Version: cvs Component: theme system Category: bug reports Priority: normal Assigned to: moshe weitzman Reported by: moshe weitzman Updated by: Dries -Status: patch (ready to be committed) +Status: patch (code needs work) Can we write "normal" if-statements with curly brackets? It's more consistent with the rest of the template(s). It's not clean to mix both styles. Dries Previous comments: ------------------------------------------------------------------------ Sun, 21 Aug 2005 01:49:53 +0000 : moshe weitzman Attachment: http://drupal.org/files/issues/logo.patch (830 bytes) if admin chooses no logo in theme config, bluemarine still lays down an img tag which points back to the site home page. thats a sure way to increase your load and bandwidth. yikes! ------------------------------------------------------------------------ Sun, 21 Aug 2005 01:53:19 +0000 : moshe weitzman pushbutton already avoids this mistake so i copied its code. ------------------------------------------------------------------------ Sun, 21 Aug 2005 11:51:27 +0000 : Uwe Hermann Attachment: http://drupal.org/files/issues/logo_0.patch (791 bytes) The patch is broken. Here's a fixed one. ------------------------------------------------------------------------ Sun, 21 Aug 2005 12:01:01 +0000 : fajerstarter When your at it: shouldn't .., .. etc. be wrapped with to remove the empty when not in use. Cleaner XHTML output. Or is it too expesive to check that for each div? ------------------------------------------------------------------------ Sun, 21 Aug 2005 12:06:46 +0000 : fajerstarter My mistake :) I meant: Shouldn't <div class='site-slogan'>... </div> , <div id="secondary">... </div> etc. be wrapped with to remove the empty when not in use. Cleaner XHTML output. Or is it too expesive to check that for each div? ------------------------------------------------------------------------ Sun, 21 Aug 2005 12:08:33 +0000 : fajerstarter ... wrapped with <?php if ... ?> ... sorry... BTW, the preview function doesn't work :) ------------------------------------------------------------------------ Sun, 21 Aug 2005 13:07:19 +0000 : Uwe Hermann Attachment: http://drupal.org/files/issues/logo_1.patch (1.43 KB) Sure, why not. I don't think performance will suffer too much. Here's an updated patch. ------------------------------------------------------------------------ Sun, 21 Aug 2005 22:30:36 +0000 : chx Attachment: http://drupal.org/files/issues/logo_2.patch (1.48 KB) The patch was OK but was not from Drupal directory. I rerolled and I think it's a go.

Issue status update for http://drupal.org/node/29283 Post a follow up: http://drupal.org/project/comments/add/29283 Project: Drupal Version: cvs Component: theme system Category: bug reports Priority: normal Assigned to: moshe weitzman Reported by: moshe weitzman Updated by: adrian Status: patch (code needs work) When i wrote the first template, i took the time to read up on other systems using php for templating, and from what i've read .. and from my experience ... the : method is simpler for newbies to understand. There's no specific reason we can't use {} , other than counting brackets inside tag soup can get hairy =) adrian Previous comments: ------------------------------------------------------------------------ Sun, 21 Aug 2005 01:49:53 +0000 : moshe weitzman Attachment: http://drupal.org/files/issues/logo.patch (830 bytes) if admin chooses no logo in theme config, bluemarine still lays down an img tag which points back to the site home page. thats a sure way to increase your load and bandwidth. yikes! ------------------------------------------------------------------------ Sun, 21 Aug 2005 01:53:19 +0000 : moshe weitzman pushbutton already avoids this mistake so i copied its code. ------------------------------------------------------------------------ Sun, 21 Aug 2005 11:51:27 +0000 : Uwe Hermann Attachment: http://drupal.org/files/issues/logo_0.patch (791 bytes) The patch is broken. Here's a fixed one. ------------------------------------------------------------------------ Sun, 21 Aug 2005 12:01:01 +0000 : fajerstarter When your at it: shouldn't .., .. etc. be wrapped with to remove the empty when not in use. Cleaner XHTML output. Or is it too expesive to check that for each div? ------------------------------------------------------------------------ Sun, 21 Aug 2005 12:06:46 +0000 : fajerstarter My mistake :) I meant: Shouldn't <div class='site-slogan'>... </div> , <div id="secondary">... </div> etc. be wrapped with to remove the empty when not in use. Cleaner XHTML output. Or is it too expesive to check that for each div? ------------------------------------------------------------------------ Sun, 21 Aug 2005 12:08:33 +0000 : fajerstarter ... wrapped with <?php if ... ?> ... sorry... BTW, the preview function doesn't work :) ------------------------------------------------------------------------ Sun, 21 Aug 2005 13:07:19 +0000 : Uwe Hermann Attachment: http://drupal.org/files/issues/logo_1.patch (1.43 KB) Sure, why not. I don't think performance will suffer too much. Here's an updated patch. ------------------------------------------------------------------------ Sun, 21 Aug 2005 22:30:36 +0000 : chx Attachment: http://drupal.org/files/issues/logo_2.patch (1.48 KB) The patch was OK but was not from Drupal directory. I rerolled and I think it's a go. ------------------------------------------------------------------------ Mon, 22 Aug 2005 05:14:12 +0000 : Dries Can we write "normal" if-statements with curly brackets? It's more consistent with the rest of the template(s). It's not clean to mix both styles. ------------------------------------------------------------------------ Mon, 22 Aug 2005 08:47:33 +0000 : Bèr Kessels I have only ever seen the <?php if ($foo): ?> veriosn in phptemplate themes, Dries. CHXs version only follows this de-facto coding-standard.

Issue status update for http://drupal.org/node/29283 Post a follow up: http://drupal.org/project/comments/add/29283 Project: Drupal Version: cvs Component: theme system Category: bug reports Priority: normal Assigned to: moshe weitzman Reported by: moshe weitzman Updated by: Uwe Hermann -Status: patch (code needs work) +Status: patch (ready to be committed) OK, lets see if Dries agrees. Uwe Hermann Previous comments: ------------------------------------------------------------------------ Sun, 21 Aug 2005 01:49:53 +0000 : moshe weitzman Attachment: http://drupal.org/files/issues/logo.patch (830 bytes) if admin chooses no logo in theme config, bluemarine still lays down an img tag which points back to the site home page. thats a sure way to increase your load and bandwidth. yikes! ------------------------------------------------------------------------ Sun, 21 Aug 2005 01:53:19 +0000 : moshe weitzman pushbutton already avoids this mistake so i copied its code. ------------------------------------------------------------------------ Sun, 21 Aug 2005 11:51:27 +0000 : Uwe Hermann Attachment: http://drupal.org/files/issues/logo_0.patch (791 bytes) The patch is broken. Here's a fixed one. ------------------------------------------------------------------------ Sun, 21 Aug 2005 12:01:01 +0000 : fajerstarter When your at it: shouldn't .., .. etc. be wrapped with to remove the empty when not in use. Cleaner XHTML output. Or is it too expesive to check that for each div? ------------------------------------------------------------------------ Sun, 21 Aug 2005 12:06:46 +0000 : fajerstarter My mistake :) I meant: Shouldn't <div class='site-slogan'>... </div> , <div id="secondary">... </div> etc. be wrapped with to remove the empty when not in use. Cleaner XHTML output. Or is it too expesive to check that for each div? ------------------------------------------------------------------------ Sun, 21 Aug 2005 12:08:33 +0000 : fajerstarter ... wrapped with <?php if ... ?> ... sorry... BTW, the preview function doesn't work :) ------------------------------------------------------------------------ Sun, 21 Aug 2005 13:07:19 +0000 : Uwe Hermann Attachment: http://drupal.org/files/issues/logo_1.patch (1.43 KB) Sure, why not. I don't think performance will suffer too much. Here's an updated patch. ------------------------------------------------------------------------ Sun, 21 Aug 2005 22:30:36 +0000 : chx Attachment: http://drupal.org/files/issues/logo_2.patch (1.48 KB) The patch was OK but was not from Drupal directory. I rerolled and I think it's a go. ------------------------------------------------------------------------ Mon, 22 Aug 2005 05:14:12 +0000 : Dries Can we write "normal" if-statements with curly brackets? It's more consistent with the rest of the template(s). It's not clean to mix both styles. ------------------------------------------------------------------------ Mon, 22 Aug 2005 08:47:33 +0000 : Bèr Kessels I have only ever seen the <?php if ($foo): ?> veriosn in phptemplate themes, Dries. CHXs version only follows this de-facto coding-standard. ------------------------------------------------------------------------ Mon, 22 Aug 2005 13:44:49 +0000 : adrian When i wrote the first template, i took the time to read up on other systems using php for templating, and from what i've read .. and from my experience ... the : method is simpler for newbies to understand. There's no specific reason we can't use {} , other than counting brackets inside tag soup can get hairy =) ------------------------------------------------------------------------ Mon, 22 Aug 2005 13:54:50 +0000 : robertDouglass +1 for the : syntax. I'd like to see it used everywhere.

Issue status update for http://drupal.org/node/29283 Post a follow up: http://drupal.org/project/comments/add/29283 Project: Drupal Version: cvs Component: theme system Category: bug reports Priority: normal Assigned to: moshe weitzman Reported by: moshe weitzman Updated by: Uwe Hermann Status: patch (ready to be committed) Attachment: http://drupal.org/files/issues/logo_3.patch (1.68 KB) OK, new patch only using { }. Uwe Hermann Previous comments: ------------------------------------------------------------------------ Sun, 21 Aug 2005 01:49:53 +0000 : moshe weitzman Attachment: http://drupal.org/files/issues/logo.patch (830 bytes) if admin chooses no logo in theme config, bluemarine still lays down an img tag which points back to the site home page. thats a sure way to increase your load and bandwidth. yikes! ------------------------------------------------------------------------ Sun, 21 Aug 2005 01:53:19 +0000 : moshe weitzman pushbutton already avoids this mistake so i copied its code. ------------------------------------------------------------------------ Sun, 21 Aug 2005 11:51:27 +0000 : Uwe Hermann Attachment: http://drupal.org/files/issues/logo_0.patch (791 bytes) The patch is broken. Here's a fixed one. ------------------------------------------------------------------------ Sun, 21 Aug 2005 12:01:01 +0000 : fajerstarter When your at it: shouldn't .., .. etc. be wrapped with to remove the empty when not in use. Cleaner XHTML output. Or is it too expesive to check that for each div? ------------------------------------------------------------------------ Sun, 21 Aug 2005 12:06:46 +0000 : fajerstarter My mistake :) I meant: Shouldn't <div class='site-slogan'>... </div> , <div id="secondary">... </div> etc. be wrapped with to remove the empty when not in use. Cleaner XHTML output. Or is it too expesive to check that for each div? ------------------------------------------------------------------------ Sun, 21 Aug 2005 12:08:33 +0000 : fajerstarter ... wrapped with <?php if ... ?> ... sorry... BTW, the preview function doesn't work :) ------------------------------------------------------------------------ Sun, 21 Aug 2005 13:07:19 +0000 : Uwe Hermann Attachment: http://drupal.org/files/issues/logo_1.patch (1.43 KB) Sure, why not. I don't think performance will suffer too much. Here's an updated patch. ------------------------------------------------------------------------ Sun, 21 Aug 2005 22:30:36 +0000 : chx Attachment: http://drupal.org/files/issues/logo_2.patch (1.48 KB) The patch was OK but was not from Drupal directory. I rerolled and I think it's a go. ------------------------------------------------------------------------ Mon, 22 Aug 2005 05:14:12 +0000 : Dries Can we write "normal" if-statements with curly brackets? It's more consistent with the rest of the template(s). It's not clean to mix both styles. ------------------------------------------------------------------------ Mon, 22 Aug 2005 08:47:33 +0000 : Bèr Kessels I have only ever seen the <?php if ($foo): ?> veriosn in phptemplate themes, Dries. CHXs version only follows this de-facto coding-standard. ------------------------------------------------------------------------ Mon, 22 Aug 2005 13:44:49 +0000 : adrian When i wrote the first template, i took the time to read up on other systems using php for templating, and from what i've read .. and from my experience ... the : method is simpler for newbies to understand. There's no specific reason we can't use {} , other than counting brackets inside tag soup can get hairy =) ------------------------------------------------------------------------ Mon, 22 Aug 2005 13:54:50 +0000 : robertDouglass +1 for the : syntax. I'd like to see it used everywhere. ------------------------------------------------------------------------ Tue, 23 Aug 2005 01:37:55 +0000 : Uwe Hermann OK, lets see if Dries agrees. ------------------------------------------------------------------------ Tue, 23 Aug 2005 04:57:49 +0000 : Dries I think using { } is better but I don't feel strong about it. My point is that we're mixing both styles: <?php if ($sidebar_right) { ?> <?php print $sidebar_right ?> <?php } ?> (Random example taken from the same template file, themes/bluemarine/page.tpl.php.)

Issue status update for http://drupal.org/node/29283 Post a follow up: http://drupal.org/project/comments/add/29283 Project: Drupal Version: cvs Component: theme system Category: bug reports Priority: normal Assigned to: moshe weitzman Reported by: moshe weitzman Updated by: kbahey Status: patch (ready to be committed) I find that when writing code in php-only modules, the curly brace is more natural, and clear. <?php if ($something { } ?> But when writing PHP intermixed with HTML (e.g. phptemplate theme, and others themes), the alternate syntax is much more readable <?php if ($something) : ?> HTML Code goes here <?php endif; ?> I would rather that themes use the alternate syntax just because it is easier to see than a single }. kbahey Previous comments: ------------------------------------------------------------------------ Sun, 21 Aug 2005 01:49:53 +0000 : moshe weitzman Attachment: http://drupal.org/files/issues/logo.patch (830 bytes) if admin chooses no logo in theme config, bluemarine still lays down an img tag which points back to the site home page. thats a sure way to increase your load and bandwidth. yikes! ------------------------------------------------------------------------ Sun, 21 Aug 2005 01:53:19 +0000 : moshe weitzman pushbutton already avoids this mistake so i copied its code. ------------------------------------------------------------------------ Sun, 21 Aug 2005 11:51:27 +0000 : Uwe Hermann Attachment: http://drupal.org/files/issues/logo_0.patch (791 bytes) The patch is broken. Here's a fixed one. ------------------------------------------------------------------------ Sun, 21 Aug 2005 12:01:01 +0000 : fajerstarter When your at it: shouldn't .., .. etc. be wrapped with to remove the empty when not in use. Cleaner XHTML output. Or is it too expesive to check that for each div? ------------------------------------------------------------------------ Sun, 21 Aug 2005 12:06:46 +0000 : fajerstarter My mistake :) I meant: Shouldn't <div class='site-slogan'>... </div> , <div id="secondary">... </div> etc. be wrapped with to remove the empty when not in use. Cleaner XHTML output. Or is it too expesive to check that for each div? ------------------------------------------------------------------------ Sun, 21 Aug 2005 12:08:33 +0000 : fajerstarter ... wrapped with <?php if ... ?> ... sorry... BTW, the preview function doesn't work :) ------------------------------------------------------------------------ Sun, 21 Aug 2005 13:07:19 +0000 : Uwe Hermann Attachment: http://drupal.org/files/issues/logo_1.patch (1.43 KB) Sure, why not. I don't think performance will suffer too much. Here's an updated patch. ------------------------------------------------------------------------ Sun, 21 Aug 2005 22:30:36 +0000 : chx Attachment: http://drupal.org/files/issues/logo_2.patch (1.48 KB) The patch was OK but was not from Drupal directory. I rerolled and I think it's a go. ------------------------------------------------------------------------ Mon, 22 Aug 2005 05:14:12 +0000 : Dries Can we write "normal" if-statements with curly brackets? It's more consistent with the rest of the template(s). It's not clean to mix both styles. ------------------------------------------------------------------------ Mon, 22 Aug 2005 08:47:33 +0000 : Bèr Kessels I have only ever seen the <?php if ($foo): ?> veriosn in phptemplate themes, Dries. CHXs version only follows this de-facto coding-standard. ------------------------------------------------------------------------ Mon, 22 Aug 2005 13:44:49 +0000 : adrian When i wrote the first template, i took the time to read up on other systems using php for templating, and from what i've read .. and from my experience ... the : method is simpler for newbies to understand. There's no specific reason we can't use {} , other than counting brackets inside tag soup can get hairy =) ------------------------------------------------------------------------ Mon, 22 Aug 2005 13:54:50 +0000 : robertDouglass +1 for the : syntax. I'd like to see it used everywhere. ------------------------------------------------------------------------ Tue, 23 Aug 2005 01:37:55 +0000 : Uwe Hermann OK, lets see if Dries agrees. ------------------------------------------------------------------------ Tue, 23 Aug 2005 04:57:49 +0000 : Dries I think using { } is better but I don't feel strong about it. My point is that we're mixing both styles: <?php if ($sidebar_right) { ?> <?php print $sidebar_right ?> <?php } ?> (Random example taken from the same template file, themes/bluemarine/page.tpl.php.) ------------------------------------------------------------------------ Wed, 24 Aug 2005 00:08:42 +0000 : Uwe Hermann Attachment: http://drupal.org/files/issues/logo_3.patch (1.68 KB) OK, new patch only using { }. ------------------------------------------------------------------------ Wed, 24 Aug 2005 06:45:27 +0000 : fajerstarter As a non php coder I agree with adrian that <?php if ($foo): ?> is esier to read/understand. The closing <?php endif; ?> makes more sense than <?php } ?> Another argument could be that that wordpress themes use it... But of course whats really matters is consistency.