Yep - that patch looks correct. That's a lot simpler in code than it was in English :)
Thanks Chris, -T On Wed, Aug 5, 2009 at 5:03 PM, Christopher Brind <[email protected]>wrote: > So Todd, are you basically talking about the patch below? If so, let me > know and I'll commit. > > > Index: > > /Users/brindy/Workspaces/pivot/wtk/src/org/apache/pivot/wtkx/WTKXSerializer.java > =================================================================== > --- > > /Users/brindy/Workspaces/pivot/wtk/src/org/apache/pivot/wtkx/WTKXSerializer.java > (revision 801388) > +++ > > /Users/brindy/Workspaces/pivot/wtk/src/org/apache/pivot/wtkx/WTKXSerializer.java > (working copy) > @@ -524,7 +524,7 @@ > if > (attribute.localName.equals(INCLUDE_SRC_ATTRIBUTE)) { > src = attribute.value; > } else if > (attribute.localName.equals(INCLUDE_RESOURCES_ATTRIBUTE)) { > - resources = new > Resources(attribute.value); > + resources = new > Resources(resources, attribute.value); > } else if > (attribute.localName.equals(INCLUDE_ASYNCHRONOUS_ATTRIBUTE)) { > // TODO > throw new > UnsupportedOperationException("Asynchronous includes are not" > > > > Greg, I know you don't like creating private methods, but that readObject > method is 574 lines long really hard to read and follow. Is there any > chance we can break it down in to something more manageable, please? > > Cheers, > Chris > > > 2009/8/5 Greg Brown <[email protected]> > > > Yes - thanks! > > > > On Wednesday, August 05, 2009, at 04:13PM, "Christopher Brind" < > > [email protected]> wrote: > > >I've committed the change and a test, feel free to have a look and let > me > > >know what you think. > > > > > >I notice the WTKXSerializer class has been updated - is it OK for me to > > >change that now? > > > > > >Cheers, > > >Chris > > > > > > > > >2009/8/5 Todd Volkert <[email protected]> > > > > > >> > I've added a load of extra constructors which take a parent resource > > as > > >> the > > >> > first argument, e.g. > > >> > > > >> > > public Resources(String baseName) throws IOException, > > >> > > SerializationException { > > >> > > this(null, baseName, Locale.getDefault(), > > >> > > Charset.defaultCharset()); > > >> > > } > > >> > > > > >> > > > >> > Now has a matching constructor: > > >> > > > >> > > public Resources(Resources parent, String baseName) throws > > >> > IOException, > > >> > > SerializationException { > > >> > > this(parent, baseName, Locale.getDefault(), > > >> > > Charset.defaultCharset()); > > >> > > } > > >> > > > > >> > > > >> > Ultimately all constructors call the full argument constructor via > > this() > > >> > passing the parent as null as appropriate. > > >> > > >> > > >> That sounds great. > > >> > > >> > > >> > This is the general approach I'm taking for delegating to the parent > > >> > resources: > > >> > > > >> > Previous: > > >> > > > >> > > public String getString(String key) { > > >> > > return JSONSerializer.getString(resourceMap, key); > > >> > > } > > >> > > > > >> > > > >> > New: > > >> > > > >> > > public String getString(String key) { > > >> > > String s = JSONSerializer.getString(resourceMap, key); > > >> > > if (s == null && parent != null) { > > >> > > return parent.getString(key); > > >> > > } > > >> > > return s; > > >> > > } > > >> > > > > >> > > >> > > >> Looks good to me. > > >> > > >> Have written a test which confirms one of the new constructors and > > >> > getString() but plan to add more tests to cover all the constructors > > and > > >> > each of the getXXX types. > > >> > > > >> > Do you need anything more than that, Todd? Does any of the WTK code > > need > > >> > to > > >> > change to support your requirement? > > >> > > >> > > >> Yeah, there needs to be a small change to WTKXSerializer, where we > deal > > >> with > > >> <wtkx:include>. Right now, the logic is that we pass our existing > > >> resources > > >> down to the include serializer, unless a resources="..." attribute was > > >> found > > >> in our include tag, in which case we *replace* the resources that's > > passed > > >> to the include serializer with the specified resources. The logic > > should > > >> change such that instantiate a new resources with our existing > resources > > >> *as > > >> its parent*, then use that new resources to pass to the include > > serializer. > > >> > > >> However, I think Greg's making a somewhat significant change to > > >> WTKXSerializer right now, so you could commit the rest first, then > > update > > >> WTKXSerializer after you see Greg's update. > > >> > > >> -T > > >> > > > > > > > > >
