WordPress.org

Make WordPress Core

Opened 16 months ago

Last modified 16 months ago

#40039 new defect (bug)

Remove closing PHP tags from bundled themes

Reported by: netweb Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords: 2nd-opinion
Focuses: Cc:

Description (last modified by 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

Attachments (8)

40039.patch (269 bytes) - added by yahil 16 months ago.
Remove closing PHP tags
40039-twentyten.diff (6.0 KB) - added by milindmore22 16 months ago.
removed closing php tag from Tewnty ten theme
40039-twentyeleven.patch (3.4 KB) - added by yahil 16 months ago.
Remove closing tag for twentyeleven theme.
40039-twentysixteen.patch (1.9 KB) - added by vishalkakadiya 16 months ago.
I added patch for TwentySixteen theme.
40039-twentythirteen.patch (5.0 KB) - added by NomNom99 16 months ago.
Removed closing php tags for twentythirteen theme
40039-twentyfifteen.patch (2.9 KB) - added by manishsongirkar36 16 months ago.
Added patch for twentyfifteen theme.
40039-twentyseventeen-1.diff (495 bytes) - added by yahil 16 months ago.
Removed closing php tag from twentyseventeen theme.
40039-twentyeleven-1.diff (5.4 KB) - added by yahil 16 months ago.
Removed closing php tag from Twentyeleven theme.

Download all attachments as: .zip

Change History (18)

#1 @netweb
16 months ago

  • Description modified (diff)
Last edited 16 months ago by netweb (previous) (diff)

#2 @dd32
16 months ago

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

Last edited 16 months ago by dd32 (previous) (diff)

#3 @yahil
16 months ago

Remove closing tag for Seventeen theme.

Last edited 16 months ago by yahil (previous) (diff)

@yahil
16 months ago

Remove closing PHP tags

@milindmore22
16 months ago

removed closing php tag from Tewnty ten theme

@yahil
16 months ago

Remove closing tag for twentyeleven theme.

@vishalkakadiya
16 months ago

I added patch for TwentySixteen theme.

@NomNom99
16 months ago

Removed closing php tags for twentythirteen theme

#4 follow-up: @netweb
16 months ago

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
16 months ago

Added patch for twentyfifteen theme.

#5 follow-up: @vishalkakadiya
16 months ago

@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!

#6 in reply to: ↑ 5 ; follow-up: @netweb
16 months ago

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
16 months ago

Removed closing php tag from twentyseventeen theme.

@yahil
16 months ago

Removed closing php tag from Twentyeleven theme.

#7 in reply to: ↑ 4 @yahil
16 months ago

@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/

#8 in reply to: ↑ 6 @vishalkakadiya
16 months ago

@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

#9 follow-up: @vishalkakadiya
16 months ago

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

#10 in reply to: ↑ 9 @netweb
16 months ago

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

Note: See TracTickets for help on using tickets.