Fix Placeholder view helper return value#162
Fix Placeholder view helper return value#162thexpand wants to merge 9 commits intozendframework:masterfrom
Conversation
…m the Placeholder::getContainer() method
…endencies on PHP 7.2
src/Helper/Placeholder.php Outdated
| * @param string $name | ||
| * @throws InvalidArgumentException | ||
| * @return Placeholder\Container\AbstractContainer | ||
| * @return Placeholder\Container\AbstractContainer|Placeholder |
| public function __invoke($name = null) | ||
| { | ||
| if ($name === null) { | ||
| throw new InvalidArgumentException( |
There was a problem hiding this comment.
This particular change is most likely a BC break. Although I don't think people actually do rely on try-catching it, it is possible that they rely on the page to be crashing
There was a problem hiding this comment.
I totally agree with you. Should we move this to the 3.0.0 milestone?
There was a problem hiding this comment.
@thexpand
Next major version makes sense, because nobody uses a try-catch-block here.
| What could we do about the failing Travis build? Should I revert the changes, made to the EDIT: |
| This repository has been closed and moved to laminas/laminas-view; a new issue has been opened at laminas/laminas-view#7. |
| This repository has been moved to laminas/laminas-view. If you feel that this patch is still relevant, please re-open against that repository, and reference this issue. To re-open, we suggest the following workflow:
|
4 participants
This PR aims to fix the bug, outlined in the following issue: #16
The changes will allow
$thisas a return value of the view helper, when no name is passed as an argument, thus providing access to the methods inside the view helper. For this reason, more tests have been added toZendTest\View\Helper\PlaceholderTest.