The standard for if () statements in template files is:
if ($foo):
....
endif;
The attached patch fixes this in node.tpl.php, though it results in more lines of code (for readability), which is partly why this was split out from the patch in #471142: coding style fixes (part 2)
| Comment | File | Size | Author |
|---|---|---|---|
| coding_style_endif.patch | 1.58 KB | stella |
Comments
Comment #1
damien tournoud commentedThis "standard" seems in frontal opposition with our documented standards [1]. As far as I know, it was only one developer choice. Core was never consistent in that regard (see for example [2]) before all themes except Garland were removed, leaving this the de facto standard.
Can't we go back to something more standard?
[1] http://drupal.org/coding-standards
[2] http://cvs.drupal.org/viewvc.py/drupal/drupal/themes/bluemarine/page.tpl...
Comment #2
stella commentedAgreed, I don't believe it's documented (which I think is strange since it's in coder...). It does appear to be the de facto standard. I do remember some discussion about it before, and I think the consensus was that it would be easier for non-programmers to deal with. However, we should probably access this "de-facto" standard and decide what standard we want to use for template files, which is why I separated this patch out from the other changes.
Comment #3
dman commented<opinion class="rant">- endif makes me queasy, I personally wish it wasn't used anywhere. I don't believe it helps non-coders at all - it's just as likely to get broken by html edits as the other, and has no symmetry with any of the code you will find in the drupal.org docs. Kill it. Really.
</opinion>Even so - this patch is using the
if() : endifparadigm in a place where it is real code logic, not just an inline display switch. The line it stepped over is tiny, but IMO :- conditionally initializing a string, then printing a string is code.
- conditionally printing a sting is embedded display logic.
IF endifs are acceptable for embedded display, that still doesn't mean they should be promoted up to being used inside code logic.
The approach this patch takes - by splitting out the class string - is an OK move. But using endif in real code - arrg.
... just opinion again :-}
Comment #4
lilou commentedAdd tag.
Comment #6
stella commentedComment #7
webchickHm. Sorry, but this is won't fix for me. I'd prefer to see this addressed by #306358: Add $classes to templates with new theme_process_hook functions which is a much more themer-friendly solution to the problem.