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

Reply via email to