-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Improve UUID#inspect
to return Crystal expression
#11879
base: master
Are you sure you want to change the base?
Improve UUID#inspect
to return Crystal expression
#11879
Conversation
I understand the rationale behind this, but this could be said about many other types. |
Many types have |
I personally think the rule should be:
I personally think seeing an array of URIs like this: [URI.new("..."), URI.new("...")] instead of this: [URI("..."), URI("...")] looks very strange. Maybe we can add an [URI["..."], URI["..."]] which is more, doesn't involve method calls, and it's also valid Crystal code. |
I'd be happy with |
In Ruby methods can start with an uppercase letter, so you can do this: require "uri"
uri = URI("https://example.com") In Crystal that syntax conflicts with generics. Maybe there's a way to allow it, but I'm not sure. And maybe it would just add confusion. So we could use the But one thing that I'm noticing is that in Ruby these types don't have an #<URI::HTTPS https://hello> If you have a struct:
I'm not saying that we shouldn't do this because Ruby doesn't do it. I'm just trying to think why Ruby doesn't do it... maybe the case where everything you output becoming a valid Crystal expression is really hard. Maybe the solution is to review this case by case 🤷 |
I think it's more of a coincidence that some Ruby inspects produce Ruby code and others don't. That is, I think they chose a format for inspect that conveys the most meaning to a programmer and sometimes that format looks like code and sometimes it doesn't. The documentation even states "Returns a string containing a human-readable representation of obj". I'd argue most Ruby inspects don't produce valid code. For example, Array and Hash appear to produce valid code, but that breaks down if you have an Array of objects or Array of Dates. In my opinion, inspect is not about producing code, but producing something human-readable. If we want a method that produces valid code, perhaps a new method is in order? (offhand, thinking |
Agreed. But it doesn't hurt when the human-readable representation is also valid code. I believe there is even a good argument that a valid expression can be a very good human-readable representation. Because that's exactly what a human would write to produce an object. When a value can be represented as reasonably concise code, there's no need for an artificial syntax to describe the object. Using the already existing syntax for construction is much easier. |
I don't think there's a reason for that. Simple data objects that can be easily represented with a Crystal expression should use that for |
I think we can try using the |
@straight-shoota |
Well, inspecting means understanding. That needs an effectual representation. Serialization means turning structured data into a serialized presentation. The human-readable representation of |
Again, |
I think @straight-shoota 's point is: if we can achieve both in a way that is still pretty much human-readable, then why not? Specially considering that those humans will be Crystal programmers, if they see a valid Crystal expression they will understand it, maybe even easier than understanding a custom output. |
Just to clarify, I don't think there's anything wrong with updating specific inspects on a case-by-case basis to emit a string that happens to work directly in crystal. However, specifically for |
For primitives and container(-like) types it certainly makes sense to have a format that works both ways. For others it seems to me like trying to conflate two different concepts (inspection and serialization) into one, which is simply at best confusing and non-coherent. Next, we'll be having questions why other types are not following such pattern, and should |
@straight-shoota btw, why won't you post an RFC before opening a PR with breaking changes - as you like to advise so often? Not doing so - like on many other occasions - is creating a community of double-standards - and it's not that I believe we need an RFC for every change, it's about following the same standards you put out for others. |
If we use |
@Sija I honestly believed this is merely a simple DX improvement that doesn't warrant any prior discussion (like #11880 for example). Apparently, I was wrong about that.
What makes primitives and container-like types special in this regard? Why couldn't this same concept apply to other types?
As I already argued before, I don't think inspection and serialization are that different.
Maybe those questions would be rightfully asked? Why don't I have a simple way to inspect |
To me, the ideal case would be to have something that works for every struct consistently. And if I had the God of Syntax here with me, I'll ask they to add |
1 similar comment
To me, the ideal case would be to have something that works for every struct consistently. And if I had the God of Syntax here with me, I'll ask they to add |
@beta-ziliani I hope the gods are not favourable to you :P I don't understand why a inspect representation that also works as a Crystal expression - in whatever syntax - would be considered a code smell. It shouldn't matter whether I write an expression like |
For the same reason visibility exists in Ruby/Crystal. You want to prevent the creation of arbitrary objects. I want to respect the Gods of Valid Code. |
Elixir seems to favor valid Elixir expressions now: https://github.com/elixir-lang/elixir/blob/v1.14/CHANGELOG.md#expression-based-inspection-and-inspect-improvements |
I still agree with #11879 (comment) that we should prefer |
Co-authored-by: Sijawusz Pur Rahnama <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose nothing prevents us from defining UUID.[]
within this PR as well?
It's related, but a separate feature. I think it makes sense to add it on its own. |
Then that separate PR should come before this one, as that's the whole premise of this PR. We do not merely want the output of |
I don't think the order matters much, really. We have other inspect formats that look like valid syntax, but are not functional. |
This patch changes
UUID#inspect
to return a string representation that is also a valid Crystal expression for creating aUUID
instance. This serves as a convenience feature for developers. You can easily print a UUID (p! uuid
) and use the result to re-construct the same UUID in Crystal code.Without that, you have to manually edit the current format to form a call to
UUID.new
. Considering the diff is quite minimal, I think it's a nice treat to make this easier for users.