[drupal-devel] [bug] bluemarine mistakenly loads home page every time

[fajerstarter]

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 (ready to be committed)

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.




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?




------------------------------------------------------------------------

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 { }.