default filter is not usable
sagotch opened this issue ยท 8 comments
This is a regression introduced by 8d1073c
Because default
might be a token, it is returned as a token whenever you are in a Logic
lexer_mode.
{% set pg = default (0, env.pg) %}
will raise
Jingoo.Jg_types.SyntaxError("Error line _, col _, token default ()")
I will take a look at it when I have time.
Ok, we tested default
filter only as jg_default
.
I've added example/samples/filter_default.{jingoo, expected}
.
Fortunately, the only difference between default(in switch case statement) and default(as filter) is that only latter is followed by LPAREN.
So I've treated the default filter as special ApplyExpr
in jg_parser.mly
.
All test work fine, but if there is something wrong, point it to me.
Hmm, my fix doesn't work in set
statement...
{{ set baz = default('baz', null) }}
It causes
Jingoo.Jg_types.SyntaxError("Error line 3, col 6, token set ()")
Sorry, with my quick fix, we'll have to fix everything where ApplyExpr
comes up in jg_parlser.mly
.
This fix doesn't look good, so I'll revert it.
EDIT:
Oh, my example code was wrong! After fixing, test works fine!
-{{ set baz = default('baz', null) }}{{ baz }}
+{% set baz = default('baz', null) %}{{ baz }}
Seems a little bit hackish but if it works it is better than without the hack, thanks for taking care of this ๐
But this issue highlight another question:
Should jingoo:
- rely on a reserved keywords list
- or should we be able to define some macro/functions which use a name wich is a keyword otherwise.
i.e. should this be allowed?
{% function switch (b) %}{{ not b }}{% endfunction %}
{{ switch (true) }}
I think that 2
would be preferable and should be pretty easy to achieve but I did not try to implement it (yet?).
Okay, apparently this problem wasn't as easy as I thought it would be.
It would certainly be better to be able to use keywords in functions, macro.
But to use keyword like switch
, default
in various context, maybe we have to find out whether the token is keyword or identifier in the lexer context after all, but it's too hard task for me.
By the way, the reason I've added this fix in a rough way is that I'm using default
filter myself in my web site, haha.
(And maybe many devs are using default
filter because it's a useful filter)
I'd like to be able to declare functions that is using keyword in the future, but the default filter seems to have a big impact, so for now, I'm going to release this fix (although which isn't very good).
When you have time, feel free to remove my fix, and implement your own!
(And maybe many devs are using
default
filter because it's a useful filter)
That's how I found the bug :)
I'd like to be able to declare functions that is using keyword in the future, but the default filter seems to have a big impact, so for now, I'm going to release this fix (although which isn't very good).
When you have time, feel free to remove my fix, and implement your own!
I'll try to implement a general fix ๐