algolia / frontman

💎 A Ruby-based static website generator
https://rubygems.org/gems/frontman-ssg
MIT License
109 stars 4 forks source link

Add slim support #23

Closed westonganger closed 3 years ago

westonganger commented 4 years ago

For migrating from middleman sites it would be very helpful to support .slim templates.

Also relevant to #10. I think this should be in core (via a require("slim") rescue clause instead of gem dependency) or it could be a separate plugin

Checklist:

I am hoping someone can help with wrap_layout .

DevinCodes commented 4 years ago

Hi @westonganger!

Thank you for your contribution to Frontman! We highly appreciate it.

First of all, I very much like the idea of having Frontman support Slim templates. However, I'm not sure we should add support for this out of the box, just like Middleman doesn't support it out of the box (if I remember correctly).

I think the best way to move forward with this, is to put a solution in place for #10 , which would allow users to add custom renderers. You could then create a separate gem that exposes the Frontman::SlimRenderer you've created so that people can easily include it in their projects.

I've added a comment in #10 to propose a way of adding custom renderers; I would love to hear what you think about this, or if you have any other ideas about it.

I look forward to hearing your thoughts on this 😄

Cheers!

westonganger commented 4 years ago

Actually Middleman does support .slim templates out of the box. You simply have to require 'slim' at the top of your config.rb.

One reason why I think this should be in core is that it is the last of the big 3 templating languages for the Ruby language. Erb, Haml, Slim. Any other templating language is more obscure for Ruby and should rather be implemented as a plugin.

DevinCodes commented 4 years ago

You're right; Middleman does support .slim templates indeed. My bad, I wasn't aware of this 🙂

That, combined with the fact that Slim is indeed one of Ruby's more prominent templating languages, I agree with you that we can add it to the core.

While it would work for me to let users add require 'slim' in their config.rb if they want to use Slim, I think an idea worth exploring is already making it available (like we do with the .erb renderers for example). This way, users but can use .slim right away without adding the require statement in their config.rb. This would also remove some of the checks in the code, cleaning it up a bit. Before we would go this route, however, I think we should see what impact this has on performance in server startup and site generation.

What do you think?

westonganger commented 4 years ago

If you want to have a direct dependency that could be an option too.

To be honest I always liked the require 'haml' & require 'slim' so that we didnt have to install extra dependencies in our bundle when not needed. That being said if you take this approach I would appreciate it if this applied to both haml and slim

DevinCodes commented 4 years ago

I think we should require it by default, this way it works out of the box, and we're sure that the included version of Slim is supported by our renderer. I checked the impact on performance, and it's neglectable (1-2ms), so let's include it by default 🙂

westonganger commented 4 years ago

Ok I have now added it as a dependency. Now I just need some assistance with the remaining checklist items.

DevinCodes commented 4 years ago

Great, thank you! I can dedicate some time this Wednesday (16 September) in the morning (Paris time) to help with the partial locals and wrap_layout functionality!

DevinCodes commented 4 years ago

Sorry for the delay, it has been a super busy week.

I think we can refactor the methods related the the buffer, so that we don't have to do checks like if haml_locals and if slim_buffer, etc. Instead, maybe we can move this logic into the renderers for example, which would make this more scaleable when we want to add new renderers. This would make it easier to make the wrap_layout work as well. WDYT?

I'll try to work on a PR for that this week, and target your branch as the base 🙂

DevinCodes commented 4 years ago

Quick update: I've been working on a refactor of the buffering, you can follow the progress on the feat/refactor-buffering branch. I didn't have the time yet to work on adding wrap_layout support, I'll try to add that soon 😄

DevinCodes commented 3 years ago

I see I introduced some linting issues in my PR onto your branch, I'll open another PR to fix that today @westonganger

DevinCodes commented 3 years ago

Closed in favor of #41