Issues

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

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?

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}}.

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}}.

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:

  • "disable" would remove or comment out the application.ini entry
  • "enable" after a disable would add or uncomment the application.ini entry, and check to see if a entry exists; if not, it creates it. It also checks to see if the layout.phtml file exists; if so, it does nothing, but if not, it creates it.

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

@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?

Fix committed to trunk in r24303

Adam - looks like you really fix it. I was struggling with the following use case, but you fix it ...


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?

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.

Issued pull request on {{zendframework/zf2}} branch {{master}} https://github.com/zendframework/zf2/pull/360