I don't see any show stoppers. Going to try patching it in now and look at the generated code.
http://gwt-code-reviews.appspot.com/1420804/diff/1/user/src/com/google/gwt/uibinder/client/UiBinderUtil.java File user/src/com/google/gwt/uibinder/client/UiBinderUtil.java (right): http://gwt-code-reviews.appspot.com/1420804/diff/1/user/src/com/google/gwt/uibinder/client/UiBinderUtil.java#newcode44 user/src/com/google/gwt/uibinder/client/UiBinderUtil.java:44: element = com.google.gwt.dom.client.Document.get() no need for the long name, this is imported http://gwt-code-reviews.appspot.com/1420804/diff/1/user/src/com/google/gwt/uibinder/elementparsers/LazyPanelParser.java File user/src/com/google/gwt/uibinder/elementparsers/LazyPanelParser.java (right): http://gwt-code-reviews.appspot.com/1420804/diff/1/user/src/com/google/gwt/uibinder/elementparsers/LazyPanelParser.java#newcode38 user/src/com/google/gwt/uibinder/elementparsers/LazyPanelParser.java:38: assert writer.useAttachableStrategyMabobThing(); http://gwt-code-reviews.appspot.com/1420804/diff/1/user/src/com/google/gwt/uibinder/elementparsers/LazyPanelParser.java#newcode40 user/src/com/google/gwt/uibinder/elementparsers/LazyPanelParser.java:40: String widgetClassPath = writer.getClassPath(Widget.class); just use Widget.class.getName(), getClassPath adds no value I can see. http://gwt-code-reviews.appspot.com/1420804/diff/1/user/src/com/google/gwt/uibinder/elementparsers/LazyPanelParser.java#newcode42 user/src/com/google/gwt/uibinder/elementparsers/LazyPanelParser.java:42: String code = null; Use XMLElement#consumeSingleChildElement and you don't need your own one-and-only-one checks. http://gwt-code-reviews.appspot.com/1420804/diff/1/user/src/com/google/gwt/uibinder/elementparsers/WidgetInterpreter.java File user/src/com/google/gwt/uibinder/elementparsers/WidgetInterpreter.java (right): http://gwt-code-reviews.appspot.com/1420804/diff/1/user/src/com/google/gwt/uibinder/elementparsers/WidgetInterpreter.java#newcode95 user/src/com/google/gwt/uibinder/elementparsers/WidgetInterpreter.java:95: "com.google.gwt.uibinder.client.UiBinderUtil.LazyDomElement", elementPointer); Please use LazyDomElement.class.getName() http://gwt-code-reviews.appspot.com/1420804/diff/1/user/src/com/google/gwt/uibinder/elementparsers/WidgetInterpreter.java#newcode98 user/src/com/google/gwt/uibinder/elementparsers/WidgetInterpreter.java:98: "new com.google.gwt.uibinder.client.UiBinderUtil.LazyDomElement(%s)", ditto http://gwt-code-reviews.appspot.com/1420804/diff/1/user/src/com/google/gwt/uibinder/elementparsers/WidgetPlaceholderInterpreter.java File user/src/com/google/gwt/uibinder/elementparsers/WidgetPlaceholderInterpreter.java (right): http://gwt-code-reviews.appspot.com/1420804/diff/1/user/src/com/google/gwt/uibinder/elementparsers/WidgetPlaceholderInterpreter.java#newcode161 user/src/com/google/gwt/uibinder/elementparsers/WidgetPlaceholderInterpreter.java:161: if (uiWriter.useAttachableStrategy()) { Seems like you would have a lot less of these useAttachableStrategy blocks floating around if addAttachStatement checked it like convertFieldToGetter does. When it's false, addAttachStatement can do what UiBinderWriter#addInitStatement does. You'd have to move the initStatements array to FieldManager, and make the UiBinderWriter#addInitStatement a passthrough to it. I don't mean to slow you down, but I'm concerned that useAttachableStrategy will live for a while, and it will be hard for parser writers to know what to do. http://gwt-code-reviews.appspot.com/1420804/diff/1/user/src/com/google/gwt/uibinder/rebind/FieldManager.java File user/src/com/google/gwt/uibinder/rebind/FieldManager.java (right): http://gwt-code-reviews.appspot.com/1420804/diff/1/user/src/com/google/gwt/uibinder/rebind/FieldManager.java#newcode98 user/src/com/google/gwt/uibinder/rebind/FieldManager.java:98: public void addAttachStatement(String fieldName, String format, Object... args) Don't duplicate the FieldWriter api. To get them add a new require(fieldName) method that wraps lookup() and does the die thing. http://gwt-code-reviews.appspot.com/1420804/diff/1/user/src/com/google/gwt/uibinder/rebind/FieldManager.java#newcode102 user/src/com/google/gwt/uibinder/rebind/FieldManager.java:102: logger.die("Failing adding attach statement. FieldName %s has no FieldWriter associated.", die is for user error. If this is really developer error, make it a RuntimeException. http://gwt-code-reviews.appspot.com/1420804/diff/1/user/src/com/google/gwt/uibinder/rebind/FieldManager.java#newcode112 user/src/com/google/gwt/uibinder/rebind/FieldManager.java:112: public void addDetachStatement(String fieldName, String format, Object... args) ditto http://gwt-code-reviews.appspot.com/1420804/diff/1/user/src/com/google/gwt/uibinder/rebind/FieldManager.java#newcode126 user/src/com/google/gwt/uibinder/rebind/FieldManager.java:126: public FieldWriter addFieldStatement(String fieldName, String format, Object... args) ditto http://gwt-code-reviews.appspot.com/1420804/diff/1/user/src/com/google/gwt/uibinder/rebind/FieldManager.java#newcode143 user/src/com/google/gwt/uibinder/rebind/FieldManager.java:143: public String convertFieldToGetter(String fieldName) { Can this be a FieldWriter instance method? http://gwt-code-reviews.appspot.com/1420804/diff/1/user/src/com/google/gwt/uibinder/rebind/FieldManager.java#newcode155 user/src/com/google/gwt/uibinder/rebind/FieldManager.java:155: * There are 3 possible situations: Lists like this are more readable if you use <dl>, and drop the colons. e.g. <dl> <dt>getter never called <dd> write builder only if... </dl> http://gwt-code-reviews.appspot.com/1420804/diff/1/user/src/com/google/gwt/uibinder/rebind/FieldManager.java#newcode177 user/src/com/google/gwt/uibinder/rebind/FieldManager.java:177: Integer count = getGetterCounter(field.getName()); And this block can be an instance method on FieldWriter http://gwt-code-reviews.appspot.com/1420804/diff/1/user/src/com/google/gwt/uibinder/rebind/FieldManager.java#newcode314 user/src/com/google/gwt/uibinder/rebind/FieldManager.java:314: public void setFieldInitializer(String fieldName, String format, Object... args) ditto http://gwt-code-reviews.appspot.com/1420804/diff/1/user/src/com/google/gwt/uibinder/rebind/FieldManager.java#newcode379 user/src/com/google/gwt/uibinder/rebind/FieldManager.java:379: private int getGetterCounter(String fieldName) { Wouldn't this count be more natural as a field on the FieldWriter? http://gwt-code-reviews.appspot.com/1420804/diff/1/user/src/com/google/gwt/uibinder/rebind/FieldWriter.java File user/src/com/google/gwt/uibinder/rebind/FieldWriter.java (right): http://gwt-code-reviews.appspot.com/1420804/diff/1/user/src/com/google/gwt/uibinder/rebind/FieldWriter.java#newcode41 user/src/com/google/gwt/uibinder/rebind/FieldWriter.java:41: * Add a statement to be executed right after the current field is attached. Thanks, this is helpful. http://gwt-code-reviews.appspot.com/1420804/diff/1/user/src/com/google/gwt/uibinder/rebind/FieldWriter.java#newcode66 user/src/com/google/gwt/uibinder/rebind/FieldWriter.java:66: * Add a statement to be executed right after the current field is detached. Example: lose "Example:" http://gwt-code-reviews.appspot.com/1420804/diff/1/user/src/com/google/gwt/uibinder/rebind/FieldWriter.java#newcode155 user/src/com/google/gwt/uibinder/rebind/FieldWriter.java:155: * private WidgetX get_widgetX() { I'm wondering if some of your code size penalty will go away if you simply don't write get_ methods for only-once widgets. Make them work as if useAttachableStrategy is false. Needn't gate submitting this since you put the switch in, just something to think about. http://gwt-code-reviews.appspot.com/1420804/diff/1/user/src/com/google/gwt/uibinder/rebind/FieldWriter.java#newcode164 user/src/com/google/gwt/uibinder/rebind/FieldWriter.java:164: * Notice that there's no field class and the getter just fallback to "field class" You mean just "field"? http://gwt-code-reviews.appspot.com/1420804/diff/1/user/src/com/google/gwt/uibinder/rebind/UiBinderGenerator.java File user/src/com/google/gwt/uibinder/rebind/UiBinderGenerator.java (right): http://gwt-code-reviews.appspot.com/1420804/diff/1/user/src/com/google/gwt/uibinder/rebind/UiBinderGenerator.java#newcode50 user/src/com/google/gwt/uibinder/rebind/UiBinderGenerator.java:50: private static final String ELEMENT_FACTORY_PROPERTY = "uibinder.html.elementfactory"; You're stale, this field is gone now. http://gwt-code-reviews.appspot.com/1420804/diff/1/user/src/com/google/gwt/uibinder/rebind/UiBinderGenerator.java#newcode166 user/src/com/google/gwt/uibinder/rebind/UiBinderGenerator.java:166: // TODO(hermes): poor naming How about useLazyWidgetBuilders http://gwt-code-reviews.appspot.com/1420804/diff/1/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java File user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java (right): http://gwt-code-reviews.appspot.com/1420804/diff/1/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java#newcode129 user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java:129: public static String extractFieldFromCode(String code) { Does this really need to be public? http://gwt-code-reviews.appspot.com/1420804/diff/1/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java#newcode322 user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java:322: // I'm expliciting over-simplifying this and assuming that the input Use /* for long comments. And doesn't this comment belong in extractFieldFromCode? http://gwt-code-reviews.appspot.com/1420804/diff/1/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java#newcode329 user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java:329: } catch (UnableToCompleteException e) { Why is this okay? http://gwt-code-reviews.appspot.com/1420804/diff/1/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java#newcode364 user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java:364: * @param fieldName The name of the field being declared @param ancestorField ... http://gwt-code-reviews.appspot.com/1420804/diff/1/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java#newcode404 user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java:404: domField.setBuildPrecendence(2); comment explaining why http://gwt-code-reviews.appspot.com/1420804/diff/1/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java#newcode624 user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java:624: public String getClassPath(Class clazz) { Really? What's wrong with clazz.getName()? Why have this method at all? http://gwt-code-reviews.appspot.com/1420804/diff/1/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java#newcode713 user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java:713: return (returnGetter ? fieldManager.convertFieldToGetter(fieldName) : fieldName); Can convertFieldToGetter be an instant method on FieldWriter? If so, instead of the hacky returnGetter arg, you can make this method return the FieldWriter itself (still leaving the old one as a wrapper around it to avoid mass parser update -- maybe this is just parseElement(XMLElement)?). Now that you've made FieldManager more public might as well go whole hog. Either way, please update the javadoc here and below. http://gwt-code-reviews.appspot.com/1420804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
