-
Notifications
You must be signed in to change notification settings - Fork 26
7.1 Short Closures need better indentation #61
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
Comments
The declaration is consistent with other multiline declarations as-is. For example: # Anonymous classes
// Brace on the next line
// Constructor arguments
$instance = new class($a) extends \Foo implements
\ArrayAccess,
\Countable,
\Serializable
{
public function __construct(public int $a)
{
}
// Class content
};
# Anonymous function declaration
$noArgs_longVars = function () use (
$longVar1,
$longerVar2,
$muchLongerVar3,
) {
// body
};
# Multiline function call (Note the closing `)` has the same scope indentation as the scope it's declared in
$foo->bar(
$arg1,
function ($arg2) use ($var1) {
// body
},
$arg3,
); In general if you need a multiline short closure you should probably be doing something else anyway. |
@Nex-Otaku Your suggested change also doesn't start and finish on the same indentation level. It just changes the indentation of the closure parameters, making them inconsistent with all other function parameters. |
@theodorejb you are right, my suggested change also does not start and finish on the same indentation level. However my goal was to state the problem as much clear as I can. That's what issues are for, right? To bring some important enough topics for a discussion. It's not a Pull Request. Not to provide the best possible solution - because I dont have one. Do you? If anyone will made better suggestion I would be happy to give my voice for it. I don't care much who will fix that and how. Maybe we can just remove short closure multiline example as a whole if there will be no good enough solution. It's not me to decide. Just think about it. Do you like it as it is now? |
I think the reality here is that breaking up one of these short functions across multiple lines is basically always a bad idea, the best we can do is try to give consistent guidelines for when it might happen and hope that folks just avoid splitting short functions across multiple lines all together. For my own code I'll be using a full anonymous function rather than a short function any time I need more than one line. Thanks for the thorough write-up in this issue, I really appreciated the images as well! |
Look at the indentation in short closure multiline example.
The indentation bounces wildly, it feels not right.
It's because blocks go in form that is not consistent with all other constructs.
Better:
The text was updated successfully, but these errors were encountered: