branaway / Japid

A Java-based statically-typed fast template engine that can be used in any Java code. It has special adapter for use with the Play! Framework.
113 stars 18 forks source link

pls add support for named parameter passing when calling tags #12

Closed tunggad closed 13 years ago

tunggad commented 13 years ago

Now we only have positional parameter passing when calling tag. With simple tags with few parameter its ok, but when calling tags with many parameter it will very easy to gets confused. The semantic of the parameters are not immediate clear to the code reader/maintainer, what are being passed into the tag. I would recommend to add support for named parameter passing to tag-calling. Such a feature is very popular by all templating engine of the dynamic script languages.

I know, its abit trickier to implement this feature in pure Java. But it should be possible thanks reflection, should it? ^_^

I think its very helpful and important feature for a template engine, and hope you would see it so as well. Thank you Bing!

branaway commented 13 years ago

It's one of the things I was thinking of. I'll see what I can do in the future about this feature.

One of th most important none-functional requirement for Japid is raw Java speed. That's why I have been trying to eliminate reflection whereever in the pipeline. Tags can be used in very tight inner loops and reflection will impact the performance badly.

tunggad commented 13 years ago

Hi Bing, i have had take a look at your code to see how the engine works. Roughly, i think, i have understood how template invocation works. And i'm thinking about how to implement this feature without Java Reflection, as you said, reflection hurts raw performance. Following i would like to show you an possible performant implementation idee.

I see, main effort to implement this feature is to adapt the JapidParser and JapidXXXCompiler having them to accept the new syntax and accordingly generate the needed logic in the Java template files.

new syntax to invoke template in views

tag myTag arg1:"String Value" arg2:68

and somewhere the in the java file of the invoking temlate we should have: ... _myTag4.renderModel( new JapidModelMap().put("arg1", "String Value").put("arg2", 68) ) ...

implementation idee and codes needed to generate in myTag.java

/*
This static mapping will be later user in method renderModel to construct an proper Object[] array
which is needed to invoke the method render(Object... args) over reflection.
*/
public static Method renderMethod;
public static final HashMap<String, int> argsNameToPosMap = new HashMap<String, int> (nrOfArgs)
static {
    argsNameToPosMap.put("arg1", 0);
    argsNameToPosMap.put("arg2", 1);

    Method[] methods = Class.forName("package.path.to.myTag").getClass().getDeclaredMethods();
    for(Method m : methods) {
    if (m.getName().equals("render")) {
            renderMethod = m;
            break;
        }
    }
}

public RenderResult renderModel(JapidModelMap model) {
    // a static utils method of JapidModelMap to build up an Object[] array. Nulls are used where the args are omitted.
    Object[] args = JapidModelMap.buildArgs(model, argsNameToPosMap)
    return (RenderResult ) renderMethod.invoke(this, args);
}

invoke/render template within controller

new methods for JapidController:

Actually, the purpose of JapidModelMap is just typed marker wrapper in order to not conflict with the other two renderJapid(Object... args) and renderJapidWith(String template, Object... args).

BTW: the static method render(T t, Object... args) of RenderInvokerUtils would also fenefit from the static initialzation of the renderMethod shown above and save few CPU cycles needed for lookup of render method per reflection

I wish, i could do it myself

The main problem here is im not really familar with the japid low-level api (token parser, code generating etc.) --> i wish, i could do it myself, ... but i cant.^_^. For you it's just stuff you would do in few hour, right? Pls and pls, Bing, pls implement this feature, i really need it now. You properly know this situation, its really annoying to invoke a template with many optional parameters, we must specify mass of Nulls for these optional params, for which we dont have a value. And named parameter passing also add documentary value to the code.

Thank you very much!

branaway commented 13 years ago

Hi, thanks for the initial attacking on the problem and interesting approach. Let me think about it. Sorting out the parser compiler mess will probably take a day or two, but you're welcome to hack it around.

branaway commented 13 years ago

Hi, I have added the support in 0.8.5 in the module repo. Let me know how it works for you.

I use = sign to separate the name and the value:

@tag myTag (name="my name", age = 11)
tunggad commented 13 years ago

wooaaaww, i will try it asap!

tunggad commented 13 years ago

Hi Bing, i have had a look at your new code, especially at JapidTemplateBaseWithoutPlay.java. I could not find anywhere that you used the proposed argsNameToPositionMap or some equivalent logic.

Just to be clear, with "named parameter passing when calling/invoking template" i meaned that the parameter could be passed in every order, it must not be the same as they being declared. Only the names (arg1, arg2) must be the same as they declared in args String arg1, Integer arg2. The names (in my concept) are not everyhing what one ever want to pair with the to be passed values, ... and perhaps, thats what you have understood and implemented?

Beside documentary valua added by this design, you can also improve your eclipse plugin to give autocompletion/suggestion on passing named parameter to a tag/template. Such a feature is called "tag attribute autocomplete" in common IDE's support for web development.

PS: i only had a look at your code, could not test the new syntax because im at a friend, will do it asap.

branaway commented 13 years ago

sure, the same idea, slightly different implementation.

tunggad commented 13 years ago

index.html and myTemplate.html

#{extends "adminLayout.html" /}
#{set title:"playSHOP Dashboard" /}

`set leftCol
<div>left-col</div>
`

`set rightCol
<div>
        <div id="editor"></div>
<div>
  `tag myTemplate (arg2=29, arg1="tunggad")
</div>
</div>
`

@args String arg1, Integer arg2, Boolean arg3

This is a String = <span style="color: green;">'${arg1}'</span>
and this an Integer = <span style="color: blue;">~{arg2 + 1}</span>
and Boolean = <span style="color: #ff0000;">${!arg3}</span>

myTemplate.java

...
@Override protected void doLayout() {
//------
;// line 1
p("\n" + 
"This is a String = <span style=\"color: green;\">'");// line 1
p(arg1);// line 3
p("'</span>\n" + 
"and this an Integer = <span style=\"color: blue;\">");// line 3
p(escape(arg2 + 1));// line 4
p("</span>\n" + 
"and Boolean = <span style=\"color: #ff0000;\">");// line 4
p(!arg3);// line 5   <---------------------- NullPointerException occurs here
p("</span>");// line 5

}
...

full stacktrace when hit /Admin/index

play.exceptions.JavaExecutionException: error in running the renderer: at play.mvc.ActionInvoker.invoke(ActionInvoker.java:227) at Invocation.HTTP Request(Play!) Caused by: java.lang.RuntimeException: error in running the renderer: at cn.bran.japid.util.RenderInvokerUtils.render(RenderInvokerUtils.java:31) at cn.bran.play.JapidController.invokeRender(JapidController.java:80) at cn.bran.play.JapidController.getRenderResultWith(JapidController.java:288) at cn.bran.play.JapidController.renderJapidWith(JapidController.java:168) at cn.bran.play.JapidController.renderJapid(JapidController.java:155) at controllers.backend.Admin.index(Admin.java:16) at play.mvc.ActionInvoker.invokeWithContinuation(ActionInvoker.java:540) at play.mvc.ActionInvoker.invoke(ActionInvoker.java:498) at play.mvc.ActionInvoker.invokeControllerMethod(ActionInvoker.java:474) at play.mvc.ActionInvoker.invokeControllerMethod(ActionInvoker.java:469) at play.mvc.ActionInvoker.invoke(ActionInvoker.java:157) ... 1 more Caused by: java.lang.RuntimeException: java.lang.NullPointerException at cn.bran.japid.template.JapidTemplateBaseWithoutPlay.render(JapidTemplateBaseWithoutPlay.java:260) at japidviews.backend.Admin.index.rightCol(index.java:110) at japidviews._layouts.adminLayout.layout(adminLayout.java:101) at japidviews.backend.Admin.index.render(index.java:65) at cn.bran.japid.util.RenderInvokerUtils.render(RenderInvokerUtils.java:24) ... 11 more Caused by: java.lang.NullPointerException at japidviews.backend.Admin.myTemplate.doLayout(myTemplate.java:85) at cn.bran.japid.template.JapidTemplateBaseWithoutPlay.layout(JapidTemplateBaseWithoutPlay.java:121) at japidviews.backend.Admin.myTemplate.render(myTemplate.java:71) at cn.bran.japid.template.JapidTemplateBaseWithoutPlay.render(JapidTemplateBaseWithoutPlay.java:253) at japidviews.backend.Admin.index.rightCol(index.java:110) at japidviews._layouts.adminLayout.layout(adminLayout.java:101) at japidviews.backend.Admin.index.render(index.java:65) at cn.bran.japid.util.RenderInvokerUtils.render(RenderInvokerUtils.java:24) at cn.bran.play.JapidController.invokeRender(JapidController.java:80) at cn.bran.play.JapidController.getRenderResultWith(JapidController.java:288) at cn.bran.play.JapidController.renderJapidWith(JapidController.java:168) at cn.bran.play.JapidController.renderJapid(JapidController.java:154) ... 7 more

branaway commented 13 years ago

The Boolean value is set to null if not set by caller, thus !null gave you NPE.

I haven't implemented decent default value mechanism yet. In this case you'll need to catch the NPE, using

`suppressNull on

or the Elvis operator

${!arg3 ?: false}

tunggad commented 13 years ago

ok, it works like a charm!

Ah i see the logic of your implementation, public String[] argNamesInstance of JapidTemplateBaseWithoutPlay is the key. Thank you for the work.

I would test abit more and close this ticket when all the tests pass.

PS: just want to know, why did you not make this field String[] argNamesInstance to static? I think, its common property of each template class, not property of temlate instance.

tunggad commented 13 years ago

passing named parameters to tag's body problem

`set rightCol

        `String myName = new String("tunggad");`
        `Integer myNr = 80;`
<div>
    <div id="editor"></div>
    <div>
      `tag myTemplate (arg2=29, arg1="tunggad", arg3=null)
    </div>
    `tag templateWithBody (arg="kkkk", name=myName, nr=myNr) | String name, Integer nr
      This is the body
      `tag myTemplate (arg1=name, arg3=true, arg2=nr)`
    `
</div>
`

----> generated java code

    @Override protected void rightCol() {
    final myTemplate _myTemplate4 = new myTemplate(getOut());
    { _myTemplate4.setActionRunners(getActionRunners()); }

final templateWithBody _templateWithBody5 = new templateWithBody(getOut());
{ _templateWithBody5.setActionRunners(getActionRunners()); }

final myTemplate _myTemplate6 = new myTemplate(getOut());
{ _myTemplate6.setActionRunners(getActionRunners()); }

    // line 8
            String myName = new String("tunggad");// line 10
            Integer myNr = 80;// line 11
p("    <div>\n" + 
"        <div id=\"editor\"></div>\n" + 
"        <div>\n" + 
"          ");// line 11
_myTemplate4.render(named("arg2", 29), named("arg1", "tunggad"), named("arg3", null));
// line 15
p("        </div>\n" + 
"        ");// line 15
  // HERE IS THE PROBLEM
_templateWithBody5.render(named("arg", "kkkk"), named("name", myName), named("nr", myNr), new templateWithBody.DoBody<String, Integer>(){
public void render(final String name, final Integer nr) {
// line 17
p("          This is the body\n" + 
"          ");// line 17
_myTemplate6.render(named("arg1", name), named("arg3", true), named("arg2", nr));
// line 19

}
}
);
// line 17
p("    </div>\n" + 
"    ");// line 20
;
  }

BTW: i think, that the document for the this feature (passing parameter to tags body + tag definitions) is abit unclear. I must have tried around 2h to figure out how it works, Ok im noob ^_^. And i think, the mechanism, that the paramters which can be passed into tag's body, must always be declared, is abit limitation. Because, normally, we dont know what the templates/tags in the body should expect in later usages.

branaway commented 13 years ago

Nice catch. Fixed in 0.8.5.1.

The body thing is a little convoluted. It's a double dispatch pattern or callback pattern. The second dispatch takes place from the tag back to body part, which itself is a closure or a mini tag that has its own required parameters. The controlling side for the callback is on the body part, not on the other side.

tunggad commented 13 years ago

version 0.8.5.1 worked fine!