Skip to content
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.

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

Add HTMLFormatter in formatter.exs templates #4793

Merged
merged 8 commits into from
Apr 18, 2022

Conversation

leandrocp
Copy link
Contributor

That's an attempt to close #4749

The config is added only if @html and Elixir >= 1.13.4 to meet formatter requirements.

@josevalim
Copy link
Member

According to this commit we also need to add {:phoenix_live_view, ">= 0.0.0"} at the umbrella root with a comment that it is used exclusively by the formatter: phoenixframework/phoenix_live_view@9793894

Can you please do so?

Thank you!

@leandrocp
Copy link
Contributor Author

According to this commit we also need to add {:phoenix_live_view, ">= 0.0.0"} at the umbrella root with a comment that it is used exclusively by the formatter: phoenixframework/phoenix_live_view@9793894

Can you please do so?

Thank you!

Sure!

Co-authored-by: José Valim <jose.valim@gmail.com>
@josevalim josevalim merged commit 9480b9b into phoenixframework:master Apr 18, 2022
@josevalim
Copy link
Member

💚 💙 💜 💛 ❤️

@michallepicki
Copy link
Contributor

michallepicki commented Apr 19, 2022

For the plugin to work as expected in an umbrella project, the plugin needs to be also added to the top level formatter.exs file in umbrella template, as shown in this comment

@leandrocp
Copy link
Contributor Author

For the plugin to work as expected in an umbrella project, the plugin needs to be also added to the top level formatter.exs file in umbrella template, as shown in this comment

Thanks @michallepicki just pushed a PR #4800

@chrismccord
Copy link
Member

@leandrocp can you check the integration tests on master? I get failures locally on our tests that ensure the generated code is formatted. It fails on layout files and phx.gen.html files, because presumably they have minor tweaks with the formatter. Thanks!

chrismccord added a commit that referenced this pull request May 6, 2022
@leandrocp
Copy link
Contributor Author

@leandrocp can you check the integration tests on master? I get failures locally on our tests that ensure the generated code is formatted. It fails on layout files and phx.gen.html files, because presumably they have minor tweaks with the formatter. Thanks!

Sure. I can reproduce, working on it.

@leandrocp
Copy link
Contributor Author

leandrocp commented May 6, 2022

@chrismccord one of the changes is adding parens to template functions, eg:

<%%= live_title_tag assigns[:page_title] || "<%= @app_module %>", suffix: " · Phoenix Framework" %>
is generated and formatted as:

    <%= live_title_tag(assigns[:page_title] || "app", suffix: " · Phoenix Framework") %>

I'm wondering if such functions should be added to locals_without_parens in each related project default .formatter.exs? That would be Phoenix, LiveView, PhoenixHTML.

@chrismccord
Copy link
Member

parens is fine. We'll probably convert this to a function component for v1.7. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Have phx.new automatically include heex formatting
4 participants