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

Derive Clone for SinkInfo and SinkPortInfo #43

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

agraven
Copy link
Contributor

@agraven agraven commented Sep 24, 2021

The Clone implementation enables using data from a SinkInfo outside the callback functions where a reference to one is made available

@agraven
Copy link
Contributor Author

agraven commented Sep 24, 2021

Hm, because of how Clone is implemented for Cow this would have to do Cow::Owned(field.clone().into_owned()) on every field with a Cow to work the way I had in mind.

@ids1024
Copy link

ids1024 commented Jun 13, 2022

I've also noticed that the API lacks something like this.

I guess changing it into an owned value doesn't really have the expected semantics of a Clone, to_owned means something a bit different, and into_owned would be expected to take self...

Maybe derive Clone, but also have an into_owned(self) -> Self<'static> method to match Cow? Looks like something like https://crates.io/crates/derive-into-owned could derive that automatically.

@ids1024
Copy link

ids1024 commented Jun 13, 2022

Or... I see you already mentioned that same crate on #44.

There is a bit of complexity in SinkInfo since it has Option<Cow<_>> fields and a Vec<SinkPortInfo<'a>> field where SinkPortInfo also contains Cow types. It may require a custom macro (or maybe improvements could be made to derive-into-owned)?

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