Indentation issues

[harikt]

Recently I was trying to fix via phpcbf and noticed the issue

        $client = new Client(
            array(
            'base_url' => \SproutVideo::$base_url,
            'defaults' => array(
            'headers' => array(
            'SproutVideo-Api-Key' => \SproutVideo::$api_key,
            'Content-Type', 'application/json;charset=utf-8'
            ),
            )
            )
        );

it could have been like

        $client = new Client(
            array(
                'base_url' => \SproutVideo::$base_url,
                'defaults' => array(
                    'headers' => array(
                        'SproutVideo-Api-Key' => \SproutVideo::$api_key,
                        'Content-Type', 'application/json;charset=utf-8'
                    ),
                )
            )
        );

I did noticed something else, but not able to get it :( . Will update when I find the same.

[gsherwood]

If you don't have an array sniff in your standard, arrays wont be formatted to look good.

For example, the PSR2 standard doesn't say how arrays should be formatted, so PHP_CodeSniffer wont report errors or try and fix the formatting. If I did, I'd get a bunch of issues from people complaining that PHPCS isn't conforming to PSR2.

One thing that the indent sniff should be doing though is to maintain indentation that is already there, if it has no rules on how to fix it. So I'd be interested to see what the code originally looked like before fixing so I can make sure any existing array formatting is preserved. Are you able to provide that? Or is that what the first code block actually is?

I think a really simple sniff to format arrays using one indent level would be a good idea, but you would have to explicitly add it to a custom standard if you want to use it. Right now, the only sniff that formats arrays in a consistent way is Squiz.Arrays.ArrayDeclaration, but it probably does a lot more formatting than you want and looks to have an issue with that first code block anyway.

[harikt]

So I'd be interested to see what the code originally looked like before fixing so I can make sure any existing array formatting is preserved.

My existing array was like

    $client = new Client(
            array(
                'base_url' => \SproutVideo::$base_url,
                'defaults' => array(
                    'headers' => array(
                        'SproutVideo-Api-Key' => \SproutVideo::$api_key,
                        'Content-Type', 'application/json;charset=utf-8'
                    ),
                )
            )
        );

but with tabs. Which changed to

        $client = new Client(
            array(
            'base_url' => \SproutVideo::$base_url,
            'defaults' => array(
            'headers' => array(
            'SproutVideo-Api-Key' => \SproutVideo::$api_key,
            'Content-Type', 'application/json;charset=utf-8'
                    ),
            )
            )
        );

[Konafets]

Do you use a custom standard or one which is shipped by PHPCS?

[harikt]

I used php phpcbf.phar sproutvideo-php/lib/SproutVideo/Resource.php .

Hope that clarifies. You can see the SproutVideo/sproutvideo-php#5 and commit SproutVideo/sproutvideo-php@06211d9#diff-a0c3af6bff051809aea4bb6d7b4dcfccR77 . See arrays in the commit .

[aik099]

Having a sniff, that just checks the indentation of multi-line array declarations would be great.

I'm not sure though how such code will be handled considering, that tabs are used for indentation:

$a = array(
    'test' => 1,
);

If we indent arrays based on array( word, then code would look this way, but it's wrong:

$a = array(
         'test' => 1,
     );

Does Squiz.Arrays.ArrayDeclaration handle that?

[gsherwood]

Does Squiz.Arrays.ArrayDeclaration handle that?

It uses spaces to line things up perfectly. But indenting to the next tab stop after the array keyword is pretty easy. PHPCS already determines the tab stops (i.e., how many spaces each tab is) and so fixing might be done in 2 steps:

1. The array formatting sniff would use spaces to align the keys to the proper column based on tab stops.
2. The sniff that bans spaces for indentation would come in and replace all spaces with tabs.

Or it could be the other way; array sniff uses tabs and the sniff than bans tab indents replaces them with spaces. But that's not how the other sniffs work (they assume spaces then do tab replacement later) so it would probably be done using spaces.

[aik099]

To get this formatting:

$a = array(
    'test' => 1,
);

custom sniff can:

1. find array(
2. get 1st token on the line (except for whitespaces) and use it as indentation start
3. for each following line until array declaration end (I guess parenthesis_closer would allow me to detect ) for correct array in case of nested arrays) add 1 indentation level according to tab-width setting

Won't described above approach create issues when nested arrays are found?

[harikt]

on another note :

Does the below code

public function connect( )

need to be changed as

public function connect()

Please see the space between ( and ) .

[aik099]

@harikt , what you describe isn't related to indentation. I bet there are special sniffs to validate function declaration that would do as you say.

[gsherwood]

Sorry, just getting back to this. I tested this code:

<?php
$client = new Client(
    array(
        'base_url' => \SproutVideo::$base_url,
        'defaults' => array(
            'headers' => array(
                'SproutVideo-Api-Key' => \SproutVideo::$api_key,
                'Content-Type', 'application/json;charset=utf-8'
            ),
        )
    )
);

And ran PHPCBF over it with the PEAR and PSR2 standards, and got the same result, which was that the tabs were replaced by 4 spaces and no other formatting was changed.

So I suspect something else is causing the indentation to be removed, but I have no idea what it is. The best way to find out is to get the smallest bit of code you can to replicate the problem, then run phpcs /path/to/file.php --standard=STANDARD --report=diff -vv which will output a heap of debug lines showing exactly what is going on.

[harikt]

@gsherwood does your code have tabs and spaces ? I will run again when I get some time. Thanks.

[gsherwood]

@gsherwood does your code have tabs and spaces ?

I only used tabs for indentation in that example. Do you have mixed indentation?

[harikt]

iirc yes.

Hari K T

You can ring me : +91 9388 75 8821

http://harikt.com , https://github.com/harikt ,
http://www.linkedin.com/in/harikt , http://www.xing.com/profile/Hari_KT

Skype : kthari85
Twitter : harikt

On Wed, Apr 22, 2015 at 9:05 AM, Greg Sherwood notifications@github.com
wrote:

@gsherwood https://github.com/gsherwood does your code have tabs and
spaces ?

I only used tabs for indentation in that example. Do you have mixed
indentation?

—
Reply to this email directly or view it on GitHub
#544 (comment)
.

[glen-84]

@harikt,

Probably not related, but shouldn't:

'Content-Type', 'application/json;charset=utf-8'

Be:

'Content-Type' => 'application/json;charset=utf-8'

?

[harikt]

@glen-84 good catch. May be.

But I don't know for I could not test it now.

Thanks

[gsherwood]

The best way to find out is to get the smallest bit of code you can to replicate the problem, then run phpcs /path/to/file.php --standard=STANDARD --report=diff -vv which will output a heap of debug lines showing exactly what is going on.

I still can't replicate any problems with mixed spaces and tabs for indentation, so the above information is still something I need to try and replicate this issue. Or, a gist with the exact code causing the problem would also be fine. I may not be putting the tabs and spaces in the right locations to trigger a problem.

[harikt]

ok thank you @gsherwood .

I did tried once again, and I could not reproduce this again, we can close for now and open with more information when I notice next time. I will make sure next time I have the exact file attached.

Thank you everyone for your valuable time.

[harikt]

Hey @gsherwood I have a gist here .

https://gist.github.com/harikt/7540f6a6e073e5a06a97

Anyone can try this and see if you guys can reproduce or not .

Thank you.

[gsherwood]

Thanks for the gist. That code is working for me as well (tabs are replaced by spaces but all the code remains indented to the same level) so I'll leave this closed.

[harikt]

Hm. strange then why the indentation is different to me :/ . https://gist.github.com/harikt/7540f6a6e073e5a06a97#comment-1486628