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

Zend filter is a hard dependency #24

Closed
wants to merge 1 commit into from
Closed

Zend filter is a hard dependency #24

wants to merge 1 commit into from

Conversation

harikt
Copy link

@harikt harikt commented Dec 22, 2015

When calling PhpRenderer it cannot call render method for the

return $this->getFilterChain()->filter($this->__content); // filter output
have dependency on the Filter . The constructor doesn’t specifically mentions the Filter class is needed and throws error .
public function __construct($config = [])
.

When calling `PhpRenderer` it cannot call `render` method for the https://github.com/zendframework/zend-view/blob/develop/src/Renderer/PhpRenderer.php#L519 have dependency on the `Filter` . The constructor doesn’t specifically mentions the Filter class is needed and throws error . https://github.com/zendframework/zend-view/blob/develop/src/Renderer/PhpRenderer.php#L145 .
@danizord
Copy link
Contributor

Zend\Filter is not required when not using PhpRenderer. See #12 and #3 (comment)

@weierophinney
Copy link
Member

Closing for the rationale @danizord has stated.

@harikt harikt deleted the patch-1 branch December 22, 2015 16:11
@harikt
Copy link
Author

harikt commented Dec 22, 2015

To be frank I disagree with the statements expressed in the issues. It just brings frustration when people trying to integrate things. Sorry to say it.

@weierophinney
Copy link
Member

@harikt I get your frustration; there's a reason the issue has been raised a number of times.

We're in an awkward position: if we don't have the dependency, we have some folks upset that it "does not work out of the box." If we add the dependency, we have some folks upset that they're forced to install a dependency they aren't using. We've chosen to err on the side of not providing as many dependencies, as we can document, "to use this, you must also require this in your project;" we cannot document removing a dependency, as it's not possible.

@danizord
Copy link
Contributor

@harikt I think #25 fixes your issue.

weierophinney added a commit that referenced this pull request Jan 19, 2016
Close #25
Fixes #3
Fixes #12
Fixes #24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants