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

Add Char.to_i128 and .to_u128 #12900

Merged

Conversation

straight-shoota
Copy link
Member

Resolves #12892

Related to #12884

Also deprecates Enum.flags which is replaced by Enum.[].

@asterite
Copy link
Member

asterite commented Jan 5, 2023

What's the point of doing this with a non-flags enum? Maybe the old method should have raised a compile error in that case. And now I'm thinking flags is a better name than brackets.

@straight-shoota
Copy link
Member Author

straight-shoota commented Jan 5, 2023

Yeah, that's a legitimate question.Enum.flags already works for non-flags enums as well. And I think that's fine. Also for Enum.[].
It's definitely very useful for single arguments to provide a valid-code syntax for Enum#inspect that's unique and concise (see examples #12884).
Enum#| is also defined for non-flag enums, so even multiple arguments are valid. Not sure if there's a good use case for that, but as long as | works, there's no point in restricting the constructor IMO (ofc we could do that and allow only a single argument for non-flag enums, but I don't see much point in that).

@beta-ziliani
Copy link
Member

I'm worried about the amount of deprecation messages that we will throw in the face of the entire Crystal ecosystem's devs. I wonder if we shouldn't allow ourselves to have two ways of doing it for a while, mention in the docs that .flags is deprecated, and wait for a while until doing the actual deprecation.

@asterite
Copy link
Member

What's wrong with keeping the name flags?

@straight-shoota
Copy link
Member Author

Sure, let's keep Enum.flags for now.

@asterite It's effectively an alias for Enum.[] now, which is preferable for its conciseness.

@beta-ziliani beta-ziliani added this to the 1.8.0 milestone Jan 24, 2023
@straight-shoota straight-shoota merged commit 9fca36b into crystal-lang:master Jan 25, 2023
@straight-shoota straight-shoota deleted the feature/enum-constructor branch January 25, 2023 18:49
@straight-shoota straight-shoota changed the title Add Enum.[] convenience constructor Add Char.to_i128 and .to_u128 Apr 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improved constructor for flags enum
3 participants