Pretty cool. I wonder, though, if this really is a 2.0 gating feature. Do we really want this big a change on the release branch?
http://gwt-code-reviews.appspot.com/103806/diff/1010/1012 File user/src/com/google/gwt/uibinder/attributeparsers/LengthAttributeParser.java (right): http://gwt-code-reviews.appspot.com/103806/diff/1010/1012#newcode28 Line 28: public class LengthAttributeParser implements AttributeParser { You've broken field references. You need to extend StrictAttributeParser and give it a chance at values you don't understand. If you want to be more proactive than "I don't understand it, maybe super will" you can use FieldReferenceConverter#hasFieldReferences, e.g.: if (FieldReferenceConverter.hasFieldReferences(value)) { return super.parse(value); } http://gwt-code-reviews.appspot.com/103806/diff/1010/1012#newcode33 Line 33: static { Map<String, String> theMap = new HashMap<String, String>(); // cram the values into it http://gwt-code-reviews.appspot.com/103806/diff/1010/1012#newcode42 Line 42: unitMap.put("px", "PX"); unitMap = Collections.unmodifiableMap(theMap); http://gwt-code-reviews.appspot.com/103806/diff/1010/1012#newcode53 Line 53: // Special case: 0 requires no units. But in real life 0 can take any unit, right? Shouldn't this be equally flexible--any or none? Also confusing that this deosn't match the API http://gwt-code-reviews.appspot.com/103806/diff/1010/1013 File user/src/com/google/gwt/uibinder/elementparsers/LayoutPanelParser.java (right): http://gwt-code-reviews.appspot.com/103806/diff/1010/1013#newcode27 Line 27: public class LayoutPanelParser implements ElementParser { Many long lines, can you auto format? http://gwt-code-reviews.appspot.com/103806/diff/1010/1013#newcode55 Line 55: writer.die("Invalid constraints: too many horizontal constraints."); On each of these die calls, please prefix with "In %s%s, ", and provide elem and layerElem as the params. http://gwt-code-reviews.appspot.com/103806/diff/1010/1015 File user/src/com/google/gwt/uibinder/rebind/XMLElement.java (right): http://gwt-code-reviews.appspot.com/103806/diff/1010/1015#newcode32 Line 32: import java.util.Set; Weird that these moved. Do you have the gwt format rules installed? http://gwt-code-reviews.appspot.com/103806/diff/1010/1015#newcode457 Line 457: public String consumeLengthAttribute(String name) throws UnableToCompleteException { long line http://gwt-code-reviews.appspot.com/103806/diff/1010/1015#newcode458 Line 458: return consumeAttributeWithDefault(name, "", new JType[] { getDoubleType(), You can just call consumeAttribute, "" is the default default. http://gwt-code-reviews.appspot.com/103806/diff/1010/1016 File user/test/com/google/gwt/uibinder/UiBinderJreSuite.java (right): http://gwt-code-reviews.appspot.com/103806/diff/1010/1016#newcode45 Line 45: public class UiBinderJreSuite { Hurray for new tests! http://gwt-code-reviews.appspot.com/103806/diff/1010/1017 File user/test/com/google/gwt/uibinder/attributeparsers/LengthAttributeParserTest.java (right): http://gwt-code-reviews.appspot.com/103806/diff/1010/1017#newcode32 Line 32: parser = new LengthAttributeParser(new MockMortalLogger()); I thought I deleted MockMortalLogger? You can use MortalLogger.NULL. Once you extend StrictAttributeParser, look at IntAttributeParserTest#setUp to see how mock things out. It'll probably be something like: parser = new LengthAttributeParser(new FieldReferenceConverter(null), null, MortalLogger.NULL) http://gwt-code-reviews.appspot.com/103806/diff/1010/1017#newcode52 Line 52: If you agree that parser.parse("0px") et al should work, it'd be best to start by writing a failing testZeroHandling() here and then doing the fix. http://gwt-code-reviews.appspot.com/103806/diff/1010/1018 File user/test/com/google/gwt/uibinder/elementparsers/LayoutPanelParserTest.java (right): http://gwt-code-reviews.appspot.com/103806/diff/1010/1018#newcode39 Line 39: tester = new ElementParserTester(PARSED_TYPE, new LayoutPanelParser()); Feeling pretty good about writing ElementParserTester :-) http://gwt-code-reviews.appspot.com/103806/diff/1010/1018#newcode72 Line 72: // Lonely right. It'd be better if each of these were there own test method: public void testLonelyRight() {... That will keep one failure from masking downstream ones. http://gwt-code-reviews.appspot.com/103806/diff/1010/1018#newcode143 Line 143: public void testHappy() throws UnableToCompleteException, SAXException, Please test the other success paths too. Should see each of LayoutPanel's add*() methods in expected. http://gwt-code-reviews.appspot.com/103806/diff/1010/1019 File user/test/com/google/gwt/uibinder/test/UiJavaResources.java (right): http://gwt-code-reviews.appspot.com/103806/diff/1010/1019#newcode43 Line 43: public static final MockJavaResource LAYOUT_PANEL = new MockJavaResource( Please keep these alphabetical. http://gwt-code-reviews.appspot.com/103806 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
