Skip to content

Add configuration for PHP formatting options #29

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

Closed
felixfbecker opened this issue Sep 15, 2016 · 14 comments
Closed

Add configuration for PHP formatting options #29

felixfbecker opened this issue Sep 15, 2016 · 14 comments

Comments

@felixfbecker
Copy link
Owner

From @websquared on September 15, 2016 18:56

Great extension, but is it possible to configure formatting options to follow PSR-2 standard - like do not remove empty lines, align assignments (by = sign), do not convert "else if" to "else } if", etc.?
Thanks.

Copied from original issue: felixfbecker/vscode-php-intellisense#10

@felixfbecker
Copy link
Owner Author

This early implementation of the formatting feature is using he standard PrettyPrinter of PHPParser: https://github.com/nikic/PHP-Parser/blob/master/lib/PhpParser/PrettyPrinter/Standard.php.

I am also not satisfied with the output, for example how it handles use and declare statements and the empty lines. I'm sure the class can be customized more. We could

  • do a PR to PHPParser to allow customizing of the PrettyPrinter
  • subclass it to customize it

contributions welcome!

@kaloyan-raev
Copy link
Contributor

I heard from @mniewrzal that the PrettyPrinter can't be used for range formatting, but only for formatting the whole document.

I wonder if we can use either the CS Fixer or PHPCBF. AFAIK they already support a variety of coding standards.

I am not sure if they are better for range formatting, but perhaps it's worth taking a look.

@mniewrzal
Copy link
Contributor

Range formatting is probably less problematic than lack of configuration. PrettyPrinter should be able to format just selected range if user will select complete AST node. To make it configurable we need to extend it and add own logic.

I will take a look at CS Fixer and PHPCBF.

@mniewrzal
Copy link
Contributor

mniewrzal commented Sep 19, 2016

I played with CS Fixer today and I was able to use it with language server. If someone want to take a look here is branch with changes. Its just a fast hack ;-)

Pros:

  • configurable
  • workspace can contain configuration file .php_cs, no need for additional configuration through language server protocol
  • quite easy to integrate

Cons:

  • can be very slow, with enabled xdebug formatting 1600 LOC can take more than 10s, The same operation without xdebug takes under 1s.

@felixfbecker
Copy link
Owner Author

@mniewrzal always feel free to open a PR, it is a nice way to start a discussion :)

can be very slow, with enabled xdebug formatting 1600 LOC can take more than 10s, The same operation without xdebug takes under 1s.

I would like to implement some solution in the client that spawns the language server without XDebug, XDebug is performance killer. Some ideas here: http://stackoverflow.com/questions/38506140/how-to-run-composer-without-xdebug-when-installed-with-homebrew
On startup, read the php.ini, remove lines containing "xdebug", write to temp folder, start php with the ini file.

@mniewrzal
Copy link
Contributor

@mniewrzal always feel free to open a PR, it is a nice way to start a discussion :)

If there is an option to overcome xdebug performance issue making such PR make sense :) I will prepare something soon.

@felixfbecker
Copy link
Owner Author

I meant that I prefer to discuss changes on an open PR that is not yet finished, because you can better comment on the changes, even when it won't get merged in the current state of the branch :)

@mniewrzal
Copy link
Contributor

Yes, I understand. I just wanted to say that without hope for better performance I didn't see a chance for using cs-fixer with language server. Actual implementation is too messy to make PR from it ;)

@kaloyan-raev
Copy link
Contributor

@mniewrzal can you check what is the performance impact if having Zend Debugger instead of Xdebug?

@mniewrzal
Copy link
Contributor

PHP CS Fixer times for https://github.com/composer/composer/blob/master/src/Composer/Installer.php:

  • no debugger: ~0.9s
  • zenddebugger: ~1.3s
  • xdebug: ~4.4s

@mniewrzal
Copy link
Contributor

I abandon my previous PR for CS-Fixer. I created PR #35 with PHPCBF integration. PHPCBF formatting looks slightly better and performance is good even with XDebug enabled. Like CS-Fixer it has some gaps to fill (e.g. whitespaces in namespace declaration) but I think it will be easier to add missing rules than writing formatter from scratch (e.g. based on PrettyPrinter).

@felixfbecker
Copy link
Owner Author

Solved with PHP_CodeSniffer

@myMartek
Copy link

myMartek commented Mar 3, 2017

And how to configure the formatting now?

Can you give an example?

@felixfbecker
Copy link
Owner Author

Please refer to the PHP CodeSniffer documentation on how to configure it with an XML file - you can also find one in this repository

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants