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

[TIMOB-23495] iOS: Add Ti.Filesystem.File "parent" property (parity) #9285

Merged
merged 11 commits into from Nov 9, 2017

Conversation

hansemannn
Copy link
Collaborator

var file = Ti.Filesystem.getFile('app.js');
// parent may be null if at root?
//should(file.parent).be.ok; // not null or undefined. should(file).not.be.null causes a stack overflow somehow.
should(file).have.a.readOnlyProperty('parent');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd assume we'd also want to verify the value is another Ti.Filesystem.File (and is a directory). Also that at some level you can't get a parent object and get null back if you try to go up a level above the app root or whatever the directory tree sandbox happens to be.

@@ -176,19 +176,6 @@ methods:
if this object doesn't identify a directory.
returns:
type: Array<String>
- name: getParent
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume we actually need to deprecate this and not just remove it? Can we have a parent property that holds a Ti.Filesystem.File return value, but also have a getParent() method that returns the path of the parent as a String?

This is a breaking change, so we need to deprecate for one major version first... I assume there are apps out there relying on this getter existing and returning a string.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only removed it because the getter is generated automatically by using - (NSString *)parent. But I assume we could match Android's behavior here whereby getParent returns a file and parent a string. @infosia how does Windows behave here? Although I don't like that the getter of a property returns something different (oddly, I'd rather match the most-useful behavior). Maybe deprecate the difference on Android?

@@ -29,45 +29,44 @@ -(id)initWithFile:(NSString*)path_
return self;
}

-(void)dealloc
- (void)dealloc
{
RELEASE_TO_NIL(fm);
RELEASE_TO_NIL(path);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hansemannn Can you Fix Indentation. I think you need to lint the file.

return [path stringByDeletingLastPathComponent];
}

- (TiFilesystemFileProxy *)parent
{
return [[[TiFilesystemFileProxy alloc] initWithFile:[path stringByDeletingLastPathComponent]] autorelease];
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sgtcoolguy Sorry to bug you on this, but to confirm:

  1. Deprecate getParent() for parity with other platforms
  2. Introduce parent property that works the same across all platforms
  3. In 8.0.0+, remove the manual getParent accessor, so getParent() will use the computed getter that matches all platforms without a deprecation.

should(file).have.a.readOnlyProperty('parent');
should(file.parent).not.be.undefined;
should(file.getParent).not.be.undefined;
should(file.getParent.be.a.Function;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hansemannn ')' is missing in above line. Please update.

@hansemannn
Copy link
Collaborator Author

@vijaysingh-axway Updated

Copy link
Contributor

@vijaysingh-axway vijaysingh-axway left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cr and FT passed.

@sgtcoolguy sgtcoolguy merged commit 2ca82ce into tidev:master Nov 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants