Revision to the concept, for design review: See http://code.google.com/p/google-web-toolkit/wiki/DefaultLocaleBinding, but the short-and-sweet is:
- Introduction of XML tag <set-property-fallback name="*propname*" value="*fallbackString*"/>. It's fine, and expected, to set this zero, one, or many times for any given property. - A new field gets added in BindingProperty, with setter and getter, and a default value of empty string - getPropertyProvider() is rewritten to return, not the literal property provider string, but the result of replacing all instances of /*-FALLBACK-*/ with the fallback string, whether empty or otherwise. - For my default locale use case, our I18N.gwt.xml sets the locale fallback to "default", and user code sets it to something real if the user doesn't like that. On Wed, Jun 10, 2009 at 11:49 AM, Freeland Abbott <fabb...@google.com>wrote: > Yes, he and I already discussed it. I was initially trying to avoid > needing a new XML tag when the existing one was such a near fit, but the > legacy linker support issues convinced me we needed it. > > > > > On Tue, Jun 9, 2009 at 5:37 PM, Bruce Johnson <br...@google.com> wrote: > >> I like what jat said. Freeland? >> >> On Tue, Jun 9, 2009 at 5:33 PM, <j...@google.com> wrote: >> >>> >>> The alternative to this would be Bruce's suggestion of defining a >>> specific fallback value for a selection property rather than using >>> config properties for it. That would narrow the scope to exactly what >>> we know we want to support and simplify the linker changes and avoids >>> the breaking API change for existing linkers (where they have to supply >>> the config properties to avoid breaking, which means the same linker >>> can't work for GWT 1.6 and GWT 2.0). >>> >>> Something like: >>> <set-property-fallback name="locale" value="default/> >>> and the last set value wins. >>> >>> >>> http://gwt-code-reviews.appspot.com/34832/diff/1/2 >>> File >>> >>> dev/core/src/com/google/gwt/core/ext/linker/impl/SelectionScriptLinker.java >>> (right): >>> >>> http://gwt-code-reviews.appspot.com/34832/diff/1/2#newcode194 >>> Line 194: new TreeSet<ConfigurationProperty>()); >>> It seems like allowing this could cause incorrect behavior. I know it >>> is a tradeoff of not breaking existing code, but if there is a linker >>> which calls this method, any code depending on config property lookup >>> will just break. >>> >>> http://gwt-code-reviews.appspot.com/34832/diff/1/2#newcode223 >>> Line 223: } >>> It looks like we are just substituting the empty string for unknown >>> properties, which is especially bad if the call is relayed through the >>> above method. >>> >>> http://gwt-code-reviews.appspot.com/34832/diff/1/4 >>> File dev/core/src/com/google/gwt/dev/cfg/StaticPropertyOracle.java >>> (right): >>> >>> http://gwt-code-reviews.appspot.com/34832/diff/1/4#newcode149 >>> Line 149: for (String v : prop.getAllowedValues()) { >>> setValues.addAll(cprop.getAllowedValues())? >>> >>> Also, maybe this should be computed in getPossibleValues() instead. >>> >>> http://gwt-code-reviews.appspot.com/34832/diff/1/5 >>> File >>> dev/core/src/com/google/gwt/dev/shell/ModuleSpacePropertyOracle.java >>> (right): >>> >>> http://gwt-code-reviews.appspot.com/34832/diff/1/5#newcode127 >>> Line 127: for (String v : cprop.getAllowedValues()) { >>> setValues.addAll(cprop.getAllowedValues())? >>> >>> Also, maybe this should be computed in getPossibleValues() instead. >>> >>> http://gwt-code-reviews.appspot.com/34832/diff/1/7 >>> File user/src/com/google/gwt/i18n/I18N.gwt.xml (right): >>> >>> http://gwt-code-reviews.appspot.com/34832/diff/1/7#newcode51 >>> Line 51: $wnd['__gwt_Locale'] = locale || >>> '/*-GWTCONFIGPROP(default.locale)-*/'; >>> I think we haven't nailed this down enough that we want anyone else >>> using it and expecting it to keep working. So, I would suggest a >>> comment here to that effect. >>> >>> http://gwt-code-reviews.appspot.com/34832/diff/1/7#newcode55 >>> Line 55: return "/*-GWTCONFIGPROP(default.locale)-*/"; >>> Since these can all be empty strings, I would suggest storing >>> "/*-GWTCONFIGPROP(default.locale)-*/" || 'default' in a variable and >>> reusing that. Things will fall over if the property provider returns a >>> value that is not valid. >>> >>> http://gwt-code-reviews.appspot.com/34832/diff/1/8 >>> File >>> user/src/com/google/gwt/i18n/rebind/AbstractLocalizableImplCreator.java >>> (right): >>> >>> http://gwt-code-reviews.appspot.com/34832/diff/1/8#newcode136 >>> Line 136: logger.log(Type.WARN, "default.locale has more than one value, >>> using " >>> Is this even possible since the config property definition says it isn't >>> multivalued? >>> >>> http://gwt-code-reviews.appspot.com/34832 >>> >>> >>> >>> >> > --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
