myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From simon <simon.kitch...@chello.at>
Subject Re: myfaces-builder-plugin notes
Date Fri, 13 Jun 2008 06:52:16 GMT

>         
>         (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.

Sorry, but I don't understand.

I see that the QDoxModelBuilder still looks for "superClass" as an
attribute on the JSFComponent tag, and the annotation class in
myfaces-builder-annotations still has the attributes. Is it ok if I
remove these or not?


>  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());
>             }
>         }

When in template mode, isn't the parent-class for the generated class
just the nearest ancestor class that is annotated with @JSFComponent? ie
the template would use:
  $clazz.parentClassName
in the "extends" clause when generating code, and superClassName would
not be used anywhere.

ClassMeta.parentClassName is set up in QDoxModelBuilder.initAncestry,
and I don't currently see any reason why we would want to use anything
else, or provide any override for that.

What am I missing?

> 
> 
>         
>         (3)
>         I'd like to clean up JSFJspProperty, as it doesn't feel quite
>         right t
>         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.

Ok, I've been looking into this (although I've not got much spare time,
so it's not rapid progress).


In Tomahawk, class AbstractHtmlDataScroller has these annotations:
 * @JSFJspProperty name = "onkeydown" tagExcluded = "true"
 * @JSFJspProperty name = "onkeypress" tagExcluded = "true"
 * @JSFJspProperty name = "onkeyup" tagExcluded = "true"
 * @JSFJspProperty name = "onmousedown" tagExcluded = "true"
 * @JSFJspProperty name = "onmousemove" tagExcluded = "true"
 * @JSFJspProperty name = "onmouseout" tagExcluded = "true"
 * @JSFJspProperty name = "onmouseover" tagExcluded = "true"
 * @JSFJspProperty name = "onmouseup" tagExcluded = "true"

I can't currently figure out what these are for. These properties are in
the Attributes map, so they don't need to be referenced in
saveState/restoreState. And because tagExcluded is true, they are not
written into the tld.

So what do these annotations do?

>         
>         == tagExcluded
>         
>         I'm also trying to figure out exactly what "tagExcluded=true"
>         means on
>         JSFJspProperty, and whether we can get rid of it.

> 
> 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)

As you've probably seen, I've defined getter/setter methods for id and
rendered on the relevant classes, and used
  @JSFProperty tagExcluded=true
on them.

Binding does indeed need to be excluded via
  JSFJspProperty tagExcluded=true
but I think that's a fine example of my proposed renaming of tagExcluded
to excludeInheritedAttributeHack. If some components don't support
binding (and it makes no sense for UIViewRoot) then binding should not
be a feature defined on the base class from which UIViewRoot inherits.
Ok, we need to support it, but we don't need to pretend it is correct.

>  
>         
>         === 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.

Ok, this appears to be something that changed between Myfaces 1.1.x and
1.2.x. For example, UIMessage has _showDetail and _showSummary members.

In 1.1.x, they are just kept as Boolean objects, and no converting is
needed when saving or restoring state. Determining whether they are set
is just a matter of testing them for null.

In 1.2.x somebody has made them primitives, and added _showDetailSet,
_showSummarySet flags too. Then stored these Set flags in the state too.
I wonder why this was done; it doesn't make any sense to me at all. It
appears to be a worse solution in all situations except where the
property is being very frequently assigned to (and is not a boolean)
which would require wrapper Objects to be created.

Even if the set flag approach is kept on the component class, there is
absolutely no reason why two objects need to be added to the state
array; storing a null for the property state implies that set=false.

So I would suggest updating the template to generate sane
saveState/restoreState implementations, which will also simplify the
template significantly.

Did you understand my point about a possible bug here? When computing
the array size, tagExcluded is checked But when iterating over the
fields to insert into the state, tagExcluded is not checked. At least I
think that is what is happening. So the numer of elements written into
the array might not be the same as the size of the array. 

Regards,
Simon



Mime
View raw message