vtourraine / AcknowList

Acknowledgements screen displaying a list of licenses, for example from CocoaPods and Swift Package Manager dependencies.
MIT License
784 stars 59 forks source link

"don’t copy the file itself, just add a reference" #33

Closed simonbromberg closed 6 years ago

simonbromberg commented 6 years ago

How do you do that? Asking for a friend. 🤷 😅

simonbromberg commented 6 years ago

Oh right, drag from Finder (Or drag within the project navigator) but don't select "copy files if needed" or just "File" > "Add files to …"

Note also have to add it to your target in the "File Inspector".

EDIT: Hm, that doesn't seem to be working for me. At runtime still not finding the list.

simonbromberg commented 6 years ago

Hm, the file generated by pod install includes the target name (e.g. "Pods-MyTargetName-acknowledgements"), but the source code is just looking for a file named "Pods-acknowledgements" without the target name. But if I rename my reference it renames it in the pod target support folder too, so the next time you run pod install it doesn't overwrite the one that is references properly.

Should the default pod name not include the target name too?

Relevant source file

vtourraine commented 6 years ago

Hello Simon, and thanks a lot for your feedback!

We should probably be more explicit about the Xcode “reference” in our documentation; as you noted, the setup can be tricky.

But to address your latest issue: you don’t have to rename the plist file, instead I would suggest that you instantiate the view controller like so:

let path = Bundle.main.path(forResource: "Pods-AcknowExample-acknowledgements", ofType: "plist")
let viewController = AcknowListViewController(acknowledgementsPlistPath: path)
simonbromberg commented 6 years ago

But then why does it use that default name that doesn't match the one the pod auto generates?

vtourraine commented 6 years ago

When I first created this library, that was the default plist file name, so it made sense. I added the option to specify the file path/name later on, but never removed the original initializer. Maybe there are cases where it’s still useful, but maybe not. If not, I should probably remove that init, or at least deprecate it.

simonbromberg commented 6 years ago

Yea, or you could change the default name to match the actual default name… (It looks like it's just the target name so you could get that using Bundle.main.infoDictionary?["CFBundleName"] as? String)

vtourraine commented 6 years ago

That’s a great idea. It’s probably more nuanced than that (dealing with spaces, capital letters, etc), but it’s definitely worth exploring. Feel free to send a pull request, otherwise I’ll try to implement that later on.

vtourraine commented 6 years ago

@simonbromberg So... I’ve implemented your idea: https://github.com/vtourraine/AcknowList/blob/35289f87d849011749d7bd639a8ba0bcec167418/Source/AcknowListViewController.swift#L152

I was worried that some apps are probably relying on the old file name, so in order not to break the current behavior, I check with FileManager if the file exists, and if not I fallback to the old behavior. I don’t really like having to check explicitly, but that seems like a good compromise overall.

Thanks again for the idea, and feel free to improve it.