2008/3/26, Cserveny Tamas <[EMAIL PROTECTED]>: > I've implemented a draft solution to the TILES-220 issue. It is > basically an extension of the DefinitionsImpl. > If a definition is registered with * in the name, it is remembered for > later use. > If a definition cannot be found, then these names will be checked, > whether any of them are matching. > If one of them matches, then a new definition will be made, and then > will be remembered, so no more matching needed.
Cool :-) Anyway, I took a look at your patch and I'd like that you give me a hand in building the final patch, since I am currently the only active developer at the moment and I cannot do it only by myself. 1) Fix your code by using the Checkstyle rules: http://svn.apache.org/repos/asf/tiles/maven/trunk/build/tiles_checks.xml If you are using Eclipse, there is a Checkstyle plugin to check it: http://eclipse-cs.sourceforge.net/ 2) Add a JUnit test 3) Modify the "tiles-test" project to use your definitions factory, adding a test page and the Selenium test. 4) Add a wiki page to show how it works: http://cwiki.apache.org/TILES/ > My implementation is basically a solution that can be used without any > modification in tiles. > I think it would be good to include this feature in later versions, > thus it needs to be integrated more tightly into Tiles. If everything goes well, it will be included in Tiles 2.1.0 > Currently it is a separate factory, with the createDefinitions() > method overridden. > I think this WildcardDefinitions should be used by the URLDefFactory > by default. Mmm... I am -1 for this, since it adds some extra complexity. I think that it is simply a matter of configuration in web.xml. I know that this means that many people can access a feature that they do not know to have :-) but I think that a good documentation could be enough. > This WildcardDefinitions could be a wrapper as well, in case it is > desired/forseen that the Definitions class could change / or other > implementations might be needed. (In this case the wildcard matching > would still be provided) In fact, some time ago we decided to not allow the configuration (through web.xml parameters) of Definitions class, since it shows an internal "gut" of the particular implementation of the Definitions factory. I think that a subclass of UrlDefinitionsFactory as you did is ok, I created the "createDefinitions" method on purpose :-) > It is also possible to include this matching in the DefinitionsImpl > itself, but may be that would complicate the DefinitionsImpl too much. This is another reason why I don't like to see it as a default. > One more issue which needs to be discussed is, that this > implementations has references to the XWork library. I don't know > whether it is a problem or not. I don't think so, it could be an optional dependency. Anyway, are you sure that you really need XWork? I notice that you do not use XWork as it is, but only a pair of classes. Probably, internally, XWork uses some other "common" classes. Can you take a look? One more thing. I noticed that you called the patch "2.0.5.patch". Does it mean that you created it starting from the 2.0.5 version? If yes, please create a patch against the trunk in SVN repository: http://svn.apache.org/repos/asf/tiles/framework/trunk/ Thanks Antonio
