Fix css/js theme generation bug #30
Conversation
| @@ -23,7 +23,7 @@ import org.stringtemplate.v4.{ STErrorListener, STRawGroupDir, ST } | |||
| import collection.concurrent.TrieMap | |||
|
|
|||
| object CachedTemplates { | |||
| def apply(dir: File, templateName: String): PageTemplate = { | |||
| def apply(dir: File, templateName: String = PageTemplate.DefaultName): PageTemplate = { | |||
There was a problem hiding this comment.
This looks risky. You could call CachedTemplates(new File("template1")) and then CachedTemplates(new File("template2")) and that second call will give you the template for "template1", because you didn't specify a "templateName".
As an aside, why did we introduce the global mutable state of CachedTemplates?
There was a problem hiding this comment.
By definition, there's no reason to redefine a new argument for dir: File, this is by default the folder containing all the defined templates (defined initially in the plugin). On the other hand, templateName: String allows to specify directly the template file used inside this folder.
If I take your example, the first call (with new File("template1")) will select the default page.st template inside the template1 folder, and the second call inside the template2, then I don't think they use the same template if this case happens since they use a different directory.
But indeed, maybe I could check if the template directory specified exists, but I'm not sure it is necessary.
There was a problem hiding this comment.
Look at the code:
cache.get(templateName) match {
case Some(t) => tIt doesn't care what directory you asked for, it caches things using templateName as the key.
Your comment only increases my fear and doubt :-/
There was a problem hiding this comment.
Oh yes my bad, you're totally right.
What I had in my mind actually is that the directory template is always the same, it's why I've implemented like this... But sure, the directory should be taken into account in the cache.
Deletion of some necessary codes in the previous PR #28 which led to non generation of css and js files associated to the generated templates.