Hi Denis, First of all, thanks a lot for the feedback on IRC and through this mail. See below,
On Mon, Sep 19, 2011 at 6:19 PM, Denis Gervalle <[email protected]> wrote: > My answer for release 3.2 is interspersed. > > On Mon, Sep 19, 2011 at 16:34, Marius Dumitru Florea < > [email protected]> wrote: > >> Hi devs, >> >> Here's the sheet resolution algorithm I have implemented in >> >> https://github.com/xwiki/xwiki-platform/blob/cbad868094b9cc9d811621670e320596ddb2f495/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/internal/sheet/DefaultSheetManager.java >> . >> >> getSheets(document, action) : List<DocumentReference> >> >> (1) If there is a "sheet" request parameter that points to a sheet >> that supports the passed action, return a reference to that sheet. >> > > +1, and I understand that this single sheet will manage all objects in the > document ? This sheet does whatever it wants. It can display all objects, a part of them or none. This step is inspired from /xwiki/bin/view/Space/Page?skin=someSkin thus /xwiki/bin/view/Space/Page?sheet=someSheet > > >> (2) For each object of type XWiki.DocumentSheetBinding attached to the >> passed document check if the "sheet" property points to a sheet that >> supports the passed action. Return the computed list, if not empty. >> > > +1, and each sheet would be managing some of the objects at their sole > discretion ? Exactly. Note that the algorithm produces a list of sheet references. The purpose of this mail is not to decide which sheet is applied. The SheetDocumentDisplayer is responsible for choosing one/more sheet(s) from the list returned by the SheetManager. Currently SheetDocumentDisplayer chooses the first sheet the current user has the right to view, so that you can have different sheets for different groups of users (with different view rights settings). We can image though an implementation that aggregates multiple sheets, but that's a different topic. > > >> (3) For each type of object attached to the passed document: >> > > For 3.2, I agree with this, but in general I wonder if it is right to put > the binding in the class document ? > Should not the class stay more clean from display related stuffs in favor of > the sheet ? > The pros is of course that it concentrate the right management on the class This was the main reason I opted for specifying the binding in the class and not in the sheet. I'd like to know what others think about this. > (if you follow my -1 below) > > >> (3.1) For each object of type XWiki.ClassSheetBinding attached to >> the document defining the class check if the "sheet" property points >> to a sheet that supports the passed action > > > +1, and so each sheet would be managing part of the object ? No. The use cases I had in mind are: * different sheets for different actions (view sheet, edit sheet etc.) * different sheets for different groups of users (administrators sheet, editors sheet, guests sheet) > > >> (3.2) If the class doesn't have any XWiki.ClassSheetBinding object >> then check if <ClassName><ActionName>Sheet exists >> > > -1, this would introduce document name convention, which is not good IMHO > The convention of document name are for WebHome, WebPreferences and > XWikiPreferences, and this is already too much. > The one mentioned by Thomas regarding XWikiServer document is even worse. > > >> (3.3) If not, then check if <ClassName>Sheet exists and supports >> the passed action >> > > -1, idem > > >> (3.4) If not, then check if "sheet.defaultClassSheetBinding" >> configuration property specifies a sheet for the class we're looking >> at >> > > -0, introduce a new space for this config is not nice IMHO. Put this in let > say XWiki.defaultSheetBindingConfig I wasn't very clear. "sheet.defaultClassSheetBinding" is a configuration property that can be set in xwiki.properties . Its default values is: sheet.defaultClassSheetBinding = XWiki.XWikiPreferences = XWiki.AdminSheet sheet.defaultClassSheetBinding = XWiki.XWikiUsers = XWiki.XWikiUserSheet sheet.defaultClassSheetBinding = XWiki.XWikiGroups = XWiki.XWikiGroupSheet > Does not this also solve the oldcore issue we have discussed, if you accept > the idea of providing this default binding with the XE xar ?(Should the old > core contains a minimal default one during empty data state ?) Indeed, I added this configuration property to bind sheets to those (mandatory) classes that are created at XWiki startup. For those that didn't follow the IRC discussion, XWiki.ClassSheetBinding is an implementation detail of DefaultSheetManager. XWiki core should work with a different SheetManager implementation that doesn't use XWiki.ClassSheetBinding objects to bind a sheet to a class. > > >> >> Return the computed list if not empty. >> > > Without duplicates ? Yep. > > >> >> (4) If the passed document is a class (holds a class definition) then >> return a reference to the default class sheet (XWiki.ClassSheet) if it >> supports the passed action. >> > > +0, why not getting this from the default binding above ? The default binding above is for specific classes. I added step (4) because I wanted to apply XWiki.ClassSheet by default to all classes without using the include macro in their content. If I wasn't very clear: Space.MyClassSheet -> displays objects of type Space.MyClass XWiki.ClassSheet -> displays xclass information (list of properties, list of sheets, the template etc.) The explicit bindings would be: XWiki.ClassSheetBinding (sheet = Space.MyClassSheet) XWiki.DocumentSheetBinding (sheet = XWiki.ClassSheet) Step (4) makes the previous line implicit. Thanks, Marius > > Otherwise return an empty list. >> >> I'd like you to vote on each of the steps of the algorithm. Are there >> steps you think I should drop? >> > > Yes definitely, but removing the need to link data with the sheet using > includes is really a big improvement. > Great job, > > >> I'm obviously +1 for each of the steps. >> >> Thanks, >> Marius >> _______________________________________________ >> devs mailing list >> [email protected] >> http://lists.xwiki.org/mailman/listinfo/devs >> > > > > -- > Denis Gervalle > SOFTEC sa - CEO > eGuilde sarl - CTO > _______________________________________________ > devs mailing list > [email protected] > http://lists.xwiki.org/mailman/listinfo/devs > _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
