-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[TwigBundle] removed the extensions setting
- Loading branch information
Showing
6 changed files
with
4 additions
and
32 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
9604573
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there no built-in way to enable these official extensions now ?
9604573
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only one
13
twig.extension
tag.9604573
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But this setting was used to make it easy to register the extensions from the official Twig-extensions repository. Currently, this require the user to define a service himself, which is less easy (and btw, its behavior was to add a tag). What about having config settings to enable the debug extension and the text extension (but only these one as user-defined extension should use the tag).
9604573
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with Fabien that it's odd to support 2 different ways of registering twig extensions: the
extensions
option for these 2 core extensions and thetwig.extension
tag otherwise. That being said, I agree with @stof that it's quite unfortunate that registering these two core extensions is much more work now. If we look at community bundles, we'll see that it's common to register a twig extension based on the presence of a semantic extension option.Perhaps it's best added back, but with a different name to represent these being "core" or "extra" extensions that are packages specifically to just this bundle:
or
Or maybe we create a separate bundle - TwigExtraBundle - and then have it be responsible for the twig-extensions. That would create a degree of separation, and we could probably do more inside that bundle, since - even if it were included in the SE - it wouldn't be core.
I don't want to make more work (which I kind of am), but food for thought.
9604573
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's just nuisance for early adopters and if you are unused to defining services in yaml. The quickest fix would be to just document this in twig section of the docs. I've done it myself but I really don't have an idea into which specific section I should put it.
e.g