Remove closing PHP tags from bundled themes

[netweb]

Previously in #12307 / r19712, #23505 / r28678, and #30534 / r34887 closing PHP tags have been removed from PHP files and is now a ​documented coding standard:

""Omitting the closing PHP tag at the end of a file is preferred. If you use the tag, make sure you remove trailing whitespace."

The ​PHP manual states:

"If a file is pure PHP code, it is preferable to omit the PHP closing tag at the end of the file. This prevents accidental whitespace or new lines being added after the PHP closing tag, which may cause unwanted effects because PHP will start output buffering when there is no intention from the programmer to send any output at that point in the script."

• Themes that exclude closing PHP tags: Twenty Fourteen and Twenty Seventeen
• Themes that include closing PHP tags: Twenty Ten, Eleven, Twelve, Thirteen, Fifteen, and Sixteen

Should the closing PHP tags be removed from the above themes?

Related: ​#bb3078

[netweb]

Note: Twenty Seventeen does have a single file with a closing PHP tag:

https://core.trac.wordpress.org/browser/trunk/src/wp-content/themes/twentyseventeen/template-parts/footer/footer-widgets.php#L32

[dd32]

IMHO changing older bundled themes doesn't gain much benefit, but we should ensure that Twenty Seventeen is consistently sticking to the guideline.

[yahil]

Remove closing tag for Seventeen theme.

[yahil]

Remove closing PHP tags

[milindmore22]

removed closing php tag from Tewnty ten theme

[yahil]

Remove closing tag for twentyeleven theme.

[vishalkakadiya]

I added patch for TwentySixteen theme.

[NomNom99]

Removed closing php tags for twentythirteen theme

[netweb]

Thanks for all the patches :)

@NomNom99, @vishalkakadiya, and @yahil could you all please refresh your patches from the "root" of the repo please.

The 40039-twentyeleven.patch​ shows the full path in the diff to the file location:

src/wp-content/themes/twentyten/

Including the full path is easier for everyone to checkout the patch to both test and when and if needed to commit :)

There's some docs on working with patches in our handbook:

​https://make.wordpress.org/core/handbook/tutorials/working-with-patches/

[manishsongirkar36]

Added patch for twentyfifteen theme.

[vishalkakadiya]

@netweb

I am making patch from github repo, where I didn't found twentysixteen theme check this ​https://github.com/WordPress/WordPress/tree/master/wp-content/themes, Can you advise on this ?

Thanks!

[netweb]

Replying to vishalkakadiya:

@netweb

I am making patch from github repo, where I didn't found twentysixteen theme check this ​https://github.com/WordPress/WordPress/tree/master/wp-content/themes, Can you advise on this ?

Thanks!

@vishalkakadiya Right you are, can you instead then create a pull request to the Twenty Sixteen theme and add a link here to this ticket to the pull request please? ​https://github.com/WordPress/twentysixteen

[yahil]

Removed closing php tag from twentyseventeen theme.

[yahil]

Removed closing php tag from Twentyeleven theme.

[yahil]

@netweb Done :)

Replying to netweb:

Thanks for all the patches :)

@NomNom99, @vishalkakadiya, and @yahil could you all please refresh your patches from the "root" of the repo please.

The 40039-twentyeleven.patch​ shows the full path in the diff to the file location:

src/wp-content/themes/twentyten/

Including the full path is easier for everyone to checkout the patch to both test and when and if needed to commit :)

There's some docs on working with patches in our handbook:

​https://make.wordpress.org/core/handbook/tutorials/working-with-patches/

[vishalkakadiya]

@netweb

Send pull request for twentysixteen, you can check on ​https://github.com/WordPress/twentysixteen/pull/497

Replying to netweb:

Replying to vishalkakadiya:

@netweb

I am making patch from github repo, where I didn't found twentysixteen theme check this ​https://github.com/WordPress/WordPress/tree/master/wp-content/themes, Can you advise on this ?

Thanks!

@vishalkakadiya Right you are, can you instead then create a pull request to the Twenty Sixteen theme and add a link here to this ticket to the pull request please? ​https://github.com/WordPress/twentysixteen

[vishalkakadiya]

@netweb
Here is the refreshed pull request on ​https://github.com/WordPress/twentysixteen/pull/498.

[netweb]

Replying to vishalkakadiya:

@netweb
Here is the refreshed pull request on ​https://github.com/WordPress/twentysixteen/pull/498.

Thanks, I've replied on the PRs

[sabernhardt]

40039.2.patch​ combines previous patches and includes more recent themes.

Trac ticket: https://core.trac.wordpress.org/ticket/40039

Putting together the previous patch in a Github PR so it's easier to review and to refresh if needed.

[audrasjb]

☝️ I put together a Github PR so it's easier to review and to refresh if needed.

Thanks! This needed to be in a PR, and it will require quite a few newlines following the opening PHP tags.

(Revisions to PR 2992)

Putting get_footer(); on its own line at the end of 63 files seems simple enough.

The value of modifying the other 20 files is a bit more questionable, particularly these 8 that have endif; at the bottom:

• twentyten\loop.php
• twentytwelve\sidebar.php
• twentythirteen\sidebar.php
• twentysixteen\sidebar.php
• twentynineteen\template-parts\footer\footer-widgets.php
• twentynineteen\template-parts\header\entry-header.php
• twentynineteen\template-parts\post\author-bio.php
• twentytwenty\template-parts\entry-author-bio.php

Additionally, this PR has a few extra changes:

• Fixes a typo in a translator comment for Twenty Eleven's category.php.
• Edits unrelated indentation earlier in the file for Twenty Ten's loop-attachment.php and loop-single.php.

Trac ticket: https://core.trac.wordpress.org/ticket/40039

[desrosj]

In 53875:

Twenty Eleven: Correct inline translator comment.

Props sabernhardt.
See #40039.

[desrosj]

I'm a bit torn here.

On one hand, I do agree with @dd32 in that I don't see much benefit to changing this in older themes.

On the other, it is important that the default themes follow all of the recommended coding standards and best practices as much as possible as they serve as examples to the greater community. People new to WordPress and theming often use these as examples and learn from them.

However, using the active install numbers to gauge overall usage, it's unlikely that a significant number of developers are using the older themes as a starting point for a new child theme. In my eyes, this decreases the amount of weight to give this argument default themes serving as examples to the greater community.

My current stance is that I think it's reasonable to make this change for more recent themes, and I'll make that commit shortly.

In my opinion, there are three groups I would place the default themes into. Block-based themes (Twenty Twenty-Two), modern themes (Twenty Seventeen through Twenty Twenty-One), older themes (Twenty Sixteen and earlier). My personal line for "more recent themes" is Twenty Seventeen and newer, so that's where I'll focus.

I'll leave this open for a bit longer to allow for some counter points arguments to fix this in Twenty Sixteen and earlier.

[desrosj]

In 53880:

Bundled Themes: Remove closing PHP tag at the end of files.

To help avoid issues with trailing whitespace, omitting the closing PHP tag at the end of a file is preferred.

Props netweb, dd32, yahil, milindmore22, vishalkakadiya, NomNom99, manishsongirkar36, sabernhardt, audrasjb, desrosj.
See #40039.

Partially merged in https://core.trac.wordpress.org/changeset/53880.

Superseded by #3018.

[audrasjb]

As per today's bug scrub, the contributors agreed that fixing this for older themes doesn't provide much value. Let's close this ticket as fixed.

Thanks everyone!