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

Allow druid_derive macros in druid crate #731

Merged
merged 2 commits into from
Mar 22, 2020
Merged

Conversation

emigr2k1
Copy link
Contributor

This makes use of feature extern_crate_self to refer to the current
crate with another name, in this case, druid. This allows druid_derive
macros to expand to something like druid::Data instead of crate::Data.

Closes #650

This makes use of feature `extern_crate_self` to refer to the current
crate with another name, in this case, druid. This allows druid_derive
macros to expand to something like `druid::Data` instead of `crate::Data`.
@emigr2k1
Copy link
Contributor Author

If putting druid in the namespace is not a good solution, I could implement something like the solution provided in this issue[1].

[1] rust-lang/rust#54647 (comment)

@cmyr
Copy link
Member

cmyr commented Mar 22, 2020

I haven't seen this solution before, can you link to any information on it? Are there any major drawbacks?

@emigr2k1
Copy link
Contributor Author

This feature was primarily implemented to solve problems like this. Here[1] is the tracking issue for the feature and here is some discussion[2]. The only drawback is that you have druid in the global namespace (not exported).
Some other crates do this too: yew[3], RustPython[4]
[1] rust-lang/rust#56409
[2] rust-lang/rust#54647
[3] https://github.com/yewstack/yew/blob/c33fe73f6caad8701a40cd488a00ea9e373930d5/src/lib.rs#L82
[4] https://github.com/RustPython/RustPython/blob/098532b283a98e8b98b70fafbed4ec4187311e97/vm/src/lib.rs#L35

@cmyr
Copy link
Member

cmyr commented Mar 22, 2020

Okay, then I have one additional request for this PR; in druid/src/widget/flex.rs we currently manually implement Data for two types; can you change those to use derive?

Comment on lines -703 to -704
// we have these impls mostly for our 'flex' example, but I could imagine
// them being broadly useful?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't know what to do with this comment

Copy link
Member

Choose a reason for hiding this comment

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

you can just delete that.

Copy link
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

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

great, thanks!

@cmyr cmyr merged commit 69f8f05 into linebender:master Mar 22, 2020
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.

cannot derive Data within druid module
2 participants