Support formatting scripts into multiple lines

[kddnewton]

For more information, see prettier/plugin-ruby#1161

[phacks]

I am looking into this issue and I think I made some progress. Let me know if you’d like me to continue looking into this (or if you already are), and if you think the direction I went for is a good one. Thanks!

Testing suite

In order to check my progress, I added a few tests to the test suite, in test/script_test.rb:

def test_script_multiline
  # Should not wrap on multiple lines for short lines
  assert_format(<<~HAML)
  = f.input :roles, as: :check_boxes
  HAML

  # Reproduction case in the issue
  assert_format(<<~HAML)
  = f.input :roles,
          as: :check_boxes,
          collection: Role.global_roles,
          label_method: lambda { |role| I18n.t(role.name, scope: "rolify.role.roles") },
          value_method: :name,
          checked: f.object.roles_name
  HAML

  # rendering a ViewComponent (a common occurrence in our codebase)
  assert_format(<<~HAML)
  = render(DesignSystem::FAIcon.new(name: "info",
      type: :uploaded,
      options: {class: "text-sm text-purple-500"}))
  HAML
end

And here’s my update to the visit_script method that passes all those tests (plus all the rest):

def visit_script(node)
  with_children(node) do
    if line = q.literal_lines[node.line]
      q.text(line)
    else
      q.text("&") if node.value[:escape_html]
      q.text(node.value[:preserve] ? "~" : "=")
      q.text(" ")

      # Changed code starts here
      formatted = SyntaxTree.format(node.value[:text].strip)

      # Valid ruby syntax does not mean valid haml syntax in a script
      # Tweak the formatted output to make sure it is valid haml
      formatted = formatted
        # if a line break happens after anything else than a comma, remove it
        .gsub(/([^,])[\r\n]/, '\1')
        # when a line break has been removed, make sure the spacing after delimiters
        # (commas, parenthesis, curly brackets) is correct. For example:
        # `foo:\n  bar` => `foo:bar` => `foo: bar`
        # `.new(\n  args) => `.new(args)` => `.new(args)`)
        .gsub(/(\S)[^\S\r\n]{2,}(\S)/, '\1 \2') # replaces multiple whitespace by one
        .gsub('( ', '(')
        .gsub(' )', ')')
        .gsub('} ', '}')
      
      q.text(formatted)
    end
  end
end

The various .gsubs to make the “valid ruby”-code become “valid haml” feel a bit hacky and error prone. Would love your ideas there to see if we can make this more elegant (or find a way to not have the issue at all).

Thanks!

[kddnewton]

I'd be super nervous about going with gsub to be honest. That's effectively treating it as if it's a regular language, when we know it's at least one level higher than that.

I think the answer is actually going to be a bit more complicated. Ideally it would parse all of the nodes in the tree that contain Ruby content together, that way we'd be able to handle

= if ...
= elsif ...

because right now that's going to throw an error any time you only are parsing one of those nodes. We'll also need to add in end keywords since they don't currently exist in HAML. Then, when everything is put back together, we should format it as normal. This will insert Breakable nodes into the doc tree. Once those are there, we should employ what we already to do remove breakables here: https://github.com/ruby-syntax-tree/prettier_print/blob/6521fadd3614b0aeacbb9baef9bb85e744b526e8/lib/prettier_print.rb#L917-L943. Except it'll have to use a slightly different algorithm because we only want to remove breakables at anything beyond the first level of indentation.

Anyway, that's the way I want to go, because I feel like that's a more complete solution. Otherwise we're going to be maintaining gsubs that may or may not be correct for all code, it's just not really possible to tell.

[phacks]

Thank you for the detailed answer!
As you mentioned, the solution will likely be more complex than my rough suggestion and will require knowledge of this gem, ASTs, and HAML that I might not currently have, and I might need to ask you for guidance and help.
I want to be respectful of your time—would you be comfortable with me looking into this issue or would you rather it be you, or someone else more experienced with all this?

[kddnewton]

By all means! Feel free. If you want to keep a PR open and just iterate there that'd be fine, happy to review it as you go.