Elixir: Figure out how .formatter.exs should work in umbrellas

Created on 29 Jan 2018  Â·  15Comments  Â·  Source: elixir-lang/elixir

Elixir Chore Intermediate Discussion Formatter

Most helpful comment

@Eiji7 that seems to hide the formatter behaviour behind configurations that is unclear where they are coming from. Let's look at the example from the forum: Mix currently generates the wrong configuration for umbrellas but anyone can easily change it. If we move the inputs behind some mysterious atoms that step becomes much harder.

I would also say that verbosity is not the problem here. The issue is rather the ambiguity that arises from the fact we have a .formatter.exs in the umbrella root and also inside each application.

It is important to remember that we want the formatter to run without requiring dependencies or the mix.exs file as much as possible. Depending on those means the formatter becomes slower. For example, how we would find out from where :old_phoenix is coming from? We would need to load and traverse all dependencies. That would slow down the formatter considerably.

We should keep everything formatter related simple and explicit. :)

In any case, thanks for the proposal!

All 15 comments

If it helps, I'm using the following:

[
  inputs: [
    "apps/*/{lib,config,test}/**/*.{ex,exs}",
    "apps/*/mix.exs",
    "mix.exs"
  ]
]

A simple idea is to support a "subdirectories" key. If this key is present, any application in those subdirectories will use the .formatters.exs in those subdirectories.

[
  subdirectories: ["apps/*"]
]

This way we delegate it to each application and we can also use it in other situations, such as migration files inside Phoenix projects, etc.

Thoughts @whatyouhide?

@josevalim what is the problem with "apps/*/{lib,config,test}/**/*.{ex,exs}", in the inputs?

I do not see the needs of adding another configuration where you could just use the current behavior.

Unless you are worry about performance?! Not quite sure

@yordis the problem that we are trying to solve (unless I totally misunderstood) is that each child application may want to have its own .formatter.exs for its own configuration.

@whatyouhide ahhhhh

Well if you run mix format in an umbrella application then checks if the sub-applications have some .formatter defined and use it, if it is not defined then use the umbrella .formatter one.

And for the umbrella just check the if the file is there.

The weird piece is that the umbrella one could re-run the whole formatting again if they use the wildcard for everything inside apps

Good call @yordis. We can also solve it like that but a generic mechanism may also be handy in other situations.

@josevalim you mean when people create folder structures as they please?

Because if not,

Right now, either umbrella apps or singular apps so I am confused with what it is a generic mechanism and which other situations outside those two.

@yordis one other use case I briefly mentioned above is migrations. We want specific formatting rules in there and we don't want that to affect the rest of the app.

@josevalim: How about introduce also input_types (or maybe app_types?) option?

For example:

  1. /.formatter.exs
  2. /apps/a/.formatter.exs
  3. /apps/a/aa/.formatter.exs
  4. /apps/b/.formatter.exs
    4.1. /apps/b/lib
    4.2. /apps/b/test
  5. /apps/ba/.formatter.exs
  6. /apps/c/.formatter.exs

So:

  1. Root app (1) and a app (2) are umbrella only projects: input_types: [:elixir_umbrella]|
  2. aa app (3), ba app (5) and c app (6) are normal apps: input_types: [:elixir_app]
  3. b app is umbrella app with lib (4.1) and test (4.2) directories: input_types: [:elixir_app, :elixir_umbrella]

and finally we could also support :old_phoenix for web directory (older versions of phoenix).

In such way developers do not need to care about default inputs (for most apps) and we can simply solve recursive umbrella apps structure without creating long matches by everyone.

Maybe also a simple way to let developers define custom project structures?

defmodule Example.MixProject do
  # …

  def project do
    [
      # …
      mix_formatters: [:example], # similar to mix_compilers option
      # ...
    ]
  end

  # …
end

formatter:

defmodule Mix.Tasks.Formatter.Example do
  # …

  def run(_args \\ []) do
    [
      inputs: [web: "**/*.{ex,exs}"], # let's say that it's a custom apps directory
      subdirectories: ["appz/*"]
    ]
  end

  # …
end

and finally something like:

> mix formatter --list
elixir_app       # default Elixir app with lib and test directories
elixir_umbrella  # Elixir umbrella app with apps directory
example          # custom formatter description …

@Eiji7 that seems to hide the formatter behaviour behind configurations that is unclear where they are coming from. Let's look at the example from the forum: Mix currently generates the wrong configuration for umbrellas but anyone can easily change it. If we move the inputs behind some mysterious atoms that step becomes much harder.

I would also say that verbosity is not the problem here. The issue is rather the ambiguity that arises from the fact we have a .formatter.exs in the umbrella root and also inside each application.

It is important to remember that we want the formatter to run without requiring dependencies or the mix.exs file as much as possible. Depending on those means the formatter becomes slower. For example, how we would find out from where :old_phoenix is coming from? We would need to load and traverse all dependencies. That would slow down the formatter considerably.

We should keep everything formatter related simple and explicit. :)

In any case, thanks for the proposal!

Other option could be to provide several formatter blocks in the .formatter.exs file. All the blocks would follow the same options as the ones currently supported by the formatter:

[
  [
    inputs: [
      "lib/**/router.ex"
    ],
    import_deps: [:phoenix]
  ],
  [
    inputs: [
      "lib/**/*.{ex,exs}"
    ],
    locals_without_parens: [
      my_function: 2
    ]
  ]
]

However, I would keep compatibility with the current one-block format by converting it internally to a list of one item. Regarding :export, I guess I would merge all the conditions into a single one or move it outside the blocks.

I currently have the following in my top level mix.exs to run mix format in each app:

  defp aliases do
    ["format": "cmd mix format"]
  end

The issue is that you cannot do mix format path/to/some/file.ex which is what editors do. If you do that, I think the file with be formatted N times, one per app in the umbrella, with different config each time. :S

Closing in favor of #7398.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

josevalim picture josevalim  Â·  3Comments

DEvil0000 picture DEvil0000  Â·  3Comments

andrewcottage picture andrewcottage  Â·  3Comments

lukaszsamson picture lukaszsamson  Â·  3Comments

Irio picture Irio  Â·  3Comments