On Thursday 15 May 2003 07:56, Conor MacNeill wrote: > On Thu, 15 May 2003 12:56 am, peter reilly wrote: > > I have merged the ant-type code into my antlib code. > > However it uses a "magic" attribute name "ant-type" to > > achieve the effect and not as discussed before - the > > namesspaced attribute name like - "ant:type". > > > > I can easily do a name-spaced attribute name for this, > > however if this is done there will be only one object in > > the xml name space uri - type. Would this be reasonable? > > I would prefer to use the XML schema attribute for this.
Mmmm, this would be confusing as the XML schema attribute has an associated URI "http://www.w3.org/2001/XMLSchema"; which implies a lot more that a reference to an ant type. > > I have done a quick review of you proposal. I wonder if we can split this > into smaller chunks to make it easier to understand and review. For > example, the onError stuff could be split out, as could the URL handling > code for separate consideration. Smaller chunks are easier to handle. This is true, but difficult to do. Some of the implementations of the different features change/improve if other features are present. For example the implementation of "onerror" uses the new anttypedefintion class. The implementation of the psuedo task "antlib" uses the add(Type) mechanism rather than explicity stating addTypedef and addTaskdef, this allowed other tasks that extend Definer to used in the antlib task (for example a definer that wraps Runnable objects as ant tasks, or a future implemation of roles). Also from a practical point of view, I find it difficult to maintain multiple patched ant versions. > > Anyway, it seems to me that you have combined the namespace URI and element > names in certain places. Examples: In component helper changes, for method > createComponent, you say > > the name of the component, if > the component is in a namespace, the > name is prefixed withe the namespace uri > dropping the "antlib:" and adding ":". > > Why not pass the URI and local name to the component helper and not have to > parse it out in componentHelper. Your approach also precludes URIs that > contain ':' (I know you disallow these elsewhere but I don't see any reason > to combine these, anyway) Initially I was going to do this, but here is a lot of places in the code that assume that a task/type is mapped from a string and not from the tuple {ns uri, local name}. J.Pietschmann suggested in http://marc.theaimsgroup.com/?l=ant-dev&m=105233644225128&w=2 that one can use a mapping of {uri, name} to a string ns-uri+"^"+name when migrating a project to namespaced XML. My current mapping is not good as it drops the antlib: prefix and thus excludes other uri prefixes, so I will change this. The current code does exclude URI that contain ":", this is a combination of a bug and by design. The bug is that I should have used lastindexof and the design is that the code only deals with NS URIs that start with "antlib:" for type/task definitions. The code for the mapping should be done in one place (maybe as a static method in ProjectHelper). > > I'm not sure where TaskAdapter went. createNewTask seems to return null if > the class is not a Task - probably handled somewhere else. This is by design and for BC reasons. If the type class is not a Task, and the type does not have an Adapter to a Task class, the CM#createNewTask() will not create the class. <taskdef/> will does this (for example the condition task), and the unit tests contain an adapter for Runnable objects. The code in CM does not treat TaskAdapter as a special case. > I'm not sure about addType methods. It usually limits only one element to > being extensible - consider a task taking two filesets for different > reasons. OTOH, I guess a lot can be done with it. The add[Configured](Type) methods are meant more for container like objects - like the antlib task, the filterchain type or the condition base class. It also allows custom extensions (e.g. new conditions) to be dropped in by third party classes without special treatment. The polymorphic feature is I think to be used for fixed elements in objects that are not containers - the javac <src/> nested element for example. <javac...> <src ant-type="..." .../> (or <src poly:type="..." .. xmlns:poly="<ant poly uri>"/>) </javac> > The element names in > IntrospectionHelper would need to have URIs as well, won't they? UnknownElement converts the {uri,name} to a "componentname" before calling IH#createElement() > > For the most part it looks OK to me. I'd need to look more closely to fully > comprehend it but thought you might like some feedback. > Thanks, and cheers. Peter
