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

Lua::from/to have confusing names #25

Closed
jonas-schievink opened this issue Jul 25, 2017 · 3 comments
Closed

Lua::from/to have confusing names #25

jonas-schievink opened this issue Jul 25, 2017 · 3 comments

Comments

@jonas-schievink
Copy link
Contributor

In Rust, T::from* and T::to* imply creating a T from another type or converting the T to another type (doesn't have to consume T).

Lua::from doesn't create a Lua object, it merely uses it to perform the conversion, producing a Value. Similarly, Lua::to doesn't convert the Lua object, it converts its argument (a Value).

I think the best way to clear this up is to remove both methods - they are merely wrappers around t.to_lua and T::from_lua respectively, which is perfectly clear.

@kyren
Copy link
Contributor

kyren commented Jul 25, 2017

The same could be said for pack / unpack, I just find them slightly awkward to use otherwise.

I use those to / from methods constantly, mostly because I get really tired of writing T::from_lua(t, lua) instead of lua.to(t).

I think this is a symptom of the awkward names of to_lua, from_lua, to_lua_multi, and from_lua_multi. Is there possibly a better, short punchy name other than 'from' and 'to' to call these that is less confusing? Is there possibly some other API design that we can use for ToLua / FromLua / ToLuaMulti / FromLuaMulti?

@kyren
Copy link
Contributor

kyren commented Aug 2, 2017

This is addressed by PR #32, but I'm not 100% sure I love the result. Now the term "packing" refers to taking any value and packaging it in the universal Any type, and unpacking refers to the opposite, and there's now pack_multi and unpack_multi.

@jugglerchris
Copy link
Collaborator

I am closing this issue as I think it's been addressed.

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

No branches or pull requests

3 participants