ZF-11516: Implement "disable layout" command in Zend_Tool
Description
The attached patch implements zf disable layout.
$ 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.
Comments
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
Adam,
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
Adam,
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 ...
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