patternlab-php-core
patternlab-php-core copied to clipboard
Special handling of Twig exists in Core
Hi Pattern-Lab folks,
I've created a Pattern Lab Fluid Edition - Fluid is the templating language of the TYPO3 and Neos CMS'es - and in that regard I came across a rather unfriendly implementation in your LineageHelper class.
It appears this class has special handling for parsing Twig lineages, which is a bit sad when you're coming to Pattern-Lab with an implementation for an additional template language and then find that you can't achieve the same level of integration because special respect was given to Twig.
In my humble opinion:
- This kind of implementation must not be part of the
corerepository. - It belongs more in the
patternengineassociated with the template engine - This really needs an implementation for such "Helper" style utilities, for example we could be allowed to configure the desired helper class names in the composer configuration we ship with our pattern engines, and ship our own helpers.
In the case of lineages in particular, this becomes important, as different template engines "include" or "render" or "extend" files using different syntaxes - which you currently are unable to support because of this hidden pseudo-dependency between your core and twig pattern engine implementations.
Hi! First of all, very exciting to learn about the Pattern Lab Fluid Edition you have created. The Pattern Lab ecosystem might seem a bit confusing at first, but it does make exactly this kind of extensibility possible.
I agree that Pattern Lab core should not have any code specific to a particular templating engine in it, and that core should not favour any templating engine over another. Does the Twig specific code in LineageHelper cause problems for your Fluid implementation?
Pattern Lab's architecture includes plugins and an event-based way to extend core. I think that especially this part of Pattern Lab should be better documented, but you can look at existing plugins for examples.
For lineage specific stuff a plugin can listen for the patternData.lineageHelperStart and patternData.lineageHelperEnd events. PatternEngines can have listeners too, so you could have a PatternLabListener.php in your patternlab-fluid-patternengine.
Hi @aleksip,
Does the Twig specific code in
LineageHelpercause problems for your Fluid implementation?
In this case, the specific problem is that PL matches a certain character, @, if it is first byte of the pattern name, and then processes the include/render statement in a special way to end up with the proper patterntype-patternname style identification of the pattern.
Fluid has no such marker and is not able to process render statements for path starting with a @ - but as far as I can tell, if PL was able to do this processing on a Fluid f:render partial="$name" syntax, the processing would result in the proper pattern name.
I'm not sure if the events will let me do what is necessary, but I'll certainly take a look. Thanks for the hint!
If it does serve my purpose then I'd suggest moving the Twig-specific chunk of processing into a similar event listener in the Twig pattern engine.
I'm not sure if the events will let me do what is necessary, but I'll certainly take a look. Thanks for the hint!
Do let us know if it works for you! I think it is great to have a PatternEngine for Fluid, and that it is important for the whole project to get it working.
If it does serve my purpose then I'd suggest moving the Twig-specific chunk of processing into a similar event listener in the Twig pattern engine.
Indeed, we should do that. Will open an issue about this.
Opened #158.
@NamelessCoder Well, I guess I was a bit too fast to open another issue, as this issue is more or less about the same thing, but maybe we can use this issue to track your progress with specific requirements related to the Fluid edition? :)
@aleksip I'll keep you updated here in this issue - but there may be a couple of days between the updates from me, I'm working on this project in smaller steps when there's time left over.
@aleksip I think I've gotten around most of the pattern resolving that was made harder by the special processing here. The implementation I have is still far from perfect though.
I've considered perhaps adding a public method to the PatternEngine component to offload this "resolve pattern name" logic, since it doesn't really fit to make an event listener responsible for this (IMHO, it should be an integral part of the PatternEngine's capabilities).
Would this be a fair way to solve that? If you agree, I would make a pull request for that (and one to follow up with refactoring the Twig handling to the Twig PatternEngine).
@NamelessCoder Thanks for the update! I must admit that I still haven't completely grasped the problem you are having, and the way you are proposing it should be fixed... Could you please flesh out your idea a bit more or (if it isn't a huge amount of work) submit a PR for further discussion? Would it be possible to implement your solution in a backwards compatible way?
@aleksip I would definitely prefer to base the discussion on a pull request, just wanted to make sure I don't waste effort. It would indeed be possible to implement it backwards compatible, and keep the (then deprecated) handling in place for as long as you need.
Rough sketch:
- Code in question: https://github.com/pattern-lab/patternlab-php-core/blob/46ae845673396ec67b24075d1ee66ac6ac10112d/src/PatternLab/PatternData/Helpers/LineageHelper.php#L56
- I would add a secondary interface for PatternEngine (for compatibility, to make this opt-in)
- I would declare a method on said interface that will process like the code above does now
- I would leave the code in place but give it the possibly transformed pattern name
- I would refactor the current code into the Twig PatternEngine, implement the new interface and bump version requirements (final step after all the above is released)
Hope that explains the idea!