Skip to content
This repository has been archived by the owner on Sep 22, 2020. It is now read-only.

Add MessageResponse derive #14

Merged
merged 2 commits into from
Mar 12, 2019
Merged

Add MessageResponse derive #14

merged 2 commits into from
Mar 12, 2019

Conversation

samrg472
Copy link
Contributor

@samrg472 samrg472 commented Mar 7, 2019

Related issue #13

@samrg472 samrg472 changed the title Add MessageResponse deriving Add MessageResponse derive Mar 8, 2019
@DoumanAsh
Copy link
Contributor

Just to be sure, did you test that derived code gets correctly compiled?

@samrg472
Copy link
Contributor Author

samrg472 commented Mar 8, 2019

I tested it against my own codebase locally by removing the manual implementations and replacing it with the derive. Both of the tests below compile successfully.

Test 1

#[derive(Clone, Default, Debug, actix::Message, MessageResponse)]
pub struct MyStruct;

Expands to

mod _impl_act_msg_rsp_mystruct {
    use super::*;
    extern crate actix;
    impl<_A: actix::Actor, _M: Message<Result = MyStruct>>
        actix::dev::MessageResponse<_A, _M> for MyStruct
    {
        fn handle<R: actix::dev::ResponseChannel<_M>>(self, _: &mut _A::Context, tx: Option<R>) {
            if let Some(tx) = tx {
                tx.send(self);
            }
        }
    }
}

Test 2

#[derive(Clone, Default, Debug, Message, MessageResponse)]
pub struct MyStruct<T: 'static> {
    _t: std::marker::PhantomData<T>
}

Expands to

mod _impl_act_msg_rsp_mystruct {
    use super::*;
    extern crate actix;
    impl<T: 'static, _A: actix::Actor, _M: actix::Message<Result = MyStruct<T>>>
        actix::dev::MessageResponse<_A, _M> for MyStruct<T>
    {
        fn handle<R: actix::dev::ResponseChannel<_M>>(self, _: &mut _A::Context, tx: Option<R>) {
            if let Some(tx) = tx {
                tx.send(self);
            }
        }
    }
}

@fafhrd91
Copy link
Member

@samrg472 could you create pr with this tests for actix repo. it has tests/derive.rs

@fafhrd91 fafhrd91 self-requested a review March 12, 2019 06:30
@fafhrd91
Copy link
Member

@samrg472 could you add entry to changes.md

@DoumanAsh could you make new release

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants