Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Closes #6306 - Console\RouteNotFoundStrategy invalid index #6307

Merged
merged 6 commits into from
Aug 7, 2014

Conversation

aeneasr
Copy link
Contributor

@aeneasr aeneasr commented May 21, 2014

Solves an issue where the consoles RouteNotFoundStrategy would throw an exception because an arrays index 0 isn't defined.

Solves an issue where the consoles RouteNotFoundStrategy would throw an exception because an arrays index 0 isn't defined.
@@ -378,7 +378,7 @@ protected function renderTable($data, $cols, $consoleWidth)
// If there is only 1 column, just concatenate it
if ($cols == 1) {
foreach ($data as $row) {
$result .= $row[0] . "\n";
$result .= isset($row[0]) ? $row[0] . "\n" : '';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no need for the else, an if statement already does the trick:

if (isset($row[0])) {
    $result .= $row[0] . "\n";
}

@Ocramius
Copy link
Member

Still needs a test case.

@aeneasr
Copy link
Contributor Author

aeneasr commented Jun 13, 2014

Sorry for the delay, there isn't a test class for RouteNotFoundStrategy yet, should I create a complete new testclass?

@aeneasr
Copy link
Contributor Author

aeneasr commented Jun 15, 2014

Tests have been added, not sure why php 5.3 fails?

$reflection = new ReflectionClass('Zend\Mvc\View\Console\RouteNotFoundStrategy');
$method = $reflection->getMethod('renderTable');
$method->setAccessible(true);
$method->invokeArgs($this->strategy, [[[]], 1, 0]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

short array syntax is available since PHP 5.4

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah that's why tests fail, thanks, I'll fix that!

@aeneasr
Copy link
Contributor Author

aeneasr commented Jun 24, 2014

Please merge this, it is so annoying :)

@@ -0,0 +1,37 @@
<?php
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use the ZF2 licence header:

/**
* Zend Framework (http://framework.zend.com/)
*
* @link http://github.com/zendframework/zf2 for the canonical source repository
* @copyright Copyright (c) 2005-2014 Zend Technologies USA Inc. (http://www.zend.com)
* @license http://framework.zend.com/license/new-bsd New BSD License
*/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh my bad, sorry!


public function setUp()
{
$this->strategy = new RouteNotFoundStrategy();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cs (spaces)

@Ocramius
Copy link
Member

Assigning to @DASPRiD

@Ocramius Ocramius added this to the 2.3.2 milestone Jul 28, 2014
weierophinney added a commit that referenced this pull request Aug 7, 2014
Closes #6306 - Console\RouteNotFoundStrategy invalid index
weierophinney added a commit that referenced this pull request Aug 7, 2014
- Added an assertion to the test, and verified that it failed before the
  change.
- Modified the foreach loop to continue if the test fails (do work
  outside of conditions/return early).
weierophinney added a commit that referenced this pull request Aug 7, 2014
@weierophinney weierophinney merged commit 01d3e35 into zendframework:master Aug 7, 2014
weierophinney added a commit that referenced this pull request Aug 7, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants