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.

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

Fix Splode.Error.message/1 #7

Merged
merged 1 commit into from
Apr 29, 2024
Merged

Fix Splode.Error.message/1 #7

merged 1 commit into from
Apr 29, 2024

Conversation

jechol
Copy link
Contributor

@jechol jechol commented Apr 29, 2024

Contributor checklist

  • Bug fixes include regression tests
  • Chores
  • Documentation changes
  • Features include unit/acceptance tests
  • Refactoring
  • Update dependencies

@zachdaniel
Copy link
Contributor

I think we should do something like "if to string is implemented, to string, else inspect"

@jechol
Copy link
Contributor Author

jechol commented Apr 29, 2024

For example, when investigated using String.Chars.impl_for/1 both [] and [:a] return String.Chars.List, but while [] can be converted to a string using to_string, [:a] cannot. I think it's always better to use inspect for the following two reasons:

  1. It is possible to use a try block to attempt to_string and fall back to inspect if it fails, but it feels a bit awkward.
  2. It feels more consistent to always use inspect rather than having to_string and inspect separated based on the type.

I'm curious about your opinion.

@zachdaniel
Copy link
Contributor

You're right :) I was thinking that this might somehow surface to end users, but that we don't use message/1 for displaying to the outside world.

@zachdaniel zachdaniel merged commit 61d76e5 into ash-project:main Apr 29, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants