Perfect your code
With built-in code review tools, GitHub makes it easy to raise the quality bar before you ship. Join the 40 million developers who've merged over 200 million pull requests.
Sign up for free See pricing for teams and enterprisesExtend clone macro #534
Extend clone macro #534
Conversation
This comment has been minimized.
This comment has been minimized.
|
You might also want to add another branch to the macro after the Did you test that all the other parts are working? The syntax looks fine to me in any case. Maybe we can add some tests for this (and fix the existing tests)? :) |
This comment has been minimized.
This comment has been minimized.
|
|
This comment has been minimized.
This comment has been minimized.
|
Oh you meant the examples maybe? Well, the PR is already open, just not up to date. |
This comment has been minimized.
This comment has been minimized.
|
I also added a check for the keyword (better than having a macro error. :) ) |
This comment has been minimized.
This comment has been minimized.
|
@GuillaumeGomez Improved the error handling a bit and also fixed a bug. This should be good to go I guess :) |
| let $variable = $variable.clone(); | ||
| ); | ||
| (@weak $variable:ident) => ( | ||
| let $variable = $crate::clone::Downgrade::downgrade(&$variable); | ||
| ); | ||
| (@ $keyword:ident $variable:ident) => ( | ||
| compile_error!("Unknown keyword, only `weak` and `strong` are allowed"); |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
The `stringify!(keyword)` wasn't working?
It was but compile_error! does not allow formatting currently, it requires a single string.
Maybe you can get around that with concat! etc, give it a try :)
…--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
|
This comment has been minimized.
This comment has been minimized.
|
Na, not worth it. Looks good to me now. Once merged, I'll update my examples PR. |
This comment has been minimized.
This comment has been minimized.
|
Merged, please fix the examples then :) Thanks! |
This comment has been minimized.
This comment has been minimized.
|
Will do before the end of the day! |
GuillaumeGomez commentedNov 14, 2019
Fixes #532.
Fixes #533.
cc @sdroege @EPashkin @antoyo