plugin-php icon indicating copy to clipboard operation
plugin-php copied to clipboard

avoid unnecessary `?>` when program doesn't have inline nodes (html)

Open titoBouzout opened this issue 7 years ago • 14 comments

In my opinion this should not be adding new lines to the end of a file.

titoBouzout avatar Sep 28 '18 08:09 titoBouzout

@titoBouzout prettier do this, PSR also recommended do this, why you can't do this? Also it is should don't break code

alexander-akait avatar Sep 28 '18 10:09 alexander-akait

My interpretation of this discussion is that the new line should be to the left of "?>" not to the right. https://groups.google.com/forum/#!topic/php-fig/PSzmcFVQst0

It looks like I could avoid the problem by omitting the closing tag "?>", so the new lines will not disturb the cookie declared after the inclusion of the code. Btw, they recommend to not use the closing tag when the file only includes php which may or not be true.. :) Anyways, Thanks for the pointer!

https://www.php-fig.org/psr/psr-2/

titoBouzout avatar Sep 28 '18 10:09 titoBouzout

@titoBouzout Maybe we can avoid ?> when file doesn't have inline nodes

alexander-akait avatar Sep 28 '18 10:09 alexander-akait

/cc @mgrip @czosel what do you think?

alexander-akait avatar Sep 28 '18 10:09 alexander-akait

We definitely shouldn’t add newlines after ?> because that would indeed break code. I’d also be open for removing ?> if the file doesn’t contain any inline nodes.

czosel avatar Sep 28 '18 11:09 czosel

@czosel we already don't add newline after ?> /cc @titoBouzout you hav example where we add newline after ?>

alexander-akait avatar Sep 28 '18 11:09 alexander-akait

https://imgur.com/a/zfK56fN

automatically removing "?>" looks like a good idea

titoBouzout avatar Sep 28 '18 11:09 titoBouzout

@titoBouzout thanks, i will investigate this in near future

alexander-akait avatar Sep 28 '18 11:09 alexander-akait

Thanks for the example @titoBouzout! It looks like a bug/edge case to me.

Input for future reference:

<? echo 2 + 2 ?>

czosel avatar Sep 28 '18 13:09 czosel

Please vote :+1: for remove :heart: for leave, we need feedback, thanks

alexander-akait avatar Jan 18 '19 14:01 alexander-akait

If you following PSR then when the file only has PHP the end tag should be removed.

titoBouzout avatar Jan 18 '19 14:01 titoBouzout

@titoBouzout i agree for removing, but need some feedback

alexander-akait avatar Jan 18 '19 14:01 alexander-akait

For sure big vote to not add trailing ?> tags to any files.

https://stackoverflow.com/questions/4410704/why-would-one-omit-the-close-tag#4499749

WordPress - No https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/blob/develop/WordPress-Core/ruleset.xml#L188

Zend Framework - No https://github.com/zendframework/ZendSkeletonApplication/blob/master/module/Application/test/Controller/IndexControllerTest.php

Laravel - No https://github.com/laravel/laravel/blob/master/config/app.php

Symfony - No https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Console/Application.php

That has to pretty much sum up the way the wind blows on this.

RiFi2k avatar Mar 20 '19 13:03 RiFi2k

Hi.

I am using prettier in my pre-commit-hook using the php plugin, and I am also using the end-of-file hook which ensures every file ends with a newline and that there is are no trailing empty lines in text files.

Usually, both tools get along very well, but for a simple php script, which does not have a trailing ?> marker and ends with a simple echo statement, prettier insists on adding a blank line after the statement BUT the end-of-file hook insists on removing the trailing empty line... :boom:

I just got out of this endless loop by adding a ?> at the end of the file. So, if you want to enforce removing the ?> marker, that's fine. But please do not add empty lines before the final \n.

Just my 2 cents. Thanks!

avdv avatar Mar 11 '21 13:03 avdv