Wednesday, September 13, 2017, 4:20:35 AM, Woonsan Ko wrote: [snip] > I've proposed a PR for tags defined in spring.tld for now: > https://github.com/apache/incubator-freemarker/pull/34 > I'll continue to replace tags in form.tld.
Thanks, looks good, I have merged it! One day (probably when you feel it's already polished) I will go through it thoroughly. For now I have more like skimmed through it. So tell me if I should look at something more closely, like you have some doubts there. Some random remarks: - AbstractSpringTemplateDirectiveModel and its subclasses probably shouldn't be public. (Also executeInternal probably should be protected.) Generally, we should avoid exposing things as published API-s, because they all become backward compatibility constraints. (It's major difficulty with libraries that thousands of projects depend on, as I have learned the hard way with FM2...) - final MessageSourceResolvable messageResolvable = (messageResolvableModel != null) ? (MessageSourceResolvable) objectWrapperAndUnwrapper.unwrap(messageResolvableModel) : null; You can have ClassCastException here instead of a TemplateException that explains the higher level problem. Certainly you should introduce and then use something like: CallableUtil.getAndUnwrapArgument(..., MessageSourceResolvable.class, ...) - Things similar to this: <#assign expression="T(java.lang.Math).max(12.34, 56.78)" /> <div id="maxNumber">${spring.eval(expression)}</div> or this: <#assign pathInfo="http://freemarker.org/docs/index.html" /> <a href="${spring.url(pathInfo)}">Apache FreeMarker Manual</a> I think they are more readable (for most of us anyway) without the intermediate variable that's use only once. Like: <div id="maxNumber">${spring.eval("T(java.lang.Math).max(12.34, 56.78)")}</div> > Thanks, > > Woonsan > >> >> Cheers, >> >> Woonsan >> >>> >>>> (ref: >>>> https://github.com/woonsan/spring-mvc-freemarker3-demo/blob/master/src/main/webapp/WEB-INF/views/useredit.ftl) >>>> >>>> Still need to add an example using 'message' named parameter. >>>> >>>> Regards, >>>> >>>> Woonsan >>>> >>>>> >>>>>> look at `spring.message("foo")`, I think you would intuitively think >>>>>> that it prints the "foo" message. As of the others, I guess we don't >>>>>> need "text" as you can write ${spring.message("foo")!'default'}, nor >>>>>> do we need the xxxEscape parameters, as we have HTML auto-escaping and >>>>>> `spring.message("foo")?jsString`. >>>>> >>>>> Oh, cool! I didn't know we have ?jsString. Perfect. >>>>> >>>>>> >>>>>>>> >>>>>>>> Something that would be interesting if the spring.message function can >>>>>>>> somehow tell if a message is plain text or HTML. Like you can >>>>>>>> configure which messages are HTML on name patterns (or a content >>>>>>>> pattern, like all HTML values start with "<span>"). After all, without >>>>>>>> such a rule, how do the developers themselves know if a message should >>>>>>>> be HTML or plain text. Hopefully the have some exact policy for that. >>>>>>>> If spring.message knows that rule too, then for HTML messages it >>>>>>>> should return a TemplateMarkupOutputModel instead of a string, so it >>>>>>>> will be automatically non-escaped. Then people can just write >>>>>>>> ${spring.message("foo")} without worrying about `?noEsc`. And then we >>>>>>>> certainly don't need the directive "overload" either. >>>>>>> >>>>>>> As far as I know, MessageTag extends HtmlEscapingAwareTag which has >>>>>>> the following javadoc: >>>>>>> >>>>>>> * <p>Provides a "htmlEscape" property for explicitly specifying >>>>>>> whether to >>>>>>> * apply HTML escaping. If not set, a page-level default (e.g. from the >>>>>>> * HtmlEscapeTag) or an application-wide default (the >>>>>>> "defaultHtmlEscape" >>>>>>> * context-param in {@code web.xml}) is used. >>>>>> >>>>>> So then it seems we indeed need some spring library configuration >>>>>> parameter that associates an output format (as in >>>>>> http://freemarker.org/docs/dgui_misc_autoescaping.html) to each >>>>>> message, based on patterns. By default everything should be just plain >>>>>> text, to be on the safe side (i.e., auto-escaping will happen for >>>>>> them, because ${} does that). The equivalent of "defaultHtmlEscape" >>>>>> being on is that you assoicate "HTML" output format to all messages. >>>>> >>>>> +1 >>>>> >>>>>> >>>>>> This also reinforces that, at least in the first iteration, >>>>>> spring.message should be function only, not a directive. >>>>>> `${spring.message("foo")}` isn't more verbose than >>>>>> `<@spring.message "foo" />` anyway. >>>>> >>>>> +1 >>>>> >>>>>> >>>>>> BTW, I think it may worth having a special syntax for message >>>>>> insertion in the template language. For example, >>>>>> %{label.greet user, today} would be a syntactical sugar for >>>>>> ${someframewok.message('label.greet', user, today)}. But that's >>>>>> another topic. >>>>> >>>>> Sounds promising! >>>>> >>>>> Woonsan >>>>> >>>>>> >>>>>>> Regards, >>>>>>> >>>>>>> Woonsan >>>>>>> >>>>>>>> >>>>>>>>>> By the way, I'm wondering what the best practices are in naming those >>>>>>>>>> models. Should it be better with 'form', 'spring_form', >>>>>>>>>> 'spring.form', >>>>>>>>>> or something else? Prefixing with something sounds safer to me. >>>>>>>>> >>>>>>>>> In the templates they should be accessible as <@spring.bind ...> and >>>>>>>>> such. In the case of the form tags, as it's in a separate namespace in >>>>>>>>> JSP, maybe it should be <@form.input ...> etc. (Though I'm not 100% >>>>>>>>> sure if its practical the structure of the JSP taglibs so closely, and >>>>>>>> >>>>>>>> I meant: "if it's practical to follow the structure of the JSP taglibs >>>>>>>> so closely" >>>>>>>> >>>>>>>>> maybe we could just go with <@spring.input ...>. I don't know.) >>>>>>>>> >>>>>>>>>> Regards, >>>>>>>>>> >>>>>>>>>> Woonsan >>>>>>>>>> >>>>>>>>>> [1] >>>>>>>>>> https://docs.spring.io/spring/docs/current/spring-framework-reference/html/spring-tld.html >>>>>>>>>> [2] >>>>>>>>>> https://docs.spring.io/spring/docs/current/spring-framework-reference/html/spring-form-tld.html >>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>>>> Besides, just to be absolutely clear, I would merge your current >>>>>>>>>>>>> PR as >>>>>>>>>>>>> well, if it doesn't raise licensing issues, which is of course a >>>>>>>>>>>>> blocker. >>>>>>>>>>>> >>>>>>>>>>>> Sure, no worries. I was also under a concern about that and wanted >>>>>>>>>>>> to >>>>>>>>>>>> get feedbacks before doing too much. ;-) >>>>>>>>>>>> >>>>>>>>>>>> Regards, >>>>>>>>>>>> >>>>>>>>>>>> Woonsan >>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> Monday, August 7, 2017, 4:23:26 PM, Woonsan Ko wrote: >>>>>>>>>>>>> >>>>>>>>>>>>>> On Sun, Aug 6, 2017 at 6:14 AM, Daniel Dekany >>>>>>>>>>>>>> <ddek...@freemail.hu> wrote: >>>>>>>>>>>>>>> The big problem is that spring.ftl is copyrighted by some of the >>>>>>>>>>>>>>> Spring authors (or the Spring project as a whole - it's not >>>>>>>>>>>>>>> clear). So >>>>>>>>>>>>>>> certainly we can't just copy it. It has to be reimplemented >>>>>>>>>>>>>>> without >>>>>>>>>>>>>>> looking at the source, or something absurd like that. Perhaps >>>>>>>>>>>>>>> the best >>>>>>>>>>>>>>> is to start out from the spring JSP taglib, as that's the most >>>>>>>>>>>>>>> widely >>>>>>>>>>>>>>> used templating solution (I assume), so certainly that's the >>>>>>>>>>>>>>> one where >>>>>>>>>>>>>>> all the features are exposed. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> I wonder if using #import + FTL is the best way of adding >>>>>>>>>>>>>>> framework-level functionality that's used by a lot of people. >>>>>>>>>>>>>>> It's >>>>>>>>>>>>>>> surely an OK way, but it's not the highest performance way. The >>>>>>>>>>>>>>> other >>>>>>>>>>>>>>> way is using a shared variable (or some other kind of commonly >>>>>>>>>>>>>>> visible >>>>>>>>>>>>>>> variable) and implement the library in Java using >>>>>>>>>>>>>>> TemplateDirectiveModel-s and TemplateFunctionModel-s. It's less >>>>>>>>>>>>>>> convenient than doing it in FTL, but it has to be done once, >>>>>>>>>>>>>>> while you >>>>>>>>>>>>>>> save some resources everywhere where it's used. Though as most >>>>>>>>>>>>>>> of >>>>>>>>>>>>>>> these macros/functions are quite simple, I don't think the >>>>>>>>>>>>>>> performance >>>>>>>>>>>>>>> difference matters much. But, it also avoids the legal issue >>>>>>>>>>>>>>> above. I >>>>>>>>>>>>>>> mean, many of these function/macros are so trivial, that it's >>>>>>>>>>>>>>> hard to >>>>>>>>>>>>>>> implement them on a different way in FTL than as it is in the >>>>>>>>>>>>>>> Spring >>>>>>>>>>>>>>> source code, but if you implement them in Java, then it's much >>>>>>>>>>>>>>> harder >>>>>>>>>>>>>>> to accuse anyone with stealing. (A minor annoyance right now is >>>>>>>>>>>>>>> that >>>>>>>>>>>>>>> that part of the FreeMarker API is not yet settled; see >>>>>>>>>>>>>>> FREEMARKER-63, >>>>>>>>>>>>>>> FREEMARKER-64, FREEMARKER-65. But hopefully it will be in a good >>>>>>>>>>>>>>> enough shape soon. And see other thread; input is welcome!) >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> As of template aliases, at the first glance that's fine. Note >>>>>>>>>>>>>>> that >>>>>>>>>>>>>>> there's MultiTemplateLoader which does something similar, but I >>>>>>>>>>>>>>> assume >>>>>>>>>>>>>>> it would be less neat (and/or slower) to do this with that. >>>>>>>>>>>>>>> (But if >>>>>>>>>>>>>>> the spring functionality won't be #import-ed after all (as >>>>>>>>>>>>>>> above), the >>>>>>>>>>>>>>> whole thing can become unnecessary...) >>>>>>>>>>>>>> >>>>>>>>>>>>>> Thank you very much for sharing your insights. Greatly helpful >>>>>>>>>>>>>> advice. >>>>>>>>>>>>>> I agree that it might be better with template model(s) rather >>>>>>>>>>>>>> than >>>>>>>>>>>>>> library FTL in various aspects. >>>>>>>>>>>>>> Let me try with that approach again and let you know soon with a >>>>>>>>>>>>>> new PR. >>>>>>>>>>>>>> >>>>>>>>>>>>>> Thank again, >>>>>>>>>>>>>> >>>>>>>>>>>>>> Woonsan >>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Sunday, August 6, 2017, 7:22:00 AM, ASF GitHub Bot (JIRA) wrote: >>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> [ >>>>>>>>>>>>>>>> https://issues.apache.org/jira/browse/FREEMARKER-55?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16115649#comment-16115649 >>>>>>>>>>>>>>>> ] >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> ASF GitHub Bot commented on FREEMARKER-55: >>>>>>>>>>>>>>>> ------------------------------------------ >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> GitHub user woonsan opened a pull request: >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> https://github.com/apache/incubator-freemarker/pull/31 >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> FREEMARKER-55: spring.ftl marco lib support >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> - Support <#import "/spring.ftl" as spring> like Spring >>>>>>>>>>>>>>>> Framework does. >>>>>>>>>>>>>>>> - By default, the system lib from /spring.ftl is read from >>>>>>>>>>>>>>>> the >>>>>>>>>>>>>>>> specific classpath, not from app's template path. >>>>>>>>>>>>>>>> The system template aliases map can be customized >>>>>>>>>>>>>>>> through >>>>>>>>>>>>>>>> SpringResourceTemplateLoader.systemTemplateAliases property. >>>>>>>>>>>>>>>> - The same macros and functions are defined in /spring.ftl >>>>>>>>>>>>>>>> as >>>>>>>>>>>>>>>> Spring Framework's, with syntax changes to comply with FM3. >>>>>>>>>>>>>>>> - Note: As the system template lib support is handled by >>>>>>>>>>>>>>>> SpringTemplateLoader in this PR, it means developers should >>>>>>>>>>>>>>>> always >>>>>>>>>>>>>>>> use SpringTemplateLoader directly or indirectly in order to >>>>>>>>>>>>>>>> use the >>>>>>>>>>>>>>>> system macro library. Please review this decision. >>>>>>>>>>>>>>>> - TODOs: review/test/refine each macro and functions in >>>>>>>>>>>>>>>> spring.ftl. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> You can merge this pull request into a Git repository by >>>>>>>>>>>>>>>> running: >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> $ git pull https://github.com/woonsan/incubator-freemarker >>>>>>>>>>>>>>>> feature/FREEMARKER-55 >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Alternatively you can review and apply these changes as the >>>>>>>>>>>>>>>> patch at: >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> https://github.com/apache/incubator-freemarker/pull/31.patch >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> To close this pull request, make a commit to your master/trunk >>>>>>>>>>>>>>>> branch >>>>>>>>>>>>>>>> with (at least) the following in the commit message: >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> This closes #31 >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> ---- >>>>>>>>>>>>>>>> commit 8e0f33c419d982279d7fb22a9dfdc66f47abaf2c >>>>>>>>>>>>>>>> Author: Woonsan Ko <woon...@apache.org> >>>>>>>>>>>>>>>> Date: 2017-07-14T15:27:17Z >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> FREEMARKER-55: Renaming Freemarker to FreeMarker >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> commit ec8d687d4ce2c0e1bb3e3ca073b139eacc198527 >>>>>>>>>>>>>>>> Author: Woonsan Ko <woon...@apache.org> >>>>>>>>>>>>>>>> Date: 2017-07-14T15:53:51Z >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Merge branch '3' into feature/FREEMARKER-55 >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> commit e7cb6f7cfc241689c85527971bf6e1ea7ced9127 >>>>>>>>>>>>>>>> Author: Woonsan Ko <woon...@apache.org> >>>>>>>>>>>>>>>> Date: 2017-07-14T17:57:29Z >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Merge branch '3' into feature/FREEMARKER-55 >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> commit c6eb09de91e57035c1e0e3c4d3490b3b96622bab >>>>>>>>>>>>>>>> Author: Woonsan Ko <woon...@apache.org> >>>>>>>>>>>>>>>> Date: 2017-07-16T18:24:55Z >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Merge branch '3' into feature/FREEMARKER-55 >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> commit 870209fa8e0acd0bb3186053dfd549b5c758e37d >>>>>>>>>>>>>>>> Author: Woonsan Ko <woon...@apache.org> >>>>>>>>>>>>>>>> Date: 2017-07-18T00:38:03Z >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Merge branch '3' into feature/FREEMARKER-55 >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> commit 4481406a2f4eeb30d6d044a4ac158efab7ba7a7b >>>>>>>>>>>>>>>> Author: Woonsan Ko <woon...@apache.org> >>>>>>>>>>>>>>>> Date: 2017-08-06T01:28:54Z >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Merge branch '3' into feature/FREEMARKER-55 >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> commit fcd9e672ec515e3042bc5efd229b7d12c23e7d88 >>>>>>>>>>>>>>>> Author: Woonsan Ko <woon...@apache.org> >>>>>>>>>>>>>>>> Date: 2017-08-06T05:09:12Z >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> FREEMARKER-55: system template lib for spring app: >>>>>>>>>>>>>>>> spring.ftl. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> ---- >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> FM3 freemarker-spring module, Web MVC support >>>>>>>>>>>>>>>>> --------------------------------------------- >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> Key: FREEMARKER-55 >>>>>>>>>>>>>>>>> URL: >>>>>>>>>>>>>>>>> https://issues.apache.org/jira/browse/FREEMARKER-55 >>>>>>>>>>>>>>>>> Project: Apache Freemarker >>>>>>>>>>>>>>>>> Issue Type: Task >>>>>>>>>>>>>>>>> Affects Versions: 3.0.0 >>>>>>>>>>>>>>>>> Reporter: Daniel Dekany >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> Add Spring "Web MVC framework" functionality to >>>>>>>>>>>>>>>>> freemarker-spring. >>>>>>>>>>>>>>>>> This can be complex task (and the issue possibly has to be >>>>>>>>>>>>>>>>> subdivided), as it involves things like: >>>>>>>>>>>>>>>>> * Some aspects of the FreeMarker 2 integration (developed by >>>>>>>>>>>>>>>>> the Spring developers) are quite confusing >>>>>>>>>>>>>>>>> ({{FreemarerConfigurer}}, etc.), and we are looking into if >>>>>>>>>>>>>>>>> it needs to be like that. >>>>>>>>>>>>>>>>> * See if we can support {{@EnableWebMvc}} (note that >>>>>>>>>>>>>>>>> FreeMarker 2 support is hard coded into >>>>>>>>>>>>>>>>> {{ViewResolverRegistry}}, which we can't modify) >>>>>>>>>>>>>>>>> * Creating custom directives/methods to expose Spring >>>>>>>>>>>>>>>>> features like the Spring JSP Tag Library does (but in a way >>>>>>>>>>>>>>>>> that firs FreeMarker better) >>>>>>>>>>>>>>>>> * Expose JSP custom tag support from the >>>>>>>>>>>>>>>>> {{freemarker-servlet}} module. >>>>>>>>>>>>>>>>> Depends on: FREEMARKER-54 >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> -- >>>>>>>>>>>>>>>> This message was sent by Atlassian JIRA >>>>>>>>>>>>>>>> (v6.4.14#64029) >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> -- >>>>>>>>>>>>>>> Thanks, >>>>>>>>>>>>>>> Daniel Dekany >>>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> -- >>>>>>>>>>>>> Thanks, >>>>>>>>>>>>> Daniel Dekany >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> -- >>>>>>>>>>> Thanks, >>>>>>>>>>> Daniel Dekany >>>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>>> -- >>>>>>>> Thanks, >>>>>>>> Daniel Dekany >>>>>>>> >>>>>>> >>>>>> >>>>>> -- >>>>>> Thanks, >>>>>> Daniel Dekany >>>>>> >>>> >>> >>> -- >>> Thanks, >>> Daniel Dekany >>> > -- Thanks, Daniel Dekany