Prettify the output of Zend\Code\Generator\ValueGenerator for multi line arrays #5616

Merged
merged 3 commits into from Mar 4, 2014

Conversation

Projects
None yet
5 participants
Contributor

RalfEggert commented Dec 15, 2013

This fix prettifies the output for multi line arrays generated with Zend\Code\Generator\ValueGenerator

Without this fix, an array looks like this.

array(
    'controllers' => array('invokables' => array(
            'Event\Controller\IndexController' => 'Event\Controller\IndexController',
            'Event\Controller\ShopController' => 'Event\Controller\ShopController',
            )),
    'view_manager' => array('template_path_stack' => array(__DIR__ . '/../view'))
    )

With the fix, the same array will look like this:

array(
    'controllers' => array(
        'invokables' => array(
            'Event\Controller\IndexController' => 'Event\Controller\IndexController',
            'Event\Controller\ShopController' => 'Event\Controller\ShopController',
        )
    ),
    'view_manager' => array(
        'template_path_stack' => array(
            __DIR__ . '/../view'
        )
    )
)

Please note the changes for the entry for 'view_manager' and the positions of the braces.

Contributor

blanchonvincent commented Dec 15, 2013

@RalfEggert Update the unit tests please, there are some fails https://travis-ci.org/zendframework/zf2/jobs/15495156#L281

Contributor

RalfEggert commented Dec 16, 2013

Sorry, forgot about that. The unit tests are fixed now...

Contributor

RalfEggert commented Dec 16, 2013

To be honest, I don't really understand what failed there... :-(

Contributor

RalfEggert commented Dec 16, 2013

@netiul Thanks, I just checked the ValueGeneratorTest and missed that one. Now it should work...

Contributor

blanchonvincent commented Dec 16, 2013

@Maks3w Maks3w commented on the diff Jan 2, 2014

tests/ZendTest/Code/Generator/ValueGeneratorTest.php
@@ -40,9 +40,15 @@ public function testPropertyDefaultValueCanHandleStrings()
public function testPropertyDefaultValueCanHandleArray()
{
+ $expectedSource = <<<EOS
+array(
+ 'foo'
+ )
@Maks3w

Maks3w Jan 2, 2014

Member

In this case the close parenthesis should not be indented

@Maks3w Maks3w commented on the diff Jan 2, 2014

tests/ZendTest/Code/Generator/ValueGeneratorTest.php
@@ -127,7 +133,7 @@ public function testPropertyDefaultValueCanHandleArrayWithUnsortedKeys()
'c',
7 => 'd',
3 => 'e'
- )
+ )
@Maks3w

Maks3w Jan 2, 2014

Member

same here

@Maks3w Maks3w commented on the diff Jan 2, 2014

tests/ZendTest/Code/Generator/ValueGeneratorTest.php
PHP_EOL
- )
+ )
@Maks3w

Maks3w Jan 2, 2014

Member

same here

@Maks3w

Maks3w Jan 2, 2014

Member

I don't know what you interpreted. Any of that examples shows the close parenthesis indented. Instead they are aligned to the first non white character in the line.

PD: Some parts of that article are outdated and don't follow PSR-2

@RalfEggert

RalfEggert Jan 2, 2014

Contributor

I still don't get it. So your are saying, that this code block is not following the ZF2 standards?

array(
    'controllers' => array(
        'invokables' => array(
            'Event\Controller\IndexController' => 'Event\Controller\IndexController',
            'Event\Controller\ShopController' => 'Event\Controller\ShopController',
        )
    ),
    'view_manager' => array(
        'template_path_stack' => array(
            __DIR__ . '/../view'
        )
    )
)

If not, how should it look like?

@Maks3w

Maks3w Jan 3, 2014

Member

@RalfEggert Could you take a look from <<<EOS up to EOS; and tell me if the first level of the array is really how you expect?

@RalfEggert

RalfEggert Jan 3, 2014

Contributor

@Maks3w Yes, the result of the ValueGeneratorTest::testPropertyDefaultValueCanHandleArrayWithUnsortedKeys() runs as expected, since the ArrayDepth is set to 1 as default.

array(
        1 => 'a',
        0 => 'b',
        'c',
        7 => 'd',
        3 => 'e'
    )

If it would be set to 0, I would expect:

array(
    1 => 'a',
    0 => 'b',
    'c',
    7 => 'd',
    3 => 'e'
)

weierophinney added this to the 2.3.0 milestone Mar 3, 2014

@weierophinney weierophinney added a commit that referenced this pull request Mar 4, 2014

@weierophinney weierophinney Merge branch 'feature/5616' into develop
Close #5616
bcf9981

weierophinney merged commit c28f71a into zendframework:develop Mar 4, 2014

1 check passed

default The Travis CI build passed
Details

weierophinney self-assigned this Mar 4, 2014

@weierophinney weierophinney added a commit that referenced this pull request Mar 5, 2014

@weierophinney weierophinney [#5616] Revert code from #5608
Based on new tests and links to relevant specifications, the code from #5608
was invalid; header lines may never start with whitespace.

This patch reverts that code, and updates the `fromString()` logic to
capture whitespace on subsequent lines to include in the multi-line
header value. Tests have been updated where necessary to use a regex for
testing the existence of whitespace between multiline values.
16ad31c

@weierophinney weierophinney added a commit that referenced this pull request Mar 5, 2014

@weierophinney weierophinney Merge branch 'hotfix/5616'
Close #5616
16b6c1e

@weierophinney weierophinney added a commit that referenced this pull request Mar 5, 2014

@weierophinney weierophinney Merge branch 'hotfix/5616' into develop
Forward port #5616

Conflicts:
	tests/ZendTest/Http/Response/ResponseStreamTest.php
	tests/ZendTest/Http/ResponseTest.php
d02d04d

@weierophinney weierophinney added a commit to zendframework/zend-http that referenced this pull request May 15, 2015

@weierophinney weierophinney [zendframework/zendframework#5616] Revert code from zendframework/zen…
…dframework#5608

Based on new tests and links to relevant specifications, the code from zendframework/zendframework#5608
was invalid; header lines may never start with whitespace.

This patch reverts that code, and updates the `fromString()` logic to
capture whitespace on subsequent lines to include in the multi-line
header value. Tests have been updated where necessary to use a regex for
testing the existence of whitespace between multiline values.
9775572

@weierophinney weierophinney added a commit to zendframework/zend-http that referenced this pull request May 15, 2015

@weierophinney weierophinney Merge branch 'hotfix/5616' 2ce8f5e

@weierophinney weierophinney added a commit to zendframework/zend-http that referenced this pull request May 15, 2015

@weierophinney weierophinney Merge branch 'hotfix/5616' into develop
Forward port zendframework/zendframework#5616

Conflicts:
	tests/ZendTest/Http/Response/ResponseStreamTest.php
	tests/ZendTest/Http/ResponseTest.php
8311d55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment