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.

Sign up for GitHub

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

Send warnings to stderr and in yellow color in test and compiler.erlang tasks #14018

Closed
wants to merge 1 commit into from
+19 −8
Closed

Send warnings to stderr and in yellow color in test and compiler.erlang tasks #14018

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion lib/mix/lib/mix/compilers/erlang.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,9 @@ defmodule Mix.Compilers.Erlang do
for {_, warnings} <- entries,
{file, issues} <- warnings,
{location, module, message} <- issues do
IO.puts("#{file}:#{location_to_string(location)} warning: #{module.format_error(message)}")
message = "#{file}:#{location_to_string(location)} warning: #{module.format_error(message)}"

IO.puts(:stderr, IO.ANSI.format([:yellow, :bright, message]))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Elixir compiler does not wrap the the whole message in yellow, only the warning part, not in bright, and it ultimately uses another format, so I am not sure we should go for consistency here. We should probably be more consistent with the Erlang compiler warnings and error messages instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I was going to mention that, but there are some parts where the whole warning is printed in yellow. So I was going to mention this in an issue itself. I can create the issue, or we can discuss it here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those are different warnings emitted by different places, and different on purpose, since they have implications. I would prefer to stay consistent within each library: Erlang, Mix, and Elixir.

Copy link
Contributor Author

@eksperimental eksperimental Nov 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. So what is the desired format for warnings in the Erlang and Mix library?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably the formats that we have right now? :) although I will double check if the Erlang one is still relevant.

end
end

Expand Down
15 changes: 12 additions & 3 deletions lib/mix/lib/mix/tasks/test.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -683,9 +683,7 @@ defmodule Mix.Tasks.Test do
files = Mix.Utils.extract_files(test_files, warn_test_pattern) -- matched_test_files

for file <- files do
Mix.shell().info(
"warning: #{file} does not match #{inspect(test_pattern)} and won't be loaded"
)
warning("warning: #{file} does not match #{inspect(test_pattern)} and won't be loaded")
end
end

Expand Down Expand Up @@ -879,4 +877,15 @@ defmodule Mix.Tasks.Test do
[]
end
end

# Prints the given ANSI warning to the shell followed by a newline.
defp warning(message) do
Mix.shell().print_app()

IO.puts(:stderr, IO.ANSI.format(yellow(message)))
end

defp yellow(message) do
[:yellow, :bright, message]
end
end
8 changes: 4 additions & 4 deletions lib/mix/test/mix/tasks/compile.erlang_test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,13 +161,13 @@ defmodule Mix.Tasks.Compile.ErlangTest do

capture_io(fn -> Mix.Tasks.Compile.Erlang.run([]) end)

assert capture_io(fn ->
assert capture_io(:stderr, fn ->
assert {:noop, _} = Mix.Tasks.Compile.Erlang.run([])
end) =~ ~r"has_warning.erl:2:(1:)? warning: function my_fn/0 is unused\n"
end) =~ ~r"has_warning.erl:2:(1:)? warning: function my_fn/0 is unused"

assert capture_io(fn ->
assert capture_io(:stderr, fn ->
assert {:noop, _} = Mix.Tasks.Compile.Erlang.run([])
end) =~ ~r"has_warning.erl:2:(1:)? warning: function my_fn/0 is unused\n"
end) =~ ~r"has_warning.erl:2:(1:)? warning: function my_fn/0 is unused"

# Should not print old warnings after fixing
File.write!(file, """
Expand Down