See my answers inline... Sunday, November 18, 2018, 8:44:40 PM, Christoph Rüger wrote:
> Thanks Daniel for your feedback. See my answers below > > Am So., 11. Nov. 2018 um 19:14 Uhr schrieb Daniel Dekany <[email protected] >>: > >> Sunday, November 11, 2018, 11:40:50 AM, Christoph Rüger wrote: >> >> > Am So., 11. Nov. 2018 um 09:25 Uhr schrieb Daniel Dekany < >> [email protected] >> >>: >> > >> >> Saturday, November 10, 2018, 3:08:14 PM, Denis Bredelet wrote: >> >> >> >> > Hi, >> >> > >> >> > Le 9 novembre 2018 à 22:36, Christoph Rüger <[email protected]> a >> >> écrit : >> >> > >> >> > Am Fr., 9. Nov. 2018 um 22:55 Uhr schrieb Daniel Dekany < >> >> [email protected] >> >> > : >> >> > >> >> > It's certainly tricky, but as far as I see possible (but then, who >> >> > >> >> > knows what will one find when actually working on it). It's also a >> >> > feature missing a lot. It's especially missing for #list (I know that >> >> > you need it for something else), because if you filter the items >> >> > inside #list with #if-s, then #sep, ?hasNext, etc. will not be usable. >> >> > >> >> > Let me say that I disagree here. >> >> > >> >> > I do not think that closures are required for FreeMarker, nor that >> they >> >> are a good idea. >> >> > >> >> > If we add new features to the FreeMarker *tempate engine* I would >> >> > rather we focus on multi-part macro body rather than an advanced >> >> language feature like closures. >> >> > >> >> > You can add ?filter and ?map if you want, a simple expression as >> >> parameter should be enough. >> >> >> >> Yes, as I said, we certainly start with only allowing lambdas in >> >> ?filter/?map, also certainly in ?contains. >> >> >> > Would be enough in my opinion and very useful. >> > >> > Is it possiblefor you to give some pointers to the code on how this could >> > be implemented? I would maybe like to wrap my head around this a little >> bit. >> >> Please feel yourself encouraged! (: >> >> > I started looking at seq_containsBI ( >> > >> https://github.com/apache/freemarker/blob/a03a1473b65d9819674b285a0538fed824f37478/src/main/java/freemarker/core/BuiltInsForSequences.java#L291 >> ) >> > and >> > and reverseBI ( >> > >> https://github.com/apache/freemarker/blob/a03a1473b65d9819674b285a0538fed824f37478/src/main/java/freemarker/core/BuiltInsForSequences.java#L264 >> ) >> > just to find something related (seq_containsBI checks something) and >> > reverseBI returns a new sequence. >> > What I haven't found is a function which takes an Expression as a >> > parameter. >> > Is there something similar already or would that be a new thing? >> >> It's a new thing in that it will be part of the expression syntax >> (even if for now we will only allow lambdas as the parameters of a few >> built-ins, so that we can get away without closures). So it's a new >> Expression subclass, and has to be part of the parser (ftl.jj) as >> well. > > Hmm, that parser stuff is new for me, it'll take me some time to get into > it. So it's a JavaCC lexer+parser. With a few twists... sorry, but this code has history... :) >> As of lazy evaluation of parameters expressions, that's already >> done in the built-ins in BuiltInsWithParseTimeParameters, and you will >> see it's trivial to do, but the situation there is much simpler. >> > > >> >> In principle, a LambdaExpression should evaluate to a >> TemplateMethodModelEx, and then you pass that TemplateMethodModelEx to >> the called built-in or whatever it is. But with the approach of >> BuiltInsWithParseTimeParameters we can certainly even skip that, and >> just bind to the LambdaExpression directly, add a LocalContext that >> contains the lambda arguments, end evaluate the LambdaExpression right >> there, in the built-in implementation. Or at least at a very quick >> glance I think so. >> > Not sure I can follow completely but that hint with* > BuiltInsWithParseTimeParameters* got me started, but at the moment I'm > stuck as I need to get more familiar with the internals of Freemarker. I am > also not sure I am on the same page regarding the syntax we are aiming for > and why I would need to extend the parser when there is something like > BuiltInsWithParseTimeParameters.... I assumed that the syntax will be similar to the Java lambda syntax (see below why), and that's of course needs lexer/parser changes. > Here is an example what I have in mind: > I started with the ?filter() builtin. I had a syntax like this in mind: > > *Example 1: ["a","b","c"]?filter(element, element == "c")* > *Example 2: ["a","b","c"]?filter(element, element == someOtherVariable, > someOtherVariable="c")* Looking at the above one believe that the value of `element` is passed in as first argument, and the the value of `element == "c"` as the second, but that's not the case. It's much better if it's visible that you got some kind of anonymous function definition there without knowing about the "filter" built-in. Java already has a syntax for expressing this kind of thing, so it would be better to use that familiar syntax. As far as I know it doesn't conflict with ours (it kind of it does, but not fatally). Also, even if for now we only allow this in said built-ins, we shouldn't exclude the possibility of making this kind of expression accessible elsewhere as well. Then, on many places the parsed won't know what kind of value is expected (consider passing a lambda to an user defined directive for example), so a syntax like above won't work. > Not sure if that's what you have in mind too, but to me it made sense with > regards to BuiltInsWithParseTimeParameters and I could start without > touching parser stuff. BuiltInsWithParseTimeParameters doesn't affect the syntax (only a bit...), as a call to a such built-in looks like a call to any other built-in. > *1st argument 'element'* would just be the iterator variable similar to > <#list ["a","b","c"] as *element*> > 2nd argument is the filter lambda expression... aka our filter condition > 3rd+n argument are optional parameters in case used in the lambda expression #list is special, similarly as `for ... in ...` is in most languages. It's not lambda-ish either; you can't write `as element/2` or such. > So at first I was looking how <#list> works and found IteratorBlock, and > though I could reuse it somehow. > > Here is some simple pseudo code I played around for the for Example 1: > > static class filter_BI extends BuiltInWithParseTimeParameters { > > > > TemplateModel _eval(Environment env) throws TemplateException { > > // sequence > > TemplateModel targetValue = target.evalToNonMissing(env); > > > > List parameters = this.parameters; > > > > Expression iteratorAlias = (Expression) parameters.get(0); > > Expression conditionExpression = (Expression) parameters.get(1); > > > > TemplateSequenceModel seq = (TemplateSequenceModel) target.eval > (env); > > for (int i = 0; i < seq.size(); i++) { > > TemplateModel cur = seq.get(i); > > > // this is where I am stuck at the moment > > // I basically want to evaluate conditionExpression > where iteratorAlias > is basically what I passed as 'element' > > // I am not sure if or how LocalContext could come into play > here > > // basically for each iteration I would assign the current > loop element to a context variable with the name 'element' > > // and then evaluate conditionExpression with that context. > > // if conditionExpression is "true" then I would populate > add the current sequence element 'cur' > > // to a new result-List.... and return that.... something > > // I wanted to reuse IteratorBlock here somehow, but didn't > get it to work yet. > > // maybe this is a stupid idea, or we just need something > similar > > > > } > > } > > > > Ok so far for my pseudo code.... Maybe you could give some more pointers > based on that... in case this makes any sense ... For LocalContext, Environment has a pushLocalContext method. See the calls to it, like in visit(TemplateElement[], TemplateDirectiveModel, Map, List). To avoid running into more new things at once than necessary, perhaps you should start with seq?seq_contains(lambda). The end result should be like: users?contains(u -> u.paying) >> Another similarity to BuiltInsWithParseTimeParameters is that we won't >> allow separating the `?someBI` and `(arg)`. Like, with the example of >> `cond?then(1, 2)`, you aren't allowed to do this: >> >> <#assign t=cond?then> >> ${t(1, 2)} >> >> That maybe looks natural, but most other built-ins allow that. Of >> course we can't allow that for ?filter etc., because then we need >> closures again. >> >> >> Multi-part macro body is also planned. Means, I know it definitely >> >> should be added, but who knows when that's done... I mean, it's like >> >> that for what, a decade? (: It's not even decided what it exactly >> >> does, as there are many ways of approaching this. (I have my own idea >> >> about what the right compromise would be, but others has other >> >> ideas...) >> >> >> >> Filtering lists bothers me because the template language should be >> >> (and somewhat indeed is) specialized on listing things on fancy ways >> >> that used to come up when generating document-like output. (If it >> >> doesn't do things like that, you might as well use a general purpose >> >> language.) Thus, that filter is unsolved (filtering with #if is >> >> verbose and spoils #sep etc.) bothers me a lot. >> >> >> >> BTW, ?filter and ?map is also especially handy in our case as >> >> FreeMarker doesn't support building new sequences (sequences are >> >> immutable). Although it has sequence concatenation with `+`, it's not >> >> good for building a sequence one by one, unless the sequence will be >> >> quite short. >> >> >> > Good point. >> > >> > >> >> >> >> > Cheers, >> >> > -- Denis. >> >> >> >> -- >> >> Thanks, >> >> Daniel Dekany >> >> >> >> >> > >> >> -- >> Thanks, >> Daniel Dekany >> >> > Thanks > Christoph > -- Thanks, Daniel Dekany
