Mix language setting #2687
Mix language setting #2687
Conversation
|
Hm, some Logger tests failed. I didn't have those failures locally. Anyway, please hold on with merging. I want to amend the escript test to use config.exs. |
|
All done. |
|
The |
|
@ericmj done |
|
Great. LGTM. |
| [{'#{name}.beam', binary}] | ||
| end | ||
|
|
||
| defp module_body_for(:elixir) do |
There was a problem hiding this comment.
We don't need to have two bodies. Let's just have one with everything written in Erlang.
There was a problem hiding this comment.
What to do with Application.format_error? If it wasn't for it, then it is easy to have all common functions except for the main one.
There was a problem hiding this comment.
We can do: case :code.ensure_loaded(Application) do {:module, Application} -> call_it; {:error, _} -> dont_call_it end
There was a problem hiding this comment.
I just realized we do need two bodies... but we can probably share some code between them?
There was a problem hiding this comment.
Like load_config and maybe io_error unsure.
|
I have added some comments. Do we want to support |
Not in this PR. I wanted to play with a generic templating solution for |
|
@josevalim what do you think about the last commit? Only the main function differs between Elixir and Erlang. Whether to color the error output is decided at build time. And there is also a bug, I realized now: need to replace |
|
The tests didn't catch that. Maybe we should remove coloring altogether? It's not extremely important here. |
|
@josevalim fixed error formatting issues in the latest commit (I can squash it before merging). This is not tested as part of our test suite, I tried it manually with an Erlang project that fails to start. |
It has a beautiful sha. |
|
|
||
| if unquote(colors_enabled) do |
There was a problem hiding this comment.
Let's just drop the color altogether because escripts will probably never have the colors enabled because they are not started through the bin/elixir executable.
alco
force-pushed
the
mix-language-setting
branch
from
c1d614f to
f0b8bc1
Compare
|
Rebased, ready to merge. |
|
Beautiful. Merge it! |
Closes #2656 and #2657.