Tapestry
  1. Tapestry
  2. TAPESTRY-1830

Add ability to store temporary data without having to define new properties

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 5.0.5
    • Fix Version/s: 5.0.8
    • Component/s: tapestry-core
    • Labels:
      None

      Description

      index and value parameter in loop (and all components like it) often used just in page templates and there is no need to access or change them in page class.

      sharing this parameters to use directly in page templates, may help developers to write most cleaner page classes.

        Activity

        M. H. Shamsi created issue -
        Hide
        Christian E Gruber added a comment -

        There needs to be a namespace in the ${} notation for this sort of thing, unless we're going to go with some sort of cascading resolution, which I expect would have a negative performance impact if used, say, in loops a lot. $

        {temp:foo}

        was suggested on the list. $

        {local:foo}

        might be another.

        I was originally thinking of $

        {component:foo}

        but the problem is there's arbitrary nesting of components below the loop or other containing component, so there's no guarantee that you're hitting the right one. Having a page-level scratch namespace would requrie that people name variables to avoid collision (which they have to do now anyway) but keep it simple, and (probably) way easier to implement and make fast.

        Show
        Christian E Gruber added a comment - There needs to be a namespace in the ${} notation for this sort of thing, unless we're going to go with some sort of cascading resolution, which I expect would have a negative performance impact if used, say, in loops a lot. $ {temp:foo} was suggested on the list. $ {local:foo} might be another. I was originally thinking of $ {component:foo} but the problem is there's arbitrary nesting of components below the loop or other containing component, so there's no guarantee that you're hitting the right one. Having a page-level scratch namespace would requrie that people name variables to avoid collision (which they have to do now anyway) but keep it simple, and (probably) way easier to implement and make fast.
        Hide
        Christian E Gruber added a comment -

        Oh, and using this should include the namespace in the variable declaration, I should think, so you can ensure that one CAN use the page class if necessary.

        Either

        <t:loop source="anIterable" value="local:myValue>
        <p>$

        {local:myValue}

        </p>
        </t:loop>

        or, allow the current syntax to be assumed in the local scope, and make someone do this to access the page class variable.

        <t:loop source="anIterable" value="page:myValue>
        <p>$

        {page:myValue}

        </p>
        </t:loop>

        I like the former rather than the latter, because most access to variables without explicit namespace implies the page class, and that should be kept consistent, I think.

        Show
        Christian E Gruber added a comment - Oh, and using this should include the namespace in the variable declaration, I should think, so you can ensure that one CAN use the page class if necessary. Either <t:loop source="anIterable" value="local:myValue> <p>$ {local:myValue} </p> </t:loop> or, allow the current syntax to be assumed in the local scope, and make someone do this to access the page class variable. <t:loop source="anIterable" value="page:myValue> <p>$ {page:myValue} </p> </t:loop> I like the former rather than the latter, because most access to variables without explicit namespace implies the page class, and that should be kept consistent, I think.
        Hide
        Christian E Gruber added a comment -

        Just clarifying the subject a bit.

        Show
        Christian E Gruber added a comment - Just clarifying the subject a bit.
        Christian E Gruber made changes -
        Field Original Value New Value
        Summary sharing value and index parameter in loop, count ... compoentes allow local value and index parameters (i.e. not page class variables) in loop, count ... components
        Hide
        Kevin Menard added a comment -

        This seems very similar in nature to TAPESTRY-1694. It may be a good idea to pick one and go with it.

        Show
        Kevin Menard added a comment - This seems very similar in nature to TAPESTRY-1694 . It may be a good idea to pick one and go with it.
        Hide
        Nick Westgate added a comment -

        Howard has suggested we use the component's own parameters which will be made public.
        When the expression language (i.e. not the current version of "prop") can access the component it will suffice for this use case.

        Cheers,
        Nick.

        Show
        Nick Westgate added a comment - Howard has suggested we use the component's own parameters which will be made public. When the expression language (i.e. not the current version of "prop") can access the component it will suffice for this use case. Cheers, Nick.
        Hide
        Christian E Gruber added a comment -

        Hmm. True, but Howard's suggestion still requires that something be coded into the page class. (ie, the loop component itself). I'd like to see no requirement for that for purely "internal" local variables.

        when you say "can access the component", are you meaning something like $

        {component:<comp_id>:<variable>}

        ?

        Show
        Christian E Gruber added a comment - Hmm. True, but Howard's suggestion still requires that something be coded into the page class. (ie, the loop component itself). I'd like to see no requirement for that for purely "internal" local variables. when you say "can access the component", are you meaning something like $ {component:<comp_id>:<variable>} ?
        Christian E Gruber made changes -
        Link This issue duplicates TAPESTRY-1694 [ TAPESTRY-1694 ]
        Hide
        Christian E Gruber added a comment -

        This issue really does duplicate TAPESTRY-1694, so I'm linking them as duplicates. I marked the other one as "affecting" v5.0.6

        Show
        Christian E Gruber added a comment - This issue really does duplicate TAPESTRY-1694 , so I'm linking them as duplicates. I marked the other one as "affecting" v5.0.6
        Nick Westgate made changes -
        Link This issue duplicates TAPESTRY-1694 [ TAPESTRY-1694 ]
        Hide
        Nick Westgate added a comment -

        I was thinking along the lines of an OGNL expression passing in the component name, but this would still require exposing the component resources as a property and fishing around in there (which is what the component binding is shorthand for).
        Not only would this make the OGNL expression verbose, I think Howard might label it a Bad Thing.

        Your component binding suggestion is interesting if we view it as "chaining" different bindings, with default chain of prop.

        And I don't want to start an edit war but I removed the duplicate and T5 affects from TAPESTRY-1694.
        Devs need one JIRA per issue (per framework) to close. Duplicate is used for closing redundant issues.
        If you change your comment to mention this issue it will be linkage enough between the two.

        Cheers,
        Nick.

        Show
        Nick Westgate added a comment - I was thinking along the lines of an OGNL expression passing in the component name, but this would still require exposing the component resources as a property and fishing around in there (which is what the component binding is shorthand for). Not only would this make the OGNL expression verbose, I think Howard might label it a Bad Thing. Your component binding suggestion is interesting if we view it as "chaining" different bindings, with default chain of prop. And I don't want to start an edit war but I removed the duplicate and T5 affects from TAPESTRY-1694 . Devs need one JIRA per issue (per framework) to close. Duplicate is used for closing redundant issues. If you change your comment to mention this issue it will be linkage enough between the two. Cheers, Nick.
        Howard M. Lewis Ship made changes -
        Fix Version/s 5.0.6 [ 12312544 ]
        Fix Version/s 5.0.7 [ 12312802 ]
        Hide
        Kevin Menard added a comment -

        I think Christian's suggestion is on the right path, although it's a lot more verbose than I was hoping for. I'd like to just see

        $

        {loop_id.index}

        This would use the "prop" binding and search the page class first, then the component store for those defined in the template. This is similar to OGNL's usage in T4.

        For what it's worth, my inspiration for all of this is Django. Django has a lot of interesting ideas I'd like to see in Tapestry 5. At the same time, it has a whole set of drawbacks, which is why I went back to T5.

        http://www.djangoproject.com/documentation/templates/#for

        If you scroll down a bit, you can see the context variables made available right off of the "for" templatetag.

        Show
        Kevin Menard added a comment - I think Christian's suggestion is on the right path, although it's a lot more verbose than I was hoping for. I'd like to just see $ {loop_id.index} This would use the "prop" binding and search the page class first, then the component store for those defined in the template. This is similar to OGNL's usage in T4. For what it's worth, my inspiration for all of this is Django. Django has a lot of interesting ideas I'd like to see in Tapestry 5. At the same time, it has a whole set of drawbacks, which is why I went back to T5. http://www.djangoproject.com/documentation/templates/#for If you scroll down a bit, you can see the context variables made available right off of the "for" templatetag.
        Hide
        Davor Hrg added a comment - - edited

        I tried this out some time ago,
        it is actualy very simple to do.

        I added another prefix, used string until first dot
        as component id, and delegated the rest of the expression
        to prop binding and using just found component as root object.

        the only problem was that this worked for my own components,
        and not for Loop component. The reason for this is simple, Loop
        component omitted getter on purpose.

        my idea was to upgrade prop binding with a simple new syntax change
        that is familiar to all playing with css.

        $

        {#loop_id.index}

        .....

        for those interested in the code :

        package test.tapestry.services;

        import org.apache.tapestry.Binding;
        import org.apache.tapestry.ComponentResources;
        import org.apache.tapestry.TapestryConstants;
        import org.apache.tapestry.ioc.Location;
        import org.apache.tapestry.runtime.Component;
        import org.apache.tapestry.services.BindingFactory;
        import org.apache.tapestry.services.BindingSource;

        /** The "cprop:" binding prefix, which allows access to a child component's prop via

        • normal prop: expression prefixed with component id.*/
          public class ComponentPropBindingFactory implements BindingFactory
          {
          private final BindingSource _propBindingFactory;

        public ComponentPropBindingFactory(BindingSource bindingSource)

        { _propBindingFactory = bindingSource; }

        public Binding newBinding(String description, ComponentResources container, ComponentResources component,
        String expression, Location location)
        {
        int idx=expression.indexOf(".");
        ComponentResources propSource = null;
        if(idx == -1)
        throw new RuntimeException("Invalid expression: '"expression"'. You must provide component id ");
        else

        { String componentId = expression.substring(0,idx); expression = expression.substring(idx+1); Component embeddedComponent = container.getEmbeddedComponent(componentId); if(embeddedComponent == null) throw new RuntimeException("Component "+componentId+" not found"); propSource = embeddedComponent.getComponentResources(); }

        return _propBindingFactory.newBinding(description, propSource, component, TapestryConstants.PROP_BINDING_PREFIX, expression, location);
        }
        }

        of course,
        for it to work .. add this to your app module...

        public static void contributeBindingSource(
        MappedConfiguration<String, BindingFactory> configuration,
        BindingSource bindingSource
        )

        { configuration.add("cprop",new ComponentPropBindingFactory(bindingSource)); }
        Show
        Davor Hrg added a comment - - edited I tried this out some time ago, it is actualy very simple to do. I added another prefix, used string until first dot as component id, and delegated the rest of the expression to prop binding and using just found component as root object. the only problem was that this worked for my own components, and not for Loop component. The reason for this is simple, Loop component omitted getter on purpose. my idea was to upgrade prop binding with a simple new syntax change that is familiar to all playing with css. $ {#loop_id.index} ..... for those interested in the code : package test.tapestry.services; import org.apache.tapestry.Binding; import org.apache.tapestry.ComponentResources; import org.apache.tapestry.TapestryConstants; import org.apache.tapestry.ioc.Location; import org.apache.tapestry.runtime.Component; import org.apache.tapestry.services.BindingFactory; import org.apache.tapestry.services.BindingSource; /** The "cprop:" binding prefix, which allows access to a child component's prop via normal prop: expression prefixed with component id.*/ public class ComponentPropBindingFactory implements BindingFactory { private final BindingSource _propBindingFactory; public ComponentPropBindingFactory(BindingSource bindingSource) { _propBindingFactory = bindingSource; } public Binding newBinding(String description, ComponentResources container, ComponentResources component, String expression, Location location) { int idx=expression.indexOf("."); ComponentResources propSource = null; if(idx == -1) throw new RuntimeException("Invalid expression: '" expression "'. You must provide component id "); else { String componentId = expression.substring(0,idx); expression = expression.substring(idx+1); Component embeddedComponent = container.getEmbeddedComponent(componentId); if(embeddedComponent == null) throw new RuntimeException("Component "+componentId+" not found"); propSource = embeddedComponent.getComponentResources(); } return _propBindingFactory.newBinding(description, propSource, component, TapestryConstants.PROP_BINDING_PREFIX, expression, location); } } of course, for it to work .. add this to your app module... public static void contributeBindingSource( MappedConfiguration<String, BindingFactory> configuration, BindingSource bindingSource ) { configuration.add("cprop",new ComponentPropBindingFactory(bindingSource)); }
        Howard M. Lewis Ship made changes -
        Fix Version/s 5.0.8 [ 12312898 ]
        Fix Version/s 5.0.7 [ 12312802 ]
        Hide
        Howard M. Lewis Ship added a comment -

        We don't define a fix version until a fix is committed.

        Show
        Howard M. Lewis Ship added a comment - We don't define a fix version until a fix is committed.
        Howard M. Lewis Ship made changes -
        Fix Version/s 5.0.8 [ 12312898 ]
        Hide
        Howard M. Lewis Ship added a comment -

        I think this could be done. We could have a Map associated with each component, used to store these kinds of values. A "var:" binding prefix (I prefer that the "local:") would read and update the Map. The Map would be cleared after the component finished rendering.

        Because there is no type information in a Map value, we really couldn't do much with a property expression (which is based on type safe signatures). But still, a lot of simple cases (such as the looping output above) could be accomplished.

        Again, if you are doing anything complicated, you would want to define a component property to hold the value, which nails the type, making it possible to use property expressions. But if you just need to hold the value and pass it as a parameter elsewhere, this would work well.

        Show
        Howard M. Lewis Ship added a comment - I think this could be done. We could have a Map associated with each component, used to store these kinds of values. A "var:" binding prefix (I prefer that the "local:") would read and update the Map. The Map would be cleared after the component finished rendering. Because there is no type information in a Map value, we really couldn't do much with a property expression (which is based on type safe signatures). But still, a lot of simple cases (such as the looping output above) could be accomplished. Again, if you are doing anything complicated, you would want to define a component property to hold the value, which nails the type, making it possible to use property expressions. But if you just need to hold the value and pass it as a parameter elsewhere, this would work well.
        Howard M. Lewis Ship made changes -
        Summary allow local value and index parameters (i.e. not page class variables) in loop, count ... components Add ability to store temporary data without having to define new properties
        Howard M. Lewis Ship made changes -
        Assignee Howard M. Lewis Ship [ hlship ]
        Howard M. Lewis Ship made changes -
        Resolution Fixed [ 1 ]
        Status Open [ 1 ] Closed [ 6 ]
        Fix Version/s 5.0.8 [ 12312898 ]
        Mark Thomas made changes -
        Workflow jira [ 12415049 ] Default workflow, editable Closed status [ 12568379 ]
        Mark Thomas made changes -
        Workflow Default workflow, editable Closed status [ 12568379 ] jira [ 12591436 ]
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Closed Closed
        83d 4h 28m 1 Howard M. Lewis Ship 06/Jan/08 02:05

          People

          • Assignee:
            Howard M. Lewis Ship
            Reporter:
            M. H. Shamsi
          • Votes:
            3 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development