On 6/9/06, Johan Compagner <[EMAIL PROTECTED]> wrote:
> What you describe is exactly what i said
> that MarkupFragment is the ComponentTag (with the XmlTag) inside it.
not exactly. It is more like Markup.java which contains the start tag,
for containers all MarkupElements of the body and the close tag. The
reason why I prefer the Markup is because we'll need it for the render
phase anyway. The open tag is always the first MarkupElement and
easily be accessed through Markup. And IMO
Component.getMarkup().getTag() wouldn't clutter the Component API as
much. Just one single method which MarkupContainers already have.
> But those are not mutable because then you would alter the markup file
> itself which is bad.
thats true. But I'd much have that code outside Component. Component
already is a monster (not complicated but big). Ok, lets say we call
it MarkupFragmentStream (similar to the naming we currently have) and
the Stream is Component specific and refers the MarkupFragment. Than
MFS could handle the case where we need a copy of the start tag and
its parameters because any of the parameters must be modified. At
least the code is out of Component than.
>
> And i don't want to store (transient is not possible if the
> Tag/MarkupFragment where mutable!)
> the Tag/MarkupFragment completely in every component. Because that would
> mean that the 2 objects
> (ComponentTag and its child XmlTag) where cloned and hold on to for every
> component.
see above.
>
> My current implementation is. If the xmltag had attributes then i make a
> mutable copy of that
> and only that map is stored in the component. And that map is used in the
> render phase
for the reasons mentioned in the original mail, I do not prefer that
solution. IMO is half baked. We need the Markup / MarkupStream for
render anyway.
> So SimpleAttributeModifiers (just String-String things) can be removed now.
> Because you can just do
> getMarkupAttributes().put("attributename",
> "attributevalue")
>
> We need that attributemap anyway in the constructor. Because in 2.0 we
> should be able to get the markup
> attributes (like markupid/cssid) inside the constructor of the component.
> And we want to check as early as possible
> if the component can be added to the parent (if it has a component tag on
> that level in the markup)
>
that is not different in either approaches. As I mentioned in my first
response that is what is needed. I just don't agree with the
ComponentTag being added to the Component. I'd rather like to have the
MarkupFragmentStream (or whatever you want to call it). As the tag and
the markup are need heavily afterwards anyway.
> I just go now through the MarkupFragmentFinder in the constructor to check
> for the component tag
> and to get the attributes if there are attributes:
>
In case someone implemented a resolver, he now has to make sure it is
synchronized (reverse algorithm) with the one he'll for the the
MarkupFinder. That is awkward and should be avoided.
> This is what the Component base constructor looks like now:
>
>
>
> public Component(MarkupContainer<?> parent, final String id, final
> IModel<T> model)
> {
> if (parent == null)
> {
> if (!(this instanceof Page))
> {
> throw new WicketRuntimeException("component
> without a parent is not allowed.");
> }
> }
> this.parent = parent;
> setId(id);
> this.model = model;
>
>
> getApplication().notifyComponentInstantiationListeners(this);
> if (id.startsWith(AUTO_COMPONENT_PREFIX))
> {
> parent.autoAdd(this);
> }
> else
>
> {
> try
> {
> MarkupStream markupStream = MarkupFragmentFinder.find(this);
> ComponentTag tag = markupStream.getTag();
> if(tag.hasAttributes())
> {
> markupAttributes = new ValueMap(tag.getAttributes());
>
> }
> }
> catch(RuntimeException re)
> {
> throw new WicketRuntimeException("Couldn't
> find the markup of the component " + id + " in parent " +
> parent.getPageRelativePath());
> }
> parent.add(this);
> }
> }
>
> Can we optimize that find() a bit?
> For example the markup just has a map as far as i know of componentpath ->
> index of the tag? That looks fast to me..
> What is slow there?
>
I think it is the wrong apporach. As mentioned above is not only
finding it, it is a) about often do you need to call it to find the
fragment during the whole cycle and not only in the constructor and
2nd it must be more flexible. The algorithm to find the associated
fragment must be replaceble for users who are e.g. using flat
structures compared to hierarchical ones
Juergen
> johan
>
>
>
> johan
>
>
>
> On 6/8/06, Juergen Donnerstag <[EMAIL PROTECTED] > wrote:
> >
> Hopefully the javadoc still says that getMarkupAttribute() is not very
> efficient and that it should only be used in rare cases. The reason
> why is because finding the tag in the markup is yet very inefficient
> compared to using the tag parameter provided to many render XXX
> functions. But I agree that in general the approach is what we want,
> IMO just not the tag but the MarkupFragment (the piece of Markup that
> is associated with the component). And because the Container (not the
> Component though) does already have a MarkupStream, you wouldn't need
> an additional variable. More importantly may be, MarkupFragment could
> be transient as you only need to ask the markupcache to get a new
> copy. MarkupFragment provides you access to the tag which than can be
> modified. And at load time the MarkupFragment would be created and
> kept in a meaningful structure (cache) to find it more efficiently.
> The current implement is far from perfect. One more note: currently we
> use resolvers which allow to augment or replace the strictly
> hierarchical order of tags e.g. with a flat one (scan the mail archive
> for people doing that). We'd need a similar flexibility to find the
> tag in the markup. IMO that would be a more complete approach but it'd
> be much more work.
> As a side note: I'm not especially proud of that renderComponent code
> everything that is related to it. It was more a quick hack to have a
> fast result rather than an well thougt through approach. It was meant
> to be replaced in 2.0 with what I described above. IMO we shouldn't
> modify it a lot. It is like hacking a hack.
>
> Juergen
>
> On 6/8/06, Johan Compagner <[EMAIL PROTECTED]> wrote:
> > Hi,
> >
> > i am looking at this code inside Component:
> >
> > public final ValueMap getMarkupAttributes()
> > {
> > MarkupStream markupStream = MarkupFragmentFinder.find(this);
> > ValueMap attrs = new ValueMap(
> > markupStream.getTag().getAttributes());
> > attrs.makeImmutable ();
> > return attrs;
> > }
> >
> > and:
> >
> > public final void renderComponent(final MarkupStream markupStream)
> > {
> > this.markupIndex = markupStream.getCurrentIndex ();
> >
> > // Get mutable copy of next tag
> > final ComponentTag openTag = markupStream.getTag();
> > final ComponentTag tag = openTag.mutable();
> >
> > // Call any tag handler
> > onComponentTag(tag);
> >
> >
> > What i would like to do is change this behaviour and let the Component
> > creates a copy of there own Tag inside itself (at construct time)
> >
> > so that getMarkupAttributes() is not immutable anymore.
> > you can just do:
> >
> > mylabel.getMarkupAttributes().put("class", "mycssclass");
> >
> > instead of making a attribute modifer object with attribtue "class" and
> > value "mycssclass" and then add that as a behaviour/attributemodifier on
> the
> > component.
> >
> > so i want something like this: (all on Component)
> >
> >
> > public Component(MarkupContainer<?> parent, final String id, final
> > IModel<T> model)
> > {
> > if (parent == null)
> > {
> > if (!(this instanceof Page))
> > {
> > throw new
> WicketRuntimeException("component
> > without a parent is not allowed.");
> > }
> > }
> > this.parent = parent;
> > setId(id);
> >
> > try
> > {
> > MarkupStream markupStream = MarkupFragmentFinder.find(this);
> > tag = markupStream.getTag().mutable(); // GET the Tag out of
> the
> > markup and make a copy.
> > }
> > catch(RuntimeException re)
> > {
> > throw new WicketRuntimeException("Couldn't
> find
> > the markup of the component " + id + " in parent " +
> > parent.getPageRelativePath ());
> > }
> > ........
> >
> >
> > ComponentTag getTag() // Return the tag. Maybe not needed this
> method..
> > {
> > return tag;
> > }
> >
> > public final ValueMap getMarkupAttributes()
> > {
> > return tag.getAttributes(); // Directly return the mutable tags
> > attributes
> > }
> >
> > public final void renderComponent(final MarkupStream markupStream)
> > {
> > this.markupIndex = markupStream.getCurrentIndex();
> >
> > final ComponentTag tag = getTag(); // Just use the mutable tag
> > attributes.
> >
> > // Call any tag handler
> > onComponentTag(tag);
> > .................
> >
> >
> > The only thing is that we then hold a reference to a Tag in every
> > component..
> > We don't need some AttributeModifiers then because you can do that
> directly.
> > And we don't generate constant copies..
> >
> > what we also could do is not get a copy of the tag but just the attributes
> > map.
> > Then we don't hold the complete xml tree as a copy in mem.
> >
> > Then we just need to set those attributes in the component tag at render
> > time.
> > i guess this would be even much better.
> >
> > any other ideas?
> >
> > johan
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > _______________________________________________
> > Wicket-develop mailing list
> > Wicket-develop@lists.sourceforge.net
> >
> https://lists.sourceforge.net/lists/listinfo/wicket-develop
> >
> >
> >
>
>
> _______________________________________________
>
> Wicket-develop mailing list
> Wicket-develop@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/wicket-develop
>
>
>
>
> _______________________________________________
> Wicket-develop mailing list
> Wicket-develop@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/wicket-develop
>
>
>
_______________________________________________
Wicket-develop mailing list
Wicket-develop@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/wicket-develop
