Option for opening brace about plugin-php HOT 62 CLOSED

@mgrip twitter + issue

• 👍 Add
• 👎 Don't add

We have other many problem right now, new option should be added only after stable release

from plugin-php.

For me, that line break definitely is the ugliest aspect of PSR-2, so from a user perspective i'd support this 100%. From a maintainance perspective, I think it depends on the implementation complexity - if this boils down to a couple of ternarys printing hardlines or not (which it probably will), then I'd say let's go for it!
I guess we should leave the default to be PSR2-compatible, right?

from plugin-php.

You prefer K&R and some other like Allman better, like I do.

In my opinion Allman is beautiful, clearer and better, only because it brings symmetry to the table: a brace/bracket should end in the exact same position the opening one started. Yes, you get some extra lines with it, but you're not supposed to bloat your methods anyway, right? So, I don't think it's a problem.

So, if we're talking about going K&R where we now have Allman's, I think we should also do the opposite, for conditionals and loops, don't you think?

from plugin-php.

@evilebottnawi I suppose what many people would like to see is K&R style for classes and methods:

class Test {
  public function testMethod() {
  }
}

from plugin-php.

You're right they're inconsistent. I would recommend follow the current PSR-2 which is Allman for classes/method + K&R for conditions/loops.

from plugin-php.

Totally agree. I'm taking a look now and I think it should definitely be doable - I think it just boils down to classes, functions, methods, and traits - which all extend the statement type which should help. And yes definitely agree we should leave the default to be PSR2-compatible.

Ok cool I'll try and put together a PR and we can see how much complexity it adds

from plugin-php.

@evilebottnawi, beauty is a matter of taste, and sometimes, even knowing it's ugly, people feel beauty is the least important matter on some subjects. But, not having something really consistent to say about it, people said PHP was the uglier language around:

So my questions are nothing but honest, because I really think that if a community is building an opinionated software, how are the opinions forged?

As for examples, I can't provide many, but here's some code from Symfony formatted according to PSR2:

if (SortableIterator::SORT_BY_ACCESSED_TIME === $mode
    || SortableIterator::SORT_BY_CHANGED_TIME === $mode
    || SortableIterator::SORT_BY_MODIFIED_TIME === $mode) {
    if ('\\' === \DIRECTORY_SEPARATOR && SortableIterator::SORT_BY_MODIFIED_TIME !== $mode) {
        $this->markTestSkipped('Sorting by atime or ctime is not supported on Windows');
    }
    $this->assertOrderedIteratorForGroups($expected, $iterator);
} else {
    $this->assertOrderedIterator($expected, $iterator);
}

Let's make it Prettier using Allman's and one extra return (absorbed by me from the Laravel's coding style):

if (SortableIterator::SORT_BY_ACCESSED_TIME === $mode
    || SortableIterator::SORT_BY_CHANGED_TIME === $mode
    || SortableIterator::SORT_BY_MODIFIED_TIME === $mode) 
{
    if ('\\' === \DIRECTORY_SEPARATOR 
        && SortableIterator::SORT_BY_MODIFIED_TIME !== $mode) 
    {
        $this->markTestSkipped(
            'Sorting by atime or ctime is not supported on Windows'
        );
    }

    $this->assertOrderedIteratorForGroups($expected, $iterator);
} else {
    $this->assertOrderedIterator($expected, $iterator);
}

from plugin-php.

@bdlowery, welcome to the Allman's lovers group! 😆

from plugin-php.

Hi, can we maybe revisit our decision to couple this change to a post-1.0 release?

1.0 is dragging on a bit - which is fine - but this is the primary blocker for using prettier at my company.
Like the drupal folks, there are many here who "won't touch" prettier without this option. I'd really rather not have to fork in order to use.

from plugin-php.

I’d just call it „brace-style“, plain and simple 🙂

from plugin-php.

I don’t see why one would have different settings for classes and functions. I’d still propose what was mentioned above:

brace-style

• psr2 (default, as now)
• 1tbs (no line break for classes and functions)

from plugin-php.

I also like the opening brace on its own line, I find it makes the code more readable for functions and classes. It does make the code seem more verbose in conditionals, and creates unnecessary vertical space that feels inconsistent.

Hence I think the middle way would be the best. An option to allow opening braces on their own line, for functions and classes. For conditionals, the code looks more tight and less verbose with braces on the same line.

I think this would be perfect for both php and Javascript formatting.

from plugin-php.

Before:

After Prettier:

Is there still, even after 4 years, really no option to change this behavior?

from plugin-php.

I would agree with @roippi - seems like it still might be a while until we release a 1.0, and the PR to add this option would be pretty straightforward

from plugin-php.

@roippi no, only after stable release, also we need discussion about this

What more discussion are you after? I feel that this topic has been discussed to death both in this issue and #120

The only option people are asking for is #120 (comment), that is presently we have:

1. Opening brace for classes are on a new line
2. Opening brace for functions are on a new line
3. Opening brace for other blocks are on the same line
4. Braces are auto-inserted for one-line statements
   I (and others) are all asking for an option to change 1 and 2 to be on the same line, following the conventions that Phabricator (and thus I assume Facebook) and Wordpress follow.

Would you like more discussion about which options people want or discussion about this particular option (namely a single option to change 1 and 2 to be on the same line).

from plugin-php.

So probably we'd call the option brace-style with the options

• psr2 (default)
• 1tbs
  Correct?

from plugin-php.

@evilebottnawi Yep, that's what I'd strongly suggest. We can still discuss adding more options later, but that should be done in a separate issue.

from plugin-php.

WIP, will be release as 0.10.0 release

from plugin-php.

Review welcome #918

from plugin-php.

Being opinionated means you have very strong opinions. But... Where are those opinions coming from? How do you build them? And, above all, are them changeable or set in stone? If so, how do we change them?

I believe "Prettier", means it should be, first of all, Pretty. By not choosing Allman is good example of PSR2 choosing the uglier one. When the PHP community was trying to force the Laravel community (and they won), we had a beautiful coding style, Taylor Otwell was proud of using mostly Allman in Laravel core, and you should be able to see in this thread most of us in favor for keeping Allman instead of moving to PSR2, because Allman is symmetric, and pretty.

from plugin-php.

Hi ,
if you need if,else,elseif for brace style to Allman
you can modify
C:\Users\USERNAME.vscode\extensions\esbenp.prettier-vscode-5.9.2\node_modules@prettier\plugin-php\src\printer.js

Change Line: 1863

     if (node.alternate) {
        parts.push(node.shortForm ? "" : "} ");

to

      if (node.alternate) {
        parts.push(node.shortForm ? "" : "}", isFirstChildrenInlineNode(path) || !node.body ? "" : hardline );

input

if (1) {
} elseif (1) {
} else {
}

output

if (1) {
}
elseif (1) {
}
else {
}

from plugin-php.

Before:

After Prettier:

Is there still, even after 4 years, really no option to change this behavior?

They've added the option to change this behavior already. See #918

In your package.json file, add the prettier key and set the braceStyle to 1tbs:

{
  "devDependencies": {
    "@prettier/plugin-php": "^0.18.8",
    "prettier": "^2.6.2"
  },
  "prettier": {
    "braceStyle": "1tbs"
  }
}

from plugin-php.

cc @evilebottnawi @czosel

from plugin-php.

Added in #108

from plugin-php.

Reopening for more discussion - @evilebottnawi

from plugin-php.

@mgrip thanks, be good if we revert option and keep PR open

from plugin-php.

I'll put up an MR to remove it - it's just removing 1 loc of code from the printer. How do you propose we get more insight/opinions?

from plugin-php.

@evilebottnawi @czosel I still want to continue this discussion - Like I mentioned in the original issue posting I think this could scare people off from using prettier for php. Several large open-source php projects (Wordpress, Drupal, Facebook's Hack) do not put the opening brace on a new line (while plenty of others following PSR2 do)

from plugin-php.

@mgrip we should ping here WordPress, Drupal and other main constributors

from plugin-php.

Drupal dev checking in. Not supporting this means that no Drupal devs are going to touch it.

from plugin-php.

@mikecrittenden new options will be only after stable release

from plugin-php.

Allman is NOT PSR-2 nor matches any of the other defaults in prettier from the JS side. Everyone has an opinion that differs (good for you). At some point an option may be added, otherwise, don't use it. Prettier is designed to be opinionated -- and it's worked incredibly well in the JavaScript side ... I'm guessing the same thing will happen very shortly once stable is released in PHPland.

I agree an option should exist ... but defaults out of box should be close to PSR-2/JS-prettier as possible (similar to @mgrip 's initial assessment) So default would not be Allman.

from plugin-php.

PSR-2 is using Allman for classes and methods:

class Test
{
  public function testMethod()
  {
  }
}

And K&R for conditionals and loops:

if ($a === $b) {
    bar();
} elseif ($a > $b) {
    $foo->bar($arg1);
} else {
    BazClass::bar($arg2, $arg3);
}

They don't talk about Allman in the docs, but they are not consistent on their design too.

from plugin-php.

@roippi can you describe what you have what you see now and what you expected with examples

from plugin-php.

Hi @evilebottnawi, we have (and would like to keep) opening brace on same line for classes and methods, per @czosel example. Thanks.

from plugin-php.

There's no way to please everyone without an option here.

--braces <psr2|allman|kr>

from plugin-php.

Would a PR to add --braces be welcome at this point?

from plugin-php.

@roippi no, only after stable release, also we need discussion about this

from plugin-php.

@aboyton Some people ask here --braces <psr2|allman|kr>, this violates https://prettier.io/docs/en/option-philosophy.html .

What is the name of the option you offer? And what it should change (please provide examples)? This will allow us to think over the option(s) in more detail.

from plugin-php.

I linked to what I was after in the previous comment, but let me place it here on this issue. I'd love to have the following code style:

<?php
class test {
    public function another_test() {
        if (true) {
            print 'hi';
        }
    }
}

The name of this option seems to be most commonly referred to as 1TBS. Please read through the discussion on #120 rather than forcing a repeat of all of the discussion again on this issue. Many of the core contributors commented their positions.

from plugin-php.

@manspaniel option is not disabled, it is not implemented right now, we want release stable version first, anyway PR welcome

from plugin-php.

/cc @aboyton we can implement this in near future, but there a lot of difference requests, need clarify.

Places:

1. Classes (newline now)
2. Function (newline now)
3. Control structures (same line now)
4. Anonymous classes (same line now)

What we can do:

1. Implement --classes-bracket-same-line (default: false) affects only on classes/interfaces/traits.
2. Implement --function-bracket-same-line (default: false) affects only on functions/method.
3. Don't think we really need this
4. Don't think we really need this

from plugin-php.

I'm with @czosel. We don't need functions and classes to be different to each other, we'd want two options either psr2 or 1tbs.

from plugin-php.

@evilebottnawi ,

3. Don't think we really need this
4. Don't think we really need this

I think we do. At least for control structures. Newline (Alman's coding style) is the original Laravel coding style (before it moving to PSR-2), but which is still being used, so, a while ago, I wrote this PHP_CS plugin to support it: https://github.com/antonioribeiro/laravelcs.

Code in Allman's is symmetric, and, thus, Prettier:
https://github.com/laravel/framework/blob/v4.2.11/src/Illuminate/Database/Eloquent/Builder.php#L208
https://github.com/laravel/framework/blob/v4.2.11/src/Illuminate/Database/Eloquent/Builder.php#L184
https://github.com/laravel/framework/blob/v4.2.11/src/Illuminate/Database/Eloquent/Builder.php#L701

Please don't let it out of Prettier PHP.

from plugin-php.

@antonioribeiro can you provide example for 3 and 4?

@aboyton @czosel what about closure?

I am afraid what 1tbs is not enough for some developers

from plugin-php.

I'm actually quite convinced that the minimal set of options (1tbs / psr2) is the only way to stay compatible with prettier's philosophy: We should only introduce options when not doing so is a major roadblock for adoption. This is the case for 1tbs/psr2, because both styles are very popular in the community. I think the fact that laravel moved to PSR-2 actually confirms that Alman style on control structures should not be considered anymore.

from plugin-php.

@czosel So we implement 1tbs (space instead newline for functions/classes), right?

from plugin-php.

@evilebottnawi, aren't those links I sent examples for control structures and closures?

from plugin-php.

@antonioribeiro yep, just made sure I understood correctly

from plugin-php.

@czosel What better name for option? --opening-brace or --opening-brace-style? Or maybe you have better idea for name?

from plugin-php.

Depends on the options we will have, but the simpler the best: --brace-style, as said by @czosel, is great.

from plugin-php.

What about this:

class ClassName extends ParentClass implements
    VeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongFileName1
{
}

Should we use space here too? For me better use newline here for readable.

from plugin-php.

👋 Might want to just mimic JS to start (keep same-line)

from plugin-php.

You prefer K&R and some other like Allman better, like I do.

In my opinion Allman is beautiful, clearer and better, only because it brings symmetry to the table: a brace/bracket should end in the exact same position the opening one started. Yes, you get some extra lines with it, but you're not supposed to bloat your methods anyway, right? So, I don't think it's a problem.

So, if we're talking about going K&R where we now have Allman's, I think we should also do the opposite, for conditionals and loops, don't you think?

I definitely like Allan better it makes the code cleaner and easier to read and follow up.

from plugin-php.

I agree I think it should be an option for all languages. I use it for Javascript and I would love to have the freedom to choose what to use.

from plugin-php.

Please read https://prettier.io/docs/en/option-philosophy.html

from plugin-php.

@antonioribeiro we are always open for improvements, no need to quote that when someone said something, it’s better to show examples of why this is better and in what cases, we are considering the possibility of changing some things, so the more information there is, the better and more convenient we will design

from plugin-php.

@antonioribeiro lets open new issue i want to know how many people need this

from plugin-php.

+1 for brace-style options ( allman )

from plugin-php.

Ah yes I'd say I'd like a fix for that eveb more.

Forcing a closing and opening brace on the same line, misaligns the 'else' statements. It's just not as readable.

It's inconvenient as well when you refactor and you just want to move or delete one of those else/elseif statements. With eg. Vim you'd just delete a block of lines, and then pop it elsewhere. No hanging characters to worry about. If you move such a conditional higher/lower in the same containing block, it doesn't even need reformatting.

from plugin-php.

What option do I need to include in the prettierrc file to remove the Allman style code.

from plugin-php.

What option do I need to include in the prettierrc file to remove the Allman style code.

Looks like WordPress have a config file, I'm presuming that'll sort it.

https://developer.wordpress.org/block-editor/reference-guides/packages/packages-prettier-config/

from plugin-php.