This repository was archived by the owner on Jul 10, 2023. It is now read-only.
Make CTFontDescriptor::font_path() return filesystem path, not URL #85
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Prior to this commit, the return string was in the format `file:///foo/bar.otf`. We now omit the leading `file://` portion. This also properly retains characters that are not URL safe.
jdm
suggested changes
Jan 31, 2018
| @@ -250,7 +250,11 @@ impl CTFontDescriptor { | |||
| let value = CFType::wrap_under_create_rule(value); | |||
| assert!(value.instance_of::<CFURL>()); | |||
| let url = CFURL::wrap_under_get_rule(mem::transmute(value.as_CFTypeRef())); | |||
| Some(format!("{:?}", url)) | |||
| let path = CFString::wrap_under_create_rule(CFURLCopyFileSystemPath( | |||
There was a problem hiding this comment.
What if we made this return Option<PathBuf> and called url.to_path() instead?
There was a problem hiding this comment.
Oh, deja vu. I understand better now.
|
@bors-servo r+ |
|
📌 Commit 8bc5c45 has been approved by |
bors-servo
pushed a commit
that referenced
this pull request
Jan 31, 2018
Make CTFontDescriptor::font_path() return filesystem path, not URL Prior to this commit, the return string was in the format `file:///foo/bar.otf`. We now omit the leading `file://` portion. This also properly retains characters that are not URL safe. <!-- 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-text-rs/85) <!-- Reviewable:end -->
|
💔 Test failed - status-travis |
|
@bors-servo retry
|
|
@poiru: 🔑 Insufficient privileges: and not in try users |
1 similar comment
|
@poiru: 🔑 Insufficient privileges: and not in try users |
|
@bors-servo retry |
bors-servo
pushed a commit
that referenced
this pull request
Jan 31, 2018
Make CTFontDescriptor::font_path() return filesystem path, not URL Prior to this commit, the return string was in the format `file:///foo/bar.otf`. We now omit the leading `file://` portion. This also properly retains characters that are not URL safe. <!-- 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-text-rs/85) <!-- Reviewable:end -->
|
☀️ Test successful - status-travis |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Prior to this commit, the return string was in the format
file:///foo/bar.otf. We now omit the leadingfile://portion. Thisalso properly retains characters that are not URL safe.
This change is