On Tue, Jun 10, 2008 at 3:05 PM, simon <[EMAIL PROTECTED]> wrote:
> Hi,
>
> I've found a couple of free hours for myfaces, and have some more stuff
> for discussion re the builder plugin:
>
> (1)
> trunk unit tests fail:
> Tests in error:
> testGetClientIdFacesContext(javax.faces.component.UIComponentBaseTest)
> testFindComponentString(javax.faces.component.UIComponentBaseTest)
>
> I'll try to find time to look into this.
>
Here I'm not have any problem. Maybe the problem is that
myfaces-builder-plugin and myfaces-builder-annotations snapshot is not
automatically build every day, so it is necessary do a manual deploy each
time something change (I forget to do this after the last commit).
>
> (2)
> Now that the "template" property has been added to JSFComponent, can the
> following be deleted?
> parent
> superClass
>
Yes, I did a revision and deleted the usage of this attributes (front end).
But this properties right now are used inside myfaces-builder-plugin, since
the concept is valid from the backend. The code that do the magic of
template is this:
Boolean template = getBoolean(clazz,"template",props, null);
// If the componentClass is not the same as the class with
// @JSFComponent annotation, the component class is generated
if (!clazz.getFullyQualifiedName().equals(componentClass)){
if (!(template != null && template.booleanValue()))
{
//If template is false or null, the superClass of the
generated
//class is the same class, otherwise is the same as the
parent
//class.
superClassName =
getString(clazz,"superClass",props,clazz.getFullyQualifiedName());
}
}
> (3)
> I'd like to clean up JSFJspProperty, as it doesn't feel quite right to
> me.
>
> == rename it?
>
> Firstly, how about renaming it to JSFAttribute, and documenting it as
> something like:
>
> <doc>
> Defines a logical property of a component that is accessed via the
> component's Attributes map, rather than javaBean getter/setter methods.
> </doc>
>
Maybe, but we need to check the usage of JSFJspProperty and JSFAttribute and
check if this is
compatible or not.
>
> This gets away from this jsp-specific emphasis; this data is relevant
> for all sorts of technologies other than jsp.
>
> == tagExcluded
>
> I'm also trying to figure out exactly what "tagExcluded=true" means on
> JSFJspProperty, and whether we can get rid of it.
>
> We managed to get rid of it for the "rendered" and "id" properties that
> some components (esp. UIViewRoot) inherit from their parent class due to
> broken OO design in the JSF spec, by simply implementing the appropriate
> methods. But obviously that won't work for getting rid of bad
> JSFJspProperty (aka JSFAttribute) declarations that exist on the parent
> class but not on the child.
>
> So I guess we do need some kind of annotation to say "please break the
> OO rules and don't inherit the setting that was on the parent". But I
> think we should make the name as ugly as possible to deter people from
> using it, eg
> @JSFAttribute excludeInheritedAttributeHack=true
>
Really, id, binding and renderer are the only properties that sometimes uses
extensively
@JSFJspProperty tagExcluded=true, because override it could cause problems
or be imposible (binding)
>
> I see that Tomahawk uses the tagExcluded feature a lot at the moment.
> This really does feel wrong. I'll try to look into this, but as I
> mentioned before, if the problem is that some Tomahawk components have
> broken inheritance models then I would rather refactor those components
> than hack the builder plugin. Of course we can postpone that refactoring
> until after the next tomahawk release is out; I'm just saying that I
> don't see widespread use of this excludeInheritedAttributeHack as the
> right solution.
>
Sure, we can take a look at this on the next release.
>
> === tagExcluded in macro files
>
> I see tagExcluded is used in componentClass11.vm:
>
> #if($utils.isPrimitiveClass($type) && !$property.tagExcluded)
> #set ($arrayIndex = $arrayIndex + 1)
> values[$arrayIndex] = ${field}Set;
> #end
>
> Is this ${field}Set thing a Trinidad-specific feature?
>
> Incidentally, I think there *might* be a bug here: the tagExcluded test
> is done here, but not at the point a dozen lines earlier when the size
> of the values array is being computed. But I'm not entirely clear what
> is happening here, so might be wrong.
>
The code for this template right now is this:
#if($utils.isPrimitiveClass($type) && !$property.isTagExcluded() )
#set ($arrayIndex = $arrayIndex + 1)
${field}Set = ((Boolean) values[$arrayIndex]).booleanValue();
#end
It is used as an additional variable for primitive values. Example:
public void restoreState(FacesContext facesContext, Object state)
{
Object[] values = (Object[])state;
super.restoreState(facesContext,values[0]);
_actionFor = (java.lang.String) values[1];
_disabled = ((Boolean) values[2]).booleanValue();
_disabledSet = ((Boolean) values[3]).booleanValue();
disable holds the value but disableSet check if the property has been
changed programatically. myfaces-faces-plugin do this generation, so in
myfaces-builder-plugin I follow this approach. Look this:
public boolean isDisabled()
{
if (_disabledSet)
{
return _disabled;
}
ValueBinding vb = getValueBinding("disabled");
if (vb != null)
{
return ((Boolean)
vb.getValue(getFacesContext())).booleanValue();
}
return false;
}
Maybe it would be better to convert primitive types (boolean, int, long) to
the boxed counterparts (Boolean, Integer, Long) and use just one variable
(like it was before on myfaces 1.1 and tomahawk), but I'm not applied this,
because requires some changes on what we are doing right now.
Suggestions are welcome
regards
Leonardo Uribe
>
> Comments?
>
> Cheers,
> Simon
>
>
