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

quick fix for hot code reload error #755

Closed
wants to merge 2 commits into from
Closed

quick fix for hot code reload error #755

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a2a2403
quick fix for https://github.com/surface-ui/surface/issues/754
mayel Oct 9, 2024
b73c9d9
clean up
mayel Oct 9, 2024
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: 4 additions & 0 deletions lib/mix/tasks/compile/surface.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,10 @@ defmodule Mix.Tasks.Compile.Surface do
end
end

defp print_diagnostic(message, :warning, file, {line, _column}) do
print_diagnostic(message, :warning, file, line)
end

Copy link
Member

Choose a reason for hiding this comment

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

Hey @mayel! Thanks for the PR.

I wonder if could have something like this, instead:

if Version.match?(System.version(), ">= 1.14.0") do    
  defp print_diagnostic(message, :warning, file, {line, col}) do
    IO.warn(message, file: file, line: line, column: col)
  end
end

# TODO: Remove this clause in Surface v0.13 and set required elixir to >= v1.14
defp print_diagnostic(message, :warning, file, line) do
  rel_file = file |> Path.relative_to_cwd() |> to_charlist()
  IO.warn(message, [{nil, :__FILE__, 1, [file: rel_file, line: line]}])
end

This way most users could already benefit from the column info and have better diagnostics.

@tiagoefmoraes WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

Best of both worlds!

defp print_diagnostic(message, :warning, file, line) do
# Use IO.warn(message, file: ..., line: ...) on Elixir v1.14+
rel_file = file |> Path.relative_to_cwd() |> to_charlist()
Expand Down
Loading