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

Implement From<&str> for CFString #128

Merged
merged 1 commit into from Nov 22, 2017
Merged

Conversation

@faern
Copy link
Contributor

faern commented Nov 22, 2017

I'm not sure if you have any strict policies on what types you usually accept as parameters to the safe wrappers. So I'm not sure if you want this or not. But my reasons are:

I create system-configuration and system-configuration-sys crates as bindings to that API. In the SCDynamicStore module a lot of the functions take a CFStringRef as argument. It would be more ergonomic if this safe wrapper type could deal with Rust strings directly, so I can do:

let store = SCDynamicStore::create("my-dynamic-store");
let dict = store.find("State:/Network/Global/IPv4");

But at the same time I don't want to make it harder to use for people who already sit on CFString objects. So with this new From<&str> impl my API can be used both with &str and CFString if I implement the methods as <S: Into<CFString>>.


This change is Reviewable

@nox
Copy link
Member

nox commented Nov 22, 2017

@bors-servo
Copy link
Contributor

bors-servo commented Nov 22, 2017

📌 Commit b8a4536 has been approved by nox

@bors-servo
Copy link
Contributor

bors-servo commented Nov 22, 2017

Testing commit b8a4536 with merge 36fad2d...

bors-servo added a commit that referenced this pull request Nov 22, 2017
Implement From<&str> for CFString

I'm not sure if you have any strict policies on what types you usually accept as parameters to the safe wrappers. So I'm not sure if you want this or not. But my reasons are:

I create `system-configuration` and `system-configuration-sys` crates as bindings to that API. In the [`SCDynamicStore`] module a lot of the functions take a `CFStringRef` as argument. It would be more ergonomic if this safe wrapper type could deal with Rust strings directly, so I can do:
```rust
let store = SCDynamicStore::create("my-dynamic-store");
let dict = store.find("State:/Network/Global/IPv4");
```
But at the same time I don't want to make it harder to use for people who already sit on `CFString` objects. So with this new `From<&str>` impl my API can be used both with `&str` and `CFString` if I implement the methods as `<S: Into<CFString>>`.

[`SCDynamicStore`]: https://developer.apple.com/documentation/systemconfiguration/scdynamicstore?language=objc

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/core-foundation-rs/128)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Nov 22, 2017

☀️ Test successful - status-travis
Approved by: nox
Pushing 36fad2d to master...

@bors-servo bors-servo merged commit b8a4536 into servo:master Nov 22, 2017
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@faern faern deleted the faern:impl-from-cfstring branch Nov 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.