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 docs for Enum#value #11947

Merged
merged 3 commits into from
May 25, 2022

Conversation

lbguilherme
Copy link
Contributor

I noticed this method wasn't really documented, although it is used in a doc example.

Copy link
Member

@beta-ziliani beta-ziliani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@asterite
Copy link
Member

I think this could be put in struct Enum in primitives.cr. It's an actual method, not a pseudo-method.

@beta-ziliani
Copy link
Member

@asterite help me here, if it isn't listed in enum.cr nor primitive.cr, and it's not a pseudo method, what is it? 👻

@lbguilherme
Copy link
Contributor Author

I think this could be put in struct Enum in primitives.cr. It's an actual method, not a pseudo-method.

I couldn't find a def value anywhere for Enum. primitives.cr doesn't define it. So I assumed it was a pseudo-method.

@asterite
Copy link
Member

This PR is probably fine, though!

@lbguilherme
Copy link
Contributor Author

Found it! Not very documentation-friendly though.

def initialize(program, namespace, name, @base_type)
super(program, namespace, name)
add_def Def.new("value", [] of Arg, Primitive.new("enum_value", @base_type))
metaclass.as(ModuleType).add_def Def.new("new", [Arg.new("value", restriction: Path.global(@base_type.to_s))], Primitive.new("enum_new", self))
end

@beta-ziliani
Copy link
Member

Nice! But then, since it's a primitive, it might make sense to move it to primitive.cr like Ary said.

@asterite
Copy link
Member

Actually it's impossible to do because value's type depends on the enum type. Enum should somehow be generic, but currently it's not.

@asterite
Copy link
Member

asterite commented Mar 29, 2022

With that in mind, maybe doing nothing for now is better

@beta-ziliani
Copy link
Member

We need

struct Enum(T) where T < Int

Ref #934, which even has a proposed implemented according to #934 (comment)

@straight-shoota
Copy link
Member

Yes, it's a regular method and can be overriden by implementing types. It's definitely not pseudo.

The docs are fine. I think we could just move them to enum.cr and simply define #value delegating to previous_def.

@straight-shoota straight-shoota added this to the 1.5.0 milestone May 17, 2022
@straight-shoota straight-shoota changed the title Add docs for Enum#value Add docs for Enum#value May 17, 2022
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.

5 participants