Skip to content

Add Extend and FromIterator impl for rosidl_runtime_rs::{String, WString} #246

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

Closed
nnmm opened this issue Aug 14, 2022 · 4 comments
Closed

Comments

@nnmm
Copy link
Contributor

nnmm commented Aug 14, 2022

It would be nice if the string types in rosidl_runtime_rs could be built/extended from iterators, like the regular string type.

For this, we'd need implementations for Extend<char> and FromIterator<char> for both types, at least.

maspe36 added a commit to maspe36/ros2_rust that referenced this issue Nov 11, 2022
… and `FromIterator<&'a char>` for String and WString

ros2-rust#246
@maspe36
Copy link
Collaborator

maspe36 commented Nov 11, 2022

An important thing to note, is that this feature gives the string types interior mutability. So these string types would have to lose the Sync impl.

https://github.com/ros2-rust/ros2_rust/blob/main/rosidl_runtime_rs/src/string.rs#L230

@nnmm
Copy link
Contributor Author

nnmm commented Nov 11, 2022

I think we wouldn't need to lose the Sync marker, since the Extend trait takes &mut self and not &self.

@maspe36
Copy link
Collaborator

maspe36 commented Nov 12, 2022

Ahh, gotcha. Yeah even rust's String is Sync
https://doc.rust-lang.org/std/string/struct.String.html#impl-Sync-for-String

Updated in the PR

@maspe36
Copy link
Collaborator

maspe36 commented Mar 4, 2023

I believe this issue should be closed now that #293 has been merged

@nnmm nnmm closed this as completed Mar 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants