New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for braces / position_before_control_structures #4178

Open
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@jclaveau
Copy link

jclaveau commented Dec 30, 2018

This option allows to force a line break before "else", "elseif", "finally" and "catch" to preserve verticality between all the control structures of conditions and exceptions handling.

I found this kind of prettyprint in a project and realized it was really helpful when you need to duplicate or comment a whole case (several lines including braces) without making the code broken:

if (/* first case */) {
    // whatever 
}
// elseif (/* case you want to patch (commented so easy to compare) */) {
//     // what was done before your patch
// }
elseif (/* copy of the case that you're patching */) {
     // your patch experiments
}
else (/* ... */) {
    /// ...
}

This participated to make my workflow really smoother and promotes a rule trait which is rare in mainstream standards even if it's a plus for readability, I think: verticality.
It also makes it shorter to convert the "if {}" and "else {}" statements to "elseif {}" in both ways.

Hoping you'll consider this behavior and thank you very much for your work!

Jean Claveau and others added some commits Dec 30, 2018

@keradus keradus changed the base branch from 2.13 to master Dec 30, 2018

@jclaveau

This comment has been minimized.

Copy link

jclaveau commented Dec 30, 2018

There is an option corresponding to it in the configuration of Scrutinizer:
Configuration → Coding-Style → PHP / Braces / Braces in ifs / Place else on a new line

@keradus

This comment has been minimized.

Copy link
Member

keradus commented Jan 10, 2019

hi @jclaveau

is this anyhow established standard, or just your preference ?
like, it is strongly against PSR, is there other standard that ask you to follow that syntax ?

@jclaveau

This comment has been minimized.

Copy link

jclaveau commented Jan 10, 2019

hi @keradus ,

It wasn't my preference until I had to adopt in a project and realized it was shortening keymap+selection operations like copy/paste of blocks, commenting and others.

I don't know if being against one detail of one rule of PSR-2 (absent from the overview) is being strongly against it (I personally apply almost all the others) but this feature fits perfectly the current listed options of Php-CS-Fixer (which is easy to realize when you see how slight are the required changes of the code base) so it looked "natural and following the philosophy of the code" to me, pushing me to propose this simple implementation of it.

Here is the overview of PSR-2 (they require it to be on the same line in the details and this requirement is not explicit for Pear Coding Style):

  • "Opening braces for control structures MUST go on the same line, and closing braces MUST go on the next line after the body."

Pear is also not super clear about it and several comments promote linebreak after every "}" in control structure https://pear.php.net/manual/en/standards.control.php

One question by the way: is PHP-CS-Fixer a tool having as goal to force people to use PSR-2 or a linter that fixes unfollowed rules? It's not super clear in the README : "You can also define your (teams) style through configuration."
As this rule impacts only one rule of PSR-2 it seems to be a team tuning to me. Maybe I'm wrong?

I personally give a lot of importance to verticality in my code (some options of PHP-CS-Fixer are super grate for that like binary_operator_spaces) even if it's not a part of PSR. This is just an option to give more consistency to this "abstract rule" behind my coding style, not a mainstream PHP standard as long as I know. Only this one https://en.wikipedia.org/wiki/Indentation_style#Variant:_Stroustrup

Last argument, digging a little, this question comes regularly in discussions about braces with interesting arguments:
https://softwareengineering.stackexchange.com/a/65633
And someone else opened a bug some days after my PR also:
#4231

Hoping this change will seem enough slight to you to be considered interesting :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment