PR for #6866 - Allow console specific view manager configuration #6951
Conversation
…ons and display_not_found_reason ref 6866
dkemper
changed the title
dkemper
changed the title
| $this->services = new ServiceManager(); | ||
| $this->view_manager = new ViewManager(); | ||
| $this->factory = new ConsoleViewManagerFactory(); | ||
| } |
There was a problem hiding this comment.
just enhancement, I think setUp() function should be placed in first place
| $config['view_manager'] = array(); | ||
| } | ||
|
|
||
| $this->config = array_merge($config['view_manager'], $config['console']['view_manager']); |
There was a problem hiding this comment.
Console view manager config takes priority?
There was a problem hiding this comment.
Yes in the ConsoleViewManager, better suggestions? Should be covered in the "with-console" testcase.
There was a problem hiding this comment.
It's just that merging it means that console will always win over http. A bit problematic, IMO. I would try solving this problem in the ViewManagerFactory instead, no?
There was a problem hiding this comment.
To clarify: we could inject multiple configs and decide (at runtime) which one to use (depending on whether we are in console context or not)
There was a problem hiding this comment.
I am not sure if i got it right. I just put it in the ConsoleViewManager, because i thought its the right place for it. The ViewManagerFactory returns the ConsoleViewManager in console context. For the ConsoleViewManager the "console view manager configuration" is priority. The new configuration should not affect the HttpViewManager.
There was a problem hiding this comment.
| $this->config = isset($config['view_manager']) && (is_array($config['view_manager']) || $config['view_manager'] instanceof ArrayAccess) | ||
| ? $config['view_manager'] | ||
| : array(); | ||
| if (isset($config['console']['view_manager']) && (is_array($config['console']['view_manager']) || $config['console']['view_manager'] instanceof ArrayAccess)) { |
There was a problem hiding this comment.
Should probably check Console::isConsole() instead of checking what config was passed in, no?
There was a problem hiding this comment.
Is there any use case for using ConsoleViewManager in the http context? I can´t imagine one. https://github.com/zendframework/zf2/blob/master/library/Zend/Mvc/Service/ViewManagerFactory.php#L28 is checking for the console context right now.
I thought supporting the current configuration key, will not cause a breaking change, because
The default configuration for displayNotFoundReason and displayExceptions is set to true: https://github.com/zendframework/zf2/blob/master/library/Zend/Mvc/View/Console/ExceptionStrategy.php#L25 and https://github.com/zendframework/zf2/blob/master/library/Zend/Mvc/View/Console/RouteNotFoundStrategy.php#L38. Otherwise we could just remove the $config['view_manager] check or solve it within the factory. I am not familiar how use usally handle backwardcompatibility.
There was a problem hiding this comment.
Missed the fact that the view manager factory selects the correct view manager upfront: my bad. This fix is valid.
|
+1 |
…fic-view-manager-configuration-6866
…config' into develop Close #6866
Allow to override "display_exceptions" and "display_not_found_reason" for Console ViewManager #6866