TrackableEntities / EntityFrameworkCore.Scaffolding.Handlebars

Scaffold EF Core models using Handlebars templates.
MIT License
209 stars 53 forks source link

Add Handlebars Block helpers support #74

Closed omatrot closed 5 years ago

omatrot commented 5 years ago

following our discussion in this issue

tonysneed commented 5 years ago

Instead of creating a separate IHbsBlockHelperService, I would suggest you instead update the IHbsHelperService interface by adding a BlockHelpers properties and a RegisterBlockHelpers method.

omatrot commented 5 years ago

Sure. Will do.

tonysneed commented 5 years ago

We will also need some sort of test for this. So perhaps you could update the ScaffoldingSample project to add a Handlebars block helper.

tonysneed commented 5 years ago

No need to close the pull request. Just update the code and push.

omatrot commented 5 years ago

Right, just on more thing: About the HbsHelperService constructor, should it take another parameter or should I create another constructor that takes a blockHelpers parameter?

tonysneed commented 5 years ago

Just add a blockHelpers parameter to the existing constructor.

omatrot commented 5 years ago

Would you like to have 2 methods: AddHandlebarsHelpers AddHandlebarsBlockHelpers

Because they're adding a singleton, the last one wins, and the helpers added in the previous call are lost. Unless there is another way that I'm unaware off right now.

tonysneed commented 5 years ago

Now that I've thought about it more, it's cleaner to just have a separate IHbsBlockHelperService service, the way you proposed. We can then have two methods: AddHandlebarsHelpers and AddHandlebarsBlockHelpers.

omatrot commented 5 years ago

Does it mean that I have nothing more to do?

tonysneed commented 5 years ago

Just rename the AddHandlebarsHelpers method, then update the ScaffoldingSample or ScaffoldingSample.TypeScript project to add a Handlebars block helper, so that we can validate that it works as expected.

omatrot commented 5 years ago

I've pushed the code.

tonysneed commented 5 years ago

@omatrot Just a couple very small corrections, and I'll go ahead and merge the PR.