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

v3 Twig Engine Integration #279

Merged
merged 4 commits into from
Mar 8, 2016
Merged

v3 Twig Engine Integration #279

merged 4 commits into from
Mar 8, 2016

Conversation

sghoweri
Copy link
Contributor

@sghoweri sghoweri commented Mar 1, 2016

@geoffp - Can you take a quick look at this first pass? I'd ideally like to see my test patterns I've included here added as proper tests but I know that will probably take some time for me to write up.

Summary of changes:

Pass number 3 at getting the Twig engine working with the latest Pattern Engines branch

  • Adding new Twig engine option to PL build: only simply includes, embeds, and extends should be working right now. Also working with Twig: the proper lineage tree generation, shorthand pattern references and code viewer
  • Updated required dependencies to both the Grunt and Gulp PL builds for building Twig templates
  • Added a few Twig examples for testing purposes
  • Updated Grunt and Gulp pattern watch tasks to glob all files within the base pattern folder (to support more than just mustache templates out of the box)

Important Side Note:
The "expandPartials: true" was the key to getting Twig to compile properly without jumping through a million hoops. Simple standalone templates will work without this however I believe this flag is what is allowing for recursive includes / extends / embeds without huge workarounds as the contents of each referenced partial appear to be getting combined together prior to Twig compiling the final template.

I'm using this as intended, yes?

Ghoweri, Salem (BOS-GEN) added 4 commits February 29, 2016 18:04
-Fixing Grunt watch task to watch all file deps inside of the _patterns folder vs just mustache and json files
-Cleaning up missing Gulp dependencies, Twig Media object example
…directory (vs just mustache and json files)
@geoffp
Copy link
Contributor

geoffp commented Mar 1, 2016

@sghoweri thank you for doing this! I'll look at this more in depth tomorrow.

Off the top of my head: yes -- expandPartials is exactly what sort of "flattens" each partial call during the recursive processing stage. The flag was created because Mustache needs to do that (to support Pattern Parameters and the other syntax extensions) and Handlebars needs to NOT do that (because it has its own internal partial registry). To that end, for a templating system with its own internal registry such as Handlebars, we have an optional hook called registerPartial() that can be implemented by the pattern engine, and will be called by addPattern() in pattern_assembler.js to pass along each pattern that PL finds and teach Handlebars how to call it by its canonical pattern key ("atoms-button" and so on).

Make sense?

I think we should move those twig patterns out of the main pattern tree and stick 'em in a file-based unit test, examples of which you can see in there for handlebars and underscore, at least. Shouldn't take too long to copy and paste 'em.

@geoffp
Copy link
Contributor

geoffp commented Mar 8, 2016

@sghoweri I'm going to merge this! We can't ship non-mustache patterns by default, but I think I can take the example templates you have and move them into proper unit tests pretty quickly.

geoffp added a commit that referenced this pull request Mar 8, 2016
@geoffp geoffp merged commit 601c143 into pattern-lab:pattern-engines Mar 8, 2016
geoffp added a commit that referenced this pull request Feb 23, 2018
geoffp added a commit that referenced this pull request Feb 23, 2018
geoffp added a commit that referenced this pull request Feb 23, 2018
bmuenzenmeyer pushed a commit that referenced this pull request Feb 23, 2018
geoffp added a commit that referenced this pull request Feb 23, 2018
geoffp added a commit that referenced this pull request Feb 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants