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

Auto format generated templates #4939

Closed

Conversation

leandrocp
Copy link
Contributor

@leandrocp leandrocp commented Sep 9, 2022

While working on #4820 I've realized that manually formatting templates with a default config won't work well because the user's project may have a different config for line length, generating a file that won't be formatted in that project context, forcing the user to run mix format to fix it anyways. Besides that a project may have custom plugins that is expected to modify those files.

So why not format on demand? The test shows how it could work. WDYT?

@josevalim
Copy link
Member

@leandrocp in this case, it is a new app, so the user most likely won't have existing formatting rules. But I agree this removes the need for us to manually fix our templates (which is annoying) and this will be more useful for phx.gen.* templates.

Also note that both generators also inject content and we should not format files on injected content. So we still need to do some manual fixes.

Finally, formatting may fail, so I recommend wrapping the formatter call in a rescue and returning the original in such cases.

TL;DR: 👍

  • We need to apply this to phx.gen.* too
  • We need to rescue errors
  • Fix whatever is left manually

@josevalim
Copy link
Member

Actually, there are complications:

  1. On phx.new, we don't have the HEEx formatter, so we can't format them. I still think the above would be helpful for Elixir. So I suggest checking the file extension for .ex and .exs and apply automatic formatting then.

  2. For phx.gen.*, if there are plugins and the plugins are not available, then we will call compile, but the odds are low because we need phoenix compiled to invoke the generators. So it should be ok with formatting any .ex, .exs, .heex file.

@josevalim
Copy link
Member

Actually, there is no guarantee the user will have a formatter for .heex files, so we can truly only format .ex and .exs for now in both cases. It is a great help, but the rest has to be manually. :)

@chrismccord
Copy link
Member

Closing this for now as it has gone stale. We may pick this back up in the future. Thanks!

@chrismccord chrismccord closed this Aug 3, 2023
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.

None yet

3 participants