Re: [xwiki-devs] [xwiki-notifications] r9589 - xwiki-platform/xwiki-plugins/trunk/skinx/src/main/java/com/xpn/xwiki/plugin/skinx
How much duplication do you reckon there is between the 2 classes? Might be worth refactoring and have a common ancestor to share code? -Vincent On Apr 30, 2008, at 2:10 AM, sdumitriu (SVN) wrote:
Author: sdumitriu Date: 2008-04-30 02:10:27 +0200 (Wed, 30 Apr 2008) New Revision: 9589
Modified: xwiki-platform/xwiki-plugins/trunk/skinx/src/main/java/com/xpn/ xwiki/plugin/skinx/CssSkinExtensionPlugin.java xwiki-platform/xwiki-plugins/trunk/skinx/src/main/java/com/xpn/ xwiki/plugin/skinx/JsSkinExtensionPlugin.java Log: XSKINX-2: Author not set when auto-creating the extension xclasses Fixed.
Modified: xwiki-platform/xwiki-plugins/trunk/skinx/src/main/java/com/ xpn/xwiki/plugin/skinx/CssSkinExtensionPlugin.java =================================================================== --- xwiki-platform/xwiki-plugins/trunk/skinx/src/main/java/com/xpn/ xwiki/plugin/skinx/CssSkinExtensionPlugin.java 2008-04-29 17:20:37 UTC (rev 9588) +++ xwiki-platform/xwiki-plugins/trunk/skinx/src/main/java/com/xpn/ xwiki/plugin/skinx/CssSkinExtensionPlugin.java 2008-04-30 00:10:27 UTC (rev 9589) @@ -1,5 +1,6 @@ package com.xpn.xwiki.plugin.skinx;
+import org.apache.commons.lang.StringUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory;
@@ -98,9 +99,16 @@ needsUpdate |= bclass.addStaticListField("cache", "Caching policy", "long|short|default|forbid");
- String content = doc.getContent(); - if ((content == null) || (content.equals(""))) { + if (StringUtils.isBlank(doc.getAuthor())) { needsUpdate = true; + doc.setAuthor("XWiki.Admin"); + } + if (StringUtils.isBlank(doc.getCreator())) { + needsUpdate = true; + doc.setCreator("XWiki.Admin"); + } + if (StringUtils.isBlank(doc.getContent())) { + needsUpdate = true; doc.setContent("1 XWiki Stylesheet Extension Class"); }
Modified: xwiki-platform/xwiki-plugins/trunk/skinx/src/main/java/com/ xpn/xwiki/plugin/skinx/JsSkinExtensionPlugin.java =================================================================== --- xwiki-platform/xwiki-plugins/trunk/skinx/src/main/java/com/xpn/ xwiki/plugin/skinx/JsSkinExtensionPlugin.java 2008-04-29 17:20:37 UTC (rev 9588) +++ xwiki-platform/xwiki-plugins/trunk/skinx/src/main/java/com/xpn/ xwiki/plugin/skinx/JsSkinExtensionPlugin.java 2008-04-30 00:10:27 UTC (rev 9589) @@ -1,5 +1,6 @@ package com.xpn.xwiki.plugin.skinx;
+import org.apache.commons.lang.StringUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory;
@@ -98,9 +99,16 @@ needsUpdate |= bclass.addStaticListField("cache", "Caching policy", "long|short|default|forbid");
- String content = doc.getContent(); - if ((content == null) || (content.equals(""))) { + if (StringUtils.isBlank(doc.getAuthor())) { needsUpdate = true; + doc.setAuthor("XWiki.Admin"); + } + if (StringUtils.isBlank(doc.getCreator())) { + needsUpdate = true; + doc.setCreator("XWiki.Admin"); + } + if (StringUtils.isBlank(doc.getContent())) { + needsUpdate = true; doc.setContent("1 XWiki Stylesheet Extension Class"); }
_______________________________________________ notifications mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/notifications
Vincent Massol wrote:
How much duplication do you reckon there is between the 2 classes?
Might be worth refactoring and have a common ancestor to share code?
Hi, There already is a common ancestor... It just happens that these 2 extensions use a similar xclass, yet different enough not to justify a common method. At least not one common between these two, but maybe we can have a common ensureClassDocumentIsProperlyDefined method used by all the code that creates a class dynamically. I had a discussion with Jerome about the skin extensions plugin(s), the main point being: Should there be only one plugin that offers both Javascript and CSS extensions? I said yes, as SkinExtensions isn't really a plugin, but a concept, or a meta-plugin, currently implemented in JavaScript and Style extensions, but that can be further explored. For example, another idea was to have LinkExtensions, which offer the ability for a page to "advertise" some links: - a Project Description application could publish links to DOAP profiles for every project page - a Social Networking application could publish FOAF documents for each user - a slideshow application could publish first/prev/next/last/index links which would appear nicely in the navigation bar in Opera and in Firefox with the Site Navigation Bar extension; this is applicable for any structured multi-document information - link to a custom RSS feed - link to a RDF/OWL description of the document This is pretty different from Jsx and Ssx, because there's no need for xobjects. Still, the same principle is used: the content pulls in links. The format of the links is quite different, though, so having one class that generates 5 types of links (jsx, static js file, ssx, static css file, user-defined link) is not clean enough. And maybe sometime later we'll come with another type of skin extension, with a different way of working. And I think that by now we've learned that trying to make one class that should do more things is not a good idea, as otherwise we'll end up with huge classes, so it is not extensible enough to just add new methods and new if-then-else blocks whenever we want to add a new type of extensions. I'd say that the current model is good enough, as it provides a common framework and allows each extension type to define its own rules, keeping them separate (each extension type can be enabled or disabled in xwiki.cfg) Yep, just had another idea of skin extensions for the future, XBL extensions; but that is for much later, when we'll be able to use XBL files. -- Sergiu Dumitriu http://purl.org/net/sergiu/
Vincent Massol wrote:
How much duplication do you reckon there is between the 2 classes?
Might be worth refactoring and have a common ancestor to share code?
To be more detailed, the 2 classes share all the code, except getName (which is wrongly redefined in all the plugins), the method that checks that the xclass is properly defined (as the xclasses are different), and the method that generates the links (as they have a different syntax for the link). -- Sergiu Dumitriu http://purl.org/net/sergiu/
participants (2)
-
Sergiu Dumitriu -
Vincent Massol