sonata-project / SonataFormatterBundle

Symfony SonataFormatterBundle
https://docs.sonata-project.org/projects/SonataFormatterBundle
MIT License
81 stars 117 forks source link

Add media extension for formatter #696

Closed jordisala1991 closed 2 years ago

jordisala1991 commented 2 years ago

Subject

I am targeting this branch, because this is in preparation for 5.0.

Changelog

### Added
- Added `MediaExtension` to render Media inside Formatters.
- Added ability to use Twig runtimes on your Formatter Extensions via `getAllowedRuntimes`.
jordisala1991 commented 2 years ago

WDYT @VincentLanglet ? I had to add some stuff to make it work with Runtimes because otherwise don't get loaded by those custom Twig Environments.

jordisala1991 commented 2 years ago

There are 2 things in this PR:

  1. Trying to solve the MediaExtension issue: It is implemented on MediaBundle but completely broken. It should be implemented here.

  2. With Runtimes of twig we can separate extensions and their logics. The problem is that we cant use them on formatters if we dont configure the Environment. Currently without this Pr there is no way someone can add a custom extension for formatters is those use Runtimes (like the media extension does).

If we agree on implementation I can add missing docs and polish it a little bit.

VincentLanglet commented 2 years ago

If we agree on implementation I can add missing docs and polish it a little bit.

Except our little discussion, I'm fully ok with it.

VincentLanglet commented 2 years ago

Is it ready to merge ?

VincentLanglet commented 2 years ago

If I understand correctly, we should provide a PR to deprecate https://github.com/sonata-project/SonataMediaBundle/blob/4.x/src/Twig/Extension/FormatterMediaExtension.php isn't it ?

jordisala1991 commented 2 years ago

I would say we should remove it directly. That class does nothing and it is not registered on the container.

VincentLanglet commented 2 years ago

I would say we should remove it directly. That class does nothing and it is not registered on the container.

Removing something is theoretically a BC-break, but this seems ok to me.