Skip to content

Stub out new KeyPath APIs in NSSortDescriptor #1120

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

Merged
merged 1 commit into from
Jul 20, 2017
Merged

Stub out new KeyPath APIs in NSSortDescriptor #1120

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Foundation/NSSortDescriptor.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,13 @@ open class NSSortDescriptor: NSObject, NSSecureCoding, NSCopying {

open var key: String? { NSUnimplemented() }
open var ascending: Bool { NSUnimplemented() }
var keyPath: AnyKeyPath? { NSUnimplemented() }
Copy link
Member

@ikesyo ikesyo Jul 20, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

This should be open.


open func allowEvaluation() { NSUnimplemented() } // Force a sort descriptor which was securely decoded to allow evaluation

public init(key: String?, ascending: Bool, comparator cmptr: Comparator) { NSUnimplemented() }
convenience init<Root, Value>(keyPath: KeyPath<Root, Value>, ascending: Bool) { NSUnimplemented() }
convenience init<Root, Value>(keyPath: KeyPath<Root, Value>, ascending: Bool, comparator cmptr: @escaping Comparator) { NSUnimplemented() }
Copy link
Member

Choose a reason for hiding this comment

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

These should be public.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I took the declaration directly from here
Are we saying this is incorrect or that it's common practice to replace convenience with public?

Copy link
Member

@ikesyo ikesyo Jul 20, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Are we saying this is incorrect or that it's common practice to replace convenience with public?

Actually incorrect 🙂 convenience is not an access control specifier.

The symblos in the API documents are all public (or open). If you don't specify any access control it defaluts to internal visibility so consumers of the framework can't see those symbols.

You should check the documents:

Copy link
Contributor Author

@DunnCoding DunnCoding Jul 21, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Perfect, thanks for taking the time to explain that :)

I'll update shortly with the correct access controls, I'll also have a read through the documentation you've provided.

Thanks again! :)


open var comparator: Comparator { NSUnimplemented() }

Expand Down