freemarker-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Daniel Dekany <ddek...@apache.org>
Subject Re: Please test/review FreeMarker 2.3.28
Date Tue, 27 Mar 2018 08:26:29 GMT
Tuesday, March 27, 2018, 3:27:51 AM, Woonsan Ko wrote:

> On Mon, Mar 26, 2018 at 3:31 PM, Daniel Dekany <ddekany@apache.org> wrote:
>> Monday, March 26, 2018, 5:26:01 PM, Woonsan Ko wrote:
>>
>>> Hi Daniel,
>>>
>>> I've tested it again after building/installing locally for both "2.3"
>>> and "2.3-gae".
>>> The problem of splitting with empty string doesn't happy any more. That's good.
>>> But, I still need to check the nullability of the return of
>>> .get_optional_template('some_non_existing.ftl') like the following:
>>>
>>>   <#assign optTemp = .get_optional_template('some.ftl')>
>>>   <#if optTemp?? && optTemp.exists>
>>
>> I guess you misread the results here. #assign won't assign
>> null/missing to a variable. So `optTemp??` is useless, as the template
>> fails earlier than you could check if `optTemp` is missing. (Also, it
>> doesn't help if .exsists is null, which looks impossible from the
>> source code anyway.)
>
> You're right! :-)
> In our applications, we have a custom TemplateLoader based on JCR backend.
> It turned out that the custom TemplateLoader has broken the contact
> regarding IOException. It should have returned null on missing
> template instead of IOException, which was wrong in our end.

Now that you mention that, it's actually a mistake I have seen for a
few times. I have added notes to the Manual to point the user to the
right direction in that case.

> Also, the custom extension module as eaten up some exception by
> itself without giving the detail, so it contributed to my misreading
> as well.

You have kept failing at the right spot. (: It led to two improvements
(missing cause exception oversight, notes in the Manual).

>>> I got an exception message starting with "Error when trying to include
>>> template ...", which seems to come from this code:
>>> -
>>> https://github.com/apache/incubator-freemarker/blob/2.3-gae/src/main/java/freemarker/core/GetOptionalTemplateMethod.java#L140-L145
>>
>> OK, so that's maybe where the confusion comes from. That's about
>> handling IOException exceptions. So your template fails before
>> reaching that #if, because of an IOException, not because some
>> variable is null or such. What was the reason given in the exception
>> message? (BTW, I have noticed a bug there... the thrown exception
>> doesn't pack the original exception as cause. Will fix that in the
>> moment.)
>
> Thanks for the pointers! It helped me find out the root cause in our
> custom module.
>
> Now, everything looks very good. I see no problem!
>
> Thanks again for pointers and your patience!
>
> Regards,
>
> Woonsan
>
>>
>>> I suppose it shouldn't have returned a null value nor thrown an
>>> exception as the .exists seems useful in that use case.
>>>
>>> Regards,
>>>
>>> Woonsan
>>>
>>>
>>> On Sat, Mar 24, 2018 at 11:05 AM, Woonsan Ko <woonsan@apache.org> wrote:
>>>> On Sat, Mar 24, 2018 at 10:41 AM, Daniel Dekany <ddekany@apache.org>
wrote:
>>>>> Saturday, March 24, 2018, 2:44:17 PM, Woonsan Ko wrote:
>>>>>
>>>>>> On Sat, Mar 24, 2018 at 3:21 AM, Daniel Dekany <ddekany@apache.org>
wrote:
>>>>>>> Saturday, March 24, 2018, 3:13:06 AM, Woonsan Ko wrote:
>>>>>>>
>>>>>>>> When I tested with freemarker-2.3.28-incubating-SNAPSHOT.jar,
which I
>>>>>>>> built and install it to my local maven repo from the latest
"2.3"
>>>>>>>> branch myself, in my applications, I haven't found any regressions.
>>>>>>>> My applications are rather simpler probably than others as
they simply
>>>>>>>> render model objects passed from request attributes through
>>>>>>>> FreeMarkerServlet.
>>>>>>>> But when I tried the new features and bug fixes [1], I think
I found
>>>>>>>> somethings (perhaps there could be my misunderstandings):
>>>>>>>>
>>>>>>>> 1. The template example in
>>>>>>>> https://freemarker.apache.org/builds/fm2.3.28/ref_specvar.html#ref_specvar_get_optional_template
>>>>>>>>
>>>>>>>>   <#assign optTemp = .get_optional_template('some.ftl')>
>>>>>>>>   <#if optTemp.exists>
>>>>>>>>
>>>>>>>>   But if the 'some.ftl' doesn't exist, optTemp.exists fails
as it's
>>>>>>>> null. So I ended up changing it to this:
>>>>>>>>
>>>>>>>>   <#assign optTemp = .get_optional_template('some.ftl')>
>>>>>>>>   <#if optTemp?? && optTemp.exists>
>>>>>>>
>>>>>>> That's bizarre. It works for me. Also for the test suite... that
>>>>>>> passes there, right? (It can't even be explained by accidentally
using
>>>>>>> 2.3.27, as there .get_optional_template is parsing (syntax) error.)
>>>>>>
>>>>>> Perhaps my build might be wrong. I did the following locally to build
>>>>>> fresh and install it to my local maven repo. (Yesterday, I used my
>>>>>> forked/forked GitHub branch, which could have caused the issue.)
>>>>>
>>>>> OK, I see the problem. I haven't pushed the last merge commit in the
>>>>> 2.3 branch... sorry! So only the 2.3-gae branch was up to date on Git.
>>>>> (The linked binaries were fine though.)
>>>>>
>>>>>> (move to a temp folder)
>>>>>> $ git clone
>>>>>> https://git-wip-us.apache.org/repos/asf/incubator-freemarker.git
>>>>>> $ cd incubator-freemarker/
>>>>>> $ git checkout 2.3
>>>>>> (edit build.properties)
>>>>>> $ git log --oneline | head -n 2
>>>>>> edefaa2f Merge remote-tracking branch 'origin/2.3-gae' into 2.3
>>>>>> 01294537 Cleaned up more lexer/parser logic ...
>>>>>> $ ant clean maven-install
>>>>>> (confirm the log installing the jar and pom files to my maven repo
to
>>>>>> compare with deployed jar later)
>>>>>>
>>>>>> When I tested with this today, ${.version} printed: "version:
>>>>>> 2.3.28-nightly_20180324T130919Z-incubating".
>>>>>>
>>>>>> Could this local build cause a problem? (I saw differences between
>>>>>> "2.3" and "2.3-gae". See below. So I probably should have built
>>>>>> "2.3-gae" branch?)
>>>>>
>>>>> But should work, except if I screw up and forgot to push one of them,
>>>>> like now...
>>>>>
>>>>>> Anyway, in my testing with "2.3" build, I needed optTemp?? additionally.
>>>>>>
>>>>>>>> 2. Square bracket syntax through ftl directive
>>>>>>>>
>>>>>>>> It reads, "This directive also determines if the template
uses angle
>>>>>>>> bracket syntax (e.g. <#include 'foo.ftl'>) or square
bracket syntax
>>>>>>>> (e.g. [#include 'foo.ftl']). Simply, the syntax used for
this
>>>>>>>> directive will be the syntax used for the whole template,
regardless
>>>>>>>> of the FreeMarker configuration settings." [2]
>>>>>>>>
>>>>>>>> I applied it to an included .ftl template like this:
>>>>>>>>
>>>>>>>> [#ftl output_format="HTML"]
>>>>>>>> [="Hello"]
>>>>>>>> ${"Hello"}
>>>>>>>>
>>>>>>>> It prints: [="Hello"] Hello,
>>>>>>>> not: Hello ${"Hello"}
>>>>>>>
>>>>>>> There are two independent settings here: tag syntax and interpolation
>>>>>>> syntax. It's mere coincidence that both has "square bracket"
in their
>>>>>>> names. [#ftl] only sets the first. I will clarify that in the
>>>>>>
>>>>>> I see. When I tried this now,
>>>>>>
>>>>>> [#ftl output_format="HTML"]
>>>>>> [#assign a=[1,2,3]]
>>>>>> [#list a as i]${i}[#sep],[/#list]
>>>>>> [="Hello"]
>>>>>> ${"Hello"}
>>>>>>
>>>>>> I got: 1,2,3 [="Hello"] Hello
>>>>>> Thanks for clarification!
>>>>>
>>>>> Note that the square bracket *tag* syntax and that it can be activated
>>>>> with [#ftl] exists for a very long time.
>>>>>
>>>>> I will update the related Manual page in a minute...
>>>>>
>>>>>>> documentation. Or, it's a warning sign that we should rather
use
>>>>>>> {{exp}}, as that doesn't have "square brackets" in it.
>>>>>>
>>>>>> Perhaps. ;-)
>>>>>> I recently had a chance to look around other templating libraries
>>>>>> using the mustache as I have seen people loving it.
>>>>>> I didn't like the usages there in directives. For example, {{#if
>>>>>> user}} {{user.name}} {{/if}}. It's hard for me complete the if, ending
>>>>>> with {{/if}}, which is too much. ;-)
>>>>>> But the interpolation part (e.g, {{user.name}}) seems okay to me.
>>>>>> So, yes, I'm fine with mustache expression for interpolation and
it
>>>>>> might be less confusing in this specific case.
>>>>>>
>>>>>> What do others think?
>>>>>
>>>>> Let's not forget digging up the thread where it was decided. I mean,
>>>>> there are some pro/cons mentioned there.
>>>>>
>>>>>>>> 3. split with an empty string
>>>>>>>>
>>>>>>>> It says, "Bug fixed: When string?split(separator) is called
with "" as
>>>>>>>> the argument, the string will be split to characters now.
Earlier it
>>>>>>>> has thrown an IllegalArgumentException (unless the r flag
was
>>>>>>>> specified)."
>>>>>>>> But when I tried it, it throws an IllegalArgumentException:
>>>>>>>>
>>>>>>>> <#assign a="hello"?split("l")>
>>>>>>> ==>>
>>>>>>>> java.lang.IllegalArgumentException: The separator string
has 0 length
>>>>>>>> at freemarker.template.utility.StringUtil.split(StringUtil.java:752)
>>>>>>>
>>>>>>> Huh? "l" isn't even 0 long! Or you meant to copy-paste `?split("")`
>>>>>>> there? And yet again, it works for me here. Though this at least
can
>>>>>>> be that you are accidentally using 2.3.27 (try ${.version}).
>>>>>>
>>>>>> It was a copy-paste error.
>>>>>> Still I got the error: "java.lang.IllegalArgumentException: The
>>>>>> separator string has 0 length at
>>>>>> freemarker.template.utility.StringUtil.split(StringUtil.java:752)"
>>>>>>
>>>>>> Now I realize that "2.3" branch is different from "2.3-gae" branch:
>>>>>> -
>>>>>> https://github.com/apache/incubator-freemarker/blob/2.3/src/main/java/freemarker/template/utility/StringUtil.java#L752
>>>>>> -
>>>>>> https://github.com/apache/incubator-freemarker/blob/2.3-gae/src/main/java/freemarker/template/utility/StringUtil.java#L753
>>>>>>
>>>>>> Perhaps, should I have built/test from "2.3-gae"?
>>>>>
>>>>> In theory you should test both, but the differences are so tiny (GAE
>>>>> has a little change so that it can run on Google App Engine), it
>>>>> shouldn't mater. (See also:
>>>>> https://freemarker.apache.org/sourcecode.html)
>>>>
>>>> Right. Will do.
>>>>
>>>> Regards,
>>>>
>>>> Woonsan
>>>>
>>>>>
>>>>>> Thanks in advance!
>>>>>>
>>>>>> Regards,
>>>>>>
>>>>>> Woonsan
>>>>>>
>>>>>>>
>>>>>>>> Please let me know if I miss or misinterpret somethings.
>>>>>>>>
>>>>>>>> Cheers,
>>>>>>>>
>>>>>>>> Woonsan
>>>>>>>>
>>>>>>>> [1] https://freemarker.apache.org/builds/fm2.3.28/versions_2_3_28.html
>>>>>>>> [2]
>>>>>>>> https://freemarker.apache.org/builds/fm2.3.28/ref_directive_ftl.html
>>>>>>>>
>>>>>>>> On Fri, Mar 23, 2018 at 4:08 AM, Daniel Dekany <ddekany@apache.org>
wrote:
>>>>>>>>> Friday, March 23, 2018, 3:33:39 AM, Woonsan Ko wrote:
>>>>>>>>>
>>>>>>>>>> Great to see the real release without "incubating"
mark soon!
>>>>>>>>>
>>>>>>>>> In theory it was decided 2 days ago, but I saw nothing
about the board
>>>>>>>>> meeting so far. I guess they are overburdened with the
May elections
>>>>>>>>> and all that.
>>>>>>>>>
>>>>>>>>>> Also ! I've just read the Change log and everything
is awesome!
>>>>>>>>>> I'll just try out the snapshot binary tomorrow in
our applications to
>>>>>>>>>> see if there's any regression just in case.
>>>>>>>>>
>>>>>>>>> Great, thanks!
>>>>>>>>>
>>>>>>>>>> Cheers,
>>>>>>>>>>
>>>>>>>>>> Woonsan
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On Tue, Mar 20, 2018 at 6:39 PM, Daniel Dekany <ddekany@apache.org>
wrote:
>>>>>>>>>>> Before we start a VOTE on releasing 2.3.28, it
would be good if more
>>>>>>>>>>> of you can test it, or otherwise review the upcoming
2.3.28! The main
>>>>>>>>>>> point in this phase is to catch technical issues.
>>>>>>>>>>>
>>>>>>>>>>> If you intend to help, but won't have time for
it in the coming few
>>>>>>>>>>> days, please indicate that, so that I know that
I should wait!
>>>>>>>>>>>
>>>>>>>>>>> Change log (so far, but I don't plan to add more):
>>>>>>>>>>> https://freemarker.apache.org/builds/fm2.3.28/versions_2_3_28.html
>>>>>>>>>>>
>>>>>>>>>>> Binary release artifacts:
>>>>>>>>>>> https://dist.apache.org/repos/dist/dev/incubator/freemarker/engine/2.3.28-incubating-SNAPSHOT/binaries/
>>>>>>>>>>>
>>>>>>>>>>> Source release artifacts:
>>>>>>>>>>> https://dist.apache.org/repos/dist/dev/incubator/freemarker/engine/2.3.28-incubating-SNAPSHOT/source/
>>>>>>>>>>>
>>>>>>>>>>> The Maven artifacts are available from the Apache
snaphsot repository:
>>>>>>>>>>>
>>>>>>>>>>>   <repository>
>>>>>>>>>>>     <id>apache-snapshot-repository</id>
>>>>>>>>>>>     <url>https://repository.apache.org/content/repositories/snapshots/</url>
>>>>>>>>>>>     <releases><enabled>false</enabled></releases>
>>>>>>>>>>>     <snapshots><enabled>true</enabled></snapshots>
>>>>>>>>>>>   </repository>
>>>>>>>>>>>
>>>>>>>>>>> under the coordinates:
>>>>>>>>>>>
>>>>>>>>>>>   <groupId>org.freemarker</groupId>
>>>>>>>>>>>   <artifactId>freemarker</artifactId>
>>>>>>>>>>>   <version>2.3.28-incubating-SNAPSHOT</version>
>>>>>>>>>>>
>>>>>>>>>>> and for the Google App Engine compatible (GAE)
version:
>>>>>>>>>>>
>>>>>>>>>>>   <groupId>org.freemarker</groupId>
>>>>>>>>>>>   <artifactId>freemarker-gae</artifactId>
>>>>>>>>>>>   <version>2.3.28-incubating-SNAPSHOT</version>
>>>>>>>>>>>
>>>>>>>>>>> The ASF Board will decide tomorrow (21th) if
FreeMarker can graduate.
>>>>>>>>>>> If everything goes well, this release won't have
"incubating" in its
>>>>>>>>>>> version number. This will be the first such release
since mid 2015
>>>>>>>>>>> (2.3.23).
>>>>>>>>>>>
>>>>>>>>>>> --
>>>>>>>>>>> Thanks,
>>>>>>>>>>>  Daniel Dekany
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> Thanks,
>>>>>>>>>  Daniel Dekany
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Thanks,
>>>>>>>  Daniel Dekany
>>>>>>>
>>>>>>
>>>>>
>>>>> --
>>>>> Thanks,
>>>>>  Daniel Dekany
>>>>>
>>>
>>
>> --
>> Thanks,
>>  Daniel Dekany
>>
>

-- 
Thanks,
 Daniel Dekany


Mime
View raw message