PHP formatting support#3521
PHP formatting support#3521mgrip wants to merge 38 commits intoprettier:masterfrom mgrip:php-prettier
Conversation
|
Adding another one dependency will make prettier even larger which is not good. Look at python implementation #3349 without deps. |
package-lock.json
Outdated
| @@ -0,0 +1,7827 @@ | |||
| { | |||
There was a problem hiding this comment.
Remove this file.. since we are using yarn.lock
There was a problem hiding this comment.
whoops sorry about that - all set!
|
Hi! Thanks for working on this. I think we should finish HTML printing before tackling PHP, because PHP has to be embedded in a HTML document. |
|
Unless we just support PHP files without any HTML? |
|
This is really exciting! I just want to make sure that you are aware that writing a pretty printer for a full language (and one as crazy as PHP) is going to be a big undertaking. If you are onboard, here are the three overall steps:
Step 2 is absolutely critical. If your formatter outputs invalid code, people are not going to trust it, no matter how pretty the code is. |
|
@mgrip For step 2, you can let me know when you think you have something that should work – I can run it on a big-ish internal Wordpress theme / app. |
|
thanks for the info all! so i cant make any promises in terms of timeline, but definitely interested in pushing this forward. I work with a large number of engineers using php daily, so im sure I could wrangle some help there as well (and we have a very large php codebase to run it against for testing, in addition to some open source ones like wordpress or drupal). mostly wanted to post this initially to make sure we’re headed in the right direction. in terms of next steps, that makes sense - thanks @vjeux. what are your guys thoughts on the parser we’re using? that could be expensive to pivot on, so just want to make sure thats set in stone before continuing on with this. |
src/printer-php.js
Outdated
| return handleNode(n); | ||
| } | ||
|
|
||
| function handleLiteral(node) { |
There was a problem hiding this comment.
Should these be renamed print* to be similar to other printers’ methods?
There was a problem hiding this comment.
makes sense - updated!
|
👋 @mgrip, FYI: #3349 (comment). |
|
makes sense, sounds much more scalable. so the intent would be to move this PR to a new prettier/prettier-php repo with a dependency on prettier? |
src/printer-php.js
Outdated
| return true; | ||
| } | ||
| } | ||
| return false; |
There was a problem hiding this comment.
You can just do array.indexOf(val) !== -1 here.
|
anyone know why the ci job fails w/ |
Class: Add support for abstract, final, extends, implements
|
Sorry for such a question, but why have you decided to support PHP when there is already a known leader of PHP Code Style fixes: PHP-CS-Fixer ? |
|
Looks like that's a fixer (rule based edits), not a full formatter. |
|
I have copied all the commits from this pull request to a new repository using the in-progress plugin API. https://github.com/prettier/prettier-php @mgrip I have added you as an admin of the repository. |
lock
bot
added
the
locked-due-to-inactivity
9 participants
Inspiration taken from #3349 - this time for php! There's still a lot of work to be done here, but wanted to get eyes on it to make sure we're headed in the right direction. We're using https://github.com/glayzzle/php-parser for a parser - luckily its written in JS so no need for external dependencies.