Skip to content

Conversation

@davydog187
Copy link
Contributor

@davydog187 davydog187 commented Nov 20, 2024

A common complaint I hear from people is that error messages in Elixir are not pretty. Generally, this seems to be only the case when there are large structs in codebases that render in one line and are hard to read.

This PR changes the behavior to pretty inspect the structs in the case that a protocol is not implemented for some value.

The downside here is that the logging will no longer be in one line, but I think this is a fair tradeoff for legibility

Before

** (Protocol.UndefinedError) protocol String.Chars not implemented for %Foo{a: %Foo{a: nil, b: nil, c: nil, d: nil}, b: %Foo{a: nil, b: nil, c: nil, d: nil}, c: %Foo{a: nil, b: nil, c: nil, d: nil}, d: %Foo{a: nil, b: nil, c: nil, d: nil}} of type Foo (a struct)
    (elixir 1.18.0-dev) lib/string/chars.ex:3: String.Chars.impl_for!/1
    (elixir 1.18.0-dev) lib/string/chars.ex:22: String.Chars.to_string/1
    iex:2: (file)

After

** (Protocol.UndefinedError) protocol String.Chars not implemented for type Foo (a struct)

Got value:

    %Foo{
      a: %Foo{a: nil, b: nil, c: nil, d: nil},
      b: %Foo{a: nil, b: nil, c: nil, d: nil},
      c: %Foo{a: nil, b: nil, c: nil, d: nil},
      d: %Foo{a: nil, b: nil, c: nil, d: nil}
    }

    (elixir 1.18.0-dev) lib/string/chars.ex:3: String.Chars.impl_for!/1
    (elixir 1.18.0-dev) lib/string/chars.ex:22: String.Chars.to_string/1
    iex:2: (file)

A common complaint I here from people is that error messages in Elixir
are not pretty. Generally, this seems to be only the case when there are
large structs in codebases that render in one line and are hard to read.

This PR changes the behavior to pretty inspect the structs in the case
that a protocol is not implemented for some value.

The downside here is that the logging will no longer be in one line, but
I think this is a fair tradeoff for legibility
Co-authored-by: Jean Klingler <[email protected]>
@josevalim
Copy link
Member

Thank you! Can you please double check CI? I assume the tests need updating too!

@davydog187
Copy link
Contributor Author

Thanks @josevalim! tests have been fixed locally, and I changed the formatting to indent it a bit. Please run the CI and take a look :)

@josevalim josevalim merged commit 2f844fd into elixir-lang:main Nov 21, 2024
@josevalim
Copy link
Member

💚 💙 💜 💛 ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants