Manifest Builder Rewrite

[Andrew Short]

Hi All,

As mentioned earlier I've been working on rewriting the manifest system so it's more flexible and testable. I've pushed my work up to a new branch (https://github.com/ajshort/sapphire/tree/manifestbuilder-rewrite), and it all seems to be working well. There are a couple of tests left to write, but apart from that it's just about feature complete and all unit tests pass. I'm looking to gather some feedback on the new manifest implementation, and to see if anyone has any suggested improvements.

There's a new SS_FileFinder class, which is useful for finding any files that match a provided set of rules. This is sub-classed by ManifestFileFinder, which is a utility class that can find any files within a folder according to the SilverStripe module/themes structure - e.g. takes into account exclude files, tests...

This is utilised by two new pairs of classes - SS_TemplateLoader/SS_TemplateManifest and SS_ClassLoader/SS_ClassManifest. Both manifest objects are responsible for building up a manifest of all the templates and classes (along with meta information) in the project. The loader objects then handle actually loading requested resources. Both loader objects have a stack of manifest objects, which makes it easy to push on different manifests for tests.

In terms of caching the manifest information, it's now managed by the SS/Zend caching layer. This opens up the possibility in the future for using APC/XCache/Memcache to store the manifest, but this is something that needs to be explored later - at the moment the manifest is loaded too early to be easily configurable. In terms of the actual cache, the class and template manifest are stored separately, but are quite quick to load. The manifest size has been reduced from 230KB on a blank install to below 100KB combined.

There are also a stack of minor API changes included in the branch, mainly to remove code which relied on the old manifest system. Below is a quick summary that might affect people, but there are several more in the branch:

• Requirements::themedCSS() has been refactored so it takes two arguments - a $name and $module argument. It attempts to load the CSS file from the current theme, falling back to the module specified. This is slightly more explicit, but it gets rid of annoying bugs where CSS files from random modules are included. Additionally, it also entirely removes the need for a CSS manifest. When using this from within themes the API is the same, but if you are using it within a module to allow overloading the css file, you'll need to change the call.
  
• A couple of magic get vars have been removed - ?includetestmanifest, ?debugmanifest.
• ManifestBuilder functions (e.g. get_themes) have been reshuffled around to new places (in this case SSViewer). Also, the responsibility for loading up a test manifest has moved to TestRunner.
• i18n::get_owner_module() no longer supports finding the owner for a template file - it only works for classes. This wasn't actually really used anywhere
  

Cheers,
Andrew Short

[Nicolaas Thiemen Francken - Sunny Side Up]

AWESOME Andrew!  Thank you for all your work on this.  Sounds great.

[Nivanka Fonseka]

Good Stuff,

Requirements::themedCSS() will be a nice one for my new project.

thanks for the good work.

--
You received this message because you are subscribed to the Google Groups "SilverStripe Core Development" group.
To post to this group, send email to silverst...@googlegroups.com.
To unsubscribe from this group, send email to silverstripe-d...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/silverstripe-dev?hl=en.

--
Nivanka Fonseka
Senior Software Engineer

Skype: nivanka.fonseka
Twitter: @nivankafonseka

[Andrew Short]

Thanks for all the positive feedback :)

I've written the last couple of tests around the template manifest and pushed those up, so I think the branch is now feature and test complete. I'll let it sit for a couple of days so people can comment on the API, but I think it's at a stage where it can be merged back to master safely.

Andrew Short

[Simon J Welsh]

On 23/03/2011, at 10:24 PM, Andrew Short wrote:

> - A couple of magic get vars have been removed - ?includetestmanifest,
> ?debugmanifest.

I'm curious about the justification for removing debugmanifest, especially if you're planning on using some other cache mechanism that can't be checked by running the manifest files through less.
---
Simon Welsh
Admin of http://simon.geek.nz/

Who said Microsoft never created a bug-free program? The blue screen never, ever crashes!

http://www.thinkgeek.com/brain/gimme.cgi?wid=81d520e5e

[Daniel Lindkvist]

Great to see that someone is paying attention to the Manifest builder, it is needed. Good job.

 

--

[stojg]

I agree with all of the above. This is one of the things that been
nagging me from time to time during the last two years on
silverstripe.

+1

[Sam Minnée]

I think it would be good to have a debugger, however I think we need to move away from the random collection of get variables as their implementation.

I'm going to start a new thread for debug tools.

> --
> You received this message because you are subscribed to the Google Groups "SilverStripe Core Development" group.
> To post to this group, send email to silverst...@googlegroups.com.
> To unsubscribe from this group, send email to silverstripe-d...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/silverstripe-dev?hl=en.
>

-------
Sam Minnée | Chief Technology Officer
SilverStripe
http://silverstripe.com

Phone: +64 4 978 7334
Skype: sam.minnee

[Andrew Short]

You're right, ?debugmanifest on second thoughts is probably something that would be useful to diagnose edge cases and odd bugs. Do you think that just dumping an array of classes and interfaces in the manifest, as well as the template manifest, would be sufficient - or do you think it's necessary to print out the whole ClassManifest and TemplateManifest objects so things like descendant information is included? This would also include a lot of extraneous properties though.

Andrew Short

[Andrew Short]

In that case I'll hold off implementing ?debugmanifest for now :)

Andrew Short

[Hamish Friedlander]

I'll add my thanks to everyone else's for getting this sorted. I've merged this into master.

In general we're trying to merge back feature branches to master as soon as they're fairly stable and are unlikely to interfere with other development. We don't want lots of feature branches getting too divergent from each other. So we're not going to hold off merging something in just because minor features have been stripped out and not yet replaced, or there are planned features or API changes that haven't made it into the codebase yet.

A good test suite makes determining when a feature is stable much easier of course, and ideally any changes would either update or not break existing tests.

Hamish Friedlander

[Artyom]

Hi Andrew. 

THanks so much for the much needed and great work.  ONe question I had about the api re css:

Is there a good mechanism to partially override css settings?  ... something like:

R::themedCSS(blog);

R::themedCSS(blog_myChangesOnly);

Something that always gets included or looked for by the manifest builder, but is easily used for just changing a few styles.  I really don't like to have to grab the entire file into my theme's css dir just to change a few things.

would be nice to have some sort of convention, like

R::themedCSS(moduleName)

and 

R::themedCSS(moduleName_ext)

or another mechanism that achieves the same effect.

thanks again,

Artyom

On Wed, Mar 23, 2011 at 2:54 PM, Andrew Short <andrew...@gmail.com> wrote:

--

[Andrew Short]

There isn't a default auto-include by name mechanism, which IMO is a good thing - the less magic the better. Why can't you just include your custom CSS file in your controller or template on top of the existing one?

Andrew Short

[Artyom]

Well, what if i'm not subclassing the existing module code at all? --either at the php level or the templates... I could put that "mychanges.css: include in *every* page, by putting it in Page.ss say, but that is not so elegant.  

It's not a huge deal, but I think from a programming model standpoint, having the ability to override the css include (which we have currently, and which you've cleaned up and clarified further), but requiring copy and paste to get any existing css intelligence in addition to one's own changes, seems a bit backward.  My solution isn't so elegant either, but I thing some way to do that would be useful and make things crisper and cleaner.

On Fri, Mar 25, 2011 at 2:09 PM, Andrew Short <andrew...@gmail.com> wrote:

There isn't a default auto-include by name mechanism, which IMO is a good thing - the less magic the better. Why can't you just include your custom CSS file in your controller or template on top of the existing one?

Andrew Short

--

[Sam Minnée]

We're planning on adding support to apply multiple themes in a cascade. I think that will provide a better solution for this - you can define a new theme that contains only a blog.css.

[Artyom]

That is in fact a better solution : - )   I like it.   Thanks