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.

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

Add support for Jinja2's import directive. #64

Closed
wants to merge 2 commits into from
Closed

Add support for Jinja2's import directive. #64

wants to merge 2 commits into from

Conversation

kkinder
Copy link

@kkinder kkinder commented Jul 28, 2020

This is my attempt to fix #63, which adds support for Jinja2's import directive.

@codecov
Copy link

codecov bot commented Jul 28, 2020 •
edited

Codecov Report

Merging #64 into master will decrease coverage by 0.14%.
The diff coverage is 62.50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #64      +/-   ##
==========================================
- Coverage   81.04%   80.89%   -0.15%     
==========================================
  Files          17       17              
  Lines        2031     2047      +16     
  Branches      376      376              
==========================================
+ Hits         1646     1656      +10     
- Misses        274      280       +6     
  Partials      111      111
Impacted Files Coverage Δ
pypugjs/parser.py 76.25% <33.33%> (-0.47%) ⬇️
pypugjs/compiler.py 77.90% <50.00%> (-0.22%) ⬇️
pypugjs/ext/html.py 88.23% <50.00%> (-0.58%) ⬇️
pypugjs/ext/mako.py 81.33% <66.66%> (-0.62%) ⬇️
pypugjs/nodes.py 91.02% <66.66%> (-0.48%) ⬇️
pypugjs/lexer.py 89.71% <100.00%> (+0.05%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fb5a6b4...bea8f2d. Read the comment docs.

@kakulukia
Copy link
Owner

Could you please also add a test case for that import case?

@kkinder
Copy link
Author

kkinder commented Jul 29, 2020

@kakulukia, I did but I'm a bit confused. Perhaps you can help me out? When I manually run the converter, the code works as expected:

$ pypugjs/convert.py pypugjs/testsuite/cases/importing.pug -c jinja
{% import "auxfiles/legacy_jinja2.html" as legacy %}
<p>{{ legacy.legacy_macro('World') }}</p>⏎

However in the test suite, I'm seeing that it's generating the output without the import statement. Eg,

======================================================================
FAIL: test_cases.test_case_generator('importing', 'Jinja2')
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/kkinder/.local/share/virtualenvs/pypugjs-AiImhkin/lib/python3.8/site-packages/nose/case.py", line 198, in runTest
    self.test(*self.arg)
  File "/home/kkinder/Projects/pypugjs/pypugjs/testsuite/test_cases.py", line 203, in run_case
    assert processed_pugjs == html_src
AssertionError: 
-------------------- >> begin captured stdout << ---------------------
PROCESSED (41 chars)
<p>{{ legacy.legacy_macro('World') }}</p>

EXPECTED (94 chars)
{% import "auxfiles/legacy_jinja2.html" as legacy %}
<p>{{ legacy.legacy_macro('World') }}</p>

--------------------- >> end captured stdout << ----------------------

I will push my attempt at a test suite update, which includes limiting this feature to only Jinja2.

@kakulukia kakulukia closed this Jan 8, 2022
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.

Jinja2's import directive isn't supported
2 participants
@kkinder @kakulukia