files missing closing php tags

[washer]

Numerous files in the 2.9.2 'wp-includes' folder are missing closing php tags (cron.php, wp-cron.php, version.php, etc).

[nacin]

Closing PHP tags aren't required, though the general consensus among the core developers are to include them. We leave them off in files that are edited by users, which means wp-config.php and any default theme functions.php file. (The other theme files, of course, are expected to generate output.) For consistency we should review all files and add the ones that are missing.

[sorich87]

This applies to files to other wordpress files as well, not to wp-includes files only.

[sorich87]

fix closing tags in all the wordpress core files

[markmcwilliams]

Simple patch, nothing wrong I can see, another ticket we can tick off the list? Or is there a plan to go through all the files before release of 3.0 and catch them?

[kkarpieszuk]

missing closing php tag is not a bug, but coding style. IMO it's good coding style because this eliminates problem with white spaces after closing tag and common problem with error 'headers already send'

i suggest to close this issue

[nacin]

Re-opening.

It is the WordPress coding standard to include closing PHP tags on files that are not expected to be edited.

That excludes a theme's functions.php and wp-config.php.

The 'headers already sent' argument only works for those files. All core files should not be edited, and should have closing tags.

[scribu]

I vote we change the WP coding standard so that closing tags are not included in any file.

[scribu]

Also, I think there are more important issues to fix atm. Maybe this could be moved to 3.1 early?

[nacin]

Yeah, patch is stale anyway. I didn't bother applying, but since we changed include paths, it'll bork.

I vote we change the WP coding standard so that closing tags are not included in any file.

I would tend to disagree with that. It identifies the end of the file so you know the file is complete. The alternative would be to add // end. It's a fine sanity check. We're normally pretty good about them, so I'm surprised there's so many files missing them.

[dd32]

I vote ....

I'd generally agree with that :)

We're normally pretty good about them, so I'm surprised there's so many files missing them.

I'm not. If you look through the list of files, a fair chunk of them will be ones which have been written(or majorly rewritten) in the not-too-distant past. To me, to not include them has been an attraction to most developers for awhile, even if against the coding standards, There are a few things in core which go against the standards which are generally seen as ok to some.

[jacobsantos]

Well, someone had to commit patches or take them out. I know in the past that patches would not be committed that took them out and there was a huge battle over taking it out of the wp-config.php. I believe eventually reason prevailed but only after the 20th ticket[1] was created that dealt with the "headers already sent" issue.

I think it was more or less the least favorite part of the standard. The arguments on why it should be included seem a little lacking, but a standard is a standard and what can one do? Regardless, a standard should be followed or it becomes more of a guideline and then we'll have spaces instead of tabs.

The standard should at least be updated to when it is acceptable to remove the closing tag or when it is required to have it.

[1] It was more like the 6th or 7th ticket, but okay.

[hakre]

Related: #10633

[RyanMurphy]

diff as of r17488

[toscho]

wp-includes/default-filters.php has a blank line after the closing tag. Is that intended?

I think, we should drop the closing tags. They do not add any value.

[RyanMurphy]

Replying to toscho:

wp-includes/default-filters.php has a blank line after the closing tag. Is that intended?

I don't see a blank line after the closing tag - I do see one between the last command and the tag though.

I think, we should drop the closing tags. They do not add any value.

IMO, they do add value, in that you know you have the full file. When I was making the patch, I had to look at some of the files in the trac browser to make sure I had the full file, because where it ended didn't look right.

If we don't want to use the PHP closing tag, we should at least add a comment, like I did with wp-config-sample.php. But right now, the standard is to include them.

[toscho]

Replying to RyanMurphy:

Replying to toscho:

wp-includes/default-filters.php has a blank line after the closing tag. Is that intended?

I don't see a blank line after the closing tag - I do see one between the last command and the tag though.

Oh, sorry, I shouldn’t rely on SVN. The Subclipse plugin is adding blank lines. Dangerous.

IMO, they do add value, in that you know you have the full file. When I was making the patch, I had to look at some of the files in the trac browser to make sure I had the full file, because where it ended didn't look right.

That’s probably a question of style. When I see a closing tag, I expect some output afterwards.
Shouldn’t diff show you if the patch is complete?

If we don't want to use the PHP closing tag, we should at least add a comment, like I did with wp-config-sample.php.

I don’t see such a comment in ​wp-config-sample.php?

#!end :)

[ryan]

Remove ?> from EOF. Trim newlines.

[markjaquith]

Fine by me. Trailing whitespace issues are nasty, and hard to debug. I can see an argument for giving them a // EOF comment so people don't think the file terminated prematurely, but that's probably an edge case (incomplete file uploads, that is).

[westi]

I much prefer to keep them and would prefer that over a comment at the end of the file.

I also think there are much more important things to concern ourselves over that whether or not a file ends with ?>

[ryan]

In [19712]:

Lose EOF ?>. Clean up EOF newlines. fixes #12307

[hakre]

Thanks for fixing this, please change close reason in the #10633 duplicate ticket.