Wednesday, January 3, 2018, 5:36:16 AM, Woonsan Ko wrote: > Hi, > > It has been a long time since my last commit to support spring mvc > functions/directives (FREEMARKER-55). > Today, I've opened a pull request, containing an initial form related > work (only form:form and form:input directives for now, but those are > parts of the crucial basement): > - https://github.com/apache/incubator-freemarker/pull/41 > > Please feel free to take a look and let me know if you have any > comments or remarks. > > Some pointers: > - Initially I tried to leverage dynamic varargs for almost every html > attribute, but I ended up defining each available attribute as member > like Spring taglibs do, which seems clearer in both argument layout > and maintenance.
It's also faster (unless you end up with a really long mostly-empty argument array). > - Arguments for html attributes are kind of inherited from base > classes such as AbstractHtmlElementTemplateDirectiveModel. I ended up > merging named arg list in child directives by accessing static > members, which is not totally clean - static members are visible > anywhere, but that's what I think is the best for now. Yeah, I have noticed some quite involved "argument index arithmetic" there. Maybe, instead of SomeSuperTemplateDirectiveModel.NAMED_ARGS_ENTRY_LIST, your could just expose ARGS_LAYOUT, and get the highest named argument index from that. Then NAMED_ARGS_ENTRY_LIST need not exist at all (especially as a List instead of as an array), unless I miss something there. > - spring:* tags are mapped to "spring.*" models, whereas form:* tags > are mapped to "spring.form.*" models. I didn't want to expose "form" > model separately in a higher level. I found out it's convenient to > write <#assign form=spring.form /> in order to use <@form.input .../> > instead of <@spring.form.input .../>. While that's the theoretically correct way, I think people will either end up starting the templates with <#assign form=spring.form />, or dislike FreeMarker for forcing them to write "spring.form" again and again. Then it's better just to expose "form" directly. > - The other (remaining) tasks seem to be relatively more > straightforward: other inputs, button, textarea, select, etc. Something I have noticed that the Spring JSP taglib uses all-lower-case convention for attributes, like "onselect" instead of "onSelect". That's alien from FreeMarker 3, and from modern Java and JavaScript conventions too. So it might be worth it to change that. Then we should make error message generation more intelligent in freemarker-core, so that it tells you the correct parameter name automatically if only the case differs. > I'll probably merge the PR by myself in a day or two and continue with > other directives while being willing to hear any feedback and > suggestions. BTW, especially in this part where you are the main author, I surely don't expect you to "stage" a change before merging, so just go ahead whenever you feel like it. (The added delay just increase the chance of conflicts.) > Kind regards, > > Woonsan > -- Thanks, Daniel Dekany
