Skip to content

Commit

Permalink
HTMLFormatter - text between inline elements (#2010)
Browse files Browse the repository at this point in the history
An attempt to fix #2008

The core issue with those examples is they get into an "incomplete" state on the first format, causing `mix format --check-formatted` to fail. A couple of examples on master:

```
iex> input = """
...> <span><%= link("Edit", to: Routes.post_path(@conn, :edit, @post)) %></span> |
...> <span><%= link("Back", to: Routes.post_path(@conn, :index)) %></span>
...> """

iex> IO.puts formatted = Phoenix.LiveView.HTMLFormatter.format(input, [])
<span><%= link("Edit", to: Routes.post_path(@conn, :edit, @post)) %></span>
| <span><%= link("Back", to: Routes.post_path(@conn, :index)) %></span>

iex> IO.puts Phoenix.LiveView.HTMLFormatter.format(formatted, [])
<span><%= link("Edit", to: Routes.post_path(@conn, :edit, @post)) %></span> |
<span><%= link("Back", to: Routes.post_path(@conn, :index)) %></span>
```

```
iex> input = """
...> <span><%= @user_a %></span> X <span><%= @user_b %></span>
...> """

iex> IO.puts formatted = Phoenix.LiveView.HTMLFormatter.format(input, [line_length: 5])
<span>
  <%= @user_a %>
</span>
X
<span>
  <%= @user_b %>
</span>

iex> IO.puts Phoenix.LiveView.HTMLFormatter.format(formatted, [line_length: 5])
<span>
  <%= @user_a %>
</span>
X <span>
  <%= @user_b %>
</span>
```

/cc @feliperenan @josevalim
  • Loading branch information
leandrocp committed May 8, 2022
1 parent ed409f2 commit 34ea4d1
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 7 deletions.
26 changes: 23 additions & 3 deletions lib/phoenix_live_view/html_algebra.ex
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,19 @@ defmodule Phoenix.LiveView.HTMLAlgebra do
defp inline_break(prev_node, next_node) do
cond do
block_preserve?(prev_node) or block_preserve?(next_node) ->
if text_ends_with_space?(prev_node) or text_starts_with_space?(next_node),
do: " ",
else: ""
cond do
text_ends_with_line_break?(prev_node) ->
flex_break(" ")

text_ends_with_space?(prev_node) or text_starts_with_space?(next_node) ->
" "

true ->
""
end

text_starts_with_line_break?(next_node) and text_ends_with_line_break?(next_node) ->
break(" ")

text_ends_with_space?(prev_node) or text_starts_with_space?(next_node) ->
flex_break(" ")
Expand All @@ -111,6 +121,16 @@ defmodule Phoenix.LiveView.HTMLAlgebra do

defp text_ends_with_space?(_node), do: false

defp text_starts_with_line_break?({:text, text, _meta}) when text != "",
do: :binary.first(text) in '\n\r'

defp text_starts_with_line_break?(_node), do: false

defp text_ends_with_line_break?({:text, text, _meta}) when text != "",
do: :binary.last(text) in '\n\r'

defp text_ends_with_line_break?(_node), do: false

defp block_preserve?({:tag_block, _, _, _, %{mode: :preserve}}), do: true
defp block_preserve?({:eex, _, _}), do: true
defp block_preserve?(_node), do: false
Expand Down
60 changes: 56 additions & 4 deletions test/phoenix_live_view/html_formatter_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,19 @@ if Version.match?(System.version(), ">= 1.13.0") do
alias Phoenix.LiveView.HTMLFormatter

defp assert_formatter_output(input, expected, dot_formatter_opts \\ []) do
formatted = HTMLFormatter.format(input, dot_formatter_opts)
assert formatted == expected
assert HTMLFormatter.format(formatted, dot_formatter_opts) == expected
first_pass = HTMLFormatter.format(input, dot_formatter_opts)
assert first_pass == expected

second_pass = HTMLFormatter.format(first_pass, dot_formatter_opts)
assert second_pass == expected
end

def assert_formatter_doesnt_change(code, dot_formatter_opts \\ []) do
assert HTMLFormatter.format(code, dot_formatter_opts) == code
first_pass = HTMLFormatter.format(code, dot_formatter_opts)
assert first_pass == code

second_pass = HTMLFormatter.format(first_pass, dot_formatter_opts)
assert second_pass == code
end

test "always break lines for block elements" do
Expand Down Expand Up @@ -487,6 +493,52 @@ if Version.match?(System.version(), ">= 1.13.0") do
)
end

test "text between inline elements" do
assert_formatter_doesnt_change(
"""
<span><%= @user_a %></span>
X
<span><%= @user_b %></span>
""",
line_length: 27
)

assert_formatter_doesnt_change(
"""
<span><%= @user_a %></span>
X
<span><%= @user_b %></span>
"""
)

assert_formatter_doesnt_change("""
<span><%= @user_a %></span> X <span><%= @user_b %></span>
""")

assert_formatter_output(
"""
<span><%= @user_a %></span> X <span><%= @user_b %></span>
""",
"""
<span>
<%= @user_a %>
</span>
X
<span>
<%= @user_b %>
</span>
""",
line_length: 5
)

assert_formatter_doesnt_change(
"""
<span><%= link("Edit", to: Routes.post_path(@conn, :edit, @post)) %></span> |
<span><%= link("Back", to: Routes.post_path(@conn, :index)) %></span>
"""
)
end

test "handle eex cond statement" do
input = """
<div>
Expand Down

0 comments on commit 34ea4d1

Please sign in to comment.