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

[45243] NSObject Binding methods should use NSString for the binding, not string #992

Merged
merged 2 commits into from Nov 29, 2016
Merged

[45243] NSObject Binding methods should use NSString for the binding, not string #992

merged 2 commits into from Nov 29, 2016

Conversation

timrisi
Copy link
Contributor

@timrisi timrisi commented Oct 13, 2016

No description provided.

@monojenkins
Copy link
Collaborator

Build success

@spouliot spouliot added the do-not-merge Do not merge this pull request label Oct 14, 2016
Copy link
Contributor

@spouliot spouliot left a comment

Choose a reason for hiding this comment

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

The patch is incorrect because using [Wrap] won't keep the existing API virtual, which means breaking changes. See API diff output (full list jenkins) like:

Modified methods:

public virtual void Bind (string binding, NSObject observable, string keyPath, NSDictionary options)
public virtual NSDictionary BindingInfo (string binding)
public virtual NSObject[] BindingOptionDescriptions (string aBinding)
public virtual MonoMac.ObjCRuntime.Class BindingValueClass (string binding)
public virtual void Unbind (string binding)

NSString[] ExposedBindings ();
#else
[Export ("exposedBindings")]
NSString[] ExposedBindings;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you're missing a { get; } there

@timrisi
Copy link
Contributor Author

timrisi commented Oct 14, 2016

Is there a way to keep the old versions and still bind the new ones to be available at the same time?

@spouliot
Copy link
Contributor

For the API listed above (i.e. the static one are fine) you can rework this by using [Sealed] on the new API (and drop the [Wrap]). That will allow you to re-use (kind of) the same [Export] for the NSString variant - they won't be virtual so they won't be registered (so the "use only once" rule does not apply).

IOW it's the new NSString overloads that will be non-virtual (for!XAMCORE_4_0). The main drawback is that it won't be possible to override them.

@monojenkins
Copy link
Collaborator

Build success

@spouliot spouliot removed the do-not-merge Do not merge this pull request label Nov 3, 2016
@spouliot
Copy link
Contributor

spouliot commented Nov 3, 2016

@chamons macOS specific fix, please review too :)

@chamons
Copy link
Contributor

chamons commented Nov 16, 2016

@timrisi Assuming API diff says you didn't break anything 👍

@spouliot
Copy link
Contributor

@timrisi please follow up on last comment, thanks!

@timrisi
Copy link
Contributor Author

timrisi commented Nov 29, 2016

Namespace Foundation

Type Changed: Foundation.NSObject

Obsoleted methods:

[Obsolete ("Use Bind (NSString binding, NSObject observable, string keyPath, [NullAllowed] NSDictionary options) instead")]
public virtual void Bind (string binding, NSObject observable, string keyPath, NSDictionary options);
[Obsolete ("Use GetBindingInfo (NSString binding) instead")]
public virtual NSDictionary BindingInfo (string binding);
[Obsolete ("Use GetBindingOptionDescriptions (NSString aBinding) instead")]
public virtual NSObject[] BindingOptionDescriptions (string aBinding);
[Obsolete ("Use GetBindingValueClass (NSString binding) instead")]
public virtual ObjCRuntime.Class BindingValueClass (string binding);
[Obsolete ("Use SetDefaultPlaceholder (NSObject placeholder, NSObject marker, NSString binding) instead")]
public static void SetDefaultPlaceholder (NSObject placeholder, NSObject marker, string binding);
[Obsolete ("Use Unbind (NSString binding) instead")]
public virtual void Unbind (string binding);

Added methods:

public void Bind (NSString binding, NSObject observable, string keyPath, NSDictionary options);
public NSDictionary GetBindingInfo (NSString binding);
public NSObject[] GetBindingOptionDescriptions (NSString aBinding);
public ObjCRuntime.Class GetBindingValueClass (NSString binding);
public static NSObject GetDefaultPlaceholder (NSObject marker, NSString binding);
public static void SetDefaultPlaceholder (NSObject placeholder, NSObject marker, NSString binding);
public void Unbind (NSString binding);

@spouliot spouliot merged commit 7d88592 into xamarin:master Nov 29, 2016
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.

None yet

5 participants