Macro resolving issues

0 views
Skip to first unread message

Hannes Wallnoefer

unread,
Dec 18, 2008, 5:51:17 AM12/18/08
to Helma
Warning: long and winding posting ahead. It's a really important
issue, though.

It took some time for me to realize that the fix I committed for bug
617 [1] had some unintended effects, and we'll need to fix macro
handler lookup again for 1.7, and possibly 1.6.4.

[1] http://helma.org/bugs/show_bug.cgi?id=617

The problem is that the fix was intended to only change macro handler
lookup in the parent objects of the this-object, but in fact it also
affects the this-object itself. Thus, in helma 1.6.3 and svn trunk, a
macro handler only resolves to the this-object if the handler name is
either "this" or the exact prototype name of the this-object.
Inherited prototypes are not resolved to the this-object, which broke
at least a few places in Gobi, and probably other apps to.

So one more effort is needed to fix this problem deeply, and in the
process define macro handler resolution (behavior has always been
foggy, to say the least).

I start with describing the old (pre-1.6.3) behavior, then the current
(1.6.3/trunk) behavior, and then what options we have to get
consistent and predictable behavior in the future.

In releases propr to 1.6.3, macro handlers were always resolved
against the this-object and all parent objects of the this object,
including inherited prototype names in the process. Only then did the
code check res.handlers for registered macro handlers (pre-populated
with the objects in the request path).

The (undocumented) rationale for this behavior was that it should be
possible to (temporarily) override res.handlers when calling a macro
on an object not in the request path, and have it behave like it was
in the request path (including its parent objects).

As I said, the behaviour in 1.6.3 and svn trunk is to only resolve
against the this-object and its parents if the prototype name matches
exactly, which is a kind of weird middle-of-the-road behaviour. It
takes something of the old "implicit-this-object-as-handler" idea but
only implements it half-ways. I don't think it's a solution we should
stick with.

Finally, the options we have for fixing this.

The first one, which I'll call the raw-and-simple solution, is to drop
the implicit-this-object-as-handler idea altogether and only resolve
this-macros against the this-object, everything else against
res.handlers. This would be simple and transparent behaviour. The
downside of this solution would be that existing applications would
have to be rewritten (replacing prototype names with "this" where
skins are invoked on objects not in the request path). It would also
make it harder to expose the parent objects of the this-object to
skins where this is really required. I don't think this would be the
case too often.

The second, which I'll call the multi-layered approach, is to break up
the macro resolver process further: First resolve against the this-
object, taking into consideration its inherited prototype names. Than
resolve against res.handlers, which by default contains the request
path keyed by prototype names. Finally, resolve against the this-
objects's parent chain, again including inherited prototype names.
This would make the algorithm a bit more complex, but it would
probably be more intuitive and natural, and would require less changes
to existing code than the raw-and simple solution.

In case we choose the raw-and-simple solution we could also find some
tricks to make it simpler to temporarily override res.handlers with
the this-object's parent path. One idea is to add a boolean argument
to the renderSkin* methods that indicates whether the this-object's
parent chain should override res.handlers: moo.renderSkin("foo",
params, true); would then resolve macros against moo and its parent
objects before checking res.handlers.

If you followed me through here: thanks and congratulations! I suggest
we may talk about this on the #helma IRC channel today. I'm online and
waiting for your comments and suggestions.

Hannes

fpm...@gmail.com

unread,
Dec 19, 2008, 6:13:31 AM12/19/08
to Helma
Thank you for your explanations, now I understand the myths of
macrohandler resolving.

> Finally, the options we have for fixing this.
>
> The first one, which I'll call the raw-and-simple solution, is to drop
> the implicit-this-object-as-handler idea altogether and only resolve
> this-macros against the this-object, everything else against
> res.handlers. This would be simple and transparent behaviour. The
> downside of this solution would be that existing applications would
> have to be rewritten (replacing prototype names with "this" where
> skins are invoked on objects not in the request path). It would also
> make it harder to expose the parent objects of the this-object to
> skins where this is really required. I don't think this would be the
> case too often.

-99 sorry this is not an option and is not realy OO style.

> The second, which I'll call the multi-layered approach, is to break up
> the macro resolver process further: First resolve against the this-
> object, taking into consideration its inherited prototype names. Than
> resolve against res.handlers, which by default contains the request
> path keyed by prototype names. Finally, resolve against the this-
> objects's parent chain, again including inherited prototype names.
> This would make the algorithm a bit more complex, but it would
> probably be more intuitive and natural, and would require less changes
> to existing code than the raw-and simple solution.

-1 for that to. Again it's not very OOPish.

IMHO we have two cases of macros:

1) there is a macro <% this.foo %>: this should only resolve to the
current object and then going down the _extend chain and nothing else.
f.e.: this macro is called in Comment so look in Comment Prototype
for the macro, if not found look if Comment extends an other Prototype
f.e.: Story than look in Story for the foo_macro. Going down the
prototype chain. And so on but never resolve agains res.handlers.

2) there are macros like <% story.foo %>: This is the interesting
part.
I think this should resolve the following way:

first look in res.handlers.story
second (if not found in res.handlers) lookup current object where
renderSkin is called if it's prototype is story or an extending Object
like Comment.
Than resolve it down the _extend chain

I too think that res.handlers should not be populated by default
req.path objects because this leads to strange behavior. Maybe there
should be a object path in res.handlers where these object will be
stored f.e.: <% path.story.title %>. I don't know if this leads to any
limitations, but I like the way of explicit call an object from the
req.path.

> In case we choose the raw-and-simple solution we could also find some
> tricks to make it simpler to temporarily override res.handlers with
> the this-object's parent path. One idea is to add a boolean argument
> to the renderSkin* methods that indicates whether the this-object's
> parent chain should override res.handlers: moo.renderSkin("foo",
> params, true); would then resolve macros against moo and its parent
> objects before checking res.handlers.

Imho res.handlers should be the first to be checked if not this. is
used. Adding a param to renderSkin, hmm, don't think this would help.

> If you followed me through here: thanks and congratulations! I suggest
> we may talk about this on the #helma IRC channel today. I'm online and
> waiting for your comments and suggestions.

Thanxs to you for writing such a long post. I too think this should go
to helma 1.7 because it is a massive change.

> Hannes

Philipp

Hannes Wallnoefer

unread,
Dec 19, 2008, 9:06:00 AM12/19/08
to Helma
On Dec 19, 12:13 pm, "fpm...@gmail.com" <fpm...@gmail.com> wrote:
> On 18 Dez., 11:51, Hannes Wallnoefer <hann...@gmail.com> wrote:
>
> > Finally, the options we have for fixing this.
>
> > The first one, which I'll call the raw-and-simple solution, is to drop
> > the implicit-this-object-as-handler idea altogether and only resolve
> > this-macros against the this-object, everything else against
> > res.handlers. This would be simple and transparent behaviour. The
> > downside of this solution would be that existing applications would
> > have to be rewritten (replacing prototype names with "this" where
> > skins are invoked on objects not in the request path). It would also
> > make it harder to expose the parent objects of the this-object to
> > skins where this is really required. I don't think this would be the
> > case too often.
>
> -99 sorry this is not an option and is not realy OO style.

I don't understand: how does this relate to OO style? and why would it
be bad not to follow OO style?

> > The second, which I'll call the multi-layered approach, is to break up
> > the macro resolver process further: First resolve against the this-
> > object, taking into consideration its inherited prototype names. Than
> > resolve against res.handlers, which by default contains the request
> > path keyed by prototype names. Finally, resolve against the this-
> > objects's parent chain, again including inherited prototype names.
> > This would make the algorithm a bit more complex, but it would
> > probably be more intuitive and natural, and would require less changes
> > to existing code than the raw-and simple solution.
>
> -1 for that to. Again it's not very OOPish.

Again, I don't understand your objection here.

> IMHO we have two cases of macros:
>
> 1) there is a macro <% this.foo %>: this should only resolve to the
> current object and then going down the _extend chain and nothing else.
>  f.e.: this macro is called in Comment so look in Comment Prototype
> for the macro, if not found look if Comment extends an other Prototype
> f.e.: Story than look in Story for the foo_macro. Going down the
> prototype chain. And so on but never resolve agains res.handlers.
>
> 2) there are macros like <% story.foo %>: This is the interesting
> part.
> I think this should resolve the following way:

Yes, these are the two basic cases we have to cover and reconcile.

> first look in res.handlers.story
> second (if not found in res.handlers) lookup current object where
> renderSkin is called if it's prototype is story or an extending Object
> like Comment.
> Than resolve it down the _extend chain

I don't understand this - how would you explain this behaviour of
looking at the this-object after res.handlers?

> I too think that res.handlers should not be populated by default
> req.path objects because this leads to strange behavior. Maybe there
> should be a object path in res.handlers where these object will be
> stored f.e.: <% path.story.title %>. I don't know if this leads to any
> limitations, but I like the way of explicit call an object from the
> req.path.

I think that's off-limits. This would break practically every Helma
application in existence. And I really think that it's very smart to
have res.handlers populated with the path objects. Can you give some
explanation or examples to underpin your proposal?

> > In case we choose the raw-and-simple solution we could also find some
> > tricks to make it simpler to temporarily override res.handlers with
> > the this-object's parent path. One idea is to add a boolean argument
> > to the renderSkin* methods that indicates whether the this-object's
> > parent chain should override res.handlers: moo.renderSkin("foo",
> > params, true); would then resolve macros against moo and its parent
> > objects before checking res.handlers.
>
> Imho res.handlers should be the first to be checked if not this. is
> used. Adding a param to renderSkin, hmm, don't think this would help.

The problem with res.handlers is that it is very cumbersome to
manipulate on a per-render basis. It is global data, so you have to
make sure you reset it to its previous state after you are finished
rendering your skin. If we want to always enforce explicit definition
of macro handlers, it would be much smarter to pass the macro handlers
directly to the renderSkin methods (like the param object, but without
requiring the param handler prefix in macros). I actually was going to
propose that as a way to override res.handlers, but dropped the idea
because changing the behaviour of the renderSkin param argument would
break apps. But maybe we shoul bite the bullet and choose that option.

Hannes

p3k

unread,
Dec 19, 2008, 9:06:24 AM12/19/08
to Helma
hi hannes

i think the issue you described occurred with Antville, too; I solved
it by replacing some macro handlers of skins of extended prototypes
with "this".

it did not occur to me that the current way poses any misconceptions
(except the one for extended prototypes), it looks quite consistent to
me.

nevertheless, i might have been become uesd to helma's macro resolving
features in the meantime...

generally, i'd give +1 for a change that does not break antville or
makes me to rewrite parts of it (of course).

and of course, i am +2 for leaving this issue as is. (sorry.)

unfortunately, i have to pop off right now, thus, won't be available
in irc.
:(

good luck,
tobi

Hannes Wallnoefer

unread,
Dec 19, 2008, 9:17:15 AM12/19/08
to he...@googlegroups.com
On Fri, Dec 19, 2008 at 3:06 PM, p3k <inte...@p3k.org> wrote:
>
> hi hannes
>
> i think the issue you described occurred with Antville, too; I solved
> it by replacing some macro handlers of skins of extended prototypes
> with "this".
>
> it did not occur to me that the current way poses any misconceptions
> (except the one for extended prototypes), it looks quite consistent to
> me.

I don't think so. I've seen a good bit of confusion caused by it, and
I've been confused myself.

> nevertheless, i might have been become uesd to helma's macro resolving
> features in the meantime...
>
> generally, i'd give +1 for a change that does not break antville or
> makes me to rewrite parts of it (of course).
>
> and of course, i am +2 for leaving this issue as is. (sorry.)

Not breaking apps is important in the 1.6 branch, so it's likely we'll
not introduce big changes here. But for Helma 1.7, I'd like to come up
with a solution that provides more transparency.

hannes

Franz Philipp Moser

unread,
Jan 7, 2009, 5:58:31 AM1/7/09
to he...@googlegroups.com
Hi list,

sorry for this late reply.

On this whole issue my thoughts again:

The current implementation is ok with one small drawback. You can't
overwrite a macro handler something like this

<% foo.bar %>

res.handlers.foo = new foo();

if its in the request path. Maybe its possible to configure this in
app.properties: resHandlersOverwrites = true, so the res.handlers are
the first things that resolve. There may be a better name for the
property for resHandlersOverwrites ;)

This I think should be the only fixes needed.

Forget about OOP and the other things.

cu Philipp

2008/12/19 Hannes Wallnoefer <han...@gmail.com>:

Hannes Wallnoefer

unread,
Jan 12, 2009, 5:17:22 AM1/12/09
to Helma
In lack of more feedback, I'll probably implement the second solution
which I dubbed "multi-layered approach". This is simply the approach
that is likely to breal the least applications, both old and new/
adapted.

On Dec 18 2008, 11:51 am, Hannes Wallnoefer <hann...@gmail.com> wrote:
> The second, which I'll call the multi-layered approach, is to break up
> the macro resolver process further: First resolve against the this-
> object, taking into consideration its inherited prototype names. Than
> resolve against res.handlers, which by default contains the request
> path keyed by prototype names. Finally, resolve against the this-
> objects's parent chain, again including inherited prototype names.
> This would make the algorithm a bit more complex, but it would
> probably be more intuitive and natural, and would require less changes
> to existing code than the raw-and simple solution.
>

In case you disagree, you still have a few days to voice your opinion.
I'm not in a hurry with this.

Hannes

Maksim Lin

unread,
Jan 14, 2009, 5:38:33 PM1/14/09
to he...@googlegroups.com
Hi Hannes,

Sorry to have taken so long to comment on this, but I actually hadn't
realised (prior to your post) how complex macro resolution is and I'm
still not quite sure I do completely, hence my comments below also
have a few questions in them.

With resolution of "this" handler, I think it should continue to
resolve to both the current object and the prototypes it extends. I
was actually unaware that pre1.6.3 "this" would also resolve to all of
the current objects parent prototypes. I am assuming when you say
"parents" you mean that if we have a url like:

/authors/2/books/5/details

and the details action renders a skin on the book object and the skin
includes a macro:

<% this.showname %>

then the pre1.6.3 behaviour would have resolved the firstname macro to
a macro showname_macro in the Author prototype because it happens to
be the parent of Book objects, is that correct?

If the above is true, then I'd actually be happy for that behaviour to
Not be re-introduced post1.6.3 as I think its confusing and the
documentation in (http://helma.org/docs/guide/framework/) which says:

"...In our this.name example, we got acquainted with the this macro
handler that always refers to the object that is currently rendering
the skin. Outside the special case, macro handlers are usually
specified by their prototype name and resolved against the request
path. This means that a macro handler "Foo" will be resolved to the
object with prototype "Foo" in the request path. If this applies to
more than one object in the request path, the last (right most) object
is chosen. The place where Helma registers objects in the request path
as macro handlers is the res.handlers object..."

Makes it hard to understand that it is describing the behaviour of
"this" macros resolving to parent objects.
Ahen I read the above docs, I took it to mean that if I wanted to
access a macro in a books author parent object prototype, I could only
do it using :
<% author.showname %>

Also what I find confusing is the fact that the request path objects
are automatically added to res.handlers keyed on the prototype name of
each object, so why would you want to do lookups in the currents
objects "parents chain" using a "this" macro instead of using a "named
prototype" macro? Or am I missing an important use case for that
scenario?

Resolving macros against inherited prototypes using the "this" macro
on the otherhand makes perfect sense to me and if in the above
example, Author inherited from User prototype and the User prototype
had a macro called showlastlogin then I would expect the following to
work in the above macro:
<% this.showlastlogin %>

and Not having to use:
<% user.showlastlogin %>

In regards to res.handlers overriding the request path, again I'm not
sure if I understand the intended behaviour, but it would be nice to
have the option of overriding objects in the request path so that the
above action could do something like:

res.handlers.author = getAliasFor Author(res.handlers.author);

but to be honest I've not yet come across a neeed to do something like
that in any apps I've written til now, so its really just an idle
thought at this point.


I hope that above makes sense and any corrections to my understanding
of the previous/current/proposed behaviour is most welcome as I am
still slowly working on my project of documenting all this.

thanks,
Maks.

Hannes Wallnoefer

unread,
Jan 15, 2009, 10:15:39 AM1/15/09
to Helma
Hi Maks,

On Jan 14, 11:38 pm, Maksim Lin <maksim....@gmail.com> wrote:
> Hi Hannes,
>
> Sorry to have taken so long to comment on this, but I actually hadn't
> realised (prior to your post) how complex macro resolution is and I'm
> still not quite sure I do completely, hence my comments below also
> have a few questions in them.
>
> With resolution of "this" handler, I think it should continue to
> resolve to both the current object and the prototypes it extends. I
> was actually unaware that pre1.6.3 "this" would also resolve to all of
> the current objects parent prototypes. I am assuming when you say
> "parents" you mean that if we have a url like:

That's a misunderstanding. The "this" macro handler always and only
resolves to the object on which the renderSkin method has been
invoked.

Everything regarding the parents of the this-object only applies to
prototype names, e.g "book" or "author".

I'm sorry if I was unclear in my explanations. As far as I can tell,
the rest of your posting is based on that false assumption that this
may resolve to anything else than the this-object. I hope I cleared
things up now, let me know if you have any further questions.

Hannes

Maksim Lin

unread,
Jan 15, 2009, 5:54:10 PM1/15/09
to he...@googlegroups.com
> That's a misunderstanding. The "this" macro handler always and only
> resolves to the object on which the renderSkin method has been
> invoked.
>
> Everything regarding the parents of the this-object only applies to
> prototype names, e.g "book" or "author".


Thanks for clearing that up Hannes. That makes sense.

> I'm sorry if I was unclear in my explanations. As far as I can tell,
> the rest of your posting is based on that false assumption that this
> may resolve to anything else than the this-object. I hope I cleared
> things up now, let me know if you have any further questions.
>
> Hannes
>

Re-reading your original email I think I now understand the point, but
please correct me if I'm wrong again, but the idea was that if you
rendered a skin against an object, it (used to be) possible to refer
to objects in its parent-chain via their prototype names, is that
correct?

If so then I think that even though that behaviour is kind of handy,
I'd prefer your "raw-and-simple solution" and have macro resolution
only look up objects specifically in res.handlers and then make
available if needed the "override" functionality that you describe for
renderSkin() with the extra parameter.

For myself I can say that because I was unaware of the implicit this's
parent-chain as handlers, I've have not got any apps that make use of
the functionality :-)

thanks,
Maks,

Reply all
Reply to author
Forward
0 new messages