padrino / padrino-framework

Padrino is a full-stack ruby framework built upon Sinatra.
http://www.padrinorb.com
MIT License
3.36k stars 508 forks source link

Padrino-helpers ERB vs Haml/Slim syntax inconsistency #1839

Closed Arcovion closed 9 years ago

Arcovion commented 9 years ago

In 0.12.0 helpers were updated to use the equals sign in Haml/Slim but not ERB: PR with the changes ERB sample Haml sample Slim sample

So Slim/Haml now use = for all helpers, but ERB users still need <% %> instead of <%= %> for some of them. Usually - and = in Haml represent/are equivalent to <% %> and <%= %> in ERB, but Padrino is breaking that rule so a lot of users were caught out when they updated Haml/Slim templates: - wrap_layout= wrap_layout but not <% wrap_layout %><%= wrap_layout %>

Relevant issues/comments: https://github.com/middleman/middleman-guides/pull/387#issuecomment-65543093 https://github.com/middleman/middleman-guides/pull/353#issuecomment-63485377 https://github.com/middleman/middleman/issues/1275#issuecomment-42736148 https://github.com/middleman/middleman-blog/issues/207#issuecomment-40430539 https://github.com/middleman/middleman-blog/issues/221#issuecomment-66201460

I think to be consistent across ERB/Haml/Slim Padrino helpers should just use <%= %> and = if possible. Currently it's not easy/intuitive to translate the ERB examples in the Middleman guide to Haml/Slim.

nesquena commented 9 years ago

Just curious, if you use erubis instead of erb is this behavior the same? My understanding is that it's actually not easily possible to support the syntax you are suggesting in ERB alone. Instead, erubis must be used (as rails does). It's possible this is outdated knowledge but that is what I recall.

Arcovion commented 9 years ago

I get the same error with both (was Middleman::Renderers::ERb::Template, just tested Tilt::ErubisTemplate) if trying to use <%= %> with the wrap_layout helper, both work fine with <% %>.

ujifgc commented 9 years ago

Should we totally ignore ERB syntax and build our own ERB with equal signs and smart capturing like Rails did so translating ERB examples to Haml/Slim would be more easy/intuitive?

Arcovion commented 9 years ago

The Padrino documentation only includes Haml examples, so it's not immediately obvious how to use the helpers in ERB. How would you change the docs to explain the current behaviour? Have multiple examples for ERB/Slim/Haml on one page? Have a note explaining that some helpers are different in ERB?

Anything that clears the confusion works for me.

Arcovion commented 9 years ago

How about a deprecation warning for people who try to use the old Haml/Slim helpers? Just print something telling the user to change - wrap_layout= wrap_layout for example. Right now people are just getting a blank page and no message in the console, same with those trying to use <%= wrap_layout %> in their ERB files which results in an (unhelpful) error.

ujifgc commented 9 years ago

How would you change the docs to explain the current behaviour? Have multiple examples for ERB/Slim/Haml on one page? Have a note explaining that some helpers are different in ERB?

Yes, I would write good documentation with examples in all the supported templating languages.

How about a deprecation warning for people who try to use the old Haml/Slim helpers?

What would be a valid thing to do if we messed up the deprecation more than a year ago? Should we bring the old wrong behavior not following Haml and Slim syntax? If you know a way to detect the misuse of -, please tell.

Right now people are just getting a blank page and no message in the console, same with those trying to use <%= wrap_layout %> in their ERB files which results in an (unhelpful) error.

Yes, <%= block do %> error in ERB is scary. ERB and Erubis should be able to fail with a reasonable error instead of unexplained SyntaxError.

Arcovion commented 9 years ago

a way to detect the misuse of -

Didn't even think about that, I guess it's impossible. =[

I added a note regarding wrap_layout (which is the main 'gotcha') in our docs, more examples will follow in future. The Haml/Slim implementation is fine for me... it's just a shame ERB isn't consistent and is causing this much confusion.

Rails style ERB does sound good, we're actually forcing Erubis for .erb files already if that's any use. It sounds like a patch for this would be difficult to write though.

Yes, <%= block do %> error in ERB is scary. ERB and Erubis should be able to fail with a reasonable error instead of unexplained SyntaxError.

I thought this would be a quick fix but it doesn't look that way, things like this are annoying all around.