PatternEngines: support multiple template engines, v2

[geoffp]

NOT READY FOR MERGE YET, this PR is only for review.

Hey, @bmuenzenmeyer -- this one is merged and up to date as of the time you created the pattern-engines branch, which this PR is pointed at. It should merge, build, and it passes all the tests now, though it silently fails to build pseudo-patterns. I'm looking in to that today.

Once again -- there's kind of a lot here, and it's WIP, so if there's anything I can do to make this more reviewable, let me know.

TODO:

• Fix pseudo-pattern generation
  • Generate pseudo-patterns
  • Investigate why they're being placed in the menus out of order
• Finish factoring out all direct uses of mustache
• Hunt down all Mustache-specific "finder" regexes in the hunters and move them into the Mustache plugin
  • finders in pattern_assembler
  • finders elsewhere?
• Use or remove the util module
• Migrate unit tests to use "real" instantiated oPatterns
  • Machinery to create empty/custom oPatterns programatically
  • Replace object-literal fake oPatterns in unit tests with real oPatterns
• Take out console.log()s someday, we like them for now

[bmuenzenmeyer]

Thanks @geoffp will try to look at this soon!

[bmuenzenmeyer]

Hi @geoffp

I am going to look at this next, specifically to have this as part of 1.0 or to kick off a post 1.0 world. Can you make sure the TODO list above it up to date? console logs are fine as they might be helpful to me yet.

[geoffp]

Sure thing. I made some more progress factoring out mustache-specific regexes and beefed up a bunch of unit tests yesterday that I'll be checking in today. Here's how it's shaking out:

This architecture depends on oPattern objects getting smart about what kind of template they are, so an oPattern figures out when its get created what PatternEngine it needs to be able to render itself, based on file extension, then goes and looks for a PatternEngine plugin that reports that it handles that file extension. If it finds one, it retains a reference to that plugin. Methods on the oPattern's prototype (findPartials, findPartialsWithStyleModifiers, etc.) then delegate to functions provided by the pattern engine.

The overall goal of this is that the bulk of the application doesn't need to care what the backing PatternEngine is; oPattern provides a uniform API and delegates functionality to the appropriate plugin.

There are multiple "safeties" that fall back to Mustache in the event that anything gets weird or is unaccounted for. I expect that as the gaps start to close we won't need those much anymore, but it helps with the transition.

[sghoweri]

@geoffp and @bmuenzenmeyer For what it's worth, I'm also checking this out right now to see how straightforward it's going to be to add in new rendering engines (like Twig) with this direction you guys are heading. Already got a good bit of experience dealing with the new PL2 setup so I'm just reviewing to see how I might be able to help out with this as well.

[geoffp]

Right on. I'll push the latest after lunch so you can review. I think I only have a couple list item hunter unit tests left to fix up before I pass again.

[bmuenzenmeyer]

Also note - I keep updating dev now to 0.15.1 which hopefully doesn't break/conflict too much.

As of right now I see this PR as out of scope for a v1.0.0 release but I'd love to see it there!

[geoffp]

I'll pull the latest dev and see what's what.

I wouldn't want to rush this work or gloss over consensus just to get into
v1.0. I feel it won't really be proven until we've implemented a
non-Mustache plugin and written some patterns with it. That said, I am free
to spend lots of work time on it now.

Question: am I right in thinking that pattern parameters are essentially an
extension to Mustache syntax? And therefore, should the Mustache plugin be
the thing that knows how to handle that syntax?

On Wed, Nov 25, 2015 at 12:06 PM Brian Muenzenmeyer <
notifications@github.com> wrote:

Also note - I keep updating dev now to 0.15.1 which hopefully doesn't
break/conflict too much.

As of right now I see this PR as out of scope for a v1.0.0 release
https://github.com/pattern-lab/patternlab-node/issues?q=is%3Aopen+is%3Aissue+milestone%3Av1.0.0
but I'd love to see it there!

—
Reply to this email directly or view it on GitHub
#191 (comment)
.

[geoffp]

cc @ajfuller

[bmuenzenmeyer]

@geoffp Yes patternParameters and styleModifiers are really extensions/hacks/patternlab-special-sauce applied before vanilla mustache parsing. As long a templating engine supports variables, a plugin for that language should be able to support pattern parameters that make since for them.

Handlebars is obvious (I think) since it's relationship to mustache

<h1>{{title}}</h1>

Twig (before mentioned) seems to have this too, but with more flavor:

{{ var }}
{{ var|escape }}
{{ var|e }} 

Thinking about this quickly, I agree that the mustache plugin should be responsible for how to parse these, as it wouldn't know what to do with var|escape but a theoretical implementation of a twig parser would. Hope this makes sense.

I am also in agreement that this shouldn't be rushed into 1.0.0 - 1.0.0 as it currently stands is not too far off, and getting pattern engines fully baked and tested requires robust thought.

I really, REALLY, REALLY appreciate everyone's bandwidth on this. Cannot emphasize enough how energizing it is 🚀

[bmuenzenmeyer]

Naive git question, should I accept this PR so others can work on it easier, or can everyone just as effectively add a remote to geoffp:pattern-engines?

[sghoweri]

I'm fine either or, so long as we can keep track where the latest patternengine code is living (although merging the PR into this branch would probably make the most sense using the Git Flow model of feature development).

Small update from my end: I'm still working out some bugs (like why the lineage logic isn't showing up) but just wanted to mention that using @geoffp's "old" code from about a week ago, I was able to get a few test twig templates to render properly! Still lots of work to do but things are looking really promising.

[geoffp]

Yeah, that makes perfect sense. That's what it looked like from reading the code, so it's good to confirm. That also explains the oPattern.template/oPattern.extendedTemplate split. So then, the .extendedTemplate copy is the Mustache with all of that secret sauce sort of desugared and ready to be passed to the Mustache renderer?

I'm not sure what the best Git thing to do is. If people clone my geoffp:pattern-engines, then I think they'd be submitting pull requests to me, and then I'd periodically send them to you. I think if you give me push access (if you're comfortable with that) this repo/branch can be the main line of development, and anyone else who wants to contribute can submit pull requests against this branch. There would just need to be an understanding that I never push to anything but feature branches, and you're the guy in the middle who merges PRs.

[geoffp]

@sghoweri that's awesome! I just checked in the code that "should" make stuff like lineage work, though I haven't fully tested it. But it's much closer to complete, and I'm passing all the unit tests, including some new ones, except for the list item hunter stuff, which looks like it's going to be tricky (seems tightly coupled to Mustache). All the regexes that hunt for that stuff are factored out into the Mustache plugin.

I see no problem with merging the pull request and going from there, except for the fact that this PR is a nice place to talk. But there's always another PR.

[bmuenzenmeyer]

I am in favor of merging. Will do so after this comment. I like the notion of attempt to push smaller changes to this branch during development. I am also super pumped that you are all interested in unit tests. These came later in the total port process and that's why not everything is well structured for them - some, especially in pattern_assembler, are more akin to integration tests, but useful nonetheless. If you could keep in mind any coverage you are adding against this list please let me know!

So then, the .extendedTemplate copy is the Mustache with all of that secret sauce sort of desugared and ready to be passed to the Mustache renderer?

I did this to keep the template a pristine copy of actual file contents, and then in the case of templates with partials, the extendedTemplate is a place that "unfurls" or glues everything together before one big render. It seemed like the best way to negotiate some of the data.son and pattern.json requirements that are inherent in pattern lab functionality - but does make some things interesting indeed. Let me know if this doesn't make sense.

[geoffp]

No, that makes total sense; I probably would have done it the same way. Though, I remember some parts of the code that depend explicitly on extendedTemplate, and I can't remember if all patterns are guaranteed to have an extendedTemplate. I think they are.

One more question: the list item hunter is a challenge -- it seems pretty bound up with Mustache syntax, and I'm not 100% clear on what the syntax is right now, or what it should be in other template engines. What do you think about that?

[bmuenzenmeyer]

and I can't remember if all patterns are guaranteed to have an extendedTemplate. I think they are.

To my knowledge all patterns should have an extendedTemplate, even if it's just a straight copy of the template, for consistency's sake.

This is most prominently illustrated here:

patternlab-node/builder/pattern_assembler.js

Line 191 in e8f060f

currentPattern.extendedTemplate = currentPattern.template;

where the template is copied to the specific instance of the pattern (before anything else really happens) so it may start anew and not be tainted by previous runs of the partial.

One more question: the list item hunter is a challenge -- it seems pretty bound up with Mustache syntax, and I'm not 100% clear on what the syntax is right now, or what it should be in other template engines. What do you think about that?

http://patternlab.io/docs/data-listitems.html is the authoritative resource on how these function as far as I am concerned, and it was quite the challenge for me to create this (probably my own failing). I did it in isolation and my best interpretation of what Dave did over in PHP. These are, perhaps, the biggest of the extensions or pre-parsings of mustache, as we scrape the template for a listitem string, and then copy the contents N number of times into the extended template. I could see both the implementation and the syntax being flexible (even across engines), as long as we can do something approachable to users of each template syntax and hopefully read the same listItems.json file.