10000 Jinja by aelaguiz · Pull Request #1 · cratejoy/html5lib-python · GitHub
[go: up one dir, main page]
Skip to content

Jinja #1

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Jinja #1

wants to merge 9 commits into from

Conversation

aelaguiz
Copy link
Member
  • Parses jinja more fully

@expl0ratory
Copy link
Member

This looks good so far. When I was trying to build on this previously I got caught up on jinja within html tags, things like

<a href="/whatever" class="headerlink {% if on_whatever %}active{% endif %}">Whatever</a>
<input type="checkbox" name="mycheck" {% if mycheck_selected %}checked{% endif %}/>

That really spidered out places that needed to expect a jinja statement start. You've got a test that should cover the first case, what about the second?

If someone does something odd like

<{{ lol_tags }}>Things</{{ lol_tags }}>

Is it easy enough to make the error message "Invalid cratejoy syntax at line: " I can't think of a legit reason for that one, but it'd be valid jinja and potentially a pain in the ass to interpret.

@aelaguiz
Copy link
Member Author

They parse fine, although the jinja in that class isn't actually parsed. My goal isn't really to build the world's best parser but just to catch additional types of errors.

I've tested this against all of our merchants themes and it works fine / catches the things I want it to catch. It definitely can miss things, but I'm open to it being improved over time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@aelaguiz @expl0ratory
0