leighmcculloch / AppSandboxFileAccess

A class that wraps up writing and accessing files outside a Mac apps App Sandbox files into a simple interface. The class will request permission from the user with a simple to understand dialog consistent with Apple's documentation and persist permissions across application runs.
BSD 2-Clause "Simplified" License
114 stars 23 forks source link

Convert to Swift #8

Closed ConfusedVorlon closed 4 years ago

ConfusedVorlon commented 5 years ago
ConfusedVorlon commented 5 years ago

Any interest in this? If so, I can tweak the docs to be swifty - and perhaps to mention the objective C version available.

Otherwise, I can just spin this off as a separate swift pod.

ConfusedVorlon commented 4 years ago

ping

demitri commented 4 years ago

Just a comment as a user of the code but not owner of the project. This PR effectively deletes the current functionality and replaces it with something that for some is completely different. I use this code in an Objective-C project and have no plans to convert my program to Swift. By deleting the Obj-C files, you effectively delete their history as well - they become "archived" in the repo and prevent anyone from making patches.

A Swift version is of course useful to some, but I would recommend leaving the Obj-C files alone and creating a new directory for the Swift files.

ConfusedVorlon commented 4 years ago

It doesn't look like this is going anywhere - but to the obj-c points. It would be easy enough to a) make an obj-c branch. This would make the current state easily accessible and allow updates if needed. b) just note in the readme that of the obj-c version, you should get podspec version xx

demitri commented 4 years ago

Yes, that's a good solution. I might suggest deleting this PR then and creating a new one with a Swift branch. I am revising this repo for other reasons (you might be interested), but I'll put those comments in a new issue.

Also, I like some of the new features you put in the Swift version - it would be nice to see those in the Obj-C files as well.

ConfusedVorlon commented 4 years ago

@demitri this does come from a separate branch. I think it is up to the owner how they merge it (though I'm no git expert).

fwiw - I'd suggest merging merging the swift version into master and keeping the obj-c version in a separate branch.

ConfusedVorlon commented 4 years ago

I did a little digging - if you prefer this in another branch, then I can change the target of the pull request. However you'd need to create the branch first.

(I still think the default should be swift though !)

leighmcculloch commented 4 years ago

Thanks for the discussion here. This PR includes 4 changes that appear to be largely unrelated to each other. I'd like to understand the individual 'why' for each change. What is motivating each change and why is the library better off with it. @ConfusedVorlon could you share this?

I think backwards compatibility is really important and there should be a compelling reason to break compatibility for the folks already using the library.

This change bundles some interesting things together as noted by the PR description:

The last two things I think would be easily backwards compatible and are really interesting and cool features. The first two things are not backwards compatible from what I can tell.

I'd love to understand why switch to Swift specifically. This library should already be compatible with Swift applications. Is it not? Would Objective-C programs be able to import this if it is written in Swift?

demitri commented 4 years ago

I'll just comment on the last point. An Obj-C program can include Swift code. However, in earlier OS's doing so with even a single file would pull in an entire Swift runtime into the app. This is a high price to pay for just a few files. I think this has changed with a newer version of Swift with ABI stability (the runtime being included in the system). I think it's version 5? But that won't apply to earlier OS's.

It seems that's not exactly right, see this page. Apps are still compiled with a binary runtime, but for Swift 5 and macOS 10.14.4 it will be ignored in favor of the OS version. So it still pulls in the runtime.

For that reason, I don't want to include Swift code in my app yet. That said, I'm in favor of a Swift version and certainly would like to see the new features written by @ConfusedVorlon in the Obj-C version!

leighmcculloch commented 4 years ago

I'm in favor of a Swift version

What are the benefits of having a separate Swift version of this library?

ConfusedVorlon commented 4 years ago

for me, the benefits of swift are in two areas

1) that's the way the world is moving, so increasingly swift is easier for people to understand, inspect, update, etc to be honest - I just thought everyone was moving to swift now! I wrote in Obj-C for about 10 years, but I'm rapidly losing fluency.

2) default values in function declarations allow for a cleaner API so, for example

public func accessFileURL(_ fileURL: URL, askIfNecessary:Bool = true, persistPermission persist: Bool, with block:AppSandboxFileAccessBlock? = nil) -> Bool 

//can be called
var result = sandbox.accessFileURL(url,persistPermission:false)
//or
var result = sandbox.accessFileURL(url,askIfNecessary:false,persistPermission:false){
//do something with result
}

Swift libraries definitely can be accessed from Obj-C. It probably needs some notations in the code to enable that though.

Looking at the code again, I haven't really updated the signatures to by 'swifty' So for example in swift it would normally be

var result = sandbox.access(fileUrl,persistPermission:false)

I believe this can still be done whilst maintaining the old obj-c signatures for backwards compatibility.

demitri commented 4 years ago

What are the benefits of having a separate Swift version of this library?

Honestly, for me? None. But I'm one data point.

that's the way the world is moving, so increasingly swift is easier for people to understand, inspect, update, etc to be honest - I just thought everyone was moving to swift now!

I think new developers are, and I think new projects will be too. I started my app in 2011/12 and have over 50K lines of code (I'm sure more by now). It would take me many months to convert my project to Swift, and more for the dust to settle. At the end of that I won't have gained any new features for my app, no speed benefits, etc. I think you'll find a lot of the larger codebases that predate Swift haven't changed for this reason. The cost/benefit isn't there. There are a lot of us in Obj-C!

Swift libraries definitely can be accessed from Obj-C.

The reverse is of course true, but without the high cost of embedding a Swift runtime in the app.

I suppose my (again, data point of one) position is that I'm not opposed to a Swift version, but I need an Obj-C version. Given how small this library is (which is a good thing!), I don't see any real drawback to keeping it Obj-C.

ConfusedVorlon commented 4 years ago

I think new developers are, and I think new projects will be too. I started my app in 2011/12 and have over 50K lines of code (I'm sure more by now). It would take me many months to convert my project to Swift, and more for the dust to settle. At the end of that I won't have gained any new features for my app, no speed benefits, etc.

I think you might be surprised! I have found that converting* a lot of my old codebase to swift has given me much more maintainable, cleaner, more correct code. The swift compiler really does find a lot more than obj-c. And of course you don't have to do it in one go, you can convert progressively, or just add new features in swift. It is certainly work - but I have found it worth the effort.

but - that's not really a debate for here.

@leighmcculloch this is really back to you with my original genuine question;

Do you have any interest in this?

It would be perfectly reasonable for you to decide that you're happy with obj-c. In that case, I can just start a swift fork. Alternatively, if you want to make the jump to swift - I'm happy to work with you to make it work in this repo.

* note, converting doesn't mean manually re-writing. https://swiftify.com/ can get you a lot of the way there automatically.

ConfusedVorlon commented 4 years ago

ping

ConfusedVorlon commented 4 years ago

It looks like forking is probably the best bet. That way I can swiftify the API without worrying too much about backwards compatibility.

demitri commented 4 years ago

I can't speak for @leighmcculloch, but my macOS work is something that I only have intermittent time to work on. I develop in bursts as time allows (and it often doesn't). My recommendation would be to write a Swift/Obj-C bridge file and just use the Obj-C code. There is no disadvantage to this; all Swift code sits on a mountain of Obj-C libraries, and this won't change anytime soon. Swift is by no means more "pure". The reverse cost is higher - a pure-Obj-C program would require a whole Swift runtime to be embedded into an app, which isn't worth it for a few files.

The other obvious disadvantage of forking is that it will lead to disparate functionality. This kind of code in particular is tedious plumbing - while I agree with the ideals behind the sandbox, Apple has made it #$&ing horrible to deal with. I'd want to spend the minimum* time possible to solve these problems so I can spend real time on actual functionality of my app. This feels like it would be optimized with contributions from several people.

That said, if you feel like you'll only be happy with a pure Swift solution, forking sounds like the swiftest option (pun painfully inflicted).

ConfusedVorlon commented 4 years ago

one key disadvantage: I'd have to work with Obj-C code. I avoid that where I can!

new fork now available for anyonw who is interested : https://github.com/ConfusedVorlon/SwiftySandboxFileAccess

leighmcculloch commented 4 years ago

It looks like forking is probably the best bet.

I think so. I think if there's an Obj-C and Swift versions that sounds good and I don't have capacity to maintain a Swift version here, so forking makes sense. Nice work!!! 🎉