On Dec 9, 2011, at 1:59 PM, Denis Gervalle wrote: > On Fri, Dec 9, 2011 at 13:49, Vincent Massol <[email protected]> wrote: > >> >> On Dec 9, 2011, at 1:41 PM, Denis Gervalle wrote: >> >>> On Fri, Dec 9, 2011 at 10:17, Denis Gervalle <[email protected]> wrote: >>> >>>> Hi Devs, >>>> >>>> I am looking at now using the new Locale added in DocumentReference into >>>> the implementation of XWikiDocument. >>>> I have already deprecated language related stuff in XWikiDocument, and I >>>> have introduce a XWikiDocument#getLocale and an >> XWikiDocument#isTranslation >>>> helper since the deprecation of defaultLanguage will increase the need >> of >>>> it. I have also added XWikiDocument#getTranslatedDocument() with Locale >> in >>>> place of language. All the changes are almost backward compatible, >> which is >>>> nice (there is some subtleties with default, "" and null that is now >> more >>>> equivalent, but should not have consequences). >> >> Make sure you move the deprecated sutff in -legacy if you can. >> >>>> The is however one change that is not backward compatible, which is the >>>> change of the document reference. Therefore, >>>> XWikiDocument#getDocumentReference does not return the same reference >> than >>>> it does before, because this reference now contains the Locale. This >> cause >>>> breakage in several places. >> >> Why? It should be the same since the new ref only adds the optional locale. >> >>>> I see some option to fix this: >>>> >>>> A. Fix all places broken. This may be too long for me, and not trivial. >> >> +1 if I understand correctly. >> >>>> B. Introduce a new XWikiDocument#getDocumentReferenceWithLocale() and >>>> have XWikiDocument#getDocumentRefence() returns without Locale. Very >> easy, >>>> but not nice. >> >> Not nice -0 >> >>>> C. Introduce a new XWikiDocument#getDocumentReferenceWithoutLocale() and >>>> change all existing calls to XWikiDocument#getDocumentRefence() in >> platform >>>> to use this one. Nicer, but this is not fully backward compatible. >> >> -0 till I understand the problem above. >> >> I don't understand the issue. >> > > The issue is simple, the DocumentReference will now contains one more > information, the Locale, and this could be unexpected in some place where a > more generic reference (without locale) is required. Therefore, in some > place, the comparison of a DocumentReference, with the one coming from a > document may return false negative, because the comparison is done with a > reference without Locale. > > I should have precise that I am applying C with a some review of each > usage, to not replace massively where this seems not necessary, and keep as > much as possible getDocumentReference in preference to > getDocumentReferenceWithoutLocale.
hmmm I'm ok with C but I don't see why you need to introduce a new API. Why not offer an API to extract the document reference without the locale part? I only recall we talked about a serializer for this (or a resolver, don't remember). You may even have introduced an extractXXX method in DocumentReference to return a refernce without the locale, I don't remember. So why not update the code to use that instead of introducing a new "temporary" API? Thanks -Vincent >> Thanks >> -Vincent >> >>>> Since I am on it right now, I would appreciate your opinion quickly. >>>> WDYT ? >>>> >>>> I am undecided between B and C >>>> >>> >>> Well after more thinking, there is no simple rules and B is not fully >> safe >>> in some places. So I am more in favor of C, since it is the cleanest on >> the >>> long term. I am also thinking to implement a cache of the reference >> without >>> local for better efficiency. >>> Please give me your comment asap, especially if you strongly disagree on >> C >>> since I will start coding this. >>> _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
