Skip to content

Bind new mac-only APIs from xcode 8.3 - #1602

Merged
timrisi merged 1 commit into
dotnet:xcode8.3from
timrisi:MacApis
Feb 6, 2017
Merged

Bind new mac-only APIs from xcode 8.3#1602
timrisi merged 1 commit into
dotnet:xcode8.3from
timrisi:MacApis

Conversation

@timrisi

@timrisi timrisi commented Jan 31, 2017

Copy link
Copy Markdown
Contributor

No description provided.

@monojenkins

Copy link
Copy Markdown
Contributor

Build failure

@spouliot spouliot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems there was not much added in 10.12.4 :)

Comment thread src/safariservices.cs Outdated
[Mac (10, 12, 4)]
[Async (ResultTypeName="SFValidationResult")]
[Export ("validateContextMenuItemWithCommand:inPage:userInfo:validationHandler:")]
void ValidateContextMenuItem (string command, SFSafariPage page, [NullAllowed] NSDictionary<NSString, NSObject> userInfo, Action<bool, NSString> validationHandler);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please create a delegate type instead of Action<bool, NSString> (and drop the ResultTypeName). That should be the case for > 1 parameter, unless the 2nd one is an NSError.

The former is easier to use, because the parameters are named. The later requires extra documentation to be useful.

There will soon be a test to catch those (at least in master) once I'm finished adding missing Async.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It won't let me do it without the ResultTypeName (or ResultType) on the Async:

error BI1023: bmac: Async method Void ValidateContextMenuItem(System.String, SafariServices.SFSafariPage, Foundation.NSDictionary`2[Foundation.NSString,Foundation.NSObject], SafariServices.SFValidationResult) with more than one result parameter in the callback by neither ResultTypeName or ResultType

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@timrisi you right, it's still needed :) However the current generated code is

public class SFValidationResult {
	// constructors
	public SFValidationResult (bool arg1, Foundation.NSString arg2);
	// properties
	public bool Arg1 { get; set; }
	public Foundation.NSString Arg2 { get; set; }
}

By using a delegate the properties will be named based on the parameters.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I understand the benefit of using the delegate instead, and am happy to do that, I just wanted to make sure I wasnt' missing something since you said said to remove the ResultTypeName and it wouldn't let me :).

@dalexsoto dalexsoto left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ok once Sebastien's comment is fixed

@spouliot spouliot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Stop! this PR targets cycle9 not xcode8.3 branch

@monojenkins

Copy link
Copy Markdown
Contributor

Build success

@timrisi
timrisi changed the base branch from cycle9 to xcode8.3 February 1, 2017 23:41
@timrisi

timrisi commented Feb 1, 2017

Copy link
Copy Markdown
Contributor Author

@spouliot Moved to be based on the xcode8.3 branch instead of cycle 9 and updated the target for the PR

@monojenkins

Copy link
Copy Markdown
Contributor

Build failure

@timrisi

timrisi commented Feb 2, 2017

Copy link
Copy Markdown
Contributor Author

Build

@monojenkins

Copy link
Copy Markdown
Contributor

Build failure

1 similar comment
@monojenkins

Copy link
Copy Markdown
Contributor

Build failure

@timrisi

timrisi commented Feb 6, 2017

Copy link
Copy Markdown
Contributor Author

Known failure

@timrisi
timrisi merged commit 602a102 into dotnet:xcode8.3 Feb 6, 2017
spouliot pushed a commit to spouliot/xamarin-macios that referenced this pull request Mar 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants