Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Add support for file URLs in MTLURLValueTransformerName #262

Closed
wants to merge 2 commits into from

3 participants

@mrh-is

MTLURLValueTransformerName currently returns nil if a file URL is passed in. This also allows for file URLs to be used.

@robb
Owner

Thanks :sparkles:

Can you add a test, too?

...SValueTransformer+MTLPredefinedTransformerAdditions.m
@@ -23,7 +23,7 @@ + (void)load {
MTLValueTransformer *URLValueTransformer = [MTLValueTransformer
reversibleTransformerWithForwardBlock:^ id (NSString *str) {
if (![str isKindOfClass:NSString.class]) return nil;
- return [NSURL URLWithString:str];
+ return ([NSURL URLWithString:str] ?: [NSURL fileURLWithPath:str]);
@robb Owner
robb added a note
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@robb robb commented on the diff
MantleTests/MTLPredefinedTransformerAdditionsSpec.m
@@ -25,6 +25,11 @@
expect([transformer transformedValue:URLString]).to.equal([NSURL URLWithString:URLString]);
expect([transformer reverseTransformedValue:[NSURL URLWithString:URLString]]).to.equal(URLString);
+ // Write temporary file for use in testing file URLs
+ NSURL *fileURL = [NSURL fileURLWithPath:[NSTemporaryDirectory() stringByAppendingPathComponent:@"file.txt"]];
@robb Owner
robb added a note

Let's use a fixed String and URL as input and not rely on the test environment's temp dir.

It also looks like there a spaces in the beginning of the line.

@mrh-is
mrh-is added a note

I was trying that, but if you don't give it a real path, +fileURLWithPath: gives you nil. Perhaps I just couldn't find the right way though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jspahrsummers

I'm not sold on this. If you're expecting a path, I think you should create a transformer that does this explicitly. Otherwise, invalid URLs could end up as paths on the user's filesystem.

@robb
Owner

That's a good point, especially when combined with implicit transformers

@mrh-is

Hmm, true @jspahrsummers. Probably a better idea. I shall do so forthwith. Thanks for the suggestion!

@mrh-is mrh-is closed this
@jspahrsummers

No problem! Sorry for the terseness of my previous reply. I really appreciate you taking the time to open this as a pull request for discussion. :sparkles:

@mrh-is

No, I appreciate the feedback! I hadn't thought of those perspectives, and I'm glad to learn from you guys.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 7, 2014
  1. @mrh-is
Commits on Mar 8, 2014
  1. @mrh-is
This page is out of date. Refresh to see the latest.
View
2  Mantle/NSValueTransformer+MTLPredefinedTransformerAdditions.m
@@ -23,7 +23,7 @@ + (void)load {
MTLValueTransformer *URLValueTransformer = [MTLValueTransformer
reversibleTransformerWithForwardBlock:^ id (NSString *str) {
if (![str isKindOfClass:NSString.class]) return nil;
- return [NSURL URLWithString:str];
+ return [NSURL URLWithString:str] ?: [NSURL fileURLWithPath:str];
}
reverseBlock:^ id (NSURL *URL) {
if (![URL isKindOfClass:NSURL.class]) return nil;
View
5 MantleTests/MTLPredefinedTransformerAdditionsSpec.m
@@ -25,6 +25,11 @@
expect([transformer transformedValue:URLString]).to.equal([NSURL URLWithString:URLString]);
expect([transformer reverseTransformedValue:[NSURL URLWithString:URLString]]).to.equal(URLString);
+ // Write temporary file for use in testing file URLs
+ NSURL *fileURL = [NSURL fileURLWithPath:[NSTemporaryDirectory() stringByAppendingPathComponent:@"file.txt"]];
@robb Owner
robb added a note

Let's use a fixed String and URL as input and not rely on the test environment's temp dir.

It also looks like there a spaces in the beginning of the line.

@mrh-is
mrh-is added a note

I was trying that, but if you don't give it a real path, +fileURLWithPath: gives you nil. Perhaps I just couldn't find the right way though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ expect([transformer transformedValue:fileURL.absoluteString]).to.equal(fileURL);
+ expect([transformer reverseTransformedValue:fileURL]).to.equal(fileURL.absoluteString);
+
expect([transformer transformedValue:nil]).to.beNil();
expect([transformer reverseTransformedValue:nil]).to.beNil();
});
Something went wrong with that request. Please try again.