Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow variables with the include tag #75

Closed
jvanzyl opened this issue Feb 20, 2018 · 7 comments
Closed

Allow variables with the include tag #75

jvanzyl opened this issue Feb 20, 2018 · 7 comments

Comments

@jvanzyl
Copy link
Collaborator

jvanzyl commented Feb 20, 2018

We have a layout that works in Jekyll where there is an include that uses a variable defined in the front-matter to select which navigation to include. Here's an example:

---                                                                                                                                                                             
layout: default                                                                                                                                                             
---                                                                                                                                                                             
<div class="row margin-bottom-50">                                                                                                                                                                                                                                                                                                                              
  {% include {{page.side-navigation}} %}                                                                                                                                                                                                                                                                                                                        
</div>

Seems like the ANTRL grammar needs to be altered to allow {{XXX}}, and do a variable lookup in the include tag.

@bkiers
Copy link
Owner

bkiers commented Feb 20, 2018

What is allowed in a Jekyll {% include ... %} ?

Only simple variable lookup like this:

'{{' id ('.' id)* '}}'

or a full-blown output like this:

'{{' expr filter* '}}'

@jvanzyl
Copy link
Collaborator Author

jvanzyl commented Feb 20, 2018

Sure, so can we be compatible with Jekyll? Happy to work on it on the weekend, just wanted to get the green light.

@bkiers
Copy link
Owner

bkiers commented Feb 21, 2018

Sure, so can we be compatible with Jekyll?

Yeah, in that case the tree rewriting needs to be modified from:

// parser grammar
include_tag
 : TagStart Include ( {this.flavor == Flavor.JEKYLL}?=>
                      file_name_as_str TagEnd    -> ^(INCLUDE file_name_as_str ^(WITH    ))
                    | a=Str (With b=Str)? TagEnd -> ^(INCLUDE $a               ^(WITH $b?))
                    )
 ;

to something like this:

// parser grammar
include_tag
 : TagStart Include ( {this.flavor == Flavor.JEKYLL}?=>
                      file_name_or_output TagEnd -> ^(INCLUDE file_name_or_output ^(WITH    ))
                    | a=Str (With b=Str)? TagEnd -> ^(INCLUDE ^(FILE_NAME_OR_OUTPUT ... ) ^(WITH $b?))
                    )
 ;

file_name_or_output
 : ... -> ^(FILE_NAME_OR_OUTPUT ... )
 ;

And the tree walker grammar will then alse need to be modified into something like this:

// tree walker grammar
include_tag returns [LNode node]
 : ^(INCLUDE ^(FILE_NAME_OR_OUTPUT ... ) ^(WITH (with=Str)?))
     ...
 ;

Happy to work on it on the weekend, just wanted to get the green light.

Of course, a PR is more than welcome! The tree-walking and -rewriting can be a bit confusing in ANTLR v3. Not surprising Terence dropped it in ANTLR v4. Let me know if I can be of assistance.

@bkiers
Copy link
Owner

bkiers commented Mar 2, 2018

@jvanzyl this is probably trickier than I had thought. The lexer keeps track of a boolean inTag whenever it sees a {{ or {% and produces tokens based on that flag.

Since we're now trying to support {{ ... }} in an include, that means the a flag isn't sufficient anymore (there's a tag in a tag now). Changing the flag into a stack that keeps track level of nesting didn't work: ANTLR3's lexer couldn't handle the lexer rules and their predicates.

There are 2 options:

  1. rewrite the lexer to handle nested Out/Tags
  2. replace the ANTLR3 grammar with an ANTLR4 grammar

Number 2 is not much more work than number 1 (the current Nodes can be re-used, I only need to write a new Walker/Visitor). And it's not at all sure that I can get ANTLR3's lexer to cooperate without some nasty hacks (and probably taking a noticeable performance hit for larger scripts). ANTLR4's parser and (most noticeable) its lexer are quite a bit more powerful compared to the v3 version.

So, I'll go and create a v4 grammar and visitor to replace the old v3 stuff (and being backwards compatible, of course!). But it won't be ready in a couple of days (I have little dev-time at the moment).

@bkiers
Copy link
Owner

bkiers commented Mar 2, 2018

See: #76

@jvanzyl
Copy link
Collaborator Author

jvanzyl commented Jul 22, 2018

I just got around to trying this (sorry it's so late), and this appears to be allowed in Jekyll but will not parse with Liqp:

Exception in thread "main" java.lang.RuntimeException: could not parse input: {% include wmt/html-head.html %}

<div class="wrap">
  {% include wmt/header.html %}
  <div class="margin-bottom-20"></div>
  <div class="container">
    {{content}}
  </div><!-- END container -->
</div><!-- END wrap -->

{% include wmt/footer.html %}

Shall I make a new issue?

@bkiers
Copy link
Owner

bkiers commented Aug 2, 2018

I've just got back from 2 weeks Scotland, hence the delayed response.

No problemo @jvanzyl, and yes, please create a new issue for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants