Conflict with NSViewController in macOS 10.13 / Swift 4

[lhc70000]

First of all thanks a lot for this project.

As stated in README, there is a workaround for implementing the property identifier in Swift:

When using Swift you need to override the identifier from MASPreferencesViewController the following to be compatible with the mutable identifier String? in NSViewController

However in macOS 10.13 SDK and Swift 4, the type of identifier from NSViewController has been changed from String? to NSUserInterfaceItemIdentifier?, which made the above trick no longer working.

[shpakovski]

Thanks a lot for the heads up! So we should rename identifier into toolbarItemIdentifier, right?

Update: Plus maybe go Swift and v2.0.

Update 2: Or change the return type to id and demand NSUserInterfaceItemIdentifier on modern macOS?

Update 3: Ah, the NSUserInterfaceItemIdentifier is an alias for NSString! So should we just modify the README.md? 😃

[lhc70000]

Yes, it should be a possible solution.

Alternatively, simply changing

@property (nonatomic, readonly) NSString* identifier;

to

@property (nonatomic, readonly) NSUserInterfaceItemIdentifier identifier;

may be a quicker (but dirtier?) way, since NSUserInterfaceItemIdentifier in Objective-C is typedef-ed as NSString*.

[lhc70000]

I'd love to see a swift version of this project! And if you want, I can help translating it to Swift :D

[lhc70000]

Ah, the NSUserInterfaceItemIdentifier is an alias for NSString! So should we just modify the README.md?

No, in Swift NSUserInterfaceItemIdentifier is a struct, so it's impossible to use it without changing the code in MASPreferences.

[shpakovski]

If we replace NSString * with NSUserInterfaceItemIdentifier it won’t ever build in Xcode 8.0. So it’s probably better to use id (Any).

I'd love to see a swift version of this project!

Let’s wait for the official release of 4.0 and Xcode 9.0 🙂

[lhc70000]

Alright, thanks in advance!

[saagarjha]

@shpakovski How about something like this? It retains type safety and works in both Xcode 8 and 9.

[shpakovski]

@lhc70000 @saagarjha Thanks a lot, this should be fixed in 7b656d2!

[sorbits]

Hi @shpakovski, I am late to this one, but I think your original suggestion of renaming identifier to toolbarItemIdentifier (or similar) is the best solution, despite breaking compatibility, but when people upgrade, they will get a compiler warning, so it should cause minimal problems.

The reason is that at run-time we now have two protocols declaring and using the same property, which means that anyone implementing identifier as read-only (for MASPreferencesViewController conformance) are effectively breaking NSUserInterfaceItemIdentification (where the property is read/write).

[shpakovski]

Hey Alan! Should the identifier remain of type NSUserInterfaceItemIdentifier? I quite like it 🙂

[sorbits]

I think we should keep it as NSString because we use it e.g. to generate user defaults keys (which are strings), so calling it something other than a string to me is confusing and I actually wonder why Apple decided to introduce a new type alias for their identifier when this one is also very obviously meant to be a string (that you can enter in Interface Builder).

[sorbits]

As a follow-up, I do not think identifier should be nullable (as done in dc30f9b), as we e.g. store them in arrays w/o any checks.

As for the name, I am a bit torn here regarding toolbarItemIdentifier because we use them to identify preferences views (as well as toolbar items), so it is really a preferencesViewIdentifier but there is no precedence for calling anything preferencesView, it also seems slightly redundant to have a preferencesViewIdentifier in a MASPreferencesViewController protocol implementation, but I guess that is Objective-C namespacing (and there is precedence for such name redundancy).

I guess preferencesViewIdentifier is OK… if you give me the thumbs up, I can push a commit with this change, and also remove the nullable.

[shpakovski]

Thanks a lot, how about viewIdentifier?

[sorbits]

OK, I went with viewIdentifier.

I did not update the demo project, would appreciate if you can take care of that.

[shpakovski]

I will, cheers!