Skip to content

Improve formatting of multiline ERB #56

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Improve formatting of multiline ERB #56

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
255e26e
fix: improve formatting of multiline ERB
pelletencate Jul 4, 2024
e886b33
fix: block types weren't mutually exclusive, so reinstate the case st…
pelletencate Jul 4, 2024
c07db1c
doc: add commentary
pelletencate Jul 4, 2024
e2b337a
fix: add support for multiline comments
pelletencate Jul 5, 2024
538a8c3
test: update all tests
pelletencate Jul 5, 2024
6ec6d89
Merge branch 'nebulab:main' into multiline_erb
pelletencate Aug 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 41 additions & 24 deletions lib/erb/formatter.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class Error < StandardError; end
ATTR = Regexp.union(SINGLE_QUOTE_ATTR, DOUBLE_QUOTE_ATTR, UNQUOTED_ATTR, UNQUOTED_VALUE)
MULTILINE_ATTR_NAMES = %w[class data-action]

ERB_TAG = %r{(<%(?:==|=|-|))\s*(.*?)\s*(-?%>)}m
ERB_TAG = %r{(<%(?:=|-|#)*)(?:(?!\n)\s)*(.*?)\s*(-?%>)}m
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fyi: (?:(?!\n)\s) matches all whitespace except for newlines. Done this in order to preserve comment indentation on the first line

ERB_PLACEHOLDER = %r{erb[a-z0-9]+tag}

TAG_NAME = /[a-z0-9_:-]+/u
Expand Down Expand Up @@ -273,15 +273,19 @@ def format_ruby(code, autoclose: false)
SyntaxTree::Command.prepend SyntaxTreeCommandPatch

code = begin
SyntaxTree.format(code, @line_width)
# TODO: For single-lines, 7 should be subtracted instead of 2: 3 for opening, 2 for closing and 2 surrounding spaces
# Subtract 2 for multiline indentation or for the surrounding tags
# Then subtract twice the tag_stack size to respect indentation
width = @line_width - 2 - tag_stack.size * 2
SyntaxTree.format(code, width)
rescue SyntaxTree::Parser::ParseError => error
p RUBY_PARSE_ERROR: error if @debug
code
end

lines = code.strip.lines
lines = lines[0...-1] if autoclose
code = lines.map { |l| indented(l.chomp("\n"), strip: false) }.join.strip
code = lines.map { |l| indented(l.chomp("\n"), strip: false) }.join
p RUBY_OUT: code if @debug
code
end
Expand All @@ -307,31 +311,44 @@ def format_erb_tags(string)
format_text(erb_pre_match)

erb_open, ruby_code, erb_close = ERB_TAG.match(erb_code).captures
erb_open << ' ' unless ruby_code.start_with?('#')

case ruby_code
when RUBY_STANDALONE_BLOCK
block_type =
if erb_open.include?('#')
:comment
else
case ruby_code
when RUBY_STANDALONE_BLOCK then :standalone
when RUBY_CLOSE_BLOCK then :close
when RUBY_REOPEN_BLOCK then :reopen
when RUBY_OPEN_BLOCK then :open
else :other
end
end

# Format Ruby code, and indent if it's multiline
if %i[standalone other].include? block_type
ruby_code = format_ruby(ruby_code, autoclose: false)
full_erb_tag = "#{erb_open}#{ruby_code} #{erb_close}"
html << (erb_pre_match.match?(/\s+\z/) ? indented(full_erb_tag) : full_erb_tag)
when RUBY_CLOSE_BLOCK
full_erb_tag = "#{erb_open}#{ruby_code} #{erb_close}"
tag_stack_pop('%erb%', ruby_code)
html << (erb_pre_match.match?(/\s+\z/) ? indented(full_erb_tag) : full_erb_tag)
when RUBY_REOPEN_BLOCK
full_erb_tag = "#{erb_open}#{ruby_code} #{erb_close}"
tag_stack_pop('%erb%', ruby_code)
html << (erb_pre_match.match?(/\s+\z/) ? indented(full_erb_tag) : full_erb_tag)
tag_stack_push('%erb%', ruby_code)
when RUBY_OPEN_BLOCK
full_erb_tag = "#{erb_open}#{ruby_code} #{erb_close}"
html << (erb_pre_match.match?(/\s+\z/) ? indented(full_erb_tag) : full_erb_tag)
tag_stack_push('%erb%', ruby_code)
ruby_code.gsub!(/^/, ' ') if ruby_code.strip.include?("\n")
end

# Remove the first line if it only has whitespace
ruby_code.sub!(/\A((?!\n)\s)*\n/, '')

# Reset "common" indentation of multi-line comments
if block_type == :comment && ruby_code.strip.include?("\n")
# Leave comments intact, but if they're multiline, replace common indentation
ruby_code.gsub!(/^#{ruby_code.scan(/^ */).min_by(&:length)}/, ' ')
end

if ruby_code.strip.include?("\n")
full_erb_tag = "#{erb_open}\n#{ruby_code}#{indented(erb_close)}"
else
ruby_code = format_ruby(ruby_code, autoclose: false)
full_erb_tag = "#{erb_open}#{ruby_code} #{erb_close}"
html << (erb_pre_match.match?(/\s+\z/) ? indented(full_erb_tag) : full_erb_tag)
full_erb_tag = "#{erb_open} #{ruby_code.strip} #{erb_close}"
end

tag_stack_pop('%erb%', ruby_code) if %i[close reopen].include? block_type
html << (erb_pre_match.match?(/\s+\z/) ? indented(full_erb_tag) : full_erb_tag)
tag_stack_push('%erb%', ruby_code) if %i[reopen open].include? block_type
else
p ERB_REST: erb_scanner.rest if @debug
rest = erb_scanner.rest.to_s
Expand Down
16 changes: 10 additions & 6 deletions test/erb/test_formatter.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,12 +146,16 @@ def test_tagnames_with_dashes

def test_format_ruby
assert_equal(
"<div>\n" \
" <%= render MyComponent.new(\n" \
" foo: barbarbarbarbarbarbarbar,\n" \
" bar: bazbazbazbazbazbazbazbaz,\n" \
" ) %>\n" \
"</div>\n",
<<~ERB,
<div>
<%=
render MyComponent.new(
foo: barbarbarbarbarbarbarbar,
bar: bazbazbazbazbazbazbazbaz,
)
%>
</div>
ERB
ERB::Formatter.format("<div> <%=render MyComponent.new(foo:barbarbarbarbarbarbarbar,bar:bazbazbazbazbazbazbazbaz)%> </div>"),
)
end
Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/comments-2.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@
<%-# link_to 'Approve', some_path, class: 'something', disabled: disabled %>

<%# if smth %>
<%#else %>
<%# else %>
<%# end %>
2 changes: 1 addition & 1 deletion test/fixtures/comments-2.html.expected.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@
<%-# link_to 'Approve', some_path, class: 'something', disabled: disabled %>

<%# if smth %>
<%#else %>
<%# else %>
<%# end %>
2 changes: 1 addition & 1 deletion test/fixtures/comments.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,4 @@ This fails

<%#
hey
%>
%>
112 changes: 61 additions & 51 deletions test/fixtures/comments.html.expected.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,64 +1,74 @@
<%#
This fails
hey
hey
hey
hey %>
This fails
hey
hey
hey
hey
%>

<%#
This fails
hey
hey
hey
hey %>
This fails
hey
hey
hey
hey
%>

<%#
This fails
hey
hey
hey
hey %>
This fails
hey
hey
hey
hey
%>

<%# This fails
This fails
hey
hey
hey
hey %>

<%# This fails
This fails
hey
hey
hey
hey %>

<%# This fails
This fails
hey
hey
hey
hey %>

<%#This fails
This fails
hey
hey
hey
hey %>
<%#
This fails
This fails
hey
hey
hey
hey
%>

<%# This fails
This fails
hey
hey
hey
hey %>
<%#
This fails
This fails
hey
hey
hey
hey
%>

<%#
hey %>
This fails
This fails
hey
hey
hey
hey
%>

<%#
hey %>
This fails
This fails
hey
hey
hey
hey
%>

<%#
hey %>
This fails
This fails
hey
hey
hey
hey
%>

<%# hey %>

<%# hey %>

<%# hey %>
12 changes: 7 additions & 5 deletions test/fixtures/complex_case_when.html.expected.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@
<% when 'Foo::PaymentMethod' %>
<span><%= t(".payment.stripe_invoice") %></span>
<% else %>
<% Rails.logger.error.report(
StandardError.new(
"No human readable name found for payment method #{payment_method.class}",
),
) %>
<%
Rails.logger.error.report(
StandardError.new(
"No human readable name found for payment method #{payment_method.class}",
),
)
%>
<% end %>
<% else %>
<%= t(".payment.no_payment_method_found") %>
Expand Down
16 changes: 9 additions & 7 deletions test/fixtures/formatted-2.html.expected.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@
<%= react_component("HelloWorld", { greeting: "Hello from react-rails." }) %>

<%= react_component("HelloWorld", { greeting: "Hello from react-rails." }) %>
<%= react_component(
"HelloWorld",
{ greeting: "Hello from react-rails." },
{ greeting: "Hello from react-rails." },
{ greeting: "Hello from react-rails." },
{ greeting: "Hello from react-rails." },
) %>
<%=
react_component(
"HelloWorld",
{ greeting: "Hello from react-rails." },
{ greeting: "Hello from react-rails." },
{ greeting: "Hello from react-rails." },
{ greeting: "Hello from react-rails." },
)
%>

</div>
14 changes: 8 additions & 6 deletions test/fixtures/formatted.html.expected.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
<% link_to "Very long string here and there",
very_very_very_long_long_long_pathhhhhh_here,
opt: "212",
options: "222sdasdasd",
class: " 322 ",
dis: diss %>
<%
link_to "Very long string here and there",
very_very_very_long_long_long_pathhhhhh_here,
opt: "212",
options: "222sdasdasd",
class: " 322 ",
dis: diss
%>
24 changes: 14 additions & 10 deletions test/fixtures/if_then_else.html.expected.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,21 @@
<% eeee ? "b" : c %>
<% eeee ? a : c %>

<% if longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong
a
else
c
end %>
<%
if longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong
a
else
c
end
%>

<% if longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong
"a"
else
"c"
end %>
<%
if longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong
"a"
else
"c"
end
%>

<div <% if eeee then "b" else c end %>></div>
<div <% if eeee then a else c end %>></div>
Expand Down
28 changes: 15 additions & 13 deletions test/fixtures/long_deep_nested-2.html.expected.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,21 @@
<div>
<div>
<div>
<%= react_component(
"HelloWorld",
{
greeting:
"Hello from react-rails react-rails react-rails react-rails react-rails react-rails react-rails.",
},
{
greeting:
"Hello from react-rails react-rails react-rails react-rails react-rails react-rails react-rails.",
},
{ greeting: "Hello from react-rails." },
{ greeting: "Hello from react-rails." },
) %>
<%=
react_component(
"HelloWorld",
{
greeting:
"Hello from react-rails react-rails react-rails react-rails react-rails react-rails react-rails.",
},
{
greeting:
"Hello from react-rails react-rails react-rails react-rails react-rails react-rails react-rails.",
},
{ greeting: "Hello from react-rails." },
{ greeting: "Hello from react-rails." },
)
%>
</div>
</div>
</div>
Expand Down
Loading