search

sighmon / mjml-rails

MJML + ERb/Haml/Slim view template
https://mjml.io
Other
296 stars 64 forks source link

Haml #3

Closed manuelmeurer closed 7 years ago

manuelmeurer commented 8 years ago

I'm trying to use Haml instead of Erb to write my MJML templates. When I simply keep the mjml suffix (mail.mjml) the Haml is rendered as-is (not converted to HTML). When I use any combination of mjml, haml and html (e.g. mail.mjml.haml) the template cannot be found. How can I accomplish this?

kdiogenes commented 8 years ago

I started a branch to add haml support: https://github.com/fnix/mjml-rails/tree/haml-handler

From my understanding of how Rails render a template, the last extension determine the handler to be used, so adding .haml to the end will not append a handler as occur with assets.

I'd also would like to ask for help with the code in my branch. When running tests I'm get the following error:

/home/kadu/.rvm/rubies/ruby-2.2.4/bin/ruby -w -I"lib:lib:test" -I"/home/kadu/.rvm/gems/ruby-2.2.4@mjml-rails/gems/rake-11.1.2/lib" "/home/kadu/.rvm/gems/ruby-2.2.4@mjml-rails/gems/rake-11.1.2/lib/rake/rake_test_loader.rb" "test/**/*_test.rb" 
/home/kadu/programming/hackerspace/mjml-rails/lib/generators/mjml/mailer/mailer_generator.rb:2:in `<top (required)>': undefined method `application' for Rails:Module (NoMethodError)
    from /home/kadu/.rvm/gems/ruby-2.2.4@mjml-rails/gems/activesupport-4.2.6/lib/active_support/dependencies.rb:274:in `require'
    from /home/kadu/.rvm/gems/ruby-2.2.4@mjml-rails/gems/activesupport-4.2.6/lib/active_support/dependencies.rb:274:in `block in require'
    from /home/kadu/.rvm/gems/ruby-2.2.4@mjml-rails/gems/activesupport-4.2.6/lib/active_support/dependencies.rb:240:in `load_dependency'
    from /home/kadu/.rvm/gems/ruby-2.2.4@mjml-rails/gems/activesupport-4.2.6/lib/active_support/dependencies.rb:274:in `require'
    from /home/kadu/programming/hackerspace/mjml-rails/test/generator_test.rb:2:in `<top (required)>'
    from /home/kadu/.rvm/gems/ruby-2.2.4@mjml-rails/gems/rake-11.1.2/lib/rake/rake_test_loader.rb:10:in `require'
    from /home/kadu/.rvm/gems/ruby-2.2.4@mjml-rails/gems/rake-11.1.2/lib/rake/rake_test_loader.rb:10:in `block (2 levels) in <main>'
    from /home/kadu/.rvm/gems/ruby-2.2.4@mjml-rails/gems/rake-11.1.2/lib/rake/rake_test_loader.rb:9:in `each'
    from /home/kadu/.rvm/gems/ruby-2.2.4@mjml-rails/gems/rake-11.1.2/lib/rake/rake_test_loader.rb:9:in `block in <main>'
    from /home/kadu/.rvm/gems/ruby-2.2.4@mjml-rails/gems/rake-11.1.2/lib/rake/rake_test_loader.rb:4:in `select'
    from /home/kadu/.rvm/gems/ruby-2.2.4@mjml-rails/gems/rake-11.1.2/lib/rake/rake_test_loader.rb:4:in `<main>'
rake aborted!
Command failed with status (1): [ruby -w -I"lib:lib:test" -I"/home/kadu/.rvm/gems/ruby-2.2.4@mjml-rails/gems/rake-11.1.2/lib" "/home/kadu/.rvm/gems/ruby-2.2.4@mjml-rails/gems/rake-11.1.2/lib/rake/rake_test_loader.rb" "test/**/*_test.rb" ]
/home/kadu/.rvm/gems/ruby-2.2.4@mjml-rails/bin/ruby_executable_hooks:15:in `eval'
/home/kadu/.rvm/gems/ruby-2.2.4@mjml-rails/bin/ruby_executable_hooks:15:in `<main>'
Tasks: TOP => default => test
(See full trace by running task with --trace)

I'm using Rails.application.config.generators.options[:rails][:template_engine] == :haml to determine if haml is being used as the default template engine, but in the context of the gem there isn't a Rails application yet.

A configurable is the better option for this case? http://stackoverflow.com/questions/24104246/how-to-use-activesupportconfigurable-with-rails-engine/24151439#24151439

kdiogenes commented 8 years ago

After struggling a lot with this I don't think it's practical to have ERb and HAML support in the same gem, so I forked it: https://github.com/fnix/mjml-haml

manuelmeurer commented 8 years ago

@cerdiogenes But it seems like in your fork you are actually implementing ERb and Haml support...?

kdiogenes commented 8 years ago

I will do a cleanup soon, I will support only haml.

manuelmeurer commented 8 years ago

But why? 😄 It seems to be entirely possible to use ERb or Haml according to Rails.application.config.generators.options[:rails][:template_engine].

kdiogenes commented 8 years ago

Yes, but it's only the runtime part. I didn't find a way to use haml in the tests.

kdiogenes commented 8 years ago

@manuelmeurer I just published mjml-rails: https://rubygems.org/gems/mjml-haml

manuelmeurer commented 8 years ago

Thanks for your work on this, I'm not sure a separate gem that duplicates all the logic is the right approach though to be honest. Are the tests the only reason this cannot be implemented in mjml-rails? @sighmon What is your opinion about this issue?

sighmon commented 8 years ago

@cerdiogenes nice work. I do agree with @manuelmeurer though - I'd rather we fixed the tests in mjml-rails than split it into another gem if the majority of the logic is the same. I'll try and have a play with your solution over the weekend.

kdiogenes commented 8 years ago

@sighmon the main issue is with this commit: https://github.com/fnix/mjml-haml/commit/bcbbfc4d9d3bf1475f99eb58eae78e8c29b261a8

The code in lib/generators/mjml/mailer/mailer_generator.rb is executed in the gem initialization, so it's occur very early during tests without giving us a chance to mock/stub what is necessary to the generator use haml.

Sorry for the duplicated work, your gem is very well maintained and I also would like to keep it togheter, but I just don't have the time/expertise to keep it together. I also thought in make my gem only monkey patch yours, but I also have to figure out how to use your tests. If you find any good solution, let me know, I will be glad to keep haml support here.

kdiogenes commented 8 years ago

I had an insight in how I could add HAML support and resolved to scratch the itch. The branch https://github.com/fnix/mjml-rails/tree/haml share the test database and have generators working for HAML and ERB.

@sighmon I would like that you review how I organized the code and if you have any comments so I can finish it and make a pull request.

Some points that I also would like to hear comments:

Finally I don't used these changes in a Rails application without the haml-rails gem and I'm requiring some haml files, so this will probably break your application. This is the next thing I will address.

sighmon commented 7 years ago

@manuelmeurer @cerdiogenes Does v2.4.0 work for you with :haml? @mswiszcz wrote a fix.

manuelmeurer commented 7 years ago

Sorry, I'm not using MJML anymore...

kdiogenes commented 7 years ago

I'm actually using my own gem: https://github.com/fnix/mjml-haml

I can't wait so much time for feedback :pensive:

sighmon commented 7 years ago

@cerdiogenes No worries - sorry for the delay (I met a girl 😔).