Allow redefining None to 0 for @[Flags] enum #6160
Conversation
|
In general I'm +1 to this, not sure I like |
|
Mentioning I would be very happy to have just |
|
Yeah I know @oprypin I just wanted to add some context, and I needed to mention |
|
I just added a better empty enum check that ignores None & All in flags enums, now that they are allowed |
| @@ -569,7 +569,13 @@ class Crystal::TopLevelVisitor < Crystal::SemanticVisitor | |||
| is_flags: enum_type.flags?) | |||
| end | |||
|
|
|||
| if enum_type.types.empty? | |||
| nb_members = enum_type.types.size | |||
There was a problem hiding this comment.
What's nb_members supposed to mean? I'd suggest to use a more descriptive name.
There was a problem hiding this comment.
it's the number of members, maybe number_of_members then? it just felt too long ^^
There was a problem hiding this comment.
There is no "too long" for names, only "too short" 😄 num_members would be fine, tough. Or members_size, enum_size.
There was a problem hiding this comment.
num_members is good 😃 ty
|
There is no change in behavior of |
|
Yeah sorry if it wasn't clear @bcardiff, the title only mention the possibility to redefine None, not All. The PR inital msg is providing context and explanations, maybe it should have gone in an issue. |
|
Thank you! |
|
No, thank you, @bew =) |
Followup on #4395 (comment)
#4395 now prevents
All&Nonein@[Flags]enums.But as @oprypin pointed out / demonstrated, redefining
None&Allcan be useful for documentation purpose.To this end:
Noneshould always be0, and All should always be all the values OR-ed together.So something like this should work:
It's easy to handle
None, just need to check it's0.About
All, first it's very tedious to repeat all members, second it's harder to detect if there are more members after it. To simplifyAlland still be able to documente it, there could be a special value likeAll = :allthat will skip this member but retain its doc somehow...As handling
Noneis easy I implemented it in this PR.Also, I don't think
Allis common when using enum flags, so I think it's ok to not be able to document it.WDYT?