Issue Type: Bug Created: 2011-06-29T20:18:56.000+0000 Last Updated: 2011-08-28T19:38:06.000+0000 Status: Resolved Fix version(s): - 1.11.11 (29/Sep/11)
Reporter: Kim Blomqvist (kblomqvist) Assignee: Adam Lundrigan (adamlundrigan) Tags: - Zend_Tool
Related issues: Attachments: - ZF-11516.patch
The attached patch implements zf disable layout.
<pre class="highlight"> $ zf create project ZF-11516 $ cd ZF-11516 $ zf enable layout Layouts have been enabled, and a default layout created at /home/kim/workspace/ZF-11516/application/layouts/scripts/layout.phtml A layout entry has been added to the application config file. Updating project profile '/home/kim/workspace/ZF-11516/.zfproject.xml' $ zf disable layout Layouts have been disabled. $ zf disable layout A layout resource already disabled.
Posted by Adam Lundrigan (adamlundrigan) on 2011-06-29T22:44:02.000+0000
Kim, I've cleaned up your patch a little (modified formatting and output messages to match existing code, removed some unnecessary lines) and added code to remove the layoutsDirectory entry from .zfproject.xml when zf disable layout is called.
I've attached the patch, and will commit it soon. Can you try it out for me first to make sure the changes I added are OK?
Posted by Kim Blomqvist (kblomqvist) on 2011-06-30T05:11:19.000+0000
I will try this later but it seems like the layout directory is deleted too. I think we don't want to do that. A good layout may end up to be deleted accidentally. There should be a separate action for this, for example zf remove layout.
Posted by Kim Blomqvist (kblomqvist) on 2011-06-30T14:26:26.000+0000
I have now tested your patch. I see that it only removes these lines from .zfproject.xml
and leaves the /application/layouts directory untouched. It's good that it really does not remove that directory. However, the .zfproject.xml is now out of sync what's the real directory structure. I still think that disabling layout should only remove the layout resource from application/configs/application.ini and leave the project structure untouched - including .zfproject.xml.
Posted by Matthew Weier O'Phinney (matthew) on 2011-06-30T14:47:11.000+0000
I agree with Kim on this -- disabling is not the same as removing, and I'd worry that re-enabling later would try to overwrite files.
Speaking of which: what happens when you call "zf enable layout" after calling "zf disable layout"? I would expect in this case the following:
Posted by Adam Lundrigan (adamlundrigan) on 2011-07-05T15:45:07.000+0000
Valid point; I hadn't considered that .zfproject.xml would end up out of sync with the actual project directory structure.
In the patch I attached zf disable layout removes the layout config entry from application.ini and associated layout information from .zfproject.xml, but leaves the layout scripts intact.
When you call zf enable layout after calling zf disable layout it will add the resources.layout.layoutPath entry in [production] section. Unless run in pretend mode, zf enable layout always calls Zend_Tool_Project_Provider_Layout::createResource, which updates the .zfproject.xml file with the structure Kim outlined in his comment on 30/Jun/2011 @ 2:26PM. If i'm reading the code correctly, then zf enable layout will always overwrite the existing layout script file, ( Zend_Tool_Project_Provider_Layout::enable always calls $layoutScriptFile->create())
Should we consider changing that behaviour? ie: zf disable layout comments out layout lines in application.ini, zf enable layout enables commented-out lines (or creates new ones if they don't already exist) and doesn't overwrite an existing layout file
Posted by Adam Lundrigan (adamlundrigan) on 2011-07-30T00:29:27.000+0000
@Matthew Based on your comments I have rejigged the fix: * Don't overwrite an existing layout file when calling zf enable layout after zf disable layout * Don't remove layoutsDirectory tree from .zfproject.xml
Should we also implement a zf remove layout function as Kim mentioned above, ie: deletes all traces of zf enable layout having ever been run?
Posted by Adam Lundrigan (adamlundrigan) on 2011-07-30T00:33:04.000+0000
Fix committed to trunk in r24303
Posted by Kim Blomqvist (kblomqvist) on 2011-07-30T07:29:13.000+0000
Adam - looks like you really fix it. I was struggling with the following use case, but you fix it ...
<pre class="highlight"> zf create project ZF-11516 cd ZF-11516 zf enable layout zf disable layout rm application/layouts/scripts/layout.phtml zf enable layout # should create layout.phtml
bq. Should we also implement a zf remove layout function as Kim mentioned above, ie: deletes all traces of zf enable layout having ever been run?
I think that it's not a good idea to delete all traces of zf enable layout. For example, one may have multiple layout files under application/layouts/scripts. Thus zf remove layout should take additional parameter to tell the tool which layout file will be removed etc. Because of this additional overhead, I wouldn't implement this function as we don't know do we really need this - or what do you think?
Posted by Adam Lundrigan (adamlundrigan) on 2011-08-26T22:07:36.000+0000
Thank you both for your feedback. I've merged the fix into release-1.11 in r24408
As for a zf remove layout, I'm fine with omitting it. I didn't see a solid use case for it, but if you did I would have implemented it anyway.
Posted by Adam Lundrigan (adamlundrigan) on 2011-08-28T19:37:56.000+0000
Issued pull request on zendframework/zf2 branch master https://github.com/zendframework/zf2/pull/360
Have you found an issue?
See the Overview section for more details.