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 -~----------~----~----~----~------~----~------~--~---
