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() On 2011/04/19 21:07:20, rjrjr wrote:
no need for the long name, this is imported
Done. 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: On 2011/04/19 21:07:20, rjrjr wrote:
assert writer.useAttachableStrategyMabobThing();
Well, the parser is not registered if the flag is disabled so this point is never reached. But did the change anyway since this way I can spit out a better warning message. 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); On 2011/04/19 21:07:20, rjrjr wrote:
just use Widget.class.getName(), getClassPath adds no value I can see.
Done. 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; On 2011/04/19 21:07:20, rjrjr wrote:
Use XMLElement#consumeSingleChildElement and you don't need your own one-and-only-one checks.
Done. 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); On 2011/04/19 21:07:20, rjrjr wrote:
Please use LazyDomElement.class.getName()
Done. 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)", On 2011/04/19 21:07:20, rjrjr wrote:
ditto
Done. 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) On 2011/04/19 21:07:20, rjrjr wrote:
Don't duplicate the FieldWriter api. To get them add a new
require(fieldName)
method that wraps lookup() and does the die thing.
Done. 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.", On 2011/04/19 21:07:20, rjrjr wrote:
die is for user error. If this is really developer error, make it a RuntimeException.
Done. 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) On 2011/04/19 21:07:20, rjrjr wrote:
ditto
Done. 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) On 2011/04/19 21:07:20, rjrjr wrote:
ditto
Done. 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) { On 2011/04/19 21:07:20, rjrjr wrote:
Can this be a FieldWriter instance method?
No because convertFieldToGetter() might be called before FieldWriter is actually created, so for now this is the easier way. 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: On 2011/04/19 21:07:20, rjrjr wrote:
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>
Done. 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()); On 2011/04/19 21:07:20, rjrjr wrote:
And this block can be an instance method on FieldWriter
Done. 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) On 2011/04/19 21:07:20, rjrjr wrote:
ditto
Done. 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) { On 2011/04/19 21:07:20, rjrjr wrote:
Wouldn't this count be more natural as a field on the FieldWriter?
see my previous comment. 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. On 2011/04/19 21:07:20, rjrjr wrote:
Thanks, this is helpful.
Done. 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: On 2011/04/19 21:07:20, rjrjr wrote:
lose "Example:"
Done. 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() { On 2011/04/19 21:07:20, rjrjr wrote:
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.
OK. 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 On 2011/04/19 21:07:20, rjrjr wrote:
"field class" You mean just "field"?
Done. 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"; On 2011/04/19 21:07:20, rjrjr wrote:
You're stale, this field is gone now.
Done. 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 On 2011/04/19 21:07:20, rjrjr wrote:
How about useLazyWidgetBuilders
Done. 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) { On 2011/04/19 21:07:20, rjrjr wrote:
Does this really need to be public?
Moved to where it's called. 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 On 2011/04/19 21:07:20, rjrjr wrote:
Use /* for long comments. And doesn't this comment belong in extractFieldFromCode?
I'd rather having them together, so I'm moving the code from extractFieldFromCode to here. 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) { On 2011/04/19 21:07:20, rjrjr wrote:
Why is this okay?
Not OK, sort of done. 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 On 2011/04/19 21:07:20, rjrjr wrote:
@param ancestorField ...
Done. 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); On 2011/04/19 21:07:20, rjrjr wrote:
comment explaining why
Done. I really hated this but it's the easier way of determining which builders should come first. 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) { On 2011/04/19 21:07:20, rjrjr wrote:
Really? What's wrong with clazz.getName()? Why have this method at
all? Not needed, old shit I forgot to get rid of. 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); On 2011/04/19 21:07:20, rjrjr wrote:
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.
See my previous comment, convertFieldToGetter() cannot be FieldWriter, at least for now. Sometimes it is called before the field writer is actually created. Can I leave a TODO? We must revisit this in the future when useLazyWidgetBuilders become the main branch. http://gwt-code-reviews.appspot.com/1420804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
