Fix QVariant enum #16
Conversation
This enum does not match the one in ext/libqmlrswrapper/libqmlrswrapper.cpp (The rust version does not have a Bool slot). As a consequence, the enumeration values are different and hence QrsVariant::String has different values in rust and C. This caused a bug where the strings from QML were not recognised as strings by the Rust code (ffi::qmlrs_variant_get_type(var) != QrsVariantType::String). This commit seems to fix the issue, at least in regards to my own project.
For some reason this branch does not compile without it.
|
Regarding the QrsVariant: I guess this is my fault. My last pull request (#13) was about something totally different, but a few lines the code I used to support the boolean type (liamsi@c9356c8, see variant.rs) got committed and merged with that PR (#13). |
|
Oh, I see where it is. Yeah, enums are tricky when loading them from C because they are literal enumerations, so what gets exported is the numbers, rather than the names (i.e. order matters). I'm thinking that we should have tests for each type of variant that can be loaded, so that bugs involving to_qvariant and from_qvaraint can be found more easily. |
|
Hi, yes It totally agree on the tests. I only tested the boolean support by slightly modifying the factorial example. Maybe we should use travis or sth similar. Could you please verify that your problem gets solved by liamsi@19a340f ? And regarding #![feature(custom_attribute)]: I can reproduce it using Rust nightly (c6b148337 2015-06-12)). But it works fine without it using Rust stable. |
|
This commit isn't on your fork, so I don't really know how to make cargo checkout this particular commit. (I'm a git/rust noob, sorry.) Anyway, I'm a bit concerned that your C enum orders them {Int, Bool, String}, whereas in rust your order them {Bool, Int, String} -- I have a suspicion that this will not work as a result. Have you tried testing this commit with a slot function that takes a string as a parameter? I might be completely wrong, so I'd like to know if it would work. |
|
Oh, it's in the types branch. Never mind, testing it now.. |
|
Huh, it works. I'll close the PR now, thank you for publishing the fix so quickly. If at all possible, can you see if there is a way to make qmlrs compile on both stable and beta/nightly? (i.e. the whole #![feature(custom_attributes)] thing). It is a bit inconvenient having to swap rust versions to compile different pieces of software, and people may want to use nightly features with this library. |
|
Thank's a lot for your feedback (and for closing)!
Yes that's true. If this issue prolongs, I will have a look on it (usually I use nightly, too). Cheers, |
|
Hey, one last hint: to make it compile with rust nightly, change #[packed] with #[repr(packed)] as explained here: rust-lang/rust#25541 |
Support more variant types (bool), and fixing #16, too
This enum does not match the one in ext/libqmlrswrapper/libqmlrswrapper.cpp (The rust version does not have a Bool slot). As a consequence, the enumeration values are different and hence QrsVariant::String has different values in rust and C. This caused a bug where the strings from QML were not recognised as strings by the Rust code (ffi::qmlrs_variant_get_type(var) != QrsVariantType::String). This commit seems to fix the issue, at least in regards to my own project.