Fixer for newlines after/before curly brackets
merged branch marekkalnik/curly-brackets (PR #9)

Commits
-------

4fc0538 Fixer for newlines after/before curly brackets

Discussion
----------

Curly brackets

Hi,

Thanks for making this tool public, it's great!

I've added a fixer for curly brackets:
* add newline after class and function declarations
* remove newline after control statements

I've also added a basic test for that.

---------------------------------------------------------------------------

by fabpot at 2012-05-18T07:01:24Z

That does not work as we don't have the same rule for anonymous functions. So, you exclude those from matching.

---------------------------------------------------------------------------

by fabpot at 2012-05-18T07:05:15Z

It would close #6

---------------------------------------------------------------------------

by marekkalnik at 2012-05-18T07:43:37Z

Thanks for the comments! I'll fix it in the evening.

---------------------------------------------------------------------------

by marekkalnik at 2012-05-18T20:17:06Z

I've fixed the errors and rebased against master. Quick question - should I recompile the phar? It seems to me that phars are likely to generate multiple conflicts so I think it should be rather the repository manager that does that, to limit the negative impacts.

---------------------------------------------------------------------------

by fabpot at 2012-05-19T06:44:41Z

Yes, I'm going to manage the phar re-compilation myself.

---------------------------------------------------------------------------

by fabpot at 2012-05-19T06:50:02Z

Test your code on Symfony, you will see that it does not work as expected. It also looks like this code slows the whole process a lot.

---------------------------------------------------------------------------

by umpirsky at 2012-05-19T08:52:03Z

@fabpot Does the speed matters?

---------------------------------------------------------------------------

by fabpot at 2012-05-19T09:01:16Z

@umpirsky not really, except if it becomes overly slow. I suspect that this change makes things very slow because it changes a lot of things that it should not. So, when it works as expected, it should be fast enough. We will see.

---------------------------------------------------------------------------

by stof at 2012-05-19T11:02:29Z

@fabpot a solution to ease the phar compilation could be to do it the way Composer uses: removing the phar from the repo and compiling it using a hook. It requires having a server hosting the phar though.

---------------------------------------------------------------------------

by marekkalnik at 2012-05-19T11:59:31Z

I've ran it on a bundle of mine and I worked fine. But it was before adding the treatment of multiple spaces. I'll run it on Symfony and come back with a fix.

---------------------------------------------------------------------------

by fabpot at 2012-05-19T12:31:23Z

@stof: That's indeed a good idea. Anywhere I can see the code for Composer?

---------------------------------------------------------------------------

by marekkalnik at 2012-05-19T12:32:47Z

Fixed. I'll see what can I do for the performance, but I don't think there's much that can be done here.

I've also run in a special case - it's a test fixture (from old 2.0 so I don't know if it still exists) :
class InvalidConstraintValidator {}

I guess this should not be modified?

---------------------------------------------------------------------------

by stof at 2012-05-19T12:33:38Z

@Seldaek ping

---------------------------------------------------------------------------

by fabpot at 2012-05-19T12:45:16Z

I've just tried on Symfony master and here is change it tries to do:

    -    public function __construct($id, $name) {
    +    public function __construct($id, $name)
    +
    +
    +    {

---------------------------------------------------------------------------

by stof at 2012-05-19T14:04:11Z

@marekkalnik you should add unit tests applying the fixer on correct curly braces, to ensure it does not messes them.

---------------------------------------------------------------------------

by marekkalnik at 2012-05-19T14:15:54Z

@stof Ok, I'll add some tests on correct code (one already added). I've ran it on symfony master, seems ok. CodeSniffer reports no brackets errors but one with do {} while (). I guess I'll need a separate function for that statement.

---------------------------------------------------------------------------

by marekkalnik at 2012-05-20T13:58:10Z

I've added some tests, tested it on symfony master (seems fine) and refactored the code a bit.

---------------------------------------------------------------------------

by fabpot at 2012-05-20T14:29:31Z

I've tested against Symfony master and indeed the results look fine for me.

---------------------------------------------------------------------------

by fabpot at 2012-05-20T14:31:05Z

Before I merge, can you remove the change on the `phar` and squash your commits? Thanks.

---------------------------------------------------------------------------

by fabpot at 2012-05-20T15:27:16Z

FYI, I've just renamed the repository to `PHP-CS-Fixer`.

---------------------------------------------------------------------------

by marekkalnik at 2012-05-20T15:35:08Z

Stashed.
added missing methods for the previous merged fixer class
updated phar