Skip to content

Add type conversions for {Hash,BTree}Sets #61

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

Closed
wants to merge 1 commit into from
Closed

Add type conversions for {Hash,BTree}Sets #61

wants to merge 1 commit into from

Conversation

mmirate
Copy link

@mmirate mmirate commented Dec 27, 2017

The Lua convention for a set (S ^ S ^ ...) is that S is a table where each x is a key whose value is true.

The FromLua and ToLua implementations I'm proposing here, then. abstract-away the .iter().cloned().map(|k| (k, true)) dance needed to expose to Lua one of the Set types from std::collections.

@jonas-schievink
Copy link
Contributor

IIRC those were removed intentionally after the discussion in #11

@mmirate
Copy link
Author

mmirate commented Dec 27, 2017

Ah, interesting. Looks like the old behavior effectively constructed/deconstructed sets from vectors, and the old behavior's removal was due to the perception that the behavior in this PR was merely another valid way to interpret a std::collections Set.

I disagree, in that this behavior seems far more obvious.

... Though the conversion between a Set and a Map actually ends up doing more work than reconstructing a set-ish table from a Vec of the members (because iterating through a set or map is much less trivial than through a Vec). Quite annoying that there isn't a lower-cost way in std::collections to transform a Set + value-function to a Map nor a Map's keys to a Set; but without it, the abstraction I'm proposing does no good. (Nor does, really, using a std::collections Set with rlua in the first place.) sigh. Closing.

@mmirate mmirate closed this Dec 27, 2017
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

Successfully merging this pull request may close these issues.

2 participants