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

marked some Twig_Environment methods as being internal #1889

Merged
merged 1 commit into from
Jan 27, 2016

Conversation

fabpot
Copy link
Contributor

@fabpot fabpot commented Oct 24, 2015

This PR marks some Twig_Environment methods as being internals:

  • getFunctions(), getFilters(), getTests(), getFunction(), getFilter(), getTest(), getTokenParsers(), getTags(), getNodeVisitors(), getUnaryOperators(), getBinaryOperators() because I don't see how they can be used in any useful way and anyway, getFunctions() and getFilters() do not return all possible functions or filters. Moreover, I think that all this logic could be refactored and moved into its own class for 2.0 (could be done by deprecating those existing methods in 1.x if we agree to merge this PR).
  • getGlobals() because it should only be called in a runtime context, not in a compilation one (so limiting its usage to internal use only allows for better control of usage).
  • initGlobals(), initExtensions(), initExtension() because they are private in Twig 2.0.

@fabpot fabpot force-pushed the more-internal-methods branch 2 times, most recently from 66e842c to 12689eb Compare October 24, 2015 21:44
@stof
Copy link
Member

stof commented Oct 25, 2015

Assetic relies on getFunction currently. However, it might be possible to avoid it thanks to changes I did when adding support for Twig 2.x. I will look into it after my vacations (which start today for 1 week)

@fabpot
Copy link
Contributor Author

fabpot commented Jan 25, 2016

@stof any news on this one?

@stof
Copy link
Member

stof commented Jan 26, 2016

I haven't worked on Assetic again. I will need to do it (I forgot about this discussion)

@stof
Copy link
Member

stof commented Jan 26, 2016

@fabpot I did the change in Assetic (was even easier than I though actually). See the PR linked above

@fabpot
Copy link
Contributor Author

fabpot commented Jan 27, 2016

Thanks @stof

@fabpot fabpot merged commit 212730a into twigphp:1.x Jan 27, 2016
fabpot added a commit that referenced this pull request Jan 27, 2016
…(fabpot)

This PR was merged into the 1.x branch.

Discussion
----------

marked some Twig_Environment methods as being internal

This PR marks some `Twig_Environment` methods as being internals:

 * `getFunctions()`, `getFilters()`, `getTests()`, `getFunction()`, `getFilter()`, `getTest()`, `getTokenParsers()`, `getTags()`, `getNodeVisitors()`, `getUnaryOperators()`, `getBinaryOperators()` because I don't see how they can be used in any useful way and anyway, `getFunctions()` and `getFilters()` do not return all possible functions or filters. Moreover, I think that all this logic could be refactored and moved into its own class for 2.0 (could be done by deprecating those existing methods in 1.x if we agree to merge this PR).

 * `getGlobals()` because it should only be called in a runtime context, not in a compilation one (so limiting its usage to internal use only allows for better control of usage).

 * `initGlobals()`, `initExtensions()`, `initExtension()` because they are private in Twig 2.0.

Commits
-------

212730a marked some Twig_Environment methods as being internal
stof added a commit to kriswallsmith/assetic that referenced this pull request Nov 11, 2016
…tic functions (stof)

This PR was merged into the 1.4-dev branch.

Discussion
----------

Avoid using Twig_Environment::getFunction to identify Assetic functions

As of Assetic 1.3.0, the Twig functions are using a special node class, which can be used to identify them instead.
The `getFunction` method is intended to be marked as an internal one in twigphp/Twig#1889

Commits
-------

39b99e8 Avoid using Twig_Environment::getFunction to identify Assetic functions
javiereguiluz added a commit to EasyCorp/EasyAdminBundle that referenced this pull request Nov 17, 2021
This PR was squashed before being merged into the 3.0.x-dev branch.

Discussion
----------

imagine_filter fail when lazy mode is enabled

Hi everybody !

The problem occurs when I use the VichImageType and its form widget associated.
This line `ea_apply_filter_if_exists('imagine_filter', formTypeOptions.imagine_pattern)` fail.

I got the error `Non-static method Liip\ImagineBundle\Templating\LazyFilterRuntime::filter() cannot be called statically` when liip_imagine is configurered in lazy mode

```yaml
# liip_imagine.yaml
liip_imagine:
    twig:
        mode: 'lazy'
```

Everything works very well if this mode is disabled.

You can reproduce that problem with the current version of Symfony ( 5.3 ) with php 8.

Just a more complex problem, getFilter is marked as internal by Twig twigphp/Twig#1889
So, we shouldn't use it... but I don't see how to avoid it :-/

I also changed the strict comparison, because on version 2 getFilter can return false but on version 3 it can return null.

Thanks a lot for your review :)

Commits
-------

bb2f839 imagine_filter fail when lazy mode is enabled
@VincentLanglet
Copy link
Contributor

Hi @fabpot, @stof.

I'm working own a custom twig lexer in order to lint twig files and I need to access the unary operator and the binary operator of the TwigEnvironnement, in order to do a similar logic than the Twig Lexer
https://github.com/twigphp/Twig/blob/3.x/src/Lexer.php#L457-L458

Those methods are internal, and are just doing

$this->extensionSet->getFoo();

Would it be ok to remove @internal on those methods or to provide another way to get those ?

@stof
Copy link
Member

stof commented Dec 6, 2022

Please open a dedicated discussion instead of commenting on a PR that was merged almost 7 years ago.

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.

None yet

3 participants