Mix.Tasks.Format.formatter_opts_for_file/2 non-relative to CWD

[Jason Axelson]

Hi,

For ElixirLS (with our current architecture) we need to support formatting of a file with varying CWD. So I'd like to propose a change to `Mix.Tasks.Format.formatter_opts_for_file/2` to support not relying on the current working directory. This is the line that would be changed:

https://github.com/elixir-lang/elixir/blob/36d12b4bd4d26533c216e935903206cc3502200f/lib/mix/lib/mix/tasks/format.ex#L162

My first thought on how to change it is to pass a directory as an option, say `relative_to`. So if `:relative_to` is passed as an option then the formatter options are found relative to that file, but if it is not passed then the current behavior of using the CWD as the `relative_to` path would be kept.

I'm open to other approaches as well.

Thanks,

Jason

[José Valim]

Hi Jason!

* if you give it a relative path, then I think the relative_to_cwd will return unchanged. Have you tried it out?

* however, I wonder if the biggest issue is not picking up the file... but rather choosing which .formatters.exs to use?

--
You received this message because you are subscribed to the Google Groups "elixir-lang-core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to elixir-lang-co...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CAHMz6kzW8BHiELiEYjZT3Y3ky-9gRhjJp0REeHmj%3DLH138p9GA%40mail.gmail.com.

[Jason Axelson]

Hi José,

Indeed Path.relative_to_cwd does return relative paths unchanged, so it appears I was not looking closely enough at the code. So the problem is indeed that the wrong .formatter.exs is chosen. Looking more closely at the code it appears that there are a number of operations that depend on the current working directory global state.

-Jason

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CAGnRm4KqzeLABsKL632vQ7aj7rqMycpQ4o0a1H-OV1qmYo1%3DsA%40mail.gmail.com.

[José Valim]

It may be better for you to use File.cd and change the directory when formatting but we will be glad to add a function that has the cwd. It is simply a bigger change. :)

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CAHMz6kwgyftNO9MnQm8zkgY84FtE%2BNgQFRJtRY-1e6jubjHCHA%40mail.gmail.com.

[Jason Axelson]

I don't think we can just do File.cd because the mix compilation process is already changing the working directory, and changing it would probably break the compilation.

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CAGnRm4%2B4fyu1_vw5N%3DxAxCG4FB-J3YA7sbH5KOMrSKsYnHAR9g%40mail.gmail.com.

[depierr...@gmail.com]

I started exploring the solution here. This is going to need a bit more work than just moving the dot_formatter to a relative_to, as it seems the plugins and deps also use CWD.

To get to a clear solution, I am wondering in which circumstances we expect the formatter to run. Mostly I am wondering due to subdirectories and plugins. Deps already use Mix.Project.deps_paths as a root. Subdirectories use a prefix that I can probably reuse. For plugins, I hope that loadpaths will not need to change.

But if someone runs the formatter from inside the project and not at the top, what do we expect to happen? The documentation for the task says "the .formatter.exs in current directory", but you can also pass a path to one as a flag, which I am not sure would work rn because I am pretty sure the "relative_to_cwd" stuff would break.

I am going to go forward for now adding a relative_to option and trying to use it for the dot_formatter evaluation and the find_formatter_and_opts_for_file but I feel like I am missing something here. Would love to know what.