tategakibunko/jingoo

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.

ddb49e4

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 }}

ba4de0c

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:

  1. rely on a reserved keywords list
  2. 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 ๐Ÿ‘