Expose CDVCommandStatus enum to Swift

[dpogue]

This allows Swift plugins to write code like

CDVPluginResult(status: .error)

rather than

CDVPluginResult(status: CDVCommandStatus_ERROR)

Its use in Objective C is unchanged, but this is a API breaking change as far as Swift plugins are concerned (so we'd be looking at cordova-ios 6 to land this) I've hacked it to support both the existing name and the "friendly" name in Swift, at the cost of the code making sense 😕

[codecov-io]

Current coverage is 56.95% (diff: 100%)

Merging #254 into master will not change coverage

@@             master       #254   diff @@
==========================================
  Files            12         12          
  Lines          1085       1085          
  Methods         169        169          
  Messages          0          0          
  Branches        174        174          
==========================================
  Hits            618        618          
  Misses          467        467          
  Partials          0          0          

Powered by Codecov. Last update 4975a1f...9977c74

[shazron]

Haven't played much with Swift enums, is there a way to support both for now, and deprecate the old in Swift?

[lucatorella]

This change isn't breaking old code. The enum is still called CDVCommandStatus in Obj-C. It's just using the newer NSEnum syntax that is the recommended syntax even before Swift came out.

I would go even further and use NS_SWIFT_NAME to have names like .noResult, .ok etc. in Swift.

[shazron]

@dpogue Can you confirm what @lucatorella said?

In any case, we don't have any Swift tests yet, but that's easy to remedy for next release.

[dpogue]

Yes, the breaking change is only on the Swift side

[lucatorella]

No, because now that enum is not accessible in Swift. So if we want to make it accessible, it's better to make it accessible with proper name conventions (.noResult, .ok, ...). If you just add NSEnum then it will be available in Swift with weird names, and then if you decide in a next version to use NS_SWIFT_NAME, then you'll have a breaking change on the Swift side.

[lucatorella]

Sorry, I'm mistaken. The plain C enum is currently already available in Swift. So yes, it'll be a breaking change (just in Swift) as much as your proposal :)

[shazron]

Sorry, Swift newbie here :) (waiting for it to be stable real soon now!). This will be slated for [email protected], I'll see how to add Swift tests in the current test project...

[shazron]

@dpogue Reviewing PRs for next cordova-ios release. Am I correct to assume since this is a breaking change, it would be for a major version release (since we follow semver)?

[dpogue]

Yes, that is correct

[knight9999]

This looks good to me. Are we still planning to release this in next major and when is the best time to merge?

[dpogue]

This causes all existing Swift plugins to break with compiler errors and it's impossible to be backwards compatible and support both... so whenever we feel like that's a good idea 😬

Maybe now is the best time, since we're already changing how Bridging Headers and SwiftVersion are handled?

[dpogue]

@shazron @knight9999 Okay... I think I've come up with a (messy) solution to allow this to not be a breaking change.

The existing ObjC enum values are exposed to Swift with friendly names, as per Swift convention:
CDVCommandStatus_NO_RESULT -> CDVCommandStatus.noResult or just .noResult

Then, we make a constant variable in ObjC for each of those enum values named SWIFT_<enum_value_name>, but expose those to Swift with the original enum value name. So these are globally available in Swift, the same way the enum constants were previously.

It's very ugly and now we have a bunch of global constants sitting around that I don't think the compiler will optimize out :(

That said, it seems to work, and I was able to take one of my existing Swift plugins and run it without changing any of the CDVCommandStatus_OK uses into .ok. Both of the following work in Swift after this change:

// Old/current style:
let result = CDVPluginResult(status: CDVCommandStatus_ERROR, messageAs: "AbortError");

// New style
let result = CDVPluginResult(status: .error, messageAs: "AbortError");

[codecov-io]

Codecov Report

Merging #254 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #254   +/-   ##
=======================================
  Coverage   74.29%   74.29%           
=======================================
  Files          12       12           
  Lines        1564     1564           
=======================================
  Hits         1162     1162           
  Misses        402      402

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 23a9387...b7b9931. Read the comment docs.

[erisu]

LGTM