Download & Extend

remove endif; from theme files

Project:Drupal core
Version:7.x-dev
Component:theme system
Category:task
Priority:normal
Assigned:Unassigned
Status:closed (won't fix)
Issue tags:Needs design review

Issue Summary

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
}
?>

Comments

#1

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

#2

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

@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

I'm surprised...

-1

#5

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

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

#7

@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

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

-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

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

Status:needs review» closed (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

Status:closed (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

Status:needs review» closed (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

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.

#15

I don't really understend this request. How can curly braces are more understandable than words? Please don't remove "endif" and "elseif" shorthand! This is far more simpler than chasing braces through code.

#16

@xalexas: This was set to won't fix nearly a year ago.

Michelle

#18

Note: if this is the accepted default for template files then the coding standards should be updated to mention it too.

#19

dman: Maybe we can ask around again in a year when everyone has stopped thinking in PHP4 or whatever...

I don't want to upset anyone by reopening this, but I'm convinced curly braces are the right way to go here. I always replace endifs with curly braces when working with themes, and I'm only a beginner/intermediate PHP coder.

It is simply much easier to check syntax when using curly braces. Lots of editors permit code folding, and the ability to jump between brackets. These features play nicely with curly braces but break on endif statements.

If the object is to ensure human readability of code, then it seems to me that comments are made for this job. Why distort the code to this end?

Something like this syntax should work perfectly for everyone, even adding more information and clarity than is given by endif statements:

  <?php if ($messages) { ?>
    <div class="messages"><?php print $messages; ?></div>
  <?php } // end if ($messages) ?>

I think the use of endif should be discouraged in the Drupal coding standards on the grounds that it breaks common editor tools.

nobody click here