bkiers / Liqp

An ANTLR based 'Liquid Template' parser and rendering engine.
MIT License
165 stars 94 forks source link

Report unsupport tags/filters more nicely #220

Closed jvanzyl closed 2 years ago

jvanzyl commented 3 years ago

Right now it's really hard to tell during parsing when a tag/filter is not implemented. You just get a NPE:

java.lang.NullPointerException
    at liqp.nodes.TagNode.<init>(TagNode.java:22)
    at liqp.parser.v4.NodeVisitor.visitOther_tag(NodeVisitor.java:104)
    at liqp.parser.v4.NodeVisitor.visitOther_tag(NodeVisitor.java:24)
    at liquid.parser.v4.LiquidParser$Other_tagContext.accept(LiquidParser.java:588)
    at org.antlr.v4.runtime.tree.AbstractParseTreeVisitor.visitChildren(AbstractParseTreeVisitor.java:46)
    at liquid.parser.v4.LiquidParserBaseVisitor.visitTag(LiquidParserBaseVisitor.java:62)
    at liquid.parser.v4.LiquidParser$TagContext.accept(LiquidParser.java:453)
    at org.antlr.v4.runtime.tree.AbstractParseTreeVisitor.visitChildren(AbstractParseTreeVisitor.java:46)
    at liquid.parser.v4.LiquidParserBaseVisitor.visitAtom_tag(LiquidParserBaseVisitor.java:34)

Which is very hard to figure out what is not implemented.

msangel commented 3 years ago

code to reproduce?

jvanzyl commented 3 years ago

I'll make a test that has a template with a tag that has not been implemented and it should yield the error above. In my case it was happening as the template I was using made use of a post_url tag. Then that worked and it failed again on another tag.

Actually it tells you when a filter is missing, but I don't think the same happens with tags.

jvanzyl commented 3 years ago

So at this location:

https://github.com/bkiers/Liqp/blob/master/src/main/java/liqp/parser/v4/NodeVisitor.java#L103

Placing something like this:

   String tagName = ctx.Id().getText();
    if (tags.containsKey(tagName)) {
      throw new RuntimeException("No tag exists: " + tagName);
    }

    return new TagNode(tags.get(tagName), expressions.toArray(new LNode[expressions.size()]));

Helped me report and find the tags that are missing. Maybe this isn't the ideal way to do this but something like this to help users determine if there are tags missing. I'm testing various Jekyll sites and there are a bunch of tags I'll have to implement so some better reporting would be good.

jvanzyl commented 3 years ago

@msangel What's the recommend way to create templates with custom filters and tags?

msangel commented 3 years ago

The examples in readme are good. I can just suggest to use those variants of apply method that also accept TemplateContext.

пт, 20 серп. 2021 о 21:23 Jason van Zyl @.***> пише:

@msangel https://github.com/msangel What's the recommend way to create templates with custom filters and tags?

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/bkiers/Liqp/issues/220#issuecomment-902875640, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAF4YK2TIMHDKXZ5LMBNAZTT52MTZANCNFSM5CQXPUQA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email .

-- Надіслано з Gmail Mobile

jvanzyl commented 3 years ago

I'll try and see if there is a test for registering a new tag and using it as this seems not to be working for me. I'll make a test quickly.

msangel commented 2 years ago

Thanks for the commit!