Haml: Newlines are added to scripts with blocks in non-ugly, not in ugly

[mattwildig]

If the result of a ruby method that is passed a block is included with = then the behaviour is different in ugly and non-ugly modes.

An example with Rails’ link_to helper:

= link_to 'http://example.com/1' do
  %span Example 1

= link_to 'http://example.com/2' do
  %span Example 2

produces this in development (i.e with ugly set to false):

<a href="http://example.com/1"><span>Example 1</span>
</a>
<a href="http://example.com/2"><span>Example 2</span>
</a>

but in production (ugly true) produces:

<a href="http://example.com/1"><span>Example 1</span>
</a><a href="http://example.com/2"><span>Example 2</span>
</a>

This may not seem much, but depending on your styling can be significant.

A simpler, somewhat contrived, non-Rails example that more clearly shows the difference in results:

-def foo
  - "Hello"

=foo do
  .ignored
=foo

produces

Hello
Hello

in normal mode and

HelloHello

in ugly mode.

When compiling the push_script method adds a trailing newline in normal mode, but not in ugly mode if a block is given. If no block is given a newline is added even in ugly mode

In normal mode the script is handled by the format_script method, which strips trailing whitespace. With the newline provided by push_script the result is always a single newline. In ugly mode format_script (usually) doesn’t handle it, so the result is whatever number newlines are returned from the method, which could be zero.

I think we should _always_ add a newline after the script in ugly mode, to reduce the difference between the two modes. Making that change only causes a couple of tests to fail, where they are expecting a single trailing newline but get two.

(This issue was prompted by this SO question: http://stackoverflow.com/questions/21741015/haml-render-different-html-in-two-different-stages)

[afn]

@mattwildig Thanks for fixing this!

Less problematic, but probably related to this issue, is the fact that even when ugly mode is turned off, the indentation isn't correct for elements generated within a block. For example:

= link_to 'http://example.com/1' do
  %span Example 1
  %span Example 2

generates:

<a href="http://example.com/1"><span>Example 1</span>
<span>Example 2</span>
</a>

whereas

%a{href: 'http://example.com/1'}
  %span Example 1
  %span Example 2

yields:

<a href='http://example.com/1'>
  <span>Example 1</span>
  <span>Example 2</span>
</a>

[jess]

Just came across this issue myself and was about to submit an issue, but ran across this first. I'd already made an example app demonstrating the issue: https://github.com/jess/haml_test

I see the referenced commits are from 2014. I'm using latest version. Curious why fix hasn't made it way into the latest release.

[richkettle]

@jess did you come up with a workaround?

My current workaround is:

= link_to # do
    %span.glyphicon.glyphicon-new-window
    View
= "" #forces a new line when haml uglifies this html.
= link_to 'Edit', "#"

[ashfurrow]

Just came across this bug, any progress on #751?

[jess]

@richkettle no, I never followed through with it. Looks like you have a decent workaround. 👍

[k0kubun]

Non-ugly mode is dropped https://github.com/haml/haml/pull/894. The difference won't exist.

[k0kubun]

Hmm. While this is no longer a bug, suggested change in https://github.com/haml/haml/pull/751 looks good to me. It'll be breaking change on production but the behavior seems natural.

On the other hand, it'll make performance worse for continuous multiple scripts. So I want to know the use case if the change should be made on production.

[mattwildig]

@k0kubun the original intent of #751 was to make ugly and non-ugly modes behave (almost) the same. Now that there is only one mode it’s not needed from that point of view. I hadn’t really considered the issue of scripts with blocks behaving differently from those without.

This appears to be (loosely) related to #648 (which is in turn related to #465 which you recently commented on) as they all deal with when a script should have a newline added afterwards.

My thoughts are that we should aim for consistency if possible (i.e. always adding a newline after a script), but I don’t know if people are relying on the current behaviour. If we do make changes, now—in a major version bump—would be the best time.

[k0kubun]

My thoughts are that we should aim for consistency if possible (i.e. always adding a newline after a script), but I don’t know if people are relying on the current behaviour. If we do make changes, now—in a major version bump—would be the best time.

While I'm not sure https://github.com/haml/haml/pull/751 is the best solution for it, I agree that a major version bump is a good time to close https://github.com/haml/haml/issues/839 https://github.com/haml/haml/issues/648 https://github.com/haml/haml/issues/465.

---

If we modify whitespace behavior, it's good to remove this and this kind of runtime efforts. If we add newlines after script, we can remove it for :script in rstrip_buffer!.