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.

Sign up for GitHub

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

Jump to bottom

[macos10.13-beta1] FinderSync and MediaLibrary bindings #2219

Merged
merged 15 commits into from
Jun 28, 2017
Merged

[macos10.13-beta1] FinderSync and MediaLibrary bindings #2219

merged 15 commits into from
Jun 28, 2017

Conversation

timrisi
Copy link
Contributor

@timrisi timrisi commented Jun 14, 2017

No description provided.

rolfbjarne
rolfbjarne requested changes Jun 14, 2017
View reviewed changes
src/findersync.cs Outdated
[Mac (10,13, onlyOn64 : true)]
[Export ("lastUsedDateForItemWithURL:")]
[return: NullAllowed]
NSDate GetLastUsedDateForItem (NSUrl itemURL);
Copy link
Member

Choose a reason for hiding this comment

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

IMHO this should be GetLastUsedDate (to match SetLastUsedDate)

Copy link
Member

Choose a reason for hiding this comment

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

Also managed code uses Url, not URL

src/findersync.cs Outdated
[Mac (10,13, onlyOn64 : true)]
[Export ("tagDataForItemWithURL:")]
[return: NullAllowed]
NSData GetTagDataForItem (NSUrl itemURL);
Copy link
Member

Choose a reason for hiding this comment

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

Same: GetTagData

Copy link
Member

Choose a reason for hiding this comment

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

Same: itemUrl instead of itemURL.

src/findersync.cs Outdated

[Mac (10,13, onlyOn64 : true)]
[Export ("setLastUsedDate:forItemWithURL:completion:")]
void SetLastUsedDate (NSDate lastUsedDate, NSUrl itemURL, Action<NSError> completion);
Copy link
Member

Choose a reason for hiding this comment

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

Same: itemUrl.

src/findersync.cs Outdated
@@ -54,6 +72,21 @@ interface FIFinderSyncProtocol

[Export ("toolbarItemToolTip")]
string ToolbarItemToolTip { get; }

[Export ("supportedMessageInterfaceNamesForItemWithURL:")]
Copy link
Member

Choose a reason for hiding this comment

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

This has space indentation instead of tab indentation.

src/findersync.cs Outdated
@@ -54,6 +72,21 @@ interface FIFinderSyncProtocol

[Export ("toolbarItemToolTip")]
string ToolbarItemToolTip { get; }

[Export ("supportedMessageInterfaceNamesForItemWithURL:")]
string[] GetSupportedMessageInterfaceNamesForItem (NSUrl itemURL);
Copy link
Member

Choose a reason for hiding this comment

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

Same: itemUrl

Also I think GetSupportedMessageInterfaceNames would be simpler and still won't lose information.

src/findersync.cs Outdated

[Mac (10,13, onlyOn64 : true)]
[Export ("exportedObjectForMessageInterface:itemURL:error:")]
NSObject GetExportedObject (NSFileProviderMessageInterface messageInterface, NSUrl itemURL, [NullAllowed] out NSError error);
Copy link
Member

Choose a reason for hiding this comment

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

Same: itemUrl.

src/findersync.cs Outdated

[Mac (10,13, onlyOn64 : true)]
[Export ("valuesForAttributes:forItemWithURL:completion:")]
void ValuesForAttributes (string[] attributes, NSUrl itemURL, Action<NSDictionary<NSString, NSObject>, NSError> completion);
Copy link
Member

Choose a reason for hiding this comment

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

Just GetValues would be simpler.

Also: itemUrl.

src/medialibrary.cs Outdated

[Mac (10,13, onlyOn64 : true)]
[Field ("MLPhotosAnimatedGroupTypeIdentifier")]
NSString MLPhotosAnimatedGroupTypeIdentifier { get; }
Copy link
Member

Choose a reason for hiding this comment

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

Remove the ML prefix for all three constants.

@monojenkins
Copy link
Collaborator

Build failure

@monojenkins
Copy link
Collaborator

Build failure

1 similar comment
@monojenkins
Copy link
Collaborator

Build failure

spouliot
spouliot suggested changes Jun 14, 2017
View reviewed changes
src/findersync.cs Outdated

[Mac (10,13, onlyOn64 : true)]
[Export ("setLastUsedDate:forItemWithURL:completion:")]
void SetLastUsedDate (NSDate lastUsedDate, NSUrl itemUrl, Action<NSError> completion);
Copy link
Contributor

Choose a reason for hiding this comment

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

Async ?

src/findersync.cs Outdated

[Mac (10,13, onlyOn64 : true)]
[Export ("setTagData:forItemWithURL:completion:")]
void SetTagData ([NullAllowed] NSData tagData, NSUrl itemUrl, Action<NSError> completion);
Copy link
Contributor

Choose a reason for hiding this comment

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

A sync ?

src/findersync.cs Outdated
@@ -54,6 +72,21 @@ interface FIFinderSyncProtocol

[Export ("toolbarItemToolTip")]
string ToolbarItemToolTip { get; }

[Export ("supportedMessageInterfaceNamesForItemWithURL:")]
string[] GetSupportedMessageInterfaceNames (NSUrl itemURL);
Copy link
Contributor

Choose a reason for hiding this comment

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

Url, not URL

src/findersync.cs Outdated

[Mac (10,13, onlyOn64 : true)]
[Export ("valuesForAttributes:forItemWithURL:completion:")]
void GetValues (string[] attributes, NSUrl itemUrl, Action<NSDictionary<NSString, NSObject>, NSError> completion);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Action too complex - we can guess the usage
Better use a custom delegate type in such case, it's self documenting

@monojenkins
Copy link
Collaborator

Build failure

@monojenkins
Copy link
Collaborator

Build success

@monojenkins
Copy link
Collaborator

Build failure

@monojenkins
Copy link
Collaborator

Build failure

@monojenkins
Copy link
Collaborator

Build failure

rolfbjarne
rolfbjarne reviewed Jun 28, 2017
View reviewed changes
src/findersync.cs Outdated
@@ -83,11 +83,12 @@ interface FIFinderSyncProtocol
[Export ("supportedServiceNamesForItemWithURL:")]
string[] SupportedServiceNames (NSUrl itemUrl);

#if FALSE // TODO: Activate after 10.13 foundation APIs have been merged
Copy link
Member

Choose a reason for hiding this comment

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

Can you file a bug about this so that we don't forget?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops. I did file a bug, meant to add it to the comment. I'll update it with the bug #

@monojenkins
Copy link
Collaborator

Build failure

@timrisi
Copy link
Contributor Author

timrisi commented Jun 28, 2017

Known failure https://bugzilla.xamarin.com/show_bug.cgi?id=57762

@timrisi timrisi merged commit 86d45b0 into xamarin:xcode9 Jun 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rolfbjarne rolfbjarne rolfbjarne approved these changes

@spouliot spouliot spouliot approved these changes

@dalexsoto dalexsoto dalexsoto approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@timrisi @monojenkins @dalexsoto @rolfbjarne @spouliot @dnfclas