remove endif; from theme files

mfer - March 23, 2009 - 13:42
Project:Drupal
Version:7.x-dev
Component:theme system
Category:task
Priority:normal
Assigned:Unassigned
Status:won't fix
Issue tags:Needs design review
Description

At the request of the designers/themers we should switch from if()... endif; in the template files to the standard:

<?php
if ($something) {
 
// do something
}
else {
 
// do something else
}
?>

#1

spuky - July 6, 2009 - 20:02
Status:active» needs review
AttachmentSizeStatusTest resultOperations
remove_endif_from_tpl.patch39.89 KBIdlePassed: 11660 passes, 0 fails, 0 exceptionsView details | Re-test

#2

akahn - July 9, 2009 - 13:26

Where did the discussion about switching from endif; to } take place? I'd like to read that.

Normally I prefer the standard format, but in templates, where you will have something like

<?php
}
?>
, that is ugly and less readable/expressive than
<?php
endif;
?>
. I think the if (condition): , endif; format implies that within the block, there is going to be a chunk of non-PHP stuff. Markup and perhaps printing of variables. Template stuff.

On the other hand, standard curly brace notation, to me, implies that there will be some actual PHP, code/processing stuff going on within the block.

#3

mfer - July 9, 2009 - 18:17

@akahn - This discussion happened in more face to face venues. The two that I was directly involved in happened in the theme/design bof at drupalcon dc. This was the Saturday bof with a large group of designers and themers. The second happened at drupalcamp boston which was the first design and theming centered camp. I believe we have, also, discussed this in #drupal-design on irc.

Basically, the designers/themers were all in agreement that they wanted to use the format:

<?php if ($something) { ?>
  <!-- Do someting -->
<?php } ?>

rather than the format:

<?php if ($something): ?>
  <!-- Do someting -->
<?php endif; ?>

I added this as an issue after drupalcon dc because i took that on as a task.

#4

Jacine - July 9, 2009 - 18:22

I'm surprised...

-1

#5

JohnAlbin - July 9, 2009 - 18:28

I've been doing this too long. I actually think its more important to do whatever makes sense to the majority of new-to-PHP themers.

#6

Zarabadoo - July 9, 2009 - 18:35

I actually prefer the way it is now. So -1 for me.

#7

mfer - July 9, 2009 - 18:51

@JohnAlbin a majority of new people to PHP learn if () { } syntax. The if (): endif; is usually the second syntax someone learns if they are trying to pickup the basics of php. every php tutorial uses if () { }.

#8

akahn - July 9, 2009 - 19:38

Okay, so the reason you are citing is that new PHP users learn if () { } initially while if (): endif; is less familiar to people. I think that's valid, but since endif; is expresses what its doing using a word, I'm not sure how compelling the argument is.

Using your same logic, <?php } ?> might look like a startling string of special characters to a n00b, rather than suggesting "Oh, this closes the if block started above."

Maybe it would be helpful if someone could clarify why we use if (): endif; in the first place, maybe there was a thoughtful decision behind this.

#9

Michelle - July 9, 2009 - 19:49

-1 from me as well. I'm not a themer but I deal with template files a lot with Advanced Forum and I much prefer the endif; there. In straight code, the matching brackets make sense. In templates, when it's mixed in with a sea of HTML, the "endif" is a lot easier to find and match up.

Michelle

#10

spuky - July 13, 2009 - 10:11

I was expecting this discussion when seeing the Task since my first motivation was to do some easy tasks to get "doing Patches and the Process" down.

So I was already thinking for myself that I don't like the Patch but If themers like it that way why not :-).

But I thought If there I a patch people will be talking about the issue. And we will have a decision at least.

On the one hand taking the patch would give some consistency how code should be written... (no exception for .tpl.php files...)
on the other hand I see that here is no Improvement in the readability of the tpl.php files (some closing bracket is even harder to spot I think)

#11

Damien Tournoud - July 13, 2009 - 10:21
Status:needs review» won't fix

I find the "alternative syntax for control structure" ugly, but it is not deprecated and the consensus here is to keep using it for templates. Won't fix.

#12

dman - July 13, 2009 - 10:39
Status:won't fix» needs review

+1 from me. :-)
colon-endif is inconsistent and unlike any other current language construct. Does not balance, is some weird throwback/compromise for early PHP developers who couldn't stop thinking in Perl or Bash or something.
Trying to educate themers on yet another syntax exception is not in any way easier.

This is opinion, so if people continue to use it, it's not worth arguing about, but I make a point of getting rid of it from any theme I work on. It just looks inelegant every time I see it.

EDIT: Found the previous discussion I was thinking of #471206: coding style fixes - if statement in template files

#13

dman - July 13, 2009 - 10:33
Status:needs review» won't fix

Status change was unintentional cross-post. :-}
I'll put it back, coz I'm not into revert debates.
Maybe we can ask around again in a year when everyone has stopped thinking in PHP4 or whatever...

#14

aj045 - July 13, 2009 - 17:52

It's awfully easy to skip a teeny tiny closing bracket even with a sophisticated syntax highlighter. I'd prefer readability over consistency in this particular case. Swapping the bracket syntax with the endif syntax makes finding the end of the scope easier when skimming over code.

But I don't think Drupal has a set standard for this (correct me if I am wrong), so there's nothing really stopping anyone from using brackets instead.

 
 

Drupal is a registered trademark of Dries Buytaert.