Enable, Disable... and... Remove?

fredwu

Ok, so I've been playing with the code base for a bit.

I noticed that the 'CssThemes' plugin cannot be disabled due to its database table (actually it is its content) not being cleaned up.

So, I am proposing to add another action to the Applications and Plugins (and possibly Themes, but I haven't gone through that bit yet), 'Remove'.

And add two more class methods, so the 'action' to 'method' mapping looks like:

Enable action -> Setup()
Disable action -> OnDisable()
Remove action -> CleanUp()

I *really* hope this is something that you core developers would like to see, as I have already started working on it.

Any thoughts?

P.S. It's 6:30AM here, I'm off to get some sleep. If all things go well, I will commit the changes to my Github fork and send you guys a pull request.

luc

cssthemes has not been updated in a while, and garden has changed enough so that it's broken now, that's all.

fredwu

@bean, regardless of the state of the CssTheme plugin, my suggestion still stands.

luc

Indeed, for application/plugins and now themes (as they can hook in) with setup, it might be good idea to be able to unsetup it, and drop the data created with many warnings saying so.

fredwu

Okay, I've pushed some updates:

http://github.com/fredwu/Garden/commits/master

Plugins can now be removed, as well as having the new 'OnDisable()' and 'CleanUp()' methods.

Mark

Just looked at all of these - great work!

My one comment is: Do you think you could call the SettingsController->Remove() method "RemoveAddon" instead? It's a little misleading when taken in the context of a "Settings Controller" to just call it "Remove". Know what I mean?

fredwu

@Mark, thanks! That makes sense, I've just committed the changes. http://github.com/fredwu/Garden/commit/060ff1d022a5dae05e85f83369febb8db027b6a8