Make WordPress Core

Opened 11 years ago

Closed 11 years ago

Last modified 9 years ago

#24549 closed enhancement (wontfix)

Follow the WP Handbook Standard, and change The Loop of Core Themes to Use Brackets

Reported by: chriscct7's profile chriscct7 Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.6
Component: Bundled Theme Keywords: needs-codex has-patch
Focuses: Cc:

Description

This started as a discussion over a pull request to change the if-endif statements to brackets in the template files for Easy Digital Downloads. After lengthy discussion on that Github issue, and even more discussion in our dev chat, @pippinsplugins published a post about his beliefs on the matter.http://techcrunch.com/

The reason for this Trac ticket is to request the core's themes, which (to date) are 2010, 2011, 2012, and in 3.6 trunk, 2013, to use braces instead of if-endifs in The Loop.

At the very beginning, this may seem like a trivial matter. Instead it's far from it. This is actually a pretty deep rooted issue, caused by the ambiguity of the presence of endifs in Core, when combined with the discouragement from using them in the Codex.

The problem lies in that, (as Pippin's belief is) that most new WordPress devs first learn coding by studying their theme's loops files. For most users of WordPress, either their theme is one of the aforementioned core themes, or contains a loop based on one of them.

A plugin author, such as the EDD devs, then make template files for our plugins, which are intended to be edited by said users.

A good place to start, would be advantages to using braces over if-endif.

  • Braces are fully supported by most IDEs. Most IDE's can find the corresponding open or close brace based on the highlighting of another one. Importantly, this includes IDE's that said new developers, might be more inclined to use, such as Dreamweaver, or similar. However, if-endifs are not supported in this manner, but in only a handful of IDE's.
  • New developers will understand braces better than if-endif. Whether from their background in CSS (where braces contain rules), mathematics, or otherwise, braces are a symbol of a containing element. From Pippin's post, it also seems the vast majority of developers also prefer braces.
  • It's shorter. Trivial maybe, but still slightly shorter.
  • It's standard. The WordPress Code Handbook states to use braces. If braces are the standard, they should be used throughout core, and particularly not in important, more prominent areas like The Loop of core themes. Several other prominent open source projects follow the same guidelines (ex Drupal etc etc).
  • Save users time learning both systems. Currently, if a user looks at The Loop, they learn endif. If they then proceed to other template files of core themes, or to plugin files, they then need to learn brackets. Making one way the Core's preferred method would cut the amount of time down for a new developer to learn the control logic (eliminate the need to learn endif).

The attached patch/diff replaces all if-endif usages with braces in 2010, 2011, 2012, and 2013.

Attachments (2)

c3cbe6c0b93d08d9861112da32c2a1ff4a13ac57.patch (133.1 KB) - added by chriscct7 11 years ago.
1st Round Patch
c3cbe6c0b93d08d9861112da32c2a1ff4a13ac57.diff (128.0 KB) - added by chriscct7 11 years ago.
1st Round Diff

Download all attachments as: .zip

Change History (27)

#1 @chriscct7
11 years ago

  • Summary changed from Follow the Core Standard, and change The Loop of Core Themes to Use Brackets to Follow the WP Handbook Standard, and change The Loop of Core Themes to Use Brackets

#2 @lkraav
11 years ago

  • Cc leho@… added

#3 @sunnyratilal
11 years ago

  • Cc sunny@… added

#4 @bpetty
11 years ago

If this change was to be made, I don't see any point in doing it in any other core themes except maybe Twenty Thirteen. If we were (for whatever reason) to push out security fixes on older themes, this will make it difficult for users to filter through those changes and apply them to modified versions of those themes.

#5 @toscho
11 years ago

  • Cc info@… added

#6 @F J Kaiser
11 years ago

  • Cc 24-7@… added

#7 @anonymized_7658014
11 years ago

  • Cc caspar@… added

#8 @SergeyBiryukov
11 years ago

  • Component changed from Template to Bundled Theme

#9 @mordauk
11 years ago

  • Cc pippin@… added

#10 follow-ups: @kovshenin
11 years ago

-1. Basic and Pascal are taught in many schools and use if/endif for control structures, and CSS is not a programming language. Shorter is not necessarily faster, but is often more difficult to read, especially when a bunch of HTML is involved.

Last edited 11 years ago by kovshenin (previous) (diff)

#11 in reply to: ↑ 10 ; follow-ups: @dd32
11 years ago

Replying to kovshenin:

-1. Basic and Pascal are thought in many schools and use if/endif for control structures, and CSS is not a programming language. Shorter is not necessarily faster, but is often more difficult to read, especially when a bunch of HTML is involved.

To echo that, one of the primary reasons for using endif verses braces was to improve readability with large chunks of HTML, it's much harder to discern which brace ends a if or while than an explicit endif; and endwhile;.
I initially thought the old Coding style reflected that, but upon looking at the revisions I can't find a reference that backs it up.
Default themes have changed a bit since then though, and some of these changes concern if/endif wrapped around single lines which would be generally discouraged at present in core, but keeping it consistent through the theme should take higher precedence.

http://core.trac.wordpress.org/ticket/4779#comment:19 is the only reference I can find to preferring if/endif where exiting PHP into blocks of HTML is involved, but it's by no means a definitive discussion

#12 follow-ups: @kovshenin
11 years ago

Good argument: http://core.trac.wordpress.org/browser/tags/3.5.1/wp-includes/pluggable.php

If the point of this ticket is to "follow the WP Handbook Standard," perhaps we should rather look into adding a section to the handbook which explains where and why we use the alternative syntax.

#13 in reply to: ↑ 12 @mordauk
11 years ago

Replying to kovshenin:

Good argument: http://core.trac.wordpress.org/browser/tags/3.5.1/wp-includes/pluggable.php

If the point of this ticket is to "follow the WP Handbook Standard," perhaps we should rather look into adding a section to the handbook which explains where and why we use the alternative syntax.

I'd like to see a new section added.

#14 in reply to: ↑ 11 @chriscct7
11 years ago

Replying to dd32:

Replying to kovshenin:

-1. Basic and Pascal are thought in many schools and use if/endif for control structures, and CSS is not a programming language. Shorter is not necessarily faster, but is often more difficult to read, especially when a bunch of HTML is involved.

To echo that, one of the primary reasons for using endif verses braces was to improve readability with large chunks of HTML, it's much harder to discern which brace ends a if or while than an explicit endif; and endwhile;.
I initially thought the old Coding style reflected that, but upon looking at the revisions I can't find a reference that backs it up.
Default themes have changed a bit since then though, and some of these changes concern if/endif wrapped around single lines which would be generally discouraged at present in core, but keeping it consistent through the theme should take higher precedence.

http://core.trac.wordpress.org/ticket/4779#comment:19 is the only reference I can find to preferring if/endif where exiting PHP into blocks of HTML is involved, but it's by no means a definitive discussion

If proper indentation is used it's not hard. Plus most editors will tell you. Touch a brace, and the editor will find it's partner. That isn't true for endifs

#15 in reply to: ↑ 10 @chriscct7
11 years ago

Replying to kovshenin:

-1. Basic and Pascal are thought in many schools and use if/endif for control structures, and CSS is not a programming language. Shorter is not necessarily faster, but is often more difficult to read, especially when a bunch of HTML is involved.

-1 to the comment. Basic and Pascal are not taught in most schools anymore. It's either C/C++ or Java nowadays.

#16 in reply to: ↑ 12 @chriscct7
11 years ago

Replying to kovshenin:

Good argument: http://core.trac.wordpress.org/browser/tags/3.5.1/wp-includes/pluggable.php

If the point of this ticket is to "follow the WP Handbook Standard," perhaps we should rather look into adding a section to the handbook which explains where and why we use the alternative syntax.

Because that would still keep the downsides of using if-endifs. We should look at just using one standard, and keeping it.

#17 @nacin
11 years ago

As has been pointed out, very few PHP projects even allow if/endif. Alternative brace style is largely frowned upon.

That said, a number of us like it specifically when breaking in and out of PHP for long periods of time. This is perhaps an uncommon problem due to the fact that our templating engine *is* PHP.

Just as one example, as loops get more complicated (yet still get edited often by people who don't know or understand PHP), it is easier to pick out the endwhile than the } at the bottom of the file. Pippin said it best:

A user that doesn’t understand a single bit of PHP, and perhaps just minimal HTML, can clearly read my translation above and understand what is happening.

Are there downsides? Sure, absolutely. But they don't come anywhere close to outweighing the status quo.

The current coding standards are very basic. A number of core contributors follow (or don't follow) a large number of other specifics that have not been fully or properly enumerated in that document. I would argue that we're simply missing a stated allowance for alternative brace style when breaking into HTML — which ideally should be avoided where possible, but which is going to be common in themes.

We're not dealing with "new developers". We're dealing with users who look at if/endif, while/endwhile, the_post(), the_title(), etc., and pretty much just think that WordPress has its own templating language. They might not even realize that what they are coding is PHP. They might not know what PHP is.

Users over IDEs every day of the week. Sorry.

#18 follow-up: @F J Kaiser
11 years ago

As this discussion started in my pull request I'll leave my 2 cents as well.

Let's assume a user wants to change something in his theme. The user gets pointed at some file that could probably hold the bits our user wants to change. Then the user opens (for example) the index.php file and sees something like the following:

<?php
get_header();

( is_home() == TRUE ) AND print __( 'Welcome Home!', 'theme_translation' );

if ( have_posts() ) :
	while ( have_posts() ) :
		the_post();
                ?>

                <!-- The title of the post -->
		<h2>
                    <?php echo get_the_title(); ?>
		</h2>

                <!-- The content of the post -->
                <?php the_content(); ?>

                <!-- The date, tags, categories, etc. -->
                <p>
		    <?php echo get_the_theme_custom_meta_data(); ?>
                </p>

                <?php
	endwhile;
endif;

get_sidebar();

get_footer();

Now let's assume our user consults a support route, figures out that there's either a filter or an action for the return value of get_the_theme_custom_meta_data() or the function is a pluggable that can be overwritten in a child theme.

Now our user has a bunch of problems all related to functions like `twentythirteen_entry_meta()` that have or don't have brackets for their statements, build strings in different ways and put MarkUp inside PHP. And this user probably now starts comparing what is written inside functions.php to to what is written in the template file. And then our user is highly confused.

So (Nacin), if we say "Users over IDEs every day of the week.", then we should immediately deprecate all Template Tags that echo something directly and ban &&, || and ^ operators from every theme file. Or in general: Abandon everything that doesn't clearly speak out what is happening. To see what we'll be left with, take a look at the bogus index.php-code from above.

Final note: Having a WordPress coding "standard" is imho nonsense. Especially if every second case for a single rule is an edge case. I vote for dropping the "standard".

#19 follow-up: @wonderboymusic
11 years ago

-100. People who don't know what they're doing should probably not be cowboy'ing their theme files. And although WordPress is easy to learn, it doesn't mean we have to accomodate every noob in the way we code. At this point, I would label the brace thing as a mutual preference of the people who do most of the coding for core.

#20 in reply to: ↑ 18 @DrewAPicture
11 years ago

Replying to F J Kaiser:

So (Nacin), if we say "Users over IDEs every day of the week.", then we should immediately deprecate all Template Tags that echo something directly and ban &&, || and ^ operators from every theme file. Or in general: Abandon everything that doesn't clearly speak out what is happening. To see what we'll be left with, take a look at the bogus index.php-code from above.

Final note: Having a WordPress coding "standard" is imho nonsense. Especially if every second case for a single rule is an edge case. I vote for dropping the "standard".

This kind of hyperbole is unnecessary and doesn't advance the discussion at all. The coding standards are meant as a guide for for core, and as a suggested reference for everyone else.

+1 to adding another section to the handbook as suggested by @kovshenin.

#21 in reply to: ↑ 11 @SergeyBiryukov
11 years ago

Replying to dd32:

To echo that, one of the primary reasons for using endif verses braces was to improve readability with large chunks of HTML, it's much harder to discern which brace ends a if or while than an explicit endif; and endwhile;.

I had the same impression. Braces are fine for core itself, but even core generally uses the if/endif syntax for better readability when it comes to HTML output, so +1 to the handbook section reflecting that.

#22 in reply to: ↑ 19 @maor
11 years ago

Replying to wonderboymusic:

-100. People who don't know what they're doing should probably not be cowboy'ing their theme files. And although WordPress is easy to learn, it doesn't mean we have to accomodate every noob in the way we code. At this point, I would label the brace thing as a mutual preference of the people who do most of the coding for core.

100% agreed. And +1 for adding that new section to the coding standards guide.

#23 @c3mdigital
11 years ago

  • Keywords dev-feedback removed
  • Resolution set to wontfix
  • Status changed from new to closed

Regarding the braces vs alternate syntax argument. We require all our developers at X-Team to use alternate syntax on all front end templates and they are free to use either in back end functions etc.. I've always thought tempting with alternate syntax was kind of the "WordPress" way.

When new standards are implemented in any big project (or small for that matter) code is not normally refactored to meet the new standard especially when its as trivial as curly braces vs colons for control structures. Please read: http://make.wordpress.org/core/2011/03/23/code-refactoring/

The handbooks and coding standards documentation needs to include the use of alternate syntax. http://php.net/manual/en/control-structures.alternative-syntax.php

Last edited 11 years ago by c3mdigital (previous) (diff)

#24 @SergeyBiryukov
11 years ago

  • Milestone Awaiting Review deleted

#25 @westonruter
9 years ago

I've updated the braces section of the handbook to indicate that it is inline control structures that are prohibited, not that literal braces are required as opposed to the alternative syntax for control structures: https://make.wordpress.org/core/handbook/coding-standards/php/#brace-style

See also comment added to post: https://make.wordpress.org/core/handbook/coding-standards/php/#comment-19978

Note: See TracTickets for help on using tickets.