Skip to content

changed template cache names to take into account the Twig C extension#1844

Merged
fabpot merged 1 commit intotwigphp:1.xfrom
fabpot:cext-cache-names
Sep 24, 2015
Merged

changed template cache names to take into account the Twig C extension#1844
fabpot merged 1 commit intotwigphp:1.xfrom
fabpot:cext-cache-names

Conversation

@fabpot
Copy link
Copy Markdown
Contributor

@fabpot fabpot commented Sep 21, 2015

No description provided.

@stof
Copy link
Copy Markdown
Member

stof commented Sep 21, 2015

👍

Comment thread CHANGELOG Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be for 1.22.3 now

@Tobion
Copy link
Copy Markdown
Contributor

Tobion commented Sep 24, 2015

👍 apart from the comment. I'd like to propose another PR on top of this when merged.

@fabpot fabpot merged commit 76ecd43 into twigphp:1.x Sep 24, 2015
fabpot added a commit that referenced this pull request Sep 24, 2015
…wig C extension (fabpot)

This PR was merged into the 1.x branch.

Discussion
----------

changed template cache names to take into account the Twig C extension

Commits
-------

76ecd43 changed template cache names to take into account the Twig C extension
@stof
Copy link
Copy Markdown
Member

stof commented Sep 29, 2015

@fabpot this broke the tests when the C extension is enabled

@ninsuo
Copy link
Copy Markdown
Contributor

ninsuo commented Sep 29, 2015

I don't understand why revoking the cache if the C extension is enabled.

  • there are no difference in the generated template?
  • shouldn't we take into account the disable_c_ext option?

Edit: just read the code, the template is effectively different if the C ext is used.

lib/Twig/Node/Expression/GetAttr.php:22: $compiler->raw('twig_template_get_attributes($this, ');

@Tobion
Copy link
Copy Markdown
Contributor

Tobion commented Sep 29, 2015

The tests are going to be fixed by #1852

fabpot added a commit that referenced this pull request Oct 4, 2015
…isions (Tobion)

This PR was merged into the 1.x branch.

Discussion
----------

fix template class name generation to prevent possible collisions

- fix template class name generation to prevent possible collisions
- save one inode per file
- fix tests as they are broken with the cext since #1844

Commits
-------

f379e81 fix template class name generation to prevent possible collisions and one inode per file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants