liquid icon indicating copy to clipboard operation
liquid copied to clipboard

Fix regex for matching endraw tags

Open peterzhu2118 opened this issue 4 years ago • 1 comments

Liquid does not accept {% endraw{% f %} as a valid endraw tag but accepts {% endfor{% f %} for endfor tags. This is because the second capture group (i.e. (\w+)) matches on the f instead of the endraw.

The solution I implemented hardcodes the endraw tag into the regex. The only way I can think of that doesn't hardcode the endraw in the regex is to dynamically generate it in Liquid::Raw#parse, which would not be very efficient.

Note: the tests fail because liquid-c hasn't been updated yet.

Another problem is that the old regex would match {% endraw{% |f %} as a valid endraw tag because |f no longer matches the (\w+) capture group so it would match on endraw. But this fails in liquid-c because it would match on the f.

peterzhu2118 avatar Nov 06 '20 19:11 peterzhu2118

Liquid does not accept {% endraw{% f %} as a valid endraw tag but accepts {% endfor{% f %} for endfor tags. This is because the second capture group (i.e. (\w+)) matches on the f instead of the endraw.

Ideally, we wouldn't accept {% endraw{% f %} or {% endfor{% f %} if there isn't liquid code that depends on this. I think we could add Liquid::Usage instrumentation to see if either of these are needed. For the raw tag, this only recently became possible when it was implemented in liquid-c, so there is a good chance we don't need to support this. We should use a separate Liquid::Usage.instrument name for block delimiters in general, since this lax behaviour has been around for longer, so has a higher chance of being relied upon.

dylanahsmith avatar Nov 06 '20 22:11 dylanahsmith