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

Template inheritance problem #7

Closed
aericson opened this issue Aug 31, 2016 · 9 comments · Fixed by #8
Closed

Template inheritance problem #7

aericson opened this issue Aug 31, 2016 · 9 comments · Fixed by #8
Assignees

Comments

@aericson
Copy link
Contributor

aericson commented Aug 31, 2016

Hey,

can you please take a look at aericson@40e5a4d
This test fails.

Shouldn't this be supported?

Thanks!

@clokep
Copy link
Owner

clokep commented Aug 31, 2016

At first glance it looks like it should be, yes. I'll try to take a look at this soon!

@dkopitsa
Copy link

I have same problem.

I have two templates.
'base.email' :

{% block subject %}Email subject{% endblock %}
{% block html %}
    <html>
     ..skip..  
    {% block html_content %}{%endblock %}
   ..skip..
    </html>
{% endblock %}

and 'test.email':

{% extends "base.email" %}
{% block html_content %}
<h1>Hello world</h1>
{% endblock %}

I expect that render_block_to_string('test.email', 'html') returns full html code with text 'hello world',
but I only get html without block html_content

@clokep
Copy link
Owner

clokep commented Aug 31, 2016

I think what's happening is that we:

  • Check the sub-template for the black.
  • The black isn't found so we check the base template.
  • We find the block and render it using only data from the base template.

Clearly the last step here is wrong and that we need to be using the block from the sub-template here.

@clokep
Copy link
Owner

clokep commented Aug 31, 2016

Thanks for the bug report, by the way! I was sure I had missed some cases in my test-cases. Clearly you found one!

@clokep
Copy link
Owner

clokep commented Aug 31, 2016

I have this mostly fixed.

@clokep clokep self-assigned this Aug 31, 2016
@clokep clokep closed this as completed in #8 Aug 31, 2016
@clokep
Copy link
Owner

clokep commented Aug 31, 2016

@aericson @dkopitsa Thanks for the reports! I just merged a PR (#8) which fixes this. There's a couple of minor clean-ups I want to do, but I should be able to do a release in the next few days. (Feel free to poke me if I don't!)

@dkopitsa
Copy link

It works for me. Thank you very much!

@aericson
Copy link
Contributor Author

👏

@clokep
Copy link
Owner

clokep commented Sep 1, 2016

Just pushed version 0.5 with these changes. Thanks for confirming it works!

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

Successfully merging a pull request may close this issue.

3 participants