search

mattdonnelly / Swifter

[DEPRECATED] :bird: A Twitter framework for iOS & OS X written in Swift
MIT License
2.37k stars 387 forks source link

Support ASWebAuthenticationSession #335

Closed Kyome22 closed 3 years ago

Kyome22 commented 3 years ago

I implemented to solve this issue https://github.com/mattdonnelly/Swifter/issues/334 I also updated the SwifterDemoMac.

meteochu commented 3 years ago

Hi there, thanks for the PR. I just quickly looked at it and there seems like quite a bit of code duplication and I wonder if there's a cleaner way to support these authentication methods across different platforms.

Kyome22 commented 3 years ago

@meteochu Thank you for looking at my request. I think so too. There is much code duplication. However, because of the complexity of the conditional branching by platform and their OS versions, it is difficult for me to determine the extent to be solved at once.

By the way, do you think we should support ASWebAuthenticationSession to iOS with this pull request? I think we should do it as soon as possible according to apple's references.

meteochu commented 3 years ago

I've actually not looked at ASWebAuthenticationSession in the past. I just gave it a quick glance and it does seem quite promising. That said, I think this could be part of a big clean-up in the project that potentially supports Twitter API v2, as well as making this the default authentication to remove friction with the callbacks and to better support SwiftUI.

I don't really have the time just yet to look at all of it but am happy to explore at some point.

Kyome22 commented 3 years ago

@meteochu I've improved it to support iOS. I checked its operation used the keys of my Twitter client app. The program seems to work properly on both iOS and macOS. I tried to reduce code duplication. How about that? I want you to merge if you like this.

Kyome22 commented 3 years ago

@meteochu Thank you for reviewing. How about my updates?

meteochu commented 3 years ago

Sorry about the delay, got a bit busy wrapping up my academics. The rest of the changes look good to me, once you apply the last change (revert the version change), I'll be happy to merge it :)