Re: Management of body in PutAttributeModel and AddAttributeModel

[Nicolas Le Bas <ma...@nlebas.net>]

Hi Antonio,

So, we also have this use case where we want to make sure the nested 
definition gets rendered:
https://github.com/apache/tiles/blob/TILES_3_0_X/tiles-test-pom/tiles-test/src/main/webapp/testinsertnesteddefinition_tags.jsp

I agree that trying to defer evaluation here creates all kinds of 
challenges. But instead of creating a new tag, I wonder if we shouldn't 
remove one.

I've never been much of a fan of declaring definitions inline in a JSP, 
and inserting it later. I think, at the top level it is a bad practice. 
The only place where it makes sense to me is inside a putAttribute tag, 
because it allows to defer the evaluation of the attribute.

But then if we defer evaluation anyway, we could use insertTemplate or 
insertDefinition instead. I'd just like to double check about cascading 
attributes, making sure we don't loose the feature in the change.

Do I make any sense?

Nick

PS: if there are no other use cases, I feel it is a minor optimization. 
Inline content (that would be evaluated on "put" instead of "insert") is 
usually small in practice. If it is big, it is easy to externalize it in 
a JSP or another definition, and this would be evaluated upon insert, in 
addition to begin more readable. Do you think the optimization would be 
worth the effort?

On 05/31/2016 08:48 AM, Antonio Petrelli wrote:
> Hi Nicolas, see my answers inline.
>
> 2016-05-31 1:53 GMT+02:00 Nicolas Le Bas <ma...@nlebas.net>:
>
>> Concerning put/add-attribute... I'd like to look at some use cases.
>>
>> There is this one in our selenium test suite:
>>
>> https://github.com/apache/tiles/blob/TILES_3_0_X/tiles-test-pom/tiles-test/src/main/webapp/testinsertdefinition_inline.jsp
>> but I think we should look at something more complex.
>>
> Yes, this is a static string, so nothing important.
>
>
>> What do you think of this one:
>>
>> <c:forEach var="line" items="${orderLines}">
>>      <tiles:insertDefinition name="orderLine">
>>         <tiles:putAttribute name="description">
>>             Item ${line.number}: ${line.product.name} for ${line.price} \u20ac
>>         </tiles:putAttribute>
>>      </tiles:insertDefinition>
>> </c:forEach>
>>
>> Is this the kind of thing you mean when you talk about moving JSPFragments
>> around the request?
>>
> Yes it is.
>
>
>> As for the "body renderer"... I like it, but I don't line the map with a
>> "generated name" in the request.
>> I think we could just put the renderer as the value of the new attribute
>> object that we create in putAttribute?
>>
> The big problem is a bunch of lines in
> BasicTilesContainer.render(Attribute, Request):
>
> <snip>
>              if (!(value instanceof String)) {
>                  throw new CannotRenderException(
>                          "Cannot render an attribute that is not a string,
> toString returns: "
>                                  + value);
>              }
> </snip>
>
> In other words, I cannot render an attribute that is not a string. For this
> reason I thought of a string key -> body map.
>
> Anyway I think we need another tag. This is because putAttribute's body
> must be evaluated when there is a definition inside, so that it can be
> connected to it. A new tag, that puts a body as a value, directly or
> indirectly via a map, must be created to avoid evaluating the body twice.
>
> Antonio
>

Re: Management of body in PutAttributeModel and AddAttributeModel

[Antonio Petrelli <an...@gmail.com>]

Sorry, thinking again on it, I have several constraints for this project
I'm working on:
1. I have to work on 3.0.x branch without modifications;
2. I have to use Java 6
3. Adding code to a 3.1 or 4.0 version of Tiles has no real benefit for me
at this time.
4. What I wanted to do does not give benefit for the community itself.

So I will work on a private project that holds a special renderer and
special tags. Once it is done, I will see if I can donate the code, for you
to take "inspiration" to do the right job.

Sorry for the noise
Antonio

2016-06-02 14:23 GMT+02:00 mck <mc...@apache.org>:

>
> > Moreover, since I am emeritus and I don't know if I will contribute much,
> > what is the best thing to do for me? Should I fork a project in GitHub or
> > resurrect my account for Tiles?
>
> Great to hear from you again Antonio! Hope all is well.
> I think we'd all be happy to see you resurrect your account, regardless
> of how much you contribute.
>
>
> > I think the best thing is to create a
> > "body renderer" that acts like any other renderer. Since the
> > Renderer.render wants a string as a first parameter, I think that a map:
> > "generated name" -> body should be created in request scope.
>
>
> Do we break compatibility by adding Renderer.render(Object, Request) ?
> Is that a possibility forward?
>
> (We do if tiles-request-api is considered an spi, which it actually is.
> This can be solved by introducing it as a default method in the
> interface. That would require Java8, but maybe it's time for that.)
>
> Mick.
>

Re: Management of body in PutAttributeModel and AddAttributeModel

[mck <mc...@apache.org>]

> Moreover, since I am emeritus and I don't know if I will contribute much,
> what is the best thing to do for me? Should I fork a project in GitHub or
> resurrect my account for Tiles?

Great to hear from you again Antonio! Hope all is well.
I think we'd all be happy to see you resurrect your account, regardless
of how much you contribute.


> I think the best thing is to create a
> "body renderer" that acts like any other renderer. Since the
> Renderer.render wants a string as a first parameter, I think that a map:
> "generated name" -> body should be created in request scope. 


Do we break compatibility by adding Renderer.render(Object, Request) ?
Is that a possibility forward?

(We do if tiles-request-api is considered an spi, which it actually is.
This can be solved by introducing it as a default method in the
interface. That would require Java8, but maybe it's time for that.)

Mick.

Re: Management of body in PutAttributeModel and AddAttributeModel

[Antonio Petrelli <an...@gmail.com>]

Hello Nick

2016-06-01 3:33 GMT+02:00 Nicolas Le Bas <ma...@nlebas.net>:

> So, we also have this use case where we want to make sure the nested
> definition gets rendered:
>
> https://github.com/apache/tiles/blob/TILES_3_0_X/tiles-test-pom/tiles-test/src/main/webapp/testinsertnesteddefinition_tags.jsp



Yep.


> I agree that trying to defer evaluation here creates all kinds of
> challenges. But instead of creating a new tag, I wonder if we shouldn't
> remove one.
>

I think that these kind of decisions should be done when changing major
version number (Tiles 4?). A feature may be deprecated when incrementing
minor or patch version, not removed (AFAIK).


> I've never been much of a fan of declaring definitions inline in a JSP,
> and inserting it later. I think, at the top level it is a bad practice. The
> only place where it makes sense to me is inside a putAttribute tag, because
> it allows to defer the evaluation of the attribute.
>

It makes sense when you put a definition that extends another, more basic,
one.


> But then if we defer evaluation anyway, we could use insertTemplate or
> insertDefinition instead. I'd just like to double check about cascading
> attributes, making sure we don't loose the feature in the change.
>

The way of putting definitions via insertDefinition and via
putAttribute->definition is different. But again, before removing features,
a survey should be done (but I guess that Tiles users are not so active
anyway) and only in major versions.
I don't worry about cascading definitions, the attribute context mechanism
seems to be consistent for many use cases.


> Do I make any sense?
>

Sure :-D

Antonio